Skip to content

Commit 76a8ce5

Browse files
BridgeARBridgeAR
committed
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.
1 parent bb6de0d commit 76a8ce5

18 files changed

+209
-178
lines changed

‎lib/buffer.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -920,7 +920,7 @@ Buffer.prototype.write = function write(string, offset, length, encoding) {
920920
length = remaining;
921921

922922
if (string.length > 0 && (length < 0 || offset < 0))
923-
throw new ERR_BUFFER_OUT_OF_BOUNDS('length', true);
923+
throw new ERR_BUFFER_OUT_OF_BOUNDS();
924924
} else {
925925
// if someone is still calling the obsolete form of write(), tell them.
926926
// we don't want eg buf.write("foo", "utf8", 10) to silently turn into

‎lib/fs.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -1019,7 +1019,7 @@ fs.fchmod = function(fd, mode, callback) {
10191019
validateUint32(mode, 'mode');
10201020
// Values for mode < 0 are already checked via the validateUint32 function
10211021
if (mode > 0o777)
1022-
throw new ERR_OUT_OF_RANGE('mode');
1022+
throw new ERR_OUT_OF_RANGE('mode', undefined, mode);
10231023

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

‎lib/fs/promises.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ async function fchmod(handle, mode) {
373373
validateFileHandle(handle);
374374
validateUint32(mode, 'mode');
375375
if (mode > 0o777)
376-
throw new ERR_OUT_OF_RANGE('mode');
376+
throw new ERR_OUT_OF_RANGE('mode', undefined, mode);
377377
return binding.fchmod(handle.fd, mode, kUsePromises);
378378
}
379379

‎lib/internal/buffer.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ function boundsError(value, length, type) {
4848
}
4949

5050
if (length < 0)
51-
throw new ERR_BUFFER_OUT_OF_BOUNDS(null, true);
51+
throw new ERR_BUFFER_OUT_OF_BOUNDS();
5252

5353
throw new ERR_OUT_OF_RANGE(type || 'offset',
5454
`>= ${type ? 1 : 0} and <= ${length}`,

‎lib/internal/crypto/pbkdf2.js

+2-5
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,8 @@ function _pbkdf2(password, salt, iterations, keylen, digest, callback) {
6464
if (typeof keylen !== 'number')
6565
throw new ERR_INVALID_ARG_TYPE('keylen', 'number', keylen);
6666

67-
if (keylen < 0 ||
68-
!Number.isFinite(keylen) ||
69-
keylen > INT_MAX) {
70-
throw new ERR_OUT_OF_RANGE('keylen');
71-
}
67+
if (keylen < 0 || !Number.isInteger(keylen) || keylen > INT_MAX)
68+
throw new ERR_OUT_OF_RANGE('keylen', `>= 0 && <= ${INT_MAX}`, keylen);
7269

7370
const encoding = getDefaultEncoding();
7471

‎lib/internal/errors.js

+31-24
Original file line numberDiff line numberDiff line change
@@ -416,20 +416,30 @@ function internalAssert(condition, message) {
416416
}
417417
}
418418

419-
function message(key, args) {
419+
function message(key, args = []) {
420420
const msg = messages.get(key);
421-
internalAssert(msg, `An invalid error message key was used: ${key}.`);
422-
let fmt;
423421
if (util === undefined) util = require('util');
422+
424423
if (typeof msg === 'function') {
425-
fmt = msg;
426-
} else {
427-
fmt = util.format;
428-
if (args === undefined || args.length === 0)
429-
return msg;
430-
args.unshift(msg);
424+
internalAssert(
425+
msg.length <= args.length, // Default options do not count.
426+
`Code: ${key}; The provided arguments length (${args.length}) does not ` +
427+
`match the required ones (${msg.length}).`
428+
);
429+
return msg.apply(null, args);
431430
}
432-
return String(fmt.apply(null, args));
431+
432+
const expectedLength = (msg.match(/%[dfijoOs]/g) || []).length;
433+
internalAssert(
434+
expectedLength === args.length,
435+
`Code: ${key}; The provided arguments length (${args.length}) does not ` +
436+
`match the required ones (${expectedLength}).`
437+
);
438+
if (args.length === 0)
439+
return msg;
440+
441+
args.unshift(msg);
442+
return util.format.apply(null, args);
433443
}
434444

435445
/**
@@ -739,7 +749,7 @@ E('ERR_HTTP2_INVALID_SETTING_VALUE',
739749
'Invalid value for setting "%s": %s', TypeError, RangeError);
740750
E('ERR_HTTP2_INVALID_STREAM', 'The stream has been destroyed', Error);
741751
E('ERR_HTTP2_MAX_PENDING_SETTINGS_ACK',
742-
'Maximum number of pending settings acknowledgements (%s)', Error);
752+
'Maximum number of pending settings acknowledgements', Error);
743753
E('ERR_HTTP2_NO_SOCKET_MANIPULATION',
744754
'HTTP/2 sockets should not be directly manipulated (e.g. read and written)',
745755
Error);
@@ -792,7 +802,7 @@ E('ERR_INVALID_ARG_VALUE', (name, value, reason = 'is invalid') => {
792802
}, TypeError, RangeError);
793803
E('ERR_INVALID_ARRAY_LENGTH',
794804
(name, len, actual) => {
795-
internalAssert(typeof actual === 'number', 'actual must be a number');
805+
internalAssert(typeof actual === 'number', 'actual must be of type number');
796806
return `The array "${name}" (length ${actual}) must be of length ${len}.`;
797807
}, TypeError);
798808
E('ERR_INVALID_ASYNC_ID', 'Invalid %s value: %s', RangeError);
@@ -924,7 +934,7 @@ E('ERR_UNCAUGHT_EXCEPTION_CAPTURE_ALREADY_SET',
924934
Error);
925935
E('ERR_UNESCAPED_CHARACTERS', '%s contains unescaped characters', TypeError);
926936
E('ERR_UNHANDLED_ERROR',
927-
(err) => {
937+
(err = undefined) => {
928938
const msg = 'Unhandled error.';
929939
if (err === undefined) return msg;
930940
return `${msg} (${err})`;
@@ -959,7 +969,6 @@ E('ERR_VM_MODULE_STATUS', 'Module status %s', Error);
959969
E('ERR_ZLIB_INITIALIZATION_FAILED', 'Initialization failed', Error);
960970

961971
function invalidArgType(name, expected, actual) {
962-
internalAssert(arguments.length === 3, 'Exactly 3 arguments are required');
963972
internalAssert(typeof name === 'string', 'name must be a string');
964973

965974
// determiner: 'must be' or 'must not be'
@@ -1006,8 +1015,7 @@ function missingArgs(...args) {
10061015
}
10071016

10081017
function oneOf(expected, thing) {
1009-
internalAssert(expected, 'expected is required');
1010-
internalAssert(typeof thing === 'string', 'thing is required');
1018+
internalAssert(typeof thing === 'string', '`thing` has to be of type string');
10111019
if (Array.isArray(expected)) {
10121020
const len = expected.length;
10131021
internalAssert(len > 0,
@@ -1026,25 +1034,24 @@ function oneOf(expected, thing) {
10261034
}
10271035
}
10281036

1029-
function bufferOutOfBounds(name, isWriting) {
1030-
if (isWriting) {
1031-
return 'Attempt to write outside buffer bounds';
1032-
} else {
1037+
function bufferOutOfBounds(name = undefined) {
1038+
if (name) {
10331039
return `"${name}" is outside of buffer bounds`;
10341040
}
1041+
return 'Attempt to write outside buffer bounds';
10351042
}
10361043

1037-
function invalidChar(name, field) {
1044+
function invalidChar(name, field = undefined) {
10381045
let msg = `Invalid character in ${name}`;
1039-
if (field) {
1046+
if (field !== undefined) {
10401047
msg += ` ["${field}"]`;
10411048
}
10421049
return msg;
10431050
}
10441051

10451052
function outOfRange(name, range, value) {
10461053
let msg = `The value of "${name}" is out of range.`;
1047-
if (range) msg += ` It must be ${range}.`;
1048-
if (value !== undefined) msg += ` Received ${value}`;
1054+
if (range !== undefined) msg += ` It must be ${range}.`;
1055+
msg += ` Received ${value}`;
10491056
return msg;
10501057
}

‎lib/internal/fs.js

+9-5
Original file line numberDiff line numberDiff line change
@@ -368,9 +368,10 @@ function validateOffsetLengthRead(offset, length, bufferLength) {
368368
let err;
369369

370370
if (offset < 0 || offset >= bufferLength) {
371-
err = new ERR_OUT_OF_RANGE('offset');
371+
err = new ERR_OUT_OF_RANGE('offset', `>= 0 && <= ${bufferLength}`, offset);
372372
} else if (length < 0 || offset + length > bufferLength) {
373-
err = new ERR_OUT_OF_RANGE('length');
373+
err = new ERR_OUT_OF_RANGE('length',
374+
`>= 0 && <= ${bufferLength - offset}`, length);
374375
}
375376

376377
if (err !== undefined) {
@@ -383,9 +384,12 @@ function validateOffsetLengthWrite(offset, length, byteLength) {
383384
let err;
384385

385386
if (offset > byteLength) {
386-
err = new ERR_OUT_OF_RANGE('offset');
387-
} else if (offset + length > byteLength || offset + length > kMaxLength) {
388-
err = new ERR_OUT_OF_RANGE('length');
387+
err = new ERR_OUT_OF_RANGE('offset', `<= ${byteLength}`, offset);
388+
} else {
389+
const max = byteLength > kMaxLength ? kMaxLength : byteLength;
390+
if (length > max - offset) {
391+
err = new ERR_OUT_OF_RANGE('length', `<= ${max - offset}`, length);
392+
}
389393
}
390394

391395
if (err !== undefined) {

‎lib/internal/http2/core.js

+5-3
Original file line numberDiff line numberDiff line change
@@ -1312,8 +1312,10 @@ class ServerHttp2Session extends Http2Session {
13121312
if (origin === 'null')
13131313
throw new ERR_HTTP2_ALTSVC_INVALID_ORIGIN();
13141314
} else if (typeof originOrStream === 'number') {
1315-
if (originOrStream >>> 0 !== originOrStream || originOrStream === 0)
1316-
throw new ERR_OUT_OF_RANGE('originOrStream');
1315+
if (originOrStream >>> 0 !== originOrStream || originOrStream === 0) {
1316+
throw new ERR_OUT_OF_RANGE('originOrStream',
1317+
`> 0 && < ${2 ** 32}`, originOrStream);
1318+
}
13171319
stream = originOrStream;
13181320
} else if (originOrStream !== undefined) {
13191321
// Allow origin to be passed a URL or object with origin property
@@ -1766,7 +1768,7 @@ class Http2Stream extends Duplex {
17661768
if (typeof code !== 'number')
17671769
throw new ERR_INVALID_ARG_TYPE('code', 'number', code);
17681770
if (code < 0 || code > kMaxInt)
1769-
throw new ERR_OUT_OF_RANGE('code');
1771+
throw new ERR_OUT_OF_RANGE('code', `>= 0 && <= ${kMaxInt}`, code);
17701772
if (callback !== undefined && typeof callback !== 'function')
17711773
throw new ERR_INVALID_CALLBACK();
17721774

‎test/parallel/test-buffer-arraybuffer.js

+13-13
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use strict';
22

3-
const common = require('../common');
3+
require('../common');
44
const assert = require('assert');
55

66
const LENGTH = 16;
@@ -58,14 +58,14 @@ assert.throws(function() {
5858
buf[0] = 9;
5959
assert.strictEqual(ab[1], 9);
6060

61-
common.expectsError(() => Buffer.from(ab.buffer, 6), {
61+
assert.throws(() => Buffer.from(ab.buffer, 6), {
6262
code: 'ERR_BUFFER_OUT_OF_BOUNDS',
63-
type: RangeError,
63+
name: 'RangeError [ERR_BUFFER_OUT_OF_BOUNDS]',
6464
message: '"offset" is outside of buffer bounds'
6565
});
66-
common.expectsError(() => Buffer.from(ab.buffer, 3, 6), {
66+
assert.throws(() => Buffer.from(ab.buffer, 3, 6), {
6767
code: 'ERR_BUFFER_OUT_OF_BOUNDS',
68-
type: RangeError,
68+
name: 'RangeError [ERR_BUFFER_OUT_OF_BOUNDS]',
6969
message: '"length" is outside of buffer bounds'
7070
});
7171
}
@@ -86,14 +86,14 @@ assert.throws(function() {
8686
buf[0] = 9;
8787
assert.strictEqual(ab[1], 9);
8888

89-
common.expectsError(() => Buffer(ab.buffer, 6), {
89+
assert.throws(() => Buffer(ab.buffer, 6), {
9090
code: 'ERR_BUFFER_OUT_OF_BOUNDS',
91-
type: RangeError,
91+
name: 'RangeError [ERR_BUFFER_OUT_OF_BOUNDS]',
9292
message: '"offset" is outside of buffer bounds'
9393
});
94-
common.expectsError(() => Buffer(ab.buffer, 3, 6), {
94+
assert.throws(() => Buffer(ab.buffer, 3, 6), {
9595
code: 'ERR_BUFFER_OUT_OF_BOUNDS',
96-
type: RangeError,
96+
name: 'RangeError [ERR_BUFFER_OUT_OF_BOUNDS]',
9797
message: '"length" is outside of buffer bounds'
9898
});
9999
}
@@ -111,11 +111,11 @@ assert.throws(function() {
111111
assert.deepStrictEqual(Buffer.from(ab, [1]), Buffer.from(ab, 1));
112112

113113
// If byteOffset is Infinity, throw.
114-
common.expectsError(() => {
114+
assert.throws(() => {
115115
Buffer.from(ab, Infinity);
116116
}, {
117117
code: 'ERR_BUFFER_OUT_OF_BOUNDS',
118-
type: RangeError,
118+
name: 'RangeError [ERR_BUFFER_OUT_OF_BOUNDS]',
119119
message: '"offset" is outside of buffer bounds'
120120
});
121121
}
@@ -133,11 +133,11 @@ assert.throws(function() {
133133
assert.deepStrictEqual(Buffer.from(ab, 0, [1]), Buffer.from(ab, 0, 1));
134134

135135
// If length is Infinity, throw.
136-
common.expectsError(() => {
136+
assert.throws(() => {
137137
Buffer.from(ab, 0, Infinity);
138138
}, {
139139
code: 'ERR_BUFFER_OUT_OF_BOUNDS',
140-
type: RangeError,
140+
name: 'RangeError [ERR_BUFFER_OUT_OF_BOUNDS]',
141141
message: '"length" is outside of buffer bounds'
142142
});
143143
}

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

+12-4
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,26 @@ const n = fork(fixtures.path('child-process-spawn-node.js'), args);
3131
assert.strictEqual(n.channel, n._channel);
3232
assert.deepStrictEqual(args, ['foo', 'bar']);
3333

34-
n.on('message', function(m) {
34+
n.on('message', (m) => {
3535
console.log('PARENT got message:', m);
3636
assert.ok(m.foo);
3737
});
3838

3939
// https://github.com/joyent/node/issues/2355 - JSON.stringify(undefined)
4040
// returns "undefined" but JSON.parse() cannot parse that...
41-
assert.throws(function() { n.send(undefined); }, TypeError);
42-
assert.throws(function() { n.send(); }, TypeError);
41+
assert.throws(() => n.send(undefined), {
42+
name: 'TypeError [ERR_MISSING_ARGS]',
43+
message: 'The "message" argument must be specified',
44+
code: 'ERR_MISSING_ARGS'
45+
});
46+
assert.throws(() => n.send(), {
47+
name: 'TypeError [ERR_MISSING_ARGS]',
48+
message: 'The "message" argument must be specified',
49+
code: 'ERR_MISSING_ARGS'
50+
});
4351

4452
n.send({ hello: 'world' });
4553

46-
n.on('exit', common.mustCall(function(c) {
54+
n.on('exit', common.mustCall((c) => {
4755
assert.strictEqual(c, 0);
4856
}));

0 commit comments

Comments
 (0)