fix: handle circular references correctly in objects (closes #8663)

jestjs · Jul 15, 2019 · 2c6eb48 · 2c6eb48
1 parent b76f01b
commit 2c6eb48
Show file tree

Hide file tree

Showing 5 changed files with 286 additions and 31 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,6 +24,7 @@
 - `[jest-core]` Fix incorrect `passWithNoTests` warning ([#8595](https://github.com/facebook/jest/pull/8595))
 - `[jest-snapshots]` Fix test retries that contain snapshots ([#8629](https://github.com/facebook/jest/pull/8629))
 - `[jest-mock]` Fix incorrect assignments when restoring mocks in instances where they originally didn't exist ([#8631](https://github.com/facebook/jest/pull/8631))
+- `[expect]` Fix stack overflow when matching objects with circular references ([#8687](https://github.com/facebook/jest/pull/8687))
 
 ### Chore & Maintenance
 

diff --git a/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap b/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap
@@ -4092,6 +4092,104 @@ Expected: not <green>Set {2, 1}</>
 Received:     <red>Set {1, 2}</>"
 `;
 
+exports[`toMatchObject() circular references simple circular references {pass: false} expect({"a": "hello", "ref": [Circular]}).toMatchObject({"a": "world", "ref": [Circular]}) 1`] = `"Maximum call stack size exceeded"`;
+
+exports[`toMatchObject() circular references simple circular references {pass: false} expect({"ref": "not a ref"}).toMatchObject({"a": "hello", "ref": [Circular]}) 1`] = `
+"<dim>expect(</><red>received</><dim>).</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+<green>- Expected</>
+<red>+ Received</>
+
+<dim>  Object {</>
+<green>-   \\"a\\": \\"hello\\",</>
+<green>-   \\"ref\\": [Circular],</>
+<red>+   \\"ref\\": \\"not a ref\\",</>
+<dim>  }</>"
+`;
+
+exports[`toMatchObject() circular references simple circular references {pass: false} expect({}).toMatchObject({"a": "hello", "ref": [Circular]}) 1`] = `
+"<dim>expect(</><red>received</><dim>).</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+<green>- Expected</>
+<red>+ Received</>
+
+<green>- Object {</>
+<green>-   \\"a\\": \\"hello\\",</>
+<green>-   \\"ref\\": [Circular],</>
+<green>- }</>
+<red>+ Object {}</>"
+`;
+
+exports[`toMatchObject() circular references simple circular references {pass: true} expect({"a": "hello", "ref": [Circular]}).toMatchObject({"a": "hello", "ref": [Circular]}) 1`] = `
+"<dim>expect(</><red>received</><dim>).</>not<dim>.</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+Expected: not <green>{\\"a\\": \\"hello\\", \\"ref\\": [Circular]}</>"
+`;
+
+exports[`toMatchObject() circular references simple circular references {pass: true} expect({"a": "hello", "ref": [Circular]}).toMatchObject({}) 1`] = `
+"<dim>expect(</><red>received</><dim>).</>not<dim>.</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+Expected: not <green>{}</>
+Received:     <red>{\\"a\\": \\"hello\\", \\"ref\\": [Circular]}</>"
+`;
+
+exports[`toMatchObject() circular references transitive circular references {pass: false} expect({"a": "world", "nestedObj": {"parentObj": [Circular]}}).toMatchObject({"a": "hello", "nestedObj": {"parentObj": [Circular]}}) 1`] = `"Maximum call stack size exceeded"`;
+
+exports[`toMatchObject() circular references transitive circular references {pass: false} expect({"nestedObj": {"parentObj": "not the parent ref"}}).toMatchObject({"a": "hello", "nestedObj": {"parentObj": [Circular]}}) 1`] = `
+"<dim>expect(</><red>received</><dim>).</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+<green>- Expected</>
+<red>+ Received</>
+
+<dim>  Object {</>
+<green>-   \\"a\\": \\"hello\\",</>
+<dim>    \\"nestedObj\\": Object {</>
+<green>-     \\"parentObj\\": [Circular],</>
+<red>+     \\"parentObj\\": \\"not the parent ref\\",</>
+<dim>    },</>
+<dim>  }</>"
+`;
+
+exports[`toMatchObject() circular references transitive circular references {pass: false} expect({}).toMatchObject({"a": "hello", "nestedObj": {"parentObj": [Circular]}}) 1`] = `
+"<dim>expect(</><red>received</><dim>).</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+<green>- Expected</>
+<red>+ Received</>
+
+<green>- Object {</>
+<green>-   \\"a\\": \\"hello\\",</>
+<green>-   \\"nestedObj\\": Object {</>
+<green>-     \\"parentObj\\": [Circular],</>
+<green>-   },</>
+<green>- }</>
+<red>+ Object {}</>"
+`;
+
+exports[`toMatchObject() circular references transitive circular references {pass: true} expect({"a": "hello", "nestedObj": {"parentObj": [Circular]}}).toMatchObject({"a": "hello", "nestedObj": {"parentObj": [Circular]}}) 1`] = `
+"<dim>expect(</><red>received</><dim>).</>not<dim>.</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+Expected: not <green>{\\"a\\": \\"hello\\", \\"nestedObj\\": {\\"parentObj\\": [Circular]}}</>"
+`;
+
+exports[`toMatchObject() circular references transitive circular references {pass: true} expect({"a": "hello", "nestedObj": {"parentObj": [Circular]}}).toMatchObject({}) 1`] = `
+"<dim>expect(</><red>received</><dim>).</>not<dim>.</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+Expected: not <green>{}</>
+Received:     <red>{\\"a\\": \\"hello\\", \\"nestedObj\\": {\\"parentObj\\": [Circular]}}</>"
+`;
+
+exports[`toMatchObject() does not match properties up in the prototype chain 1`] = `
+"<dim>expect(</><red>received</><dim>).</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+<green>- Expected</>
+<red>+ Received</>
+
+<dim>  Object {</>
+<dim>    \\"other\\": \\"child\\",</>
+<green>-   \\"ref\\": [Circular],</>
+<dim>  }</>"
+`;
+
 exports[`toMatchObject() throws expect("44").toMatchObject({}) 1`] = `
 "<dim>expect(</><red>received</><dim>).</>toMatchObject<dim>(</><green>expected</><dim>)</>
 

diff --git a/packages/expect/src/__tests__/matchers.test.js b/packages/expect/src/__tests__/matchers.test.js
@@ -1537,7 +1537,91 @@ describe('toMatchObject()', () => {
     }
   }
 
-  [
+  const testNotToMatchSnapshots = tuples => {
+    tuples.forEach(([n1, n2]) => {
+      it(`{pass: true} expect(${stringify(n1)}).toMatchObject(${stringify(
+        n2,
+      )})`, () => {
+        jestExpect(n1).toMatchObject(n2);
+        expect(() =>
+          jestExpect(n1).not.toMatchObject(n2),
+        ).toThrowErrorMatchingSnapshot();
+      });
+    });
+  };
+
+  const testToMatchSnapshots = tuples => {
+    tuples.forEach(([n1, n2]) => {
+      it(`{pass: false} expect(${stringify(n1)}).toMatchObject(${stringify(
+        n2,
+      )})`, () => {
+        jestExpect(n1).not.toMatchObject(n2);
+        expect(() =>
+          jestExpect(n1).toMatchObject(n2),
+        ).toThrowErrorMatchingSnapshot();
+      });
+    });
+  };
+
+  describe('circular references', () => {
+    describe('simple circular references', () => {
+      const circularObjA1 = {a: 'hello'};
+      circularObjA1.ref = circularObjA1;
+
+      const circularObjB = {a: 'world'};
+      circularObjB.ref = circularObjB;
+
+      const circularObjA2 = {a: 'hello'};
+      circularObjA2.ref = circularObjA2;
+
+      const primitiveInsteadOfRef = {};
+      primitiveInsteadOfRef.ref = 'not a ref';
+
+      testNotToMatchSnapshots([
+        [circularObjA1, {}],
+        [circularObjA2, circularObjA1],
+      ]);
+
+      testToMatchSnapshots([
+        [{}, circularObjA1],
+        [circularObjA1, circularObjB],
+        [primitiveInsteadOfRef, circularObjA1],
+      ]);
+    });
+
+    describe('transitive circular references', () => {
+      const transitiveCircularObjA1 = {a: 'hello'};
+      transitiveCircularObjA1.nestedObj = {parentObj: transitiveCircularObjA1};
+
+      const transitiveCircularObjA2 = {a: 'hello'};
+      transitiveCircularObjA2.nestedObj = {
+        parentObj: transitiveCircularObjA2,
+      };
+
+      const transitiveCircularObjB = {a: 'world'};
+      transitiveCircularObjB.nestedObj = {
+        parentObj: transitiveCircularObjB,
+      };
+
+      const primitiveInsteadOfRef = {};
+      primitiveInsteadOfRef.nestedObj = {
+        parentObj: 'not the parent ref',
+      };
+
+      testNotToMatchSnapshots([
+        [transitiveCircularObjA1, {}],
+        [transitiveCircularObjA2, transitiveCircularObjA1],
+      ]);
+
+      testToMatchSnapshots([
+        [{}, transitiveCircularObjA1],
+        [transitiveCircularObjB, transitiveCircularObjA1],
+        [primitiveInsteadOfRef, transitiveCircularObjA1],
+      ]);
+    });
+  });
+
+  testNotToMatchSnapshots([
     [{a: 'b', c: 'd'}, {a: 'b'}],
     [{a: 'b', c: 'd'}, {a: 'b', c: 'd'}],
     [{a: 'b', t: {x: {r: 'r'}, z: 'z'}}, {a: 'b', t: {z: 'z'}}],
@@ -1560,18 +1644,9 @@ describe('toMatchObject()', () => {
     [new Error('bar'), {message: 'bar'}],
     [new Foo(), {a: undefined, b: 'b'}],
     [Object.assign(Object.create(null), {a: 'b'}), {a: 'b'}],
-  ].forEach(([n1, n2]) => {
-    it(`{pass: true} expect(${stringify(n1)}).toMatchObject(${stringify(
-      n2,
-    )})`, () => {
-      jestExpect(n1).toMatchObject(n2);
-      expect(() =>
-        jestExpect(n1).not.toMatchObject(n2),
-      ).toThrowErrorMatchingSnapshot();
-    });
-  });
+  ]);
 
-  [
+  testToMatchSnapshots([
     [{a: 'b', c: 'd'}, {e: 'b'}],
     [{a: 'b', c: 'd'}, {a: 'b!', c: 'd'}],
     [{a: 'a', c: 'd'}, {a: jestExpect.any(Number)}],
@@ -1597,16 +1672,7 @@ describe('toMatchObject()', () => {
     [[1, 2, 3], [1, 2, 2]],
     [new Error('foo'), new Error('bar')],
     [Object.assign(Object.create(null), {a: 'b'}), {c: 'd'}],
-  ].forEach(([n1, n2]) => {
-    it(`{pass: false} expect(${stringify(n1)}).toMatchObject(${stringify(
-      n2,
-    )})`, () => {
-      jestExpect(n1).not.toMatchObject(n2);
-      expect(() =>
-        jestExpect(n1).toMatchObject(n2),
-      ).toThrowErrorMatchingSnapshot();
-    });
-  });
+  ]);
 
   [
     [null, {}],
@@ -1628,4 +1694,20 @@ describe('toMatchObject()', () => {
       ).toThrowErrorMatchingSnapshot();
     });
   });
+
+  it('does not match properties up in the prototype chain', () => {
+    const a = {};
+    a.ref = a;
+
+    const b = Object.create(a);
+    b.other = 'child';
+
+    const matcher = {other: 'child'};
+    matcher.ref = matcher;
+
+    jestExpect(b).not.toMatchObject(matcher);
+    expect(() =>
+      jestExpect(b).toMatchObject(matcher),
+    ).toThrowErrorMatchingSnapshot();
+  });
 });
diff --git a/packages/expect/src/__tests__/utils.test.js b/packages/expect/src/__tests__/utils.test.js
@@ -202,6 +202,60 @@ describe('subsetEquality()', () => {
   test('undefined does not return errors', () => {
     expect(subsetEquality(undefined, {foo: 'bar'})).not.toBeTruthy();
   });
+
+  describe('matching subsets with circular references', () => {
+    test('simple circular references', () => {
+      const circularObjA1 = {a: 'hello'};
+      circularObjA1.ref = circularObjA1;
+
+      const circularObjA2 = {a: 'hello'};
+      circularObjA2.ref = circularObjA2;
+
+      const circularObjB = {a: 'world'};
+      circularObjB.ref = circularObjB;
+
+      const primitiveInsteadOfRef = {};
+      primitiveInsteadOfRef.ref = 'not a ref';
+
+      expect(subsetEquality(circularObjA1, {})).toBe(true);
+      expect(subsetEquality({}, circularObjA1)).toBe(false);
+      expect(subsetEquality(circularObjA2, circularObjA1)).toBe(true);
+      expect(subsetEquality(circularObjB, circularObjA1)).toBe(false);
+      expect(subsetEquality(primitiveInsteadOfRef, circularObjA1)).toBe(false);
+    });
+
+    test('transitive circular references', () => {
+      const transitiveCircularObjA1 = {a: 'hello'};
+      transitiveCircularObjA1.nestedObj = {parentObj: transitiveCircularObjA1};
+
+      const transitiveCircularObjA2 = {a: 'hello'};
+      transitiveCircularObjA2.nestedObj = {
+        parentObj: transitiveCircularObjA2,
+      };
+
+      const transitiveCircularObjB = {a: 'world'};
+      transitiveCircularObjB.nestedObj = {
+        parentObj: transitiveCircularObjB,
+      };
+
+      const primitiveInsteadOfRef = {};
+      primitiveInsteadOfRef.nestedObj = {
+        parentObj: 'not the parent ref',
+      };
+
+      expect(subsetEquality(transitiveCircularObjA1, {})).toBe(true);
+      expect(subsetEquality({}, transitiveCircularObjA1)).toBe(false);
+      expect(
+        subsetEquality(transitiveCircularObjA2, transitiveCircularObjA1),
+      ).toBe(true);
+      expect(
+        subsetEquality(transitiveCircularObjB, transitiveCircularObjA1),
+      ).toBe(false);
+      expect(
+        subsetEquality(primitiveInsteadOfRef, transitiveCircularObjA1),
+      ).toBe(false);
+    });
+  });
 });
 
 describe('iterableEquality', () => {

diff --git a/packages/expect/src/utils.ts b/packages/expect/src/utils.ts
@@ -268,16 +268,36 @@ export const subsetEquality = (
   object: any,
   subset: any,
 ): undefined | boolean => {
-  if (!isObjectWithKeys(subset)) {
-    return undefined;
-  }
+  // subsetEquality needs to keep track of the references
+  // it has already visited to avoid infinite loops in case
+  // there are circular references in the subset passed to it.
+  const subsetEqualityWithContext = (
+    seenReferences: WeakMap<object, boolean> = new WeakMap(),
+  ) => (object: any, subset: any): undefined | boolean => {
+    if (!isObjectWithKeys(subset)) {
+      return undefined;
+    }
 
-  return Object.keys(subset).every(
-    key =>
-      object != null &&
-      hasOwnProperty(object, key) &&
-      equals(object[key], subset[key], [iterableEquality, subsetEquality]),
-  );
+    return Object.keys(subset).every(key => {
+      if (isObjectWithKeys(subset[key])) {
+        if (seenReferences.get(subset[key])) {
+          return equals(object[key], subset[key], [iterableEquality]);
+        }
+        seenReferences.set(subset[key], true);
+      }
+
+      return (
+        object != null &&
+        hasOwnProperty(object, key) &&
+        equals(object[key], subset[key], [
+          iterableEquality,
+          subsetEqualityWithContext(seenReferences),
+        ])
+      );
+    });
+  };
+
+  return subsetEqualityWithContext()(object, subset);
 };
 
 export const typeEquality = (a: any, b: any) => {