From 54aad996cd6cab4a24e27f8af21a99e7d56a4bad Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Wed, 24 Oct 2018 15:07:21 -0400 Subject: [PATCH] Core: Ensure inherited getters are not invoked by assert.deepEqual. Previously, given the following: ```js class Foo { constructor(a = 1) { this.a = a; } } Object.defineProperty(Foo.prototype, 'b', { enumerable: true, get() { return new Foo(this.a + 1); } }) QUnit.test( "hello test", function( assert ) { assert.deepEqual(new Foo(), new Foo()); }); ``` The `assert.deepEqual` invocation would never complete (ultimately crashing the tab or node process). The changes here ensure that inherited descriptors (getter / setter _or_ value only) are compared without invoking the getter therefore preventing the issue mentioned above. --- src/equiv.js | 49 +++++++++++++++++++++++++++++++++++++----- test/main/deepEqual.js | 15 +++++++++++++ 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/src/equiv.js b/src/equiv.js index cc8ab6332..b3a9daff4 100644 --- a/src/equiv.js +++ b/src/equiv.js @@ -13,6 +13,31 @@ export default ( function() { return obj.__proto__; }; + function lookupDescriptor( obj, keyName ) { + let current = obj; + do { + const descriptor = Object.getOwnPropertyDescriptor( current, keyName ); + if ( descriptor !== undefined ) { + return descriptor; + } + current = getProto( current ); + } while ( current !== null ); + return null; + } + + function hasSharedDescriptor( a, b, keyName ) { + var aDescriptor = lookupDescriptor( a, keyName ); + var bDescriptor = lookupDescriptor( b, keyName ); + + return ( aDescriptor !== null && bDescriptor !== null ) && + aDescriptor.value === bDescriptor.value && + aDescriptor.get === bDescriptor.get && + aDescriptor.set === bDescriptor.set && + aDescriptor.writable === bDescriptor.writable && + aDescriptor.configurable === bDescriptor.configurable && + aDescriptor.enumerable === bDescriptor.enumerable; + } + function useStrictEquality( a, b ) { // This only gets called if a and b are not strict equal, and is used to compare on @@ -265,16 +290,30 @@ export default ( function() { aProperties.push( i ); // Skip OOP methods that look the same + var hasValidConstructor = a.constructor !== Object && + typeof a.constructor !== "undefined"; + if ( - a.constructor !== Object && - typeof a.constructor !== "undefined" && - typeof a[ i ] === "function" && - typeof b[ i ] === "function" && - a[ i ].toString() === b[ i ].toString() + hasValidConstructor && + ( + ( + + // Skip own functions with same definition + hasOwnProperty.call( a, i ) && + typeof a[ i ] === "function" && + typeof b[ i ] === "function" && + a[ i ].toString() === b[ i ].toString() + ) || ( + + // Skip shared inherited functions + hasSharedDescriptor( a, b, i ) + ) + ) ) { continue; } + // Compare non-containers; queue non-reference-equal containers if ( !breadthFirstCompareChild( a[ i ], b[ i ] ) ) { return false; diff --git a/test/main/deepEqual.js b/test/main/deepEqual.js index 8d1d2efdb..65d5a8b10 100644 --- a/test/main/deepEqual.js +++ b/test/main/deepEqual.js @@ -1664,6 +1664,21 @@ QUnit.test( "Compare structures with multiple references to the same containers" assert.equal( QUnit.equiv( { big: x, z: [ true ] }, { big: y, z: [ false ] } ), false, "Nonequivalent structures" ); } ); +QUnit.test( "Compare instances with getters", function( assert ) { + function Foo( a ) { + this.a = a === undefined ? 1 : a; + } + + Object.defineProperty( Foo.prototype, "b", { + enumerable: true, + get() { + return new Foo( this.a + 1 ); + } + } ); + + assert.deepEqual( new Foo(), new Foo(), "inherited getters are not included in computation" ); +} ); + QUnit.test( "Test that must be done at the end because they extend some primitive's prototype", function( assert ) {