From 21c3a402d486234fb00fcb130802b8fcabd97de8 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Thu, 3 May 2018 00:23:47 +0200
Subject: [PATCH] assert: validate input stricter
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 lib/assert.js                | 16 ++++++++++------
 test/parallel/test-assert.js | 27 +++++++++++++++++++++++----
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/lib/assert.js b/lib/assert.js
index 8bcdc8cdacb3ee..45ddbe5f817d36 100644
--- a/lib/assert.js
+++ b/lib/assert.js
@@ -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');
@@ -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({
@@ -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' &&
@@ -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) {
diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js
index dba40c0a0e3c42..d3e37afa5101cb 100644
--- a/test/parallel/test-assert.js
+++ b/test/parallel/test-assert.js
@@ -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');
@@ -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
@@ -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);