Skip to content

Commit

Permalink
assert: fix deepEqual RangeError: Maximum call stack size exceeded
Browse files Browse the repository at this point in the history
Fixes: #13314
Refs: #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: #13318
Fixes: #13314
Ref: #6416
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
rmdm authored and jasnell committed Jun 2, 2017
1 parent 592d7d2 commit b1ed55f
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 13 deletions.
29 changes: 16 additions & 13 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
10 changes: 10 additions & 0 deletions test/parallel/test-assert-deep.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]]));
Expand Down
12 changes: 12 additions & 0 deletions test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -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; })();
Expand Down

0 comments on commit b1ed55f

Please sign in to comment.