Skip to content

assert,util: fix constructor lookup in deep equal comparison #57876

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions benchmark/assert/deepequal-prims-and-objs-big-loop.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const notCircular = {};
notCircular.circular = {};

const primValues = {
'null_prototype': { __proto__: null },
'string': 'abcdef',
'number': 1_000,
'boolean': true,
Expand All @@ -24,6 +25,7 @@ const primValues = {
};

const primValues2 = {
'null_prototype': { __proto__: null },
'object': { property: 'abcdef' },
'array': [1, 2, 3],
'set_object': new Set([[1]]),
Expand All @@ -35,6 +37,7 @@ const primValues2 = {
};

const primValuesUnequal = {
'null_prototype': { __proto__: { __proto__: null } },
'string': 'abcdez',
'number': 1_001,
'boolean': false,
Expand Down
80 changes: 75 additions & 5 deletions lib/internal/util/comparisons.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,31 @@
'use strict';

const {
Array,
ArrayBuffer,
ArrayIsArray,
ArrayPrototypeFilter,
ArrayPrototypePush,
BigInt,
BigInt64Array,
BigIntPrototypeValueOf,
BigUint64Array,
Boolean,
BooleanPrototypeValueOf,
DataView,
Date,
DatePrototypeGetTime,
Error,
Float32Array,
Float64Array,
Function,
Int16Array,
Int32Array,
Int8Array,
Map,
Number,
NumberPrototypeValueOf,
Object,
ObjectGetOwnPropertyDescriptor,
ObjectGetOwnPropertySymbols: getOwnSymbols,
ObjectGetPrototypeOf,
Expand All @@ -17,18 +34,67 @@ const {
ObjectPrototypeHasOwnProperty: hasOwn,
ObjectPrototypePropertyIsEnumerable: hasEnumerable,
ObjectPrototypeToString,
Promise,
RegExp,
SafeSet,
Set,
String,
StringPrototypeValueOf,
Symbol,
SymbolPrototypeValueOf,
TypedArrayPrototypeGetByteLength: getByteLength,
TypedArrayPrototypeGetSymbolToStringTag,
Uint16Array,
Uint32Array,
Uint8Array,
Uint8ClampedArray,
WeakMap,
WeakSet,
globalThis: { Float16Array },
} = primordials;

const { compare } = internalBinding('buffer');
const assert = require('internal/assert');
const { isURL } = require('internal/url');
const { isError } = require('internal/util');
const { Buffer } = require('buffer');

const wellKnownConstructors = new SafeSet()
.add(Array)
.add(ArrayBuffer)
.add(BigInt)
.add(BigInt64Array)
.add(BigUint64Array)
.add(Boolean)
.add(Buffer)
.add(DataView)
.add(Date)
.add(Error)
.add(Float32Array)
.add(Float64Array)
.add(Function)
.add(Int16Array)
.add(Int32Array)
.add(Int8Array)
.add(Map)
.add(Number)
.add(Object)
.add(Promise)
.add(RegExp)
.add(Set)
.add(String)
.add(Symbol)
.add(Uint16Array)
.add(Uint32Array)
.add(Uint8Array)
.add(Uint8ClampedArray)
.add(WeakMap)
.add(WeakSet);

if (Float16Array) { // TODO(BridgeAR): Remove when regularly supported
wellKnownConstructors.add(Float16Array);
}

const types = require('internal/util/types');
const {
isAnyArrayBuffer,
Expand Down Expand Up @@ -198,11 +264,15 @@ function innerDeepEqual(val1, val2, mode, memos) {
}

function objectComparisonStart(val1, val2, mode, memos) {
if (mode === kStrict &&
(val1.constructor !== val2.constructor ||
(val1.constructor === undefined &&
ObjectGetPrototypeOf(val1) !== ObjectGetPrototypeOf(val2)))) {
return false;
if (mode === kStrict) {
if (wellKnownConstructors.has(val1.constructor) ||
(val1.constructor !== undefined && !hasOwn(val1, 'constructor'))) {
if (val1.constructor !== val2.constructor) {
return false;
}
} else if (ObjectGetPrototypeOf(val1) !== ObjectGetPrototypeOf(val2)) {
return false;
}
}

const val1Tag = ObjectPrototypeToString(val1);
Expand Down
45 changes: 36 additions & 9 deletions lib/internal/util/inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,18 @@ const {
ArrayPrototypeSplice,
ArrayPrototypeUnshift,
BigIntPrototypeValueOf,
Boolean,
BooleanPrototype,
BooleanPrototypeValueOf,
DataView,
DataViewPrototype,
Date,
DatePrototype,
DatePrototypeGetTime,
DatePrototypeToISOString,
DatePrototypeToString,
Error,
ErrorPrototype,
ErrorPrototypeToString,
Function,
FunctionPrototype,
Expand All @@ -47,6 +55,7 @@ const {
NumberIsNaN,
NumberParseFloat,
NumberParseInt,
NumberPrototype,
NumberPrototypeToString,
NumberPrototypeValueOf,
Object,
Expand All @@ -63,9 +72,12 @@ const {
ObjectPrototypePropertyIsEnumerable,
ObjectSeal,
ObjectSetPrototypeOf,
Promise,
PromisePrototype,
ReflectApply,
ReflectOwnKeys,
RegExp,
RegExpPrototype,
RegExpPrototypeExec,
RegExpPrototypeSymbolReplace,
RegExpPrototypeSymbolSplit,
Expand All @@ -78,6 +90,7 @@ const {
SetPrototypeGetSize,
SetPrototypeValues,
String,
StringPrototype,
StringPrototypeCharCodeAt,
StringPrototypeCodePointAt,
StringPrototypeEndsWith,
Expand Down Expand Up @@ -106,6 +119,10 @@ const {
TypedArrayPrototypeGetLength,
TypedArrayPrototypeGetSymbolToStringTag,
Uint8Array,
WeakMap,
WeakMapPrototype,
WeakSet,
WeakSetPrototype,
globalThis,
uncurryThis,
} = primordials;
Expand Down Expand Up @@ -608,21 +625,31 @@ function isInstanceof(object, proto) {
}

// Special-case for some builtin prototypes in case their `constructor` property has been tampered.
const wellKnownPrototypes = new SafeMap();
wellKnownPrototypes.set(ArrayPrototype, { name: 'Array', constructor: Array });
wellKnownPrototypes.set(ArrayBufferPrototype, { name: 'ArrayBuffer', constructor: ArrayBuffer });
wellKnownPrototypes.set(FunctionPrototype, { name: 'Function', constructor: Function });
wellKnownPrototypes.set(MapPrototype, { name: 'Map', constructor: Map });
wellKnownPrototypes.set(ObjectPrototype, { name: 'Object', constructor: Object });
wellKnownPrototypes.set(SetPrototype, { name: 'Set', constructor: Set });
wellKnownPrototypes.set(TypedArrayPrototype, { name: 'TypedArray', constructor: TypedArray });
const wellKnownPrototypes = new SafeMap()
.set(ArrayPrototype, { name: 'Array', constructor: Array })
.set(ArrayBufferPrototype, { name: 'ArrayBuffer', constructor: ArrayBuffer })
.set(FunctionPrototype, { name: 'Function', constructor: Function })
.set(MapPrototype, { name: 'Map', constructor: Map })
.set(SetPrototype, { name: 'Set', constructor: Set })
.set(ObjectPrototype, { name: 'Object', constructor: Object })
.set(TypedArrayPrototype, { name: 'TypedArray', constructor: TypedArray })
.set(RegExpPrototype, { name: 'RegExp', constructor: RegExp })
.set(DatePrototype, { name: 'Date', constructor: Date })
.set(DataViewPrototype, { name: 'DataView', constructor: DataView })
.set(ErrorPrototype, { name: 'Error', constructor: Error })
.set(BooleanPrototype, { name: 'Boolean', constructor: Boolean })
.set(NumberPrototype, { name: 'Number', constructor: Number })
.set(StringPrototype, { name: 'String', constructor: String })
.set(PromisePrototype, { name: 'Promise', constructor: Promise })
.set(WeakMapPrototype, { name: 'WeakMap', constructor: WeakMap })
.set(WeakSetPrototype, { name: 'WeakSet', constructor: WeakSet });

function getConstructorName(obj, ctx, recurseTimes, protoProps) {
let firstProto;
const tmp = obj;
while (obj || isUndetectableObject(obj)) {
const wellKnownPrototypeNameAndConstructor = wellKnownPrototypes.get(obj);
if (wellKnownPrototypeNameAndConstructor != null) {
if (wellKnownPrototypeNameAndConstructor !== undefined) {
const { name, constructor } = wellKnownPrototypeNameAndConstructor;
if (FunctionPrototypeSymbolHasInstance(constructor, tmp)) {
if (protoProps !== undefined && firstProto !== obj) {
Expand Down
131 changes: 96 additions & 35 deletions test/parallel/test-assert-deep.js
Original file line number Diff line number Diff line change
Expand Up @@ -1527,46 +1527,107 @@ test('Detects differences in deeply nested arrays instead of seeing a new object
);
});

// check URL
{
const a = new URL('http://foo');
const b = new URL('http://bar');
test('URLs', () => {
// check URL
{
const a = new URL('http://foo');
const b = new URL('http://bar');

assertNotDeepOrStrict(a, b);
}
assertNotDeepOrStrict(a, b);
}

{
const a = new URL('http://foo');
const b = new URL('http://foo');
{
const a = new URL('http://foo');
const b = new URL('http://foo');

assertDeepAndStrictEqual(a, b);
}

{
const a = new URL('http://foo');
const b = new URL('http://foo');
a.bar = 1;
b.bar = 2;
assertNotDeepOrStrict(a, b);
}

{
const a = new URL('http://foo');
const b = new URL('http://foo');
a.bar = 1;
b.bar = 1;
assertDeepAndStrictEqual(a, b);
}

{
const a = new URL('http://foo');
const b = new URL('http://bar');
assert.throws(
() => assert.deepStrictEqual(a, b),
{
code: 'ERR_ASSERTION',
name: 'AssertionError',
message: /http:\/\/bar/
}
);
}
});

test('Own property constructor properties should check against the original prototype', () => {
const a = { constructor: { name: 'Foo' } };
const b = { constructor: { name: 'Foo' } };
assertDeepAndStrictEqual(a, b);
}

{
const a = new URL('http://foo');
const b = new URL('http://foo');
a.bar = 1;
b.bar = 2;
assertNotDeepOrStrict(a, b);
}
let prototype = {};
Object.setPrototypeOf(a, prototype);
Object.setPrototypeOf(b, prototype);
assertDeepAndStrictEqual(a, b);

{
const a = new URL('http://foo');
const b = new URL('http://foo');
a.bar = 1;
b.bar = 1;
Object.setPrototypeOf(b, {});
assertNotDeepOrStrict(a, {});

prototype = { __proto__: null };
Object.setPrototypeOf(a, prototype);
Object.setPrototypeOf(b, prototype);
assertDeepAndStrictEqual(a, b);
}

{
const a = new URL('http://foo');
const b = new URL('http://bar');
assert.throws(
() => assert.deepStrictEqual(a, b),
{
code: 'ERR_ASSERTION',
name: 'AssertionError',
message: /http:\/\/bar/
}
);
}
Object.setPrototypeOf(b, { __proto__: null });
assert.notDeepStrictEqual(a, b);
assert.notDeepStrictEqual(b, a);

// Turn off no-restricted-properties because we are testing deepEqual!
/* eslint-disable no-restricted-properties */
assert.deepEqual(a, b);
assert.deepEqual(b, a);
});

test('Inherited null prototype without own constructor properties should check the correct prototype', () => {
const a = { foo: { name: 'Foo' } };
const b = { foo: { name: 'Foo' } };
assertDeepAndStrictEqual(a, b);

let prototype = {};
Object.setPrototypeOf(a, prototype);
Object.setPrototypeOf(b, prototype);
assertDeepAndStrictEqual(a, b);

Object.setPrototypeOf(b, {});
assertNotDeepOrStrict(a, {});

prototype = { __proto__: null };
Object.setPrototypeOf(a, prototype);
Object.setPrototypeOf(b, prototype);
assertDeepAndStrictEqual(a, b);

Object.setPrototypeOf(b, { __proto__: null });
assert.notDeepStrictEqual(a, b);
assert.notDeepStrictEqual(b, a);

assert.notDeepStrictEqual({ __proto__: null }, { __proto__: { __proto__: null } });
assert.notDeepStrictEqual({ __proto__: { __proto__: null } }, { __proto__: null });

// Turn off no-restricted-properties because we are testing deepEqual!
/* eslint-disable no-restricted-properties */
assert.deepEqual(a, b);
assert.deepEqual(b, a);
});
Loading