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

@@ -636,12 +636,22 @@ fs.readFileSync = function(path, options) {
 };
 
 fs.close = function(fd, 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');
+
+  const req = new FSReqWrap();
   req.oncomplete = makeCallback(callback);
   binding.close(fd, req);
 };
 
 fs.closeSync = function(fd) {
+  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');
+
   return binding.close(fd);
 };
 
@@ -854,7 +864,14 @@ fs.ftruncate = function(fd, len, callback) {
   } else if (len === undefined) {
     len = 0;
   }
-  var req = new FSReqWrap();
+  if (typeof fd !== 'number')
+    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 len !== 'number')
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'len', 'number');
+  len = Math.max(0, len);
+  const req = new FSReqWrap();
   req.oncomplete = makeCallback(callback);
   binding.ftruncate(fd, len, req);
 };
@@ -863,6 +880,13 @@ fs.ftruncateSync = function(fd, len) {
   if (len === undefined) {
     len = 0;
   }
+  if (typeof fd !== 'number')
+    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 len !== 'number')
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'len', 'number');
+  len = Math.max(0, len);
   return binding.ftruncate(fd, len);
 };
 
@@ -883,22 +907,38 @@ fs.rmdirSync = function(path) {
 };
 
 fs.fdatasync = function(fd, callback) {
-  var req = new FSReqWrap();
+  if (typeof fd !== 'number')
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'fd', 'number');
+  if (fd < 0 || fd > 0xFFFFFFFF)
+    throw new errors.RangeError('ERR_OUT_OF_RANGE', 'fd');
+  const req = new FSReqWrap();
   req.oncomplete = makeCallback(callback);
   binding.fdatasync(fd, req);
 };
 
 fs.fdatasyncSync = function(fd) {
+  if (typeof fd !== 'number')
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'fd', 'number');
+  if (fd < 0 || fd > 0xFFFFFFFF)
+    throw new errors.RangeError('ERR_OUT_OF_RANGE', 'fd');
   return binding.fdatasync(fd);
 };
 
 fs.fsync = function(fd, callback) {
-  var req = new FSReqWrap();
+  if (typeof fd !== 'number')
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'fd', 'number');
+  if (fd < 0 || fd > 0xFFFFFFFF)
+    throw new errors.RangeError('ERR_OUT_OF_RANGE', 'fd');
+  const req = new FSReqWrap();
   req.oncomplete = makeCallback(callback);
   binding.fsync(fd, req);
 };
 
 fs.fsyncSync = function(fd) {
+  if (typeof fd !== 'number')
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'fd', 'number');
+  if (fd < 0 || fd > 0xFFFFFFFF)
+    throw new errors.RangeError('ERR_OUT_OF_RANGE', 'fd');
   return binding.fsync(fd);
 };
 
@@ -941,7 +981,11 @@ fs.readdirSync = function(path, options) {
 };
 
 fs.fstat = function(fd, callback) {
-  var req = new FSReqWrap();
+  if (typeof fd !== 'number')
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'fd', 'number');
+  if (fd < 0 || fd > 0xFFFFFFFF)
+    throw new errors.RangeError('ERR_OUT_OF_RANGE', 'fd');
+  const req = new FSReqWrap();
   req.oncomplete = makeStatsCallback(callback);
   binding.fstat(fd, req);
 };
@@ -967,6 +1011,10 @@ fs.stat = function(path, callback) {
 };
 
 fs.fstatSync = function(fd) {
+  if (typeof fd !== 'number')
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'fd', 'number');
+  if (fd < 0 || fd > 0xFFFFFFFF)
+    throw new errors.RangeError('ERR_OUT_OF_RANGE', 'fd');
   binding.fstat(fd);
   return statsFromValues();
 };
@@ -1098,13 +1146,32 @@ fs.unlinkSync = function(path) {
 };
 
 fs.fchmod = function(fd, mode, callback) {
-  var req = new FSReqWrap();
+  mode = modeNum(mode);
+  if (typeof fd !== 'number')
+    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')
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'mode', 'number');
+  if (mode < 0 || mode > 0o777)
+    throw new errors.RangeError('ERR_OUT_OF_RANGE', 'mode');
+
+  const req = new FSReqWrap();
   req.oncomplete = makeCallback(callback);
-  binding.fchmod(fd, modeNum(mode), req);
+  binding.fchmod(fd, mode, req);
 };
 
 fs.fchmodSync = function(fd, mode) {
-  return binding.fchmod(fd, modeNum(mode));
+  mode = modeNum(mode);
+  if (typeof fd !== 'number')
+    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')
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'mode', 'number');
+  if (mode < 0 || mode > 0o777)
+    throw new errors.RangeError('ERR_OUT_OF_RANGE', 'mode');
+  return binding.fchmod(fd, mode);
 };
 
 if (constants.O_SYMLINK !== undefined) {

src/node_file.cc

@@ -464,10 +464,7 @@ void Access(const FunctionCallbackInfo<Value>& args) {
 void Close(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  if (args.Length() < 1)
-    return TYPE_ERROR("fd is required");
-  if (!args[0]->IsInt32())
-    return TYPE_ERROR("fd must be a file descriptor");
+  CHECK(args[0]->IsInt32());
 
   int fd = args[0]->Int32Value();
 

test/parallel/test-fs-close-errors.js

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