fs: move type checking for fs.fchown to js

PR-URL: https://github.com/nodejs/node/pull/17334
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

lib/fs.js

@@ -1249,12 +1249,38 @@ if (constants.O_SYMLINK !== undefined) {
 }
 
 fs.fchown = function(fd, uid, gid, callback) {
-  var req = new FSReqWrap();
+  if (!Number.isInteger(fd))
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'fd', 'number');
+  if (fd < 0 || fd > 0xFFFFFFFF)
+    throw new errors.RangeError('ERR_OUT_OF_RANGE', 'fd');
+  if (!Number.isInteger(uid))
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'uid', 'number');
+  if (uid < 0)
+    throw new errors.RangeError('ERR_OUT_OF_RANGE', 'uid');
+  if (!Number.isInteger(gid))
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'gid', 'number');
+  if (gid < 0)
+    throw new errors.RangeError('ERR_OUT_OF_RANGE', 'gid');
+
+  const req = new FSReqWrap();
   req.oncomplete = makeCallback(callback);
   binding.fchown(fd, uid, gid, req);
 };
 
 fs.fchownSync = function(fd, uid, gid) {
+  if (!Number.isInteger(fd))
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'fd', 'number');
+  if (fd < 0 || fd > 0xFFFFFFFF)
+    throw new errors.RangeError('ERR_OUT_OF_RANGE', 'fd');
+  if (!Number.isInteger(uid))
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'uid', 'number');
+  if (uid < 0)
+    throw new errors.RangeError('ERR_OUT_OF_RANGE', 'uid');
+  if (!Number.isInteger(gid))
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'gid', 'number');
+  if (gid < 0)
+    throw new errors.RangeError('ERR_OUT_OF_RANGE', 'gid');
+
   return binding.fchown(fd, uid, gid);
 };
 

src/node_file.cc

@@ -1306,19 +1306,9 @@ static void Chown(const FunctionCallbackInfo<Value>& args) {
 static void FChown(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  int len = args.Length();
+  CHECK(args[0]->IsInt32());
-  if (len < 1)
+  CHECK(args[1]->IsUint32());
-    return TYPE_ERROR("fd required");
+  CHECK(args[2]->IsUint32());
-  if (len < 2)
-    return TYPE_ERROR("uid required");
-  if (len < 3)
-    return TYPE_ERROR("gid required");
-  if (!args[0]->IsInt32())
-    return TYPE_ERROR("fd must be an int");
-  if (!args[1]->IsUint32())
-    return TYPE_ERROR("uid must be an unsigned int");
-  if (!args[2]->IsUint32())
-    return TYPE_ERROR("gid must be an unsigned int");
 
   int fd = args[0]->Int32Value();
   uv_uid_t uid = static_cast<uv_uid_t>(args[1]->Uint32Value());

test/parallel/test-fs-fchown.js

@@ -0,0 +1,110 @@
+'use strict';
+
+const common = require('../common');
+const fs = require('fs');
+
+['', false, null, undefined, {}, []].forEach((i) => {
+  common.expectsError(
+    () => fs.fchown(i),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      type: TypeError,
+      message: 'The "fd" argument must be of type number'
+    }
+  );
+  common.expectsError(
+    () => fs.fchownSync(i),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      type: TypeError,
+      message: 'The "fd" argument must be of type number'
+    }
+  );
+
+  common.expectsError(
+    () => fs.fchown(1, i),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      type: TypeError,
+      message: 'The "uid" argument must be of type number'
+    }
+  );
+  common.expectsError(
+    () => fs.fchownSync(1, i),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      type: TypeError,
+      message: 'The "uid" argument must be of type number'
+    }
+  );
+
+  common.expectsError(
+    () => fs.fchown(1, 1, i),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      type: TypeError,
+      message: 'The "gid" argument must be of type number'
+    }
+  );
+  common.expectsError(
+    () => fs.fchownSync(1, 1, i),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      type: TypeError,
+      message: 'The "gid" argument must be of type number'
+    }
+  );
+});
+
+[-1, 0xFFFFFFFF + 1].forEach((i) => {
+  common.expectsError(
+    () => fs.fchown(i),
+    {
+      code: 'ERR_OUT_OF_RANGE',
+      type: RangeError,
+      message: 'The "fd" argument is out of range'
+    }
+  );
+  common.expectsError(
+    () => fs.fchownSync(i),
+    {
+      code: 'ERR_OUT_OF_RANGE',
+      type: RangeError,
+      message: 'The "fd" argument is out of range'
+    }
+  );
+});
+
+common.expectsError(
+  () => fs.fchown(1, -1),
+  {
+    code: 'ERR_OUT_OF_RANGE',
+    type: RangeError,
+    message: 'The "uid" argument is out of range'
+  }
+);
+common.expectsError(
+  () => fs.fchownSync(1, -1),
+  {
+    code: 'ERR_OUT_OF_RANGE',
+    type: RangeError,
+    message: 'The "uid" argument is out of range'
+  }
+);
+
+common.expectsError(
+  () => fs.fchown(1, 1, -1),
+  {
+    code: 'ERR_OUT_OF_RANGE',
+    type: RangeError,
+    message: 'The "gid" argument is out of range'
+  }
+);
+common.expectsError(
+  () => fs.fchownSync(1, 1, -1),
+  {
+    code: 'ERR_OUT_OF_RANGE',
+    type: RangeError,
+    message: 'The "gid" argument is out of range'
+  }
+);