errors: validate input arguments

This makes sure the input arguments get validated so implementation
errors will be caught early. It also improves a couple of error
messages by providing more detailed information and fixes errors
detected by the new functionality. Besides that a error type got
simplified and tests got refactored.

PR-URL: nodejs#19924
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

lib/buffer.js

@@ -920,7 +920,7 @@ Buffer.prototype.write = function write(string, offset, length, encoding) {
       length = remaining;
 
     if (string.length > 0 && (length < 0 || offset < 0))
-      throw new ERR_BUFFER_OUT_OF_BOUNDS('length', true);
+      throw new ERR_BUFFER_OUT_OF_BOUNDS();
   } else {
     // if someone is still calling the obsolete form of write(), tell them.
     // we don't want eg buf.write("foo", "utf8", 10) to silently turn into

lib/fs.js

@@ -1019,7 +1019,7 @@ fs.fchmod = function(fd, mode, callback) {
   validateUint32(mode, 'mode');
   // Values for mode < 0 are already checked via the validateUint32 function
   if (mode > 0o777)
-    throw new ERR_OUT_OF_RANGE('mode');
+    throw new ERR_OUT_OF_RANGE('mode', undefined, mode);
 
   const req = new FSReqWrap();
   req.oncomplete = makeCallback(callback);
@@ -1032,7 +1032,7 @@ fs.fchmodSync = function(fd, mode) {
   validateUint32(mode, 'mode');
   // Values for mode < 0 are already checked via the validateUint32 function
   if (mode > 0o777)
-    throw new ERR_OUT_OF_RANGE('mode');
+    throw new ERR_OUT_OF_RANGE('mode', undefined, mode);
   const ctx = {};
   binding.fchmod(fd, mode, undefined, ctx);
   handleErrorFromBinding(ctx);

lib/fs/promises.js

@@ -373,7 +373,7 @@ async function fchmod(handle, mode) {
   validateFileHandle(handle);
   validateUint32(mode, 'mode');
   if (mode > 0o777)
-    throw new ERR_OUT_OF_RANGE('mode');
+    throw new ERR_OUT_OF_RANGE('mode', undefined, mode);
   return binding.fchmod(handle.fd, mode, kUsePromises);
 }
 

lib/internal/buffer.js

@@ -50,7 +50,7 @@ function boundsError(value, length, type) {
   }
 
   if (length < 0)
-    throw new ERR_BUFFER_OUT_OF_BOUNDS(null, true);
+    throw new ERR_BUFFER_OUT_OF_BOUNDS();
 
   throw new ERR_OUT_OF_RANGE(type || 'offset',
                              `>= ${type ? 1 : 0} and <= ${length}`,

lib/internal/crypto/pbkdf2.js

@@ -64,11 +64,8 @@ function _pbkdf2(password, salt, iterations, keylen, digest, callback) {
   if (typeof keylen !== 'number')
     throw new ERR_INVALID_ARG_TYPE('keylen', 'number', keylen);
 
-  if (keylen < 0 ||
-      !Number.isFinite(keylen) ||
-      keylen > INT_MAX) {
-    throw new ERR_OUT_OF_RANGE('keylen');
-  }
+  if (keylen < 0 || !Number.isInteger(keylen) || keylen > INT_MAX)
+    throw new ERR_OUT_OF_RANGE('keylen', `>= 0 && <= ${INT_MAX}`, keylen);
 
   const encoding = getDefaultEncoding();
 

lib/internal/errors.js

@@ -416,20 +416,30 @@ function internalAssert(condition, message) {
   }
 }
 
-function message(key, args) {
+function message(key, args = []) {
   const msg = messages.get(key);
-  internalAssert(msg, `An invalid error message key was used: ${key}.`);
-  let fmt;
   if (util === undefined) util = require('util');
+
   if (typeof msg === 'function') {
-    fmt = msg;
-  } else {
-    fmt = util.format;
-    if (args === undefined || args.length === 0)
-      return msg;
-    args.unshift(msg);
+    internalAssert(
+      msg.length <= args.length, // Default options do not count.
+      `Code: ${key}; The provided arguments length (${args.length}) does not ` +
+        `match the required ones (${msg.length}).`
+    );
+    return msg.apply(null, args);
   }
-  return String(fmt.apply(null, args));
+
+  const expectedLength = (msg.match(/%[dfijoOs]/g) || []).length;
+  internalAssert(
+    expectedLength === args.length,
+    `Code: ${key}; The provided arguments length (${args.length}) does not ` +
+      `match the required ones (${expectedLength}).`
+  );
+  if (args.length === 0)
+    return msg;
+
+  args.unshift(msg);
+  return util.format.apply(null, args);
 }
 
 /**
@@ -740,7 +750,7 @@ E('ERR_HTTP2_INVALID_SETTING_VALUE',
   'Invalid value for setting "%s": %s', TypeError, RangeError);
 E('ERR_HTTP2_INVALID_STREAM', 'The stream has been destroyed', Error);
 E('ERR_HTTP2_MAX_PENDING_SETTINGS_ACK',
-  'Maximum number of pending settings acknowledgements (%s)', Error);
+  'Maximum number of pending settings acknowledgements', Error);
 E('ERR_HTTP2_NO_SOCKET_MANIPULATION',
   'HTTP/2 sockets should not be directly manipulated (e.g. read and written)',
   Error);
@@ -793,7 +803,7 @@ E('ERR_INVALID_ARG_VALUE', (name, value, reason = 'is invalid') => {
 }, TypeError, RangeError);
 E('ERR_INVALID_ARRAY_LENGTH',
   (name, len, actual) => {
-    internalAssert(typeof actual === 'number', 'actual must be a number');
+    internalAssert(typeof actual === 'number', 'actual must be of type number');
     return `The array "${name}" (length ${actual}) must be of length ${len}.`;
   }, TypeError);
 E('ERR_INVALID_ASYNC_ID', 'Invalid %s value: %s', RangeError);
@@ -925,7 +935,9 @@ E('ERR_UNCAUGHT_EXCEPTION_CAPTURE_ALREADY_SET',
   Error);
 E('ERR_UNESCAPED_CHARACTERS', '%s contains unescaped characters', TypeError);
 E('ERR_UNHANDLED_ERROR',
-  (err) => {
+  // Using a default argument here is important so the argument is not counted
+  // towards `Function#length`.
+  (err = undefined) => {
     const msg = 'Unhandled error.';
     if (err === undefined) return msg;
     return `${msg} (${err})`;
@@ -960,7 +972,6 @@ E('ERR_VM_MODULE_STATUS', 'Module status %s', Error);
 E('ERR_ZLIB_INITIALIZATION_FAILED', 'Initialization failed', Error);
 
 function invalidArgType(name, expected, actual) {
-  internalAssert(arguments.length === 3, 'Exactly 3 arguments are required');
   internalAssert(typeof name === 'string', 'name must be a string');
 
   // determiner: 'must be' or 'must not be'
@@ -1007,8 +1018,7 @@ function missingArgs(...args) {
 }
 
 function oneOf(expected, thing) {
-  internalAssert(expected, 'expected is required');
-  internalAssert(typeof thing === 'string', 'thing is required');
+  internalAssert(typeof thing === 'string', '`thing` has to be of type string');
   if (Array.isArray(expected)) {
     const len = expected.length;
     internalAssert(len > 0,
@@ -1027,25 +1037,28 @@ function oneOf(expected, thing) {
   }
 }
 
-function bufferOutOfBounds(name, isWriting) {
-  if (isWriting) {
-    return 'Attempt to write outside buffer bounds';
-  } else {
+// Using a default argument here is important so the argument is not counted
+// towards `Function#length`.
+function bufferOutOfBounds(name = undefined) {
+  if (name) {
     return `"${name}" is outside of buffer bounds`;
   }
+  return 'Attempt to write outside buffer bounds';
 }
 
-function invalidChar(name, field) {
+// Using a default argument here is important so the argument is not counted
+// towards `Function#length`.
+function invalidChar(name, field = undefined) {
   let msg = `Invalid character in ${name}`;
-  if (field) {
+  if (field !== undefined) {
     msg += ` ["${field}"]`;
   }
   return msg;
 }
 
 function outOfRange(name, range, value) {
   let msg = `The value of "${name}" is out of range.`;
-  if (range) msg += ` It must be ${range}.`;
-  if (value !== undefined) msg += ` Received ${value}`;
+  if (range !== undefined) msg += ` It must be ${range}.`;
+  msg += ` Received ${value}`;
   return msg;
 }

lib/internal/fs.js

@@ -368,9 +368,10 @@ function validateOffsetLengthRead(offset, length, bufferLength) {
   let err;
 
   if (offset < 0 || offset >= bufferLength) {
-    err = new ERR_OUT_OF_RANGE('offset');
+    err = new ERR_OUT_OF_RANGE('offset', `>= 0 && <= ${bufferLength}`, offset);
   } else if (length < 0 || offset + length > bufferLength) {
-    err = new ERR_OUT_OF_RANGE('length');
+    err = new ERR_OUT_OF_RANGE('length',
+                               `>= 0 && <= ${bufferLength - offset}`, length);
   }
 
   if (err !== undefined) {
@@ -383,9 +384,12 @@ function validateOffsetLengthWrite(offset, length, byteLength) {
   let err;
 
   if (offset > byteLength) {
-    err = new ERR_OUT_OF_RANGE('offset');
-  } else if (offset + length > byteLength || offset + length > kMaxLength) {
-    err = new ERR_OUT_OF_RANGE('length');
+    err = new ERR_OUT_OF_RANGE('offset', `<= ${byteLength}`, offset);
+  } else {
+    const max = byteLength > kMaxLength ? kMaxLength : byteLength;
+    if (length > max - offset) {
+      err = new ERR_OUT_OF_RANGE('length', `<= ${max - offset}`, length);
+    }
   }
 
   if (err !== undefined) {

lib/internal/http2/core.js

@@ -1312,8 +1312,10 @@ class ServerHttp2Session extends Http2Session {
       if (origin === 'null')
         throw new ERR_HTTP2_ALTSVC_INVALID_ORIGIN();
     } else if (typeof originOrStream === 'number') {
-      if (originOrStream >>> 0 !== originOrStream || originOrStream === 0)
-        throw new ERR_OUT_OF_RANGE('originOrStream');
+      if (originOrStream >>> 0 !== originOrStream || originOrStream === 0) {
+        throw new ERR_OUT_OF_RANGE('originOrStream',
+                                   `> 0 && < ${2 ** 32}`, originOrStream);
+      }
       stream = originOrStream;
     } else if (originOrStream !== undefined) {
       // Allow origin to be passed a URL or object with origin property
@@ -1764,7 +1766,7 @@ class Http2Stream extends Duplex {
     if (typeof code !== 'number')
       throw new ERR_INVALID_ARG_TYPE('code', 'number', code);
     if (code < 0 || code > kMaxInt)
-      throw new ERR_OUT_OF_RANGE('code');
+      throw new ERR_OUT_OF_RANGE('code', `>= 0 && <= ${kMaxInt}`, code);
     if (callback !== undefined && typeof callback !== 'function')
       throw new ERR_INVALID_CALLBACK();
 

test/parallel/test-buffer-arraybuffer.js

@@ -1,6 +1,6 @@
 'use strict';
 
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 
 const LENGTH = 16;
@@ -58,14 +58,14 @@ assert.throws(function() {
   buf[0] = 9;
   assert.strictEqual(ab[1], 9);
 
-  common.expectsError(() => Buffer.from(ab.buffer, 6), {
+  assert.throws(() => Buffer.from(ab.buffer, 6), {
     code: 'ERR_BUFFER_OUT_OF_BOUNDS',
-    type: RangeError,
+    name: 'RangeError [ERR_BUFFER_OUT_OF_BOUNDS]',
     message: '"offset" is outside of buffer bounds'
   });
-  common.expectsError(() => Buffer.from(ab.buffer, 3, 6), {
+  assert.throws(() => Buffer.from(ab.buffer, 3, 6), {
     code: 'ERR_BUFFER_OUT_OF_BOUNDS',
-    type: RangeError,
+    name: 'RangeError [ERR_BUFFER_OUT_OF_BOUNDS]',
     message: '"length" is outside of buffer bounds'
   });
 }
@@ -86,14 +86,14 @@ assert.throws(function() {
   buf[0] = 9;
   assert.strictEqual(ab[1], 9);
 
-  common.expectsError(() => Buffer(ab.buffer, 6), {
+  assert.throws(() => Buffer(ab.buffer, 6), {
     code: 'ERR_BUFFER_OUT_OF_BOUNDS',
-    type: RangeError,
+    name: 'RangeError [ERR_BUFFER_OUT_OF_BOUNDS]',
     message: '"offset" is outside of buffer bounds'
   });
-  common.expectsError(() => Buffer(ab.buffer, 3, 6), {
+  assert.throws(() => Buffer(ab.buffer, 3, 6), {
     code: 'ERR_BUFFER_OUT_OF_BOUNDS',
-    type: RangeError,
+    name: 'RangeError [ERR_BUFFER_OUT_OF_BOUNDS]',
     message: '"length" is outside of buffer bounds'
   });
 }
@@ -111,11 +111,11 @@ assert.throws(function() {
   assert.deepStrictEqual(Buffer.from(ab, [1]), Buffer.from(ab, 1));
 
   // If byteOffset is Infinity, throw.
-  common.expectsError(() => {
+  assert.throws(() => {
     Buffer.from(ab, Infinity);
   }, {
     code: 'ERR_BUFFER_OUT_OF_BOUNDS',
-    type: RangeError,
+    name: 'RangeError [ERR_BUFFER_OUT_OF_BOUNDS]',
     message: '"offset" is outside of buffer bounds'
   });
 }
@@ -133,11 +133,11 @@ assert.throws(function() {
   assert.deepStrictEqual(Buffer.from(ab, 0, [1]), Buffer.from(ab, 0, 1));
 
   // If length is Infinity, throw.
-  common.expectsError(() => {
+  assert.throws(() => {
     Buffer.from(ab, 0, Infinity);
   }, {
     code: 'ERR_BUFFER_OUT_OF_BOUNDS',
-    type: RangeError,
+    name: 'RangeError [ERR_BUFFER_OUT_OF_BOUNDS]',
     message: '"length" is outside of buffer bounds'
   });
 }

test/parallel/test-child-process-fork.js

@@ -31,18 +31,26 @@ const n = fork(fixtures.path('child-process-spawn-node.js'), args);
 assert.strictEqual(n.channel, n._channel);
 assert.deepStrictEqual(args, ['foo', 'bar']);
 
-n.on('message', function(m) {
+n.on('message', (m) => {
   console.log('PARENT got message:', m);
   assert.ok(m.foo);
 });
 
 // https://github.com/joyent/node/issues/2355 - JSON.stringify(undefined)
 // returns "undefined" but JSON.parse() cannot parse that...
-assert.throws(function() { n.send(undefined); }, TypeError);
-assert.throws(function() { n.send(); }, TypeError);
+assert.throws(() => n.send(undefined), {
+  name: 'TypeError [ERR_MISSING_ARGS]',
+  message: 'The "message" argument must be specified',
+  code: 'ERR_MISSING_ARGS'
+});
+assert.throws(() => n.send(), {
+  name: 'TypeError [ERR_MISSING_ARGS]',
+  message: 'The "message" argument must be specified',
+  code: 'ERR_MISSING_ARGS'
+});
 
 n.send({ hello: 'world' });
 
-n.on('exit', common.mustCall(function(c) {
+n.on('exit', common.mustCall((c) => {
   assert.strictEqual(c, 0);
 }));