fs: move type checking for fs.fchmod 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>

3 files changed, 52 insertions, 10 deletions

diff --git a/lib/fs.js b/lib/fs.js
index 1a93d0acf2..c105f5fb3a 100644
--- a/lib/fs.js
+++ b/lib/fs.js
@@ -1147,11 +1147,11 @@ fs.unlinkSync = function(path) {
 
 fs.fchmod = function(fd, mode, callback) {
   mode = modeNum(mode);
-  if (typeof fd !== 'number')
+  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 (typeof mode !== 'number')
+  if (!Number.isInteger(mode))
     throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'mode', 'number');
   if (mode < 0 || mode > 0o777)
     throw new errors.RangeError('ERR_OUT_OF_RANGE', 'mode');
@@ -1163,11 +1163,11 @@ fs.fchmod = function(fd, mode, callback) {
 
 fs.fchmodSync = function(fd, mode) {
   mode = modeNum(mode);
-  if (typeof fd !== 'number')
+  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 (typeof mode !== 'number')
+  if (!Number.isInteger(mode))
     throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'mode', 'number');
   if (mode < 0 || mode > 0o777)
     throw new errors.RangeError('ERR_OUT_OF_RANGE', 'mode');
diff --git a/src/node_file.cc b/src/node_file.cc
index c330c95fa6..2acac8c8f8 100644
--- a/src/node_file.cc
+++ b/src/node_file.cc
@@ -1254,12 +1254,8 @@ static void Chmod(const FunctionCallbackInfo<Value>& args) {
 static void FChmod(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  if (args.Length() < 2)
-    return TYPE_ERROR("fd and mode are required");
-  if (!args[0]->IsInt32())
-    return TYPE_ERROR("fd must be a file descriptor");
-  if (!args[1]->IsInt32())
-    return TYPE_ERROR("mode must be an integer");
+  CHECK(args[0]->IsInt32());
+  CHECK(args[1]->IsInt32());
 
   int fd = args[0]->Int32Value();
   int mode = static_cast<int>(args[1]->Int32Value());
diff --git a/test/parallel/test-fs-chmod.js b/test/parallel/test-fs-chmod.js
index 7d4b7a10db..9eae75c3c7 100644
--- a/test/parallel/test-fs-chmod.js
+++ b/test/parallel/test-fs-chmod.js
@@ -108,6 +108,15 @@ fs.open(file2, 'w', common.mustCall((err, fd) => {
       assert.strictEqual(mode_async, fs.fstatSync(fd).mode & 0o777);
     }
 
+    common.expectsError(
+      () => fs.fchmod(fd, {}),
+      {
+        code: 'ERR_INVALID_ARG_TYPE',
+        type: TypeError,
+        message: 'The "mode" argument must be of type number'
+      }
+    );
+
     fs.fchmodSync(fd, mode_sync);
     if (common.isWindows) {
       assert.ok((fs.fstatSync(fd).mode & 0o777) & mode_sync);
@@ -136,6 +145,43 @@ if (fs.lchmod) {
   }));
 }
 
+['', false, null, undefined, {}, []].forEach((i) => {
+  common.expectsError(
+    () => fs.fchmod(i, 0o000),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      type: TypeError,
+      message: 'The "fd" argument must be of type number'
+    }
+  );
+  common.expectsError(
+    () => fs.fchmodSync(i, 0o000),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      type: TypeError,
+      message: 'The "fd" argument must be of type number'
+    }
+  );
+});
+
+[-1, 0xFFFFFFFF + 1].forEach((i) => {
+  common.expectsError(
+    () => fs.fchmod(i, 0o000),
+    {
+      code: 'ERR_OUT_OF_RANGE',
+      type: RangeError,
+      message: 'The "fd" argument is out of range'
+    }
+  );
+  common.expectsError(
+    () => fs.fchmodSync(i, 0o000),
+    {
+      code: 'ERR_OUT_OF_RANGE',
+      type: RangeError,
+      message: 'The "fd" argument is out of range'
+    }
+  );
+});
 
 process.on('exit', function() {
   assert.strictEqual(0, openCount);