assert,util: stricter type comparison using deep equal comparisons

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]>

Author: BridgeAR

lib/internal/util/comparisons.js

@@ -19,6 +19,7 @@ const {
 } = primordials;
 
 const { compare } = internalBinding('buffer');
+const types = require('internal/util/types');
 const {
   isAnyArrayBuffer,
   isArrayBufferView,
@@ -34,8 +35,17 @@ const {
   isBigIntObject,
   isSymbolObject,
   isFloat32Array,
-  isFloat64Array
-} = require('internal/util/types');
+  isFloat64Array,
+  isUint8Array,
+  isUint8ClampedArray,
+  isUint16Array,
+  isUint32Array,
+  isInt8Array,
+  isInt16Array,
+  isInt32Array,
+  isBigInt64Array,
+  isBigUint64Array
+} = types;
 const {
   getOwnNonIndexProperties,
   propertyFilter: {
@@ -107,6 +117,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[Symbol.toStringTag]}`];
+  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
@@ -115,15 +152,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) {
@@ -164,9 +194,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;
@@ -176,25 +207,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;
@@ -223,12 +257,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);

test/parallel/test-assert-deep.js

@@ -1123,3 +1123,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);
+}