From b37a1173543b58c9524c3f23183d2a2dff196bce Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Wed, 20 Mar 2019 12:01:11 +0000 Subject: [PATCH] [expect] Fix circular references in iterable equality (#8160) * Fix circular references in iterable equality * Add changelog entry * Add circular shape checking * Fix changelog conflict * Add comment to remove from stack --- CHANGELOG.md | 1 + packages/expect/src/__tests__/utils.test.js | 92 +++++++++++++++++++++ packages/expect/src/utils.ts | 60 +++++++++++--- 3 files changed, 140 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 43ccd59c4691..512742a01103 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ ### Fixes +- `[expect]` Fix circular references in iterable equality ([#8160](https://github.com/facebook/jest/pull/8160)) - `[jest-changed-files]` Change method of obtaining git root ([#8052](https://github.com/facebook/jest/pull/8052)) - `[jest-each]` Fix test function type ([#8145](https://github.com/facebook/jest/pull/8145)) - `[jest-fake-timers]` `getTimerCount` not taking immediates and ticks into account ([#8139](https://github.com/facebook/jest/pull/8139)) diff --git a/packages/expect/src/__tests__/utils.test.js b/packages/expect/src/__tests__/utils.test.js index 180b514a1a77..7ff609a8ab87 100644 --- a/packages/expect/src/__tests__/utils.test.js +++ b/packages/expect/src/__tests__/utils.test.js @@ -15,6 +15,7 @@ const { getPath, hasOwnProperty, subsetEquality, + iterableEquality, } = require('../utils'); describe('getPath()', () => { @@ -202,3 +203,94 @@ describe('subsetEquality()', () => { expect(subsetEquality(undefined, {foo: 'bar'})).not.toBeTruthy(); }); }); + +describe('iterableEquality', () => { + test('returns true when given circular iterators', () => { + class Iter { + *[Symbol.iterator]() { + yield this; + } + } + + const a = new Iter(); + const b = new Iter(); + + expect(iterableEquality(a, b)).toBe(true); + }); + + test('returns true when given circular Set', () => { + const a = new Set(); + a.add(a); + const b = new Set(); + b.add(b); + expect(iterableEquality(a, b)).toBe(true); + }); + + test('returns true when given nested Sets', () => { + expect( + iterableEquality( + new Set([new Set([[1]]), new Set([[2]])]), + new Set([new Set([[2]]), new Set([[1]])]), + ), + ).toBe(true); + expect( + iterableEquality( + new Set([new Set([[1]]), new Set([[2]])]), + new Set([new Set([[3]]), new Set([[1]])]), + ), + ).toBe(false); + }); + + test('returns true when given circular Set shape', () => { + const a1 = new Set(); + const a2 = new Set(); + a1.add(a2); + a2.add(a1); + const b = new Set(); + b.add(b); + + expect(iterableEquality(a1, b)).toBe(true); + }); + + test('returns true when given circular key in Map', () => { + const a = new Map(); + a.set(a, 'a'); + const b = new Map(); + b.set(b, 'a'); + + expect(iterableEquality(a, b)).toBe(true); + }); + + test('returns true when given nested Maps', () => { + expect( + iterableEquality( + new Map([['hello', new Map([['world', 'foobar']])]]), + new Map([['hello', new Map([['world', 'qux']])]]), + ), + ).toBe(false); + expect( + iterableEquality( + new Map([['hello', new Map([['world', 'foobar']])]]), + new Map([['hello', new Map([['world', 'foobar']])]]), + ), + ).toBe(true); + }); + + test('returns true when given circular key and value in Map', () => { + const a = new Map(); + a.set(a, a); + const b = new Map(); + b.set(b, b); + + expect(iterableEquality(a, b)).toBe(true); + }); + + test('returns true when given circular value in Map', () => { + const a = new Map(); + a.set('a', a); + const b = new Map(); + b.set('a', b); + + expect(iterableEquality(a, b)).toBe(true); + }); +}); diff --git a/packages/expect/src/utils.ts b/packages/expect/src/utils.ts index 13047d555ff1..2f3f97ffe2ea 100644 --- a/packages/expect/src/utils.ts +++ b/packages/expect/src/utils.ts @@ -137,7 +137,13 @@ const IteratorSymbol = Symbol.iterator; const hasIterator = (object: any) => !!(object != null && object[IteratorSymbol]); -export const iterableEquality = (a: any, b: any) => { + +export const iterableEquality = ( + a: any, + b: any, + aStack: Array = [], + bStack: Array = [], +) => { if ( typeof a !== 'object' || typeof b !== 'object' || @@ -152,6 +158,22 @@ export const iterableEquality = (a: any, b: any) => { return false; } + let length = aStack.length; + while (length--) { + // Linear search. Performance is inversely proportional to the number of + // unique nested structures. + // circular references at same depth are equal + // circular reference is not equal to non-circular one + if (aStack[length] === a) { + return bStack[length] === b; + } + } + aStack.push(a); + bStack.push(b); + + const iterableEqualityWithStack = (a: any, b: any) => + iterableEquality(a, b, aStack, bStack); + if (a.size !== undefined) { if (a.size !== b.size) { return false; @@ -161,7 +183,7 @@ export const iterableEquality = (a: any, b: any) => { if (!b.has(aValue)) { let has = false; for (const bValue of b) { - const isEqual = equals(aValue, bValue, [iterableEquality]); + const isEqual = equals(aValue, bValue, [iterableEqualityWithStack]); if (isEqual === true) { has = true; } @@ -173,25 +195,29 @@ export const iterableEquality = (a: any, b: any) => { } } } - if (allFound) { - return true; - } + // Remove the first value from the stack of traversed values. + aStack.pop(); + bStack.pop(); + return allFound; } else if (isA('Map', a) || isImmutableUnorderedKeyed(a)) { let allFound = true; for (const aEntry of a) { if ( !b.has(aEntry[0]) || - !equals(aEntry[1], b.get(aEntry[0]), [iterableEquality]) + !equals(aEntry[1], b.get(aEntry[0]), [iterableEqualityWithStack]) ) { let has = false; for (const bEntry of b) { - const matchedKey = equals(aEntry[0], bEntry[0], [iterableEquality]); + const matchedKey = equals(aEntry[0], bEntry[0], [ + iterableEqualityWithStack, + ]); let matchedValue = false; if (matchedKey === true) { - matchedValue = equals(aEntry[1], bEntry[1], [iterableEquality]); + matchedValue = equals(aEntry[1], bEntry[1], [ + iterableEqualityWithStack, + ]); } - if (matchedValue === true) { has = true; } @@ -203,9 +229,10 @@ export const iterableEquality = (a: any, b: any) => { } } } - if (allFound) { - return true; - } + // Remove the first value from the stack of traversed values. + aStack.pop(); + bStack.pop(); + return allFound; } } @@ -213,13 +240,20 @@ export const iterableEquality = (a: any, b: any) => { for (const aValue of a) { const nextB = bIterator.next(); - if (nextB.done || !equals(aValue, nextB.value, [iterableEquality])) { + if ( + nextB.done || + !equals(aValue, nextB.value, [iterableEqualityWithStack]) + ) { return false; } } if (!bIterator.next().done) { return false; } + + // Remove the first value from the stack of traversed values. + aStack.pop(); + bStack.pop(); return true; };