Skip to content

Commit d9e78c0

Browse files
BridgeARRafaelGSS
BridgeAR
authored and
RafaelGSS
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. PR-URL: #57876 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
1 parent 403264c commit d9e78c0

File tree

4 files changed

+210
-49
lines changed

4 files changed

+210
-49
lines changed

‎benchmark/assert/deepequal-prims-and-objs-big-loop.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const notCircular = {};
1010
notCircular.circular = {};
1111

1212
const primValues = {
13+
'null_prototype': { __proto__: null },
1314
'string': 'abcdef',
1415
'number': 1_000,
1516
'boolean': true,
@@ -24,6 +25,7 @@ const primValues = {
2425
};
2526

2627
const primValues2 = {
28+
'null_prototype': { __proto__: null },
2729
'object': { property: 'abcdef' },
2830
'array': [1, 2, 3],
2931
'set_object': new Set([[1]]),
@@ -35,6 +37,7 @@ const primValues2 = {
3537
};
3638

3739
const primValuesUnequal = {
40+
'null_prototype': { __proto__: { __proto__: null } },
3841
'string': 'abcdez',
3942
'number': 1_001,
4043
'boolean': false,

‎lib/internal/util/comparisons.js‎

Lines changed: 75 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,31 @@
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+
Float32Array,
20+
Float64Array,
21+
Function,
22+
Int16Array,
23+
Int32Array,
24+
Int8Array,
25+
Map,
26+
Number,
1127
NumberPrototypeValueOf,
28+
Object,
1229
ObjectGetOwnPropertyDescriptor,
1330
ObjectGetOwnPropertySymbols: getOwnSymbols,
1431
ObjectGetPrototypeOf,
@@ -17,18 +34,67 @@ const {
1734
ObjectPrototypeHasOwnProperty: hasOwn,
1835
ObjectPrototypePropertyIsEnumerable: hasEnumerable,
1936
ObjectPrototypeToString,
37+
Promise,
38+
RegExp,
2039
SafeSet,
40+
Set,
41+
String,
2142
StringPrototypeValueOf,
43+
Symbol,
2244
SymbolPrototypeValueOf,
2345
TypedArrayPrototypeGetByteLength: getByteLength,
2446
TypedArrayPrototypeGetSymbolToStringTag,
47+
Uint16Array,
48+
Uint32Array,
2549
Uint8Array,
50+
Uint8ClampedArray,
51+
WeakMap,
52+
WeakSet,
53+
globalThis: { Float16Array },
2654
} = primordials;
2755

2856
const { compare } = internalBinding('buffer');
2957
const assert = require('internal/assert');
3058
const { isURL } = require('internal/url');
3159
const { isError } = require('internal/util');
60+
const { Buffer } = require('buffer');
61+
62+
const wellKnownConstructors = new SafeSet()
63+
.add(Array)
64+
.add(ArrayBuffer)
65+
.add(BigInt)
66+
.add(BigInt64Array)
67+
.add(BigUint64Array)
68+
.add(Boolean)
69+
.add(Buffer)
70+
.add(DataView)
71+
.add(Date)
72+
.add(Error)
73+
.add(Float32Array)
74+
.add(Float64Array)
75+
.add(Function)
76+
.add(Int16Array)
77+
.add(Int32Array)
78+
.add(Int8Array)
79+
.add(Map)
80+
.add(Number)
81+
.add(Object)
82+
.add(Promise)
83+
.add(RegExp)
84+
.add(Set)
85+
.add(String)
86+
.add(Symbol)
87+
.add(Uint16Array)
88+
.add(Uint32Array)
89+
.add(Uint8Array)
90+
.add(Uint8ClampedArray)
91+
.add(WeakMap)
92+
.add(WeakSet);
93+
94+
if (Float16Array) { // TODO(BridgeAR): Remove when regularly supported
95+
wellKnownConstructors.add(Float16Array);
96+
}
97+
3298
const types = require('internal/util/types');
3399
const {
34100
isAnyArrayBuffer,
@@ -198,11 +264,15 @@ function innerDeepEqual(val1, val2, mode, memos) {
198264
}
199265

200266
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;
267+
if (mode === kStrict) {
268+
if (wellKnownConstructors.has(val1.constructor) ||
269+
(val1.constructor !== undefined && !hasOwn(val1, 'constructor'))) {
270+
if (val1.constructor !== val2.constructor) {
271+
return false;
272+
}
273+
} else if (ObjectGetPrototypeOf(val1) !== ObjectGetPrototypeOf(val2)) {
274+
return false;
275+
}
206276
}
207277

208278
const val1Tag = ObjectPrototypeToString(val1);

‎lib/internal/util/inspect.js‎

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,18 @@ const {
2020
ArrayPrototypeSplice,
2121
ArrayPrototypeUnshift,
2222
BigIntPrototypeValueOf,
23+
Boolean,
24+
BooleanPrototype,
2325
BooleanPrototypeValueOf,
26+
DataView,
27+
DataViewPrototype,
28+
Date,
29+
DatePrototype,
2430
DatePrototypeGetTime,
2531
DatePrototypeToISOString,
2632
DatePrototypeToString,
33+
Error,
34+
ErrorPrototype,
2735
ErrorPrototypeToString,
2836
Function,
2937
FunctionPrototype,
@@ -47,6 +55,7 @@ const {
4755
NumberIsNaN,
4856
NumberParseFloat,
4957
NumberParseInt,
58+
NumberPrototype,
5059
NumberPrototypeToString,
5160
NumberPrototypeValueOf,
5261
Object,
@@ -63,9 +72,12 @@ const {
6372
ObjectPrototypePropertyIsEnumerable,
6473
ObjectSeal,
6574
ObjectSetPrototypeOf,
75+
Promise,
76+
PromisePrototype,
6677
ReflectApply,
6778
ReflectOwnKeys,
6879
RegExp,
80+
RegExpPrototype,
6981
RegExpPrototypeExec,
7082
RegExpPrototypeSymbolReplace,
7183
RegExpPrototypeSymbolSplit,
@@ -78,6 +90,7 @@ const {
7890
SetPrototypeGetSize,
7991
SetPrototypeValues,
8092
String,
93+
StringPrototype,
8194
StringPrototypeCharCodeAt,
8295
StringPrototypeCodePointAt,
8396
StringPrototypeEndsWith,
@@ -106,6 +119,10 @@ const {
106119
TypedArrayPrototypeGetLength,
107120
TypedArrayPrototypeGetSymbolToStringTag,
108121
Uint8Array,
122+
WeakMap,
123+
WeakMapPrototype,
124+
WeakSet,
125+
WeakSetPrototype,
109126
globalThis,
110127
uncurryThis,
111128
} = primordials;
@@ -608,21 +625,31 @@ function isInstanceof(object, proto) {
608625
}
609626

610627
// Special-case for some builtin prototypes in case their `constructor` property has been tampered.
611-
const wellKnownPrototypes = new SafeMap();
612-
wellKnownPrototypes.set(ArrayPrototype, { name: 'Array', constructor: Array });
613-
wellKnownPrototypes.set(ArrayBufferPrototype, { name: 'ArrayBuffer', constructor: ArrayBuffer });
614-
wellKnownPrototypes.set(FunctionPrototype, { name: 'Function', constructor: Function });
615-
wellKnownPrototypes.set(MapPrototype, { name: 'Map', constructor: Map });
616-
wellKnownPrototypes.set(ObjectPrototype, { name: 'Object', constructor: Object });
617-
wellKnownPrototypes.set(SetPrototype, { name: 'Set', constructor: Set });
618-
wellKnownPrototypes.set(TypedArrayPrototype, { name: 'TypedArray', constructor: TypedArray });
628+
const wellKnownPrototypes = new SafeMap()
629+
.set(ArrayPrototype, { name: 'Array', constructor: Array })
630+
.set(ArrayBufferPrototype, { name: 'ArrayBuffer', constructor: ArrayBuffer })
631+
.set(FunctionPrototype, { name: 'Function', constructor: Function })
632+
.set(MapPrototype, { name: 'Map', constructor: Map })
633+
.set(SetPrototype, { name: 'Set', constructor: Set })
634+
.set(ObjectPrototype, { name: 'Object', constructor: Object })
635+
.set(TypedArrayPrototype, { name: 'TypedArray', constructor: TypedArray })
636+
.set(RegExpPrototype, { name: 'RegExp', constructor: RegExp })
637+
.set(DatePrototype, { name: 'Date', constructor: Date })
638+
.set(DataViewPrototype, { name: 'DataView', constructor: DataView })
639+
.set(ErrorPrototype, { name: 'Error', constructor: Error })
640+
.set(BooleanPrototype, { name: 'Boolean', constructor: Boolean })
641+
.set(NumberPrototype, { name: 'Number', constructor: Number })
642+
.set(StringPrototype, { name: 'String', constructor: String })
643+
.set(PromisePrototype, { name: 'Promise', constructor: Promise })
644+
.set(WeakMapPrototype, { name: 'WeakMap', constructor: WeakMap })
645+
.set(WeakSetPrototype, { name: 'WeakSet', constructor: WeakSet });
619646

620647
function getConstructorName(obj, ctx, recurseTimes, protoProps) {
621648
let firstProto;
622649
const tmp = obj;
623650
while (obj || isUndetectableObject(obj)) {
624651
const wellKnownPrototypeNameAndConstructor = wellKnownPrototypes.get(obj);
625-
if (wellKnownPrototypeNameAndConstructor != null) {
652+
if (wellKnownPrototypeNameAndConstructor !== undefined) {
626653
const { name, constructor } = wellKnownPrototypeNameAndConstructor;
627654
if (FunctionPrototypeSymbolHasInstance(constructor, tmp)) {
628655
if (protoProps !== undefined && firstProto !== obj) {

‎test/parallel/test-assert-deep.js‎

Lines changed: 96 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1527,46 +1527,107 @@ 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

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+
});
1575+
1576+
test('Own property constructor properties should check against the original prototype', () => {
1577+
const a = { constructor: { name: 'Foo' } };
1578+
const b = { constructor: { name: 'Foo' } };
15421579
assertDeepAndStrictEqual(a, b);
1543-
}
15441580

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-
}
1581+
let prototype = {};
1582+
Object.setPrototypeOf(a, prototype);
1583+
Object.setPrototypeOf(b, prototype);
1584+
assertDeepAndStrictEqual(a, b);
15521585

1553-
{
1554-
const a = new URL('http://foo');
1555-
const b = new URL('http://foo');
1556-
a.bar = 1;
1557-
b.bar = 1;
1586+
Object.setPrototypeOf(b, {});
1587+
assertNotDeepOrStrict(a, {});
1588+
1589+
prototype = { __proto__: null };
1590+
Object.setPrototypeOf(a, prototype);
1591+
Object.setPrototypeOf(b, prototype);
15581592
assertDeepAndStrictEqual(a, b);
1559-
}
15601593

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-
}
1594+
Object.setPrototypeOf(b, { __proto__: null });
1595+
assert.notDeepStrictEqual(a, b);
1596+
assert.notDeepStrictEqual(b, a);
1597+
1598+
// Turn off no-restricted-properties because we are testing deepEqual!
1599+
/* eslint-disable no-restricted-properties */
1600+
assert.deepEqual(a, b);
1601+
assert.deepEqual(b, a);
1602+
});
1603+
1604+
test('Inherited null prototype without own constructor properties should check the correct prototype', () => {
1605+
const a = { foo: { name: 'Foo' } };
1606+
const b = { foo: { name: 'Foo' } };
1607+
assertDeepAndStrictEqual(a, b);
1608+
1609+
let prototype = {};
1610+
Object.setPrototypeOf(a, prototype);
1611+
Object.setPrototypeOf(b, prototype);
1612+
assertDeepAndStrictEqual(a, b);
1613+
1614+
Object.setPrototypeOf(b, {});
1615+
assertNotDeepOrStrict(a, {});
1616+
1617+
prototype = { __proto__: null };
1618+
Object.setPrototypeOf(a, prototype);
1619+
Object.setPrototypeOf(b, prototype);
1620+
assertDeepAndStrictEqual(a, b);
1621+
1622+
Object.setPrototypeOf(b, { __proto__: null });
1623+
assert.notDeepStrictEqual(a, b);
1624+
assert.notDeepStrictEqual(b, a);
1625+
1626+
assert.notDeepStrictEqual({ __proto__: null }, { __proto__: { __proto__: null } });
1627+
assert.notDeepStrictEqual({ __proto__: { __proto__: null } }, { __proto__: null });
1628+
1629+
// Turn off no-restricted-properties because we are testing deepEqual!
1630+
/* eslint-disable no-restricted-properties */
1631+
assert.deepEqual(a, b);
1632+
assert.deepEqual(b, a);
1633+
});

0 commit comments

Comments
 (0)