1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

assert: validate input stricter

Browse Source 
This makes sure invalid `error` objects are not ignored when using
`assert.throws` and `assert.rejects`.

PR-URL: https://github.com/nodejs/node/pull/20481
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
This commit is contained in:
Ruben Bridgewater 2018-05-03 00:23:47 +02:00
parent c072057049
commit 21c3a402d4
No known key found for this signature in database
GPG Key ID: F07496B3EB3C1762
2 changed files with 33 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
lib/assert.js
View File

@ -28,6 +28,7 @@ const {
const { codes: { const { codes: {
ERR_AMBIGUOUS_ARGUMENT, ERR_AMBIGUOUS_ARGUMENT,
ERR_INVALID_ARG_TYPE, ERR_INVALID_ARG_TYPE,
ERR_INVALID_ARG_VALUE,
ERR_INVALID_RETURN_VALUE ERR_INVALID_RETURN_VALUE
} } = require('internal/errors'); } } = require('internal/errors');
const { AssertionError, errorCache } = require('internal/assert'); const { AssertionError, errorCache } = require('internal/assert');
@ -414,12 +415,6 @@ function expectedException(actual, expected, msg) {
); );
} }
// TODO: Disallow primitives as error argument.
// This is here to prevent a breaking change.
if (typeof expected !== 'object') {
return true;
}
// Handle primitives properly. // Handle primitives properly.
if (typeof actual !== 'object' || actual === null) { if (typeof actual !== 'object' || actual === null) {
const err = new AssertionError({ const err = new AssertionError({
@ -438,6 +433,9 @@ function expectedException(actual, expected, msg) {
// as well. // as well.
if (expected instanceof Error) { if (expected instanceof Error) {
keys.push('name', 'message'); keys.push('name', 'message');
} else if (keys.length === 0) {
throw new ERR_INVALID_ARG_VALUE('error',
expected, 'may not be an empty object');
} }
for (const key of keys) { for (const key of keys) {
if (typeof actual[key] === 'string' && if (typeof actual[key] === 'string' &&
@ -527,6 +525,12 @@ function expectsError(stackStartFn, actual, error, message) {
} }
message = error; message = error;
error = undefined; error = undefined;
} else if (error != null &&
typeof error !== 'object' &&
typeof error !== 'function') {
throw new ERR_INVALID_ARG_TYPE('error',
['Object', 'Error', 'Function', 'RegExp'],
error);
} }
if (actual === NO_EXCEPTION_SENTINEL) { if (actual === NO_EXCEPTION_SENTINEL) {

27
test/parallel/test-assert.js
View File

@ -772,6 +772,21 @@ common.expectsError(
} }
); );
[
1,
false,
Symbol()
].forEach((input) => {
assert.throws(
() => assert.throws(() => {}, input),
{
code: 'ERR_INVALID_ARG_TYPE',
message: 'The "error" argument must be one of type Object, Error, ' +
`Function, or RegExp. Received type ${typeof input}`
}
);
});
{ {
const errFn = () => { const errFn = () => {
const err = new TypeError('Wrong value'); const err = new TypeError('Wrong value');
@ -871,6 +886,14 @@ common.expectsError(
); );
} }
assert.throws(
() => assert.throws(() => { throw new Error(); }, {}),
{
message: "The argument 'error' may not be an empty object. Received {}",
code: 'ERR_INVALID_ARG_VALUE'
}
);
assert.throws( assert.throws(
() => a.throws( () => a.throws(
// eslint-disable-next-line no-throw-literal // eslint-disable-next-line no-throw-literal
@ -981,7 +1004,3 @@ assert.throws(
} }
); );
} }
// TODO: This case is only there to make sure there is no breaking change.
// eslint-disable-next-line no-restricted-syntax, no-throw-literal
assert.throws(() => { throw 4; }, 4);