Skip to content

Commit 8753288

Browse files
committed
assert,util: fix constructor lookup in deep equal comparison
The latest performance optimization did not take into account that an object may have a property called constructor. This is addressed in this PR by adding a new fast path and using fallbacks.
1 parent f89baf2 commit 8753288

File tree

2 files changed

+132
-41
lines changed

2 files changed

+132
-41
lines changed

lib/internal/util/comparisons.js

Lines changed: 79 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,32 @@
11
'use strict';
22

33
const {
4+
Array,
5+
ArrayBuffer,
46
ArrayIsArray,
57
ArrayPrototypeFilter,
68
ArrayPrototypePush,
9+
BigInt,
10+
BigInt64Array,
711
BigIntPrototypeValueOf,
12+
BigUint64Array,
13+
Boolean,
814
BooleanPrototypeValueOf,
15+
DataView,
16+
Date,
917
DatePrototypeGetTime,
1018
Error,
19+
Float16Array,
20+
Float32Array,
21+
Float64Array,
22+
Function,
23+
Int16Array,
24+
Int32Array,
25+
Int8Array,
26+
Map,
27+
Number,
1128
NumberPrototypeValueOf,
29+
Object,
1230
ObjectGetOwnPropertyDescriptor,
1331
ObjectGetOwnPropertySymbols: getOwnSymbols,
1432
ObjectGetPrototypeOf,
@@ -17,18 +35,65 @@ const {
1735
ObjectPrototypeHasOwnProperty: hasOwn,
1836
ObjectPrototypePropertyIsEnumerable: hasEnumerable,
1937
ObjectPrototypeToString,
38+
Promise,
39+
RegExp,
2040
SafeSet,
41+
Set,
42+
SharedArrayBuffer,
43+
String,
2144
StringPrototypeValueOf,
45+
Symbol,
2246
SymbolPrototypeValueOf,
2347
TypedArrayPrototypeGetByteLength: getByteLength,
2448
TypedArrayPrototypeGetSymbolToStringTag,
49+
Uint16Array,
50+
Uint32Array,
2551
Uint8Array,
52+
Uint8ClampedArray,
53+
WeakMap,
54+
WeakSet,
2655
} = primordials;
2756

2857
const { compare } = internalBinding('buffer');
2958
const assert = require('internal/assert');
3059
const { isURL } = require('internal/url');
3160
const { isError } = require('internal/util');
61+
const { Buffer } = require('buffer');
62+
63+
const wellKnownConstructors = new SafeSet();
64+
wellKnownConstructors.add(Array);
65+
wellKnownConstructors.add(ArrayBuffer);
66+
wellKnownConstructors.add(BigInt);
67+
wellKnownConstructors.add(BigInt64Array);
68+
wellKnownConstructors.add(BigUint64Array);
69+
wellKnownConstructors.add(Boolean);
70+
wellKnownConstructors.add(Buffer);
71+
wellKnownConstructors.add(DataView);
72+
wellKnownConstructors.add(Date);
73+
wellKnownConstructors.add(Error);
74+
wellKnownConstructors.add(Float16Array);
75+
wellKnownConstructors.add(Float32Array);
76+
wellKnownConstructors.add(Float64Array);
77+
wellKnownConstructors.add(Function);
78+
wellKnownConstructors.add(Int16Array);
79+
wellKnownConstructors.add(Int32Array);
80+
wellKnownConstructors.add(Int8Array);
81+
wellKnownConstructors.add(Map);
82+
wellKnownConstructors.add(Number);
83+
wellKnownConstructors.add(Object);
84+
wellKnownConstructors.add(Promise);
85+
wellKnownConstructors.add(RegExp);
86+
wellKnownConstructors.add(Set);
87+
wellKnownConstructors.add(SharedArrayBuffer);
88+
wellKnownConstructors.add(String);
89+
wellKnownConstructors.add(Symbol);
90+
wellKnownConstructors.add(Uint16Array);
91+
wellKnownConstructors.add(Uint32Array);
92+
wellKnownConstructors.add(Uint8Array);
93+
wellKnownConstructors.add(Uint8ClampedArray);
94+
wellKnownConstructors.add(WeakMap);
95+
wellKnownConstructors.add(WeakSet);
96+
3297
const types = require('internal/util/types');
3398
const {
3499
isAnyArrayBuffer,
@@ -198,11 +263,20 @@ function innerDeepEqual(val1, val2, mode, memos) {
198263
}
199264

200265
function objectComparisonStart(val1, val2, mode, memos) {
201-
if (mode === kStrict &&
202-
(val1.constructor !== val2.constructor ||
203-
(val1.constructor === undefined &&
204-
ObjectGetPrototypeOf(val1) !== ObjectGetPrototypeOf(val2)))) {
205-
return false;
266+
if (mode === kStrict) {
267+
if (wellKnownConstructors.has(val1.constructor)) {
268+
if (val1.constructor !== val2.constructor) {
269+
return false;
270+
}
271+
} else if (hasOwn(val1, 'constructor')) {
272+
if (ObjectGetPrototypeOf(val1) !== ObjectGetPrototypeOf(val2)) {
273+
return false;
274+
}
275+
} else if ((val1.constructor !== val2.constructor ||
276+
(val1.constructor === undefined &&
277+
ObjectGetPrototypeOf(val1) !== ObjectGetPrototypeOf(val2)))) {
278+
return false;
279+
}
206280
}
207281

208282
const val1Tag = ObjectPrototypeToString(val1);

test/parallel/test-assert-deep.js

Lines changed: 53 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1527,46 +1527,63 @@ test('Detects differences in deeply nested arrays instead of seeing a new object
15271527
);
15281528
});
15291529

1530-
// check URL
1531-
{
1532-
const a = new URL('http://foo');
1533-
const b = new URL('http://bar');
1530+
test('URLs', () => {
1531+
// check URL
1532+
{
1533+
const a = new URL('http://foo');
1534+
const b = new URL('http://bar');
15341535

1535-
assertNotDeepOrStrict(a, b);
1536-
}
1536+
assertNotDeepOrStrict(a, b);
1537+
}
15371538

1538-
{
1539-
const a = new URL('http://foo');
1540-
const b = new URL('http://foo');
1539+
{
1540+
const a = new URL('http://foo');
1541+
const b = new URL('http://foo');
15411542

1542-
assertDeepAndStrictEqual(a, b);
1543-
}
1543+
assertDeepAndStrictEqual(a, b);
1544+
}
1545+
1546+
{
1547+
const a = new URL('http://foo');
1548+
const b = new URL('http://foo');
1549+
a.bar = 1;
1550+
b.bar = 2;
1551+
assertNotDeepOrStrict(a, b);
1552+
}
1553+
1554+
{
1555+
const a = new URL('http://foo');
1556+
const b = new URL('http://foo');
1557+
a.bar = 1;
1558+
b.bar = 1;
1559+
assertDeepAndStrictEqual(a, b);
1560+
}
1561+
1562+
{
1563+
const a = new URL('http://foo');
1564+
const b = new URL('http://bar');
1565+
assert.throws(
1566+
() => assert.deepStrictEqual(a, b),
1567+
{
1568+
code: 'ERR_ASSERTION',
1569+
name: 'AssertionError',
1570+
message: /http:\/\/bar/
1571+
}
1572+
);
1573+
}
1574+
});
15441575

1545-
{
1546-
const a = new URL('http://foo');
1547-
const b = new URL('http://foo');
1548-
a.bar = 1;
1549-
b.bar = 2;
1550-
assertNotDeepOrStrict(a, b);
1551-
}
15521576

1553-
{
1554-
const a = new URL('http://foo');
1555-
const b = new URL('http://foo');
1556-
a.bar = 1;
1557-
b.bar = 1;
1577+
test('Own property constructor properties should check against the original prototype', () => {
1578+
const a = { constructor: { name: 'Foo' } };
1579+
const b = { constructor: { name: 'Foo' } };
15581580
assertDeepAndStrictEqual(a, b);
1559-
}
15601581

1561-
{
1562-
const a = new URL('http://foo');
1563-
const b = new URL('http://bar');
1564-
assert.throws(
1565-
() => assert.deepStrictEqual(a, b),
1566-
{
1567-
code: 'ERR_ASSERTION',
1568-
name: 'AssertionError',
1569-
message: /http:\/\/bar/
1570-
}
1571-
);
1572-
}
1582+
const prototype = {};
1583+
Object.setPrototypeOf(a, prototype);
1584+
Object.setPrototypeOf(b, prototype);
1585+
assertDeepAndStrictEqual(a, b);
1586+
1587+
Object.setPrototypeOf(b, {});
1588+
assertNotDeepOrStrict(a, {});
1589+
});

0 commit comments

Comments
 (0)