fs: fix chown abort

This syncs the type assertion introduced in the referenced PR in the C++
side. Since chown, lchown, and fchown can accept -1 as a value for uid
and gid, we should also accept signed integers from the JS side.

Fixes: https://github.com/nodejs/node/issues/37995
Refs: https://github.com/nodejs/node/pull/31694

PR-URL: https://github.com/nodejs/node/pull/38004
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>

3 files changed, 25 insertions, 24 deletions

diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
index 5db2f7759c..1eac04665d 100644
--- a/lib/internal/fs/promises.js
+++ b/lib/internal/fs/promises.js
@@ -8,6 +8,9 @@ const kReadFileBufferLength = 512 * 1024;
 const kReadFileUnknownBufferLength = 64 * 1024;
 const kWriteFileMaxChunkSize = 512 * 1024;
 
+// 2 ** 32 - 1
+const kMaxUserId = 4294967295;
+
 const {
   ArrayPrototypePush,
   Error,
@@ -71,7 +74,6 @@ const {
   validateBoolean,
   validateBuffer,
   validateInteger,
-  validateUint32
 } = require('internal/validators');
 const pathModule = require('path');
 const { promisify } = require('internal/util');
@@ -615,22 +617,22 @@ async function lchmod(path, mode) {
 
 async function lchown(path, uid, gid) {
   path = getValidatedPath(path);
-  validateUint32(uid, 'uid');
-  validateUint32(gid, 'gid');
+  validateInteger(uid, 'uid', -1, kMaxUserId);
+  validateInteger(gid, 'gid', -1, kMaxUserId);
   return binding.lchown(pathModule.toNamespacedPath(path),
                         uid, gid, kUsePromises);
 }
 
 async function fchown(handle, uid, gid) {
-  validateUint32(uid, 'uid');
-  validateUint32(gid, 'gid');
+  validateInteger(uid, 'uid', -1, kMaxUserId);
+  validateInteger(gid, 'gid', -1, kMaxUserId);
   return binding.fchown(handle.fd, uid, gid, kUsePromises);
 }
 
 async function chown(path, uid, gid) {
   path = getValidatedPath(path);
-  validateUint32(uid, 'uid');
-  validateUint32(gid, 'gid');
+  validateInteger(uid, 'uid', -1, kMaxUserId);
+  validateInteger(gid, 'gid', -1, kMaxUserId);
   return binding.chown(pathModule.toNamespacedPath(path),
                        uid, gid, kUsePromises);
 }
diff --git a/src/node_file.cc b/src/node_file.cc
index 0eb882b49a..7a61ce22af 100644
--- a/src/node_file.cc
+++ b/src/node_file.cc
@@ -71,7 +71,6 @@ using v8::ObjectTemplate;
 using v8::Promise;
 using v8::String;
 using v8::Symbol;
-using v8::Uint32;
 using v8::Undefined;
 using v8::Value;
 
@@ -2184,11 +2183,11 @@ static void Chown(const FunctionCallbackInfo<Value>& args) {
   BufferValue path(env->isolate(), args[0]);
   CHECK_NOT_NULL(*path);
 
-  CHECK(args[1]->IsUint32());
-  const uv_uid_t uid = static_cast<uv_uid_t>(args[1].As<Uint32>()->Value());
+  CHECK(IsSafeJsInt(args[1]));
+  const uv_uid_t uid = static_cast<uv_uid_t>(args[1].As<Integer>()->Value());
 
-  CHECK(args[2]->IsUint32());
-  const uv_gid_t gid = static_cast<uv_gid_t>(args[2].As<Uint32>()->Value());
+  CHECK(IsSafeJsInt(args[2]));
+  const uv_gid_t gid = static_cast<uv_gid_t>(args[2].As<Integer>()->Value());
 
   FSReqBase* req_wrap_async = GetReqWrap(args, 3);
   if (req_wrap_async != nullptr) {  // chown(path, uid, gid, req)
@@ -2217,11 +2216,11 @@ static void FChown(const FunctionCallbackInfo<Value>& args) {
   CHECK(args[0]->IsInt32());
   const int fd = args[0].As<Int32>()->Value();
 
-  CHECK(args[1]->IsUint32());
-  const uv_uid_t uid = static_cast<uv_uid_t>(args[1].As<Uint32>()->Value());
+  CHECK(IsSafeJsInt(args[1]));
+  const uv_uid_t uid = static_cast<uv_uid_t>(args[1].As<Integer>()->Value());
 
-  CHECK(args[2]->IsUint32());
-  const uv_gid_t gid = static_cast<uv_gid_t>(args[2].As<Uint32>()->Value());
+  CHECK(IsSafeJsInt(args[2]));
+  const uv_gid_t gid = static_cast<uv_gid_t>(args[2].As<Integer>()->Value());
 
   FSReqBase* req_wrap_async = GetReqWrap(args, 3);
   if (req_wrap_async != nullptr) {  // fchown(fd, uid, gid, req)
@@ -2247,11 +2246,11 @@ static void LChown(const FunctionCallbackInfo<Value>& args) {
   BufferValue path(env->isolate(), args[0]);
   CHECK_NOT_NULL(*path);
 
-  CHECK(args[1]->IsUint32());
-  const uv_uid_t uid = static_cast<uv_uid_t>(args[1].As<Uint32>()->Value());
+  CHECK(IsSafeJsInt(args[1]));
+  const uv_uid_t uid = static_cast<uv_uid_t>(args[1].As<Integer>()->Value());
 
-  CHECK(args[2]->IsUint32());
-  const uv_gid_t gid = static_cast<uv_gid_t>(args[2].As<Uint32>()->Value());
+  CHECK(IsSafeJsInt(args[2]));
+  const uv_gid_t gid = static_cast<uv_gid_t>(args[2].As<Integer>()->Value());
 
   FSReqBase* req_wrap_async = GetReqWrap(args, 3);
   if (req_wrap_async != nullptr) {  // lchown(path, uid, gid, req)
diff --git a/test/parallel/test-fs-promises.js b/test/parallel/test-fs-promises.js
index e01fc2ec6a..bb9a3257e1 100644
--- a/test/parallel/test-fs-promises.js
+++ b/test/parallel/test-fs-promises.js
@@ -191,24 +191,24 @@ async function getHandle(dest) {
 
       assert.rejects(
         async () => {
-          await chown(dest, 1, -1);
+          await chown(dest, 1, -2);
         },
         {
           code: 'ERR_OUT_OF_RANGE',
           name: 'RangeError',
           message: 'The value of "gid" is out of range. ' +
-                  'It must be >= 0 && < 4294967296. Received -1'
+                  'It must be >= -1 && <= 4294967295. Received -2'
         });
 
       assert.rejects(
         async () => {
-          await handle.chown(1, -1);
+          await handle.chown(1, -2);
         },
         {
           code: 'ERR_OUT_OF_RANGE',
           name: 'RangeError',
           message: 'The value of "gid" is out of range. ' +
-                    'It must be >= 0 && < 4294967296. Received -1'
+                    'It must be >= -1 && <= 4294967295. Received -2'
         });
 
       await handle.close();