From 46acf737f9645fbb9f34c483fb899ed1391f05aa Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 27 Apr 2016 14:40:24 -0700 Subject: [PATCH] assert: allow circular references assert.deepEqual() and assert.deepStrictEqual() will no longer throw a RangeError if passed objects with circular references. PR-URL: https://github.com/nodejs/node/pull/6432 Fixes: https://github.com/nodejs/node/issues/6416 Reviewed-By: James M Snell Reviewed-By: Anna Henningsen --- lib/assert.js | 21 +++++++++++++++++---- test/parallel/test-assert.js | 32 ++++++++++++++++++++------------ 2 files changed, 37 insertions(+), 16 deletions(-) diff --git a/lib/assert.js b/lib/assert.js index 7fc9e0c342646a..8955aa8761d7c2 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -143,7 +143,7 @@ assert.deepStrictEqual = function deepStrictEqual(actual, expected, message) { } }; -function _deepEqual(actual, expected, strict) { +function _deepEqual(actual, expected, strict, memos) { // 7.1. All identical values are equivalent, as determined by ===. if (actual === expected) { return true; @@ -191,7 +191,19 @@ function _deepEqual(actual, expected, strict) { // corresponding key, and an identical 'prototype' property. Note: this // accounts for both named and indexed properties on Arrays. } else { - return objEquiv(actual, expected, strict); + memos = memos || {actual: [], expected: []}; + + const actualIndex = memos.actual.indexOf(actual); + if (actualIndex !== -1) { + if (actualIndex === memos.expected.indexOf(expected)) { + return true; + } + } + + memos.actual.push(actual); + memos.expected.push(expected); + + return objEquiv(actual, expected, strict, memos); } } @@ -199,7 +211,7 @@ function isArguments(object) { return Object.prototype.toString.call(object) == '[object Arguments]'; } -function objEquiv(a, b, strict) { +function objEquiv(a, b, strict, actualVisitedObjects) { if (a === null || a === undefined || b === null || b === undefined) return false; // if one is a primitive, the other must be same @@ -235,7 +247,8 @@ function objEquiv(a, b, strict) { //~~~possibly expensive deep test for (i = ka.length - 1; i >= 0; i--) { key = ka[i]; - if (!_deepEqual(a[key], b[key], strict)) return false; + if (!_deepEqual(a[key], b[key], strict, actualVisitedObjects)) + return false; } return true; } diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index e955c57f8ecbee..d06f7b678e714b 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -381,25 +381,33 @@ try { assert.ok(threw); -// GH-207. Make sure deepEqual doesn't loop forever on circular refs -var b = {}; -b.b = b; +// https://github.com/nodejs/node/issues/6416 +// Make sure circular refs don't throw. +{ + const b = {}; + b.b = b; -var c = {}; -c.b = c; + const c = {}; + c.b = c; -var gotError = false; -try { - assert.deepEqual(b, c); -} catch (e) { - gotError = true; -} + a.doesNotThrow(makeBlock(a.deepEqual, b, c)); + a.doesNotThrow(makeBlock(a.deepStrictEqual, b, c)); + const d = {}; + d.a = 1; + d.b = d; + + const e = {}; + e.a = 1; + e.b = e.a; + + a.throws(makeBlock(a.deepEqual, d, e), /AssertionError/); + a.throws(makeBlock(a.deepStrictEqual, d, e), /AssertionError/); +} // GH-7178. Ensure reflexivity of deepEqual with `arguments` objects. var args = (function() { return arguments; })(); a.throws(makeBlock(a.deepEqual, [], args)); a.throws(makeBlock(a.deepEqual, args, [])); -assert.ok(gotError); var circular = {y: 1};