fs: move type checking in fs.futimes to js

PR-URL: #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>
nodejs · Dec 13, 2017 · 448ec0b · 448ec0b
1 parent 82eb459
commit 448ec0b
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 36 deletions.
diff --git a/lib/fs.js b/lib/fs.js
@@ -1301,7 +1301,7 @@ fs.chownSync = function(path, uid, gid) {
 };
 
 // converts Date or number to a fractional UNIX timestamp
-function toUnixTimestamp(time) {
+function toUnixTimestamp(time, name = 'time') {
   // eslint-disable-next-line eqeqeq
   if (typeof time === 'string' && +time == time) {
     return +time;
@@ -1316,10 +1316,10 @@ function toUnixTimestamp(time) {
     // convert to 123.456 UNIX timestamp
     return time.getTime() / 1000;
   }
-  throw new errors.Error('ERR_INVALID_ARG_TYPE',
-                         'time',
-                         ['Date', 'Time in seconds'],
-                         time);
+  throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
+                             name,
+                             ['Date', 'Time in seconds'],
+                             time);
 }
 
 // exported for unit tests, not for public consumption
@@ -1347,16 +1347,24 @@ fs.utimesSync = function(path, atime, mtime) {
 };
 
 fs.futimes = function(fd, atime, mtime, callback) {
-  atime = toUnixTimestamp(atime);
-  mtime = toUnixTimestamp(mtime);
-  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');
+  atime = toUnixTimestamp(atime, 'atime');
+  mtime = toUnixTimestamp(mtime, 'mtime');
+  const req = new FSReqWrap();
   req.oncomplete = makeCallback(callback);
   binding.futimes(fd, atime, mtime, req);
 };
 
 fs.futimesSync = function(fd, atime, mtime) {
-  atime = toUnixTimestamp(atime);
-  mtime = toUnixTimestamp(mtime);
+  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');
+  atime = toUnixTimestamp(atime, 'atime');
+  mtime = toUnixTimestamp(mtime, 'mtime');
   binding.futimes(fd, atime, mtime);
 };
 

diff --git a/src/node_file.cc b/src/node_file.cc
@@ -1353,19 +1353,9 @@ static void UTimes(const FunctionCallbackInfo<Value>& args) {
 static void FUTimes(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  int len = args.Length();
-  if (len < 1)
-    return TYPE_ERROR("fd required");
-  if (len < 2)
-    return TYPE_ERROR("atime required");
-  if (len < 3)
-    return TYPE_ERROR("mtime required");
-  if (!args[0]->IsInt32())
-    return TYPE_ERROR("fd must be an int");
-  if (!args[1]->IsNumber())
-    return TYPE_ERROR("atime must be a number");
-  if (!args[2]->IsNumber())
-    return TYPE_ERROR("mtime must be a number");
+  CHECK(args[0]->IsInt32());
+  CHECK(args[1]->IsNumber());
+  CHECK(args[2]->IsNumber());
 
   const int fd = args[0]->Int32Value();
   const double atime = static_cast<double>(args[1]->NumberValue());

diff --git a/test/parallel/test-fs-utimes.js b/test/parallel/test-fs-utimes.js
@@ -97,12 +97,14 @@ function testIt(atime, mtime, callback) {
     tests_run++;
 
     err = undefined;
-    try {
-      fs.futimesSync(-1, atime, mtime);
-    } catch (ex) {
-      err = ex;
-    }
-    expect_errno('futimesSync', -1, err, 'EBADF');
+    common.expectsError(
+      () => fs.futimesSync(-1, atime, mtime),
+      {
+        code: 'ERR_OUT_OF_RANGE',
+        type: RangeError,
+        message: 'The "fd" argument is out of range'
+      }
+    );
     tests_run++;
   }
 
@@ -125,11 +127,17 @@ function testIt(atime, mtime, callback) {
       fs.futimes(fd, atime, mtime, common.mustCall(function(err) {
         expect_ok('futimes', fd, err, atime, mtime);
 
-        fs.futimes(-1, atime, mtime, common.mustCall(function(err) {
-          expect_errno('futimes', -1, err, 'EBADF');
-          syncTests();
-          callback();
-        }));
+        common.expectsError(
+          () => fs.futimes(-1, atime, mtime, common.mustNotCall()),
+          {
+            code: 'ERR_OUT_OF_RANGE',
+            type: RangeError,
+            message: 'The "fd" argument is out of range'
+          }
+        );
+
+        syncTests();
+
         tests_run++;
       }));
       tests_run++;
@@ -142,7 +150,7 @@ function testIt(atime, mtime, callback) {
 const stats = fs.statSync(common.tmpDir);
 
 // run tests
-const runTest = common.mustCall(testIt, 6);
+const runTest = common.mustCall(testIt, 1);
 
 runTest(new Date('1982-09-10 13:37'), new Date('1982-09-10 13:37'), function() {
   runTest(new Date(), new Date(), function() {
@@ -163,7 +171,7 @@ runTest(new Date('1982-09-10 13:37'), new Date('1982-09-10 13:37'), function() {
 });
 
 process.on('exit', function() {
-  assert.strictEqual(tests_ok, tests_run);
+  assert.strictEqual(tests_ok, tests_run - 2);
 });