From 47c9de9842634089f8d1fe6befc4a1e172e527e7 Mon Sep 17 00:00:00 2001 From: rmdm Date: Tue, 30 May 2017 23:44:34 +0300 Subject: [PATCH] assert: fix deepEqual RangeError: Maximum call stack size exceeded Fixes: https://github.com/nodejs/node/issues/13314 Refs: https://github.com/nodejs/node/issues/6416 This commit changes semantics of the memos cycles tracker. Before the change it was used to track all currently wisited nodes of an object tree, which is a bit shifted from its original intention of tracking cycles. The change brings intended semantics, by tracking only objects of the current branch of the object tree. PR-URL: https://github.com/nodejs/node/pull/13318 Fixes: https://github.com/nodejs/node/issues/13314 Ref: https://github.com/nodejs/node/issues/6416 Reviewed-By: James M Snell Reviewed-By: Joyee Cheung --- lib/assert.js | 29 ++++++++++++++++------------- test/parallel/test-assert-deep.js | 10 ++++++++++ test/parallel/test-assert.js | 12 ++++++++++++ 3 files changed, 38 insertions(+), 13 deletions(-) diff --git a/lib/assert.js b/lib/assert.js index 0f0f3f165aeeb2..5aca51119484b6 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -262,24 +262,27 @@ function _deepEqual(actual, expected, strict, memos) { // Use memos to handle cycles. if (!memos) { memos = { - actual: { map: new Map(), position: 0 }, - expected: { map: new Map(), position: 0 } + actual: new Map(), + expected: new Map(), + position: 0 }; - } - - const actualPosition = memos.actual.map.get(actual); - if (actualPosition !== undefined) { - if (actualPosition === memos.expected.map.get(expected)) { - return true; - } } else { - memos.actual.map.set(actual, memos.actual.position++); + memos.position++; } - if (!memos.expected.map.has(expected)) { - memos.expected.map.set(expected, memos.expected.position++); + + if (memos.actual.has(actual)) { + return memos.actual.get(actual) === memos.expected.get(expected); } - return objEquiv(actual, expected, strict, memos); + memos.actual.set(actual, memos.position); + memos.expected.set(expected, memos.position); + + const areEq = objEquiv(actual, expected, strict, memos); + + memos.actual.delete(actual); + memos.expected.delete(expected); + + return areEq; } function setHasSimilarElement(set, val1, strict, memo) { diff --git a/test/parallel/test-assert-deep.js b/test/parallel/test-assert-deep.js index e54377bdbbfeb7..f6fed8411b5213 100644 --- a/test/parallel/test-assert-deep.js +++ b/test/parallel/test-assert-deep.js @@ -158,6 +158,16 @@ assertNotDeepOrStrict(new Set([1, 2, 3, 4]), new Set([1, 2, 3])); assertDeepAndStrictEqual(new Set(['1', '2', '3']), new Set(['1', '2', '3'])); assertDeepAndStrictEqual(new Set([[1, 2], [3, 4]]), new Set([[3, 4], [1, 2]])); +const a = [ 1, 2 ]; +const b = [ 3, 4 ]; +const c = [ 1, 2 ]; +const d = [ 3, 4 ]; + +assertDeepAndStrictEqual( + { a: a, b: b, s: new Set([a, b]) }, + { a: c, b: d, s: new Set([d, c]) } +); + assertDeepAndStrictEqual(new Map([[1, 1], [2, 2]]), new Map([[1, 1], [2, 2]])); assertDeepAndStrictEqual(new Map([[1, 1], [2, 2]]), new Map([[2, 2], [1, 1]])); assertNotDeepOrStrict(new Map([[1, 1], [2, 2]]), new Map([[1, 2], [2, 1]])); diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index 4489773a40ea1b..7b492374f0518b 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -535,6 +535,18 @@ a.throws(makeBlock(thrower, TypeError), function(err) { a.throws(makeBlock(a.deepEqual, d, e), /AssertionError/); a.throws(makeBlock(a.deepStrictEqual, d, e), /AssertionError/); + + // https://github.com/nodejs/node/issues/13314 + const f = {}; + f.ref = f; + + const g = {}; + g.ref = g; + + const h = {ref: g}; + + a.throws(makeBlock(a.deepEqual, f, h), /AssertionError/); + a.throws(makeBlock(a.deepStrictEqual, f, h), /AssertionError/); } // GH-7178. Ensure reflexivity of deepEqual with `arguments` objects. const args = (function() { return arguments; })();