Skip to content

Commit

Permalink
assert,util: stricter type comparison using deep equal comparisons
Browse files Browse the repository at this point in the history 
This veryfies that both input arguments are always of the identical
type. It was possible to miss a few cases before. This change applies
to all deep equal assert functions (e.g., `assert.deepStrictEqual()`)
and to `util.isDeepStrictEqual()`.

PR-URL: #30764
Refs: #30743
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
  • Loading branch information
@BridgeAR @targos
BridgeAR authored and targos committed Jan 14, 2020
1 parent 219c8e9 commit 1f68004
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 22 deletions.
90 changes: 68 additions & 22 deletions lib/internal/util/comparisons.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ const {
Set,
StringPrototypeValueOf,
SymbolPrototypeValueOf,
SymbolToStringTag,
} = primordials;

const { compare } = internalBinding('buffer');
const types = require('internal/util/types');
const {
isAnyArrayBuffer,
isArrayBufferView,
Expand All @@ -37,8 +39,17 @@ const {
isBigIntObject,
isSymbolObject,
isFloat32Array,
isFloat64Array
} = require('internal/util/types');
isFloat64Array,
isUint8Array,
isUint8ClampedArray,
isUint16Array,
isUint32Array,
isInt8Array,
isInt16Array,
isInt32Array,
isBigInt64Array,
isBigUint64Array
} = types;
const {
getOwnNonIndexProperties,
propertyFilter: {
Expand Down Expand Up @@ -110,6 +121,33 @@ function isEqualBoxedPrimitive(val1, val2) {
return false;
}

function isIdenticalTypedArrayType(a, b) {
// Fast path to reduce type checks in the common case.
const check = types[`is${a[SymbolToStringTag]}`];
if (check !== undefined && check(a)) {
return check(b);
}
// Manipulated Symbol.toStringTag.
for (const check of [
isUint16Array,
isUint32Array,
isInt8Array,
isInt16Array,
isInt32Array,
isFloat32Array,
isFloat64Array,
isBigInt64Array,
isBigUint64Array,
isUint8ClampedArray,
isUint8Array
]) {
if (check(a)) {
return check(b);
}
}
return false;
}

// Notes: Type tags are historical [[Class]] properties that can be set by
// FunctionTemplate::SetClassName() in C++ or Symbol.toStringTag in JS
// and retrieved using Object.prototype.toString.call(obj) in JS
Expand All @@ -118,15 +156,8 @@ function isEqualBoxedPrimitive(val1, val2) {
// There are some unspecified tags in the wild too (e.g. typed array tags).
// Since tags can be altered, they only serve fast failures
//
// Typed arrays and buffers are checked by comparing the content in their
// underlying ArrayBuffer. This optimization requires that it's
// reasonable to interpret their underlying memory in the same way,
// which is checked by comparing their type tags.
// (e.g. a Uint8Array and a Uint16Array with the same memory content
// could still be different because they will be interpreted differently).
//
// For strict comparison, objects should have
// a) The same built-in type tags
// a) The same built-in type tag.
// b) The same prototypes.

function innerDeepEqual(val1, val2, strict, memos) {
Expand Down Expand Up @@ -167,9 +198,10 @@ function innerDeepEqual(val1, val2, strict, memos) {
if (val1Tag !== val2Tag) {
return false;
}

if (ArrayIsArray(val1)) {
// Check for sparse arrays and general fast path
if (val1.length !== val2.length) {
if (!ArrayIsArray(val2) || val1.length !== val2.length) {
return false;
}
const filter = strict ? ONLY_ENUMERABLE : ONLY_ENUMERABLE | SKIP_SYMBOLS;
Expand All @@ -179,25 +211,28 @@ function innerDeepEqual(val1, val2, strict, memos) {
return false;
}
return keyCheck(val1, val2, strict, memos, kIsArray, keys1);
}
if (val1Tag === '[object Object]') {
} else if (val1Tag === '[object Object]') {
return keyCheck(val1, val2, strict, memos, kNoIterator);
}
if (isDate(val1)) {
if (DatePrototypeGetTime(val1) !== DatePrototypeGetTime(val2)) {
} else if (isDate(val1)) {
if (!isDate(val2) ||
DatePrototypeGetTime(val1) !== DatePrototypeGetTime(val2)) {
return false;
}
} else if (isRegExp(val1)) {
if (!areSimilarRegExps(val1, val2)) {
if (!isRegExp(val2) || !areSimilarRegExps(val1, val2)) {
return false;
}
} else if (isNativeError(val1) || val1 instanceof Error) {
// Do not compare the stack as it might differ even though the error itself
// is otherwise identical.
if (val1.message !== val2.message || val1.name !== val2.name) {
if ((!isNativeError(val2) && !(val2 instanceof Error)) ||
val1.message !== val2.message ||
val1.name !== val2.name) {
return false;
}
} else if (isArrayBufferView(val1)) {
if (!isIdenticalTypedArrayType(val1, val2))
return false;
if (!strict && (isFloat32Array(val1) || isFloat64Array(val1))) {
if (!areSimilarFloatArrays(val1, val2)) {
return false;
Expand Down Expand Up @@ -226,12 +261,23 @@ function innerDeepEqual(val1, val2, strict, memos) {
}
return keyCheck(val1, val2, strict, memos, kIsMap);
} else if (isAnyArrayBuffer(val1)) {
if (!areEqualArrayBuffers(val1, val2)) {
if (!isAnyArrayBuffer(val2) || !areEqualArrayBuffers(val1, val2)) {
return false;
}
}
if ((isBoxedPrimitive(val1) || isBoxedPrimitive(val2)) &&
!isEqualBoxedPrimitive(val1, val2)) {
} else if (isBoxedPrimitive(val1)) {
if (!isEqualBoxedPrimitive(val1, val2)) {
return false;
}
} else if (ArrayIsArray(val2) ||
isArrayBufferView(val2) ||
isSet(val2) ||
isMap(val2) ||
isDate(val2) ||
isRegExp(val2) ||
isAnyArrayBuffer(val2) ||
isBoxedPrimitive(val2) ||
isNativeError(val2) ||
val2 instanceof Error) {
return false;
}
return keyCheck(val1, val2, strict, memos, kNoIterator);
Expand Down
66 changes: 66 additions & 0 deletions test/parallel/test-assert-deep.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1138,3 +1138,69 @@ assert.throws(
// The descriptor is not compared.
assertDeepAndStrictEqual(a, { a: 5 });
}

// Verify object types being identical on both sides.
{
let a = Buffer.from('test');
let b = Object.create(
Object.getPrototypeOf(a),
Object.getOwnPropertyDescriptors(a)
);
Object.defineProperty(b, Symbol.toStringTag, {
value: 'Uint8Array'
});
assertNotDeepOrStrict(a, b);

a = new Uint8Array(10);
b = new Int8Array(10);
Object.defineProperty(b, Symbol.toStringTag, {
value: 'Uint8Array'
});
Object.setPrototypeOf(b, Uint8Array.prototype);
assertNotDeepOrStrict(a, b);

a = [1, 2, 3];
b = { 0: 1, 1: 2, 2: 3 };
Object.setPrototypeOf(b, Array.prototype);
Object.defineProperty(b, 'length', { value: 3, enumerable: false });
Object.defineProperty(b, Symbol.toStringTag, {
value: 'Array'
});
assertNotDeepOrStrict(a, b);

a = new Date(2000);
b = Object.create(
Object.getPrototypeOf(a),
Object.getOwnPropertyDescriptors(a)
);
Object.defineProperty(b, Symbol.toStringTag, {
value: 'Date'
});
assertNotDeepOrStrict(a, b);

a = /abc/g;
b = Object.create(
Object.getPrototypeOf(a),
Object.getOwnPropertyDescriptors(a)
);
Object.defineProperty(b, Symbol.toStringTag, {
value: 'RegExp'
});
assertNotDeepOrStrict(a, b);

a = [];
b = /abc/;
Object.setPrototypeOf(b, Array.prototype);
Object.defineProperty(b, Symbol.toStringTag, {
value: 'Array'
});
assertNotDeepOrStrict(a, b);

a = Object.create(null);
b = new RangeError('abc');
Object.defineProperty(a, Symbol.toStringTag, {
value: 'Error'
});
Object.setPrototypeOf(b, null);
assertNotDeepOrStrict(a, b, assert.AssertionError);
}

0 comments on commit 1f68004

Please sign in to comment.