From 155d9326158f0e7417b3b870edaea4fc4f15a60f Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 7 Feb 2018 02:15:23 +0100 Subject: [PATCH 1/3] assert: fix infinite loop In rare cirumstances it is possible to get a identical error diff. In such a case the advances diffing runs into a infinite loop. This fixes it by properly checking for extra entries. --- lib/internal/errors.js | 2 ++ test/parallel/test-assert.js | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index a41df135407341..daa494a59f88fc 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -171,6 +171,8 @@ function createErrDiff(actual, expected, operator) { } actualLines.pop(); expectedLines.pop(); + if (actualLines.length === 0 || expectedLines.length === 0) + break; a = actualLines[actualLines.length - 1]; b = expectedLines[expectedLines.length - 1]; } diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index e422acbbfbcddd..8b5d9ee36676dc 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -31,6 +31,7 @@ const { EOL } = require('os'); const EventEmitter = require('events'); const { errorCache } = require('internal/errors'); const { writeFileSync, unlinkSync } = require('fs'); +const { inspect } = require('util'); const a = assert; function makeBlock(f) { @@ -899,6 +900,17 @@ common.expectsError( () => assert.deepEqual(Array(12).fill(1), Array(12).fill(2)), { message }); + const obj1 = {}; + const obj2 = { loop: 'forever' }; + obj2[inspect.custom] = () => '{}'; + // No infinite loop and no custom inspect. + assert.throws(() => assert.deepEqual(obj1, obj2), { + message: `${start}\n` + + `${actExp}\n` + + '\n' + + ' {}' + }); + // notDeepEqual tests message = 'Identical input passed to notDeepStrictEqual:\n[\n 1\n]'; assert.throws( From 6d0eb21e9945776f2b7353e1137b5ddfcb082d59 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 7 Feb 2018 02:23:40 +0100 Subject: [PATCH 2/3] assert: show proper differences Right now it is possible to get an AssertionError from input that has the customInspect function set to always return the same value. That way the error message is actually misleading because the output is going to look the same. This fixes it by deactivating the custom inspect function. --- lib/internal/errors.js | 4 ++-- test/parallel/test-assert.js | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index daa494a59f88fc..4aff5ce4c6c5b2 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -151,9 +151,9 @@ function createErrDiff(actual, expected, operator) { var skipped = false; const util = lazyUtil(); const actualLines = util - .inspect(actual, { compact: false }).split('\n'); + .inspect(actual, { compact: false, customInspect: false }).split('\n'); const expectedLines = util - .inspect(expected, { compact: false }).split('\n'); + .inspect(expected, { compact: false, customInspect: false }).split('\n'); const msg = `Input A expected to ${operator} input B:\n` + `${green}+ expected${white} ${red}- actual${white}`; const skippedMsg = ' ... Lines skipped'; diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index 8b5d9ee36676dc..1800b5aa035b54 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -908,7 +908,11 @@ common.expectsError( message: `${start}\n` + `${actExp}\n` + '\n' + - ' {}' + `${minus} {}\n` + + `${plus} {\n` + + `${plus} loop: 'forever',\n` + + `${plus} [Symbol(util.inspect.custom)]: [Function]\n` + + `${plus} }` }); // notDeepEqual tests From da6354661bdaa07a15ab84a4f73e2620ec5abbaf Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 10 Feb 2018 16:17:20 +0100 Subject: [PATCH 3/3] fixup --- lib/internal/errors.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 4aff5ce4c6c5b2..5bebf0f092d9af 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -297,8 +297,10 @@ class AssertionError extends Error { } else if (errorDiff === 1) { // In case the objects are equal but the operator requires unequal, show // the first object and say A equals B - const res = util - .inspect(actual, { compact: false }).split('\n'); + const res = util.inspect( + actual, + { compact: false, customInspect: false } + ).split('\n'); if (res.length > 20) { res[19] = '...';