Skip to content

Commit

Permalink
assert: validate input stricter
Browse files Browse the repository at this point in the history
This makes sure invalid `error` objects are not ignored when using
`assert.throws` and `assert.rejects`.

PR-URL: #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>
  • Loading branch information
BridgeAR committed May 10, 2018
1 parent c072057 commit 21c3a40
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 10 deletions.
16 changes: 10 additions & 6 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const {
const { codes: {
ERR_AMBIGUOUS_ARGUMENT,
ERR_INVALID_ARG_TYPE,
ERR_INVALID_ARG_VALUE,
ERR_INVALID_RETURN_VALUE
} } = require('internal/errors');
const { AssertionError, errorCache } = require('internal/assert');
Expand Down Expand Up @@ -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.
if (typeof actual !== 'object' || actual === null) {
const err = new AssertionError({
Expand All @@ -438,6 +433,9 @@ function expectedException(actual, expected, msg) {
// as well.
if (expected instanceof Error) {
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) {
if (typeof actual[key] === 'string' &&
Expand Down Expand Up @@ -527,6 +525,12 @@ function expectsError(stackStartFn, actual, error, message) {
}
message = error;
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) {
Expand Down
27 changes: 23 additions & 4 deletions test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 err = new TypeError('Wrong value');
Expand Down Expand Up @@ -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(
() => a.throws(
// eslint-disable-next-line no-throw-literal
Expand Down Expand Up @@ -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);

0 comments on commit 21c3a40

Please sign in to comment.