assert: handle enumerable symbol keys

PR-URL: https://github.com/nodejs/node/pull/15169
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>

1 files changed, 74 insertions, 51 deletions

diff --git a/lib/assert.js b/lib/assert.js
index b3e66aa74f..2aaf80eecc 100644
--- a/lib/assert.js
+++ b/lib/assert.js
@@ -24,6 +24,7 @@ const { compare } = process.binding('buffer');
 const { isSet, isMap, isDate, isRegExp } = process.binding('util');
 const { objectToString } = require('internal/util');
 const errors = require('internal/errors');
+const { propertyIsEnumerable } = Object.prototype;
 
 // The assert module provides functions that throw
 // AssertionError's when particular conditions are not met. The
@@ -165,7 +166,7 @@ function isObjectOrArrayTag(tag) {
 // For strict comparison, objects should have
 // a) The same built-in type tags
 // b) The same prototypes.
-function strictDeepEqual(actual, expected) {
+function strictDeepEqual(actual, expected, memos) {
   if (typeof actual !== 'object') {
     return typeof actual === 'number' && Number.isNaN(actual) &&
       Number.isNaN(expected);
@@ -186,12 +187,12 @@ function strictDeepEqual(actual, expected) {
     // Check for sparse arrays and general fast path
     if (actual.length !== expected.length)
       return false;
-    // Skip testing the part below and continue in the callee function.
-    return;
+    // Skip testing the part below and continue with the keyCheck.
+    return keyCheck(actual, expected, true, memos);
   }
   if (actualTag === '[object Object]') {
-    // Skip testing the part below and continue in the callee function.
-    return;
+    // Skip testing the part below and continue with the keyCheck.
+    return keyCheck(actual, expected, true, memos);
   }
   if (isDate(actual)) {
     if (actual.getTime() !== expected.getTime()) {
@@ -215,10 +216,8 @@ function strictDeepEqual(actual, expected) {
     }
     // Buffer.compare returns true, so actual.length === expected.length
     // if they both only contain numeric keys, we don't need to exam further
-    if (Object.keys(actual).length === actual.length &&
-        Object.keys(expected).length === expected.length) {
-      return true;
-    }
+    return keyCheck(actual, expected, true, memos, actual.length,
+                    expected.length);
   } else if (typeof actual.valueOf === 'function') {
     const actualValue = actual.valueOf();
     // Note: Boxed string keys are going to be compared again by Object.keys
@@ -232,15 +231,14 @@ function strictDeepEqual(actual, expected) {
         lengthActual = actual.length;
         lengthExpected = expected.length;
       }
-      if (Object.keys(actual).length === lengthActual &&
-        Object.keys(expected).length === lengthExpected) {
-        return true;
-      }
+      return keyCheck(actual, expected, true, memos, lengthActual,
+                      lengthExpected);
     }
   }
+  return keyCheck(actual, expected, true, memos);
 }
 
-function looseDeepEqual(actual, expected) {
+function looseDeepEqual(actual, expected, memos) {
   if (actual === null || typeof actual !== 'object') {
     if (expected === null || typeof expected !== 'object') {
       // eslint-disable-next-line eqeqeq
@@ -274,26 +272,10 @@ function looseDeepEqual(actual, expected) {
   } else if (isArguments(actualTag) || isArguments(expectedTag)) {
     return false;
   }
+  return keyCheck(actual, expected, false, memos);
 }
 
-function innerDeepEqual(actual, expected, strict, memos) {
-  // All identical values are equivalent, as determined by ===.
-  if (actual === expected) {
-    if (actual !== 0)
-      return true;
-    return strict ? Object.is(actual, expected) : true;
-  }
-
-  // Returns a boolean if (not) equal and undefined in case we have to check
-  // further.
-  const partialCheck = strict ?
-    strictDeepEqual(actual, expected) :
-    looseDeepEqual(actual, expected);
-
-  if (partialCheck !== undefined) {
-    return partialCheck;
-  }
-
+function keyCheck(actual, expected, strict, memos, lengthA, lengthB) {
   // For all remaining Object pairs, including Array, objects and Maps,
   // equivalence is determined by having:
   // a) The same number of owned enumerable properties
@@ -301,6 +283,51 @@ function innerDeepEqual(actual, expected, strict, memos) {
   // c) Equivalent values for every corresponding key/index
   // d) For Sets and Maps, equal contents
   // Note: this accounts for both named and indexed properties on Arrays.
+  var aKeys = Object.keys(actual);
+  var bKeys = Object.keys(expected);
+  var i;
+
+  // The pair must have the same number of owned properties.
+  if (aKeys.length !== bKeys.length)
+    return false;
+
+  if (strict) {
+    var symbolKeysA = Object.getOwnPropertySymbols(actual);
+    var symbolKeysB = Object.getOwnPropertySymbols(expected);
+    if (symbolKeysA.length !== 0) {
+      symbolKeysA = symbolKeysA.filter((k) =>
+        propertyIsEnumerable.call(actual, k));
+      symbolKeysB = symbolKeysB.filter((k) =>
+        propertyIsEnumerable.call(expected, k));
+      if (symbolKeysA.length !== symbolKeysB.length)
+        return false;
+    } else if (symbolKeysB.length !== 0 && symbolKeysB.filter((k) =>
+      propertyIsEnumerable.call(expected, k)).length !== 0) {
+      return false;
+    }
+    if (lengthA !== undefined) {
+      if (aKeys.length !== lengthA || bKeys.length !== lengthB)
+        return false;
+      if (symbolKeysA.length === 0)
+        return true;
+      aKeys = [];
+      bKeys = [];
+    }
+    if (symbolKeysA.length !== 0) {
+      aKeys.push(...symbolKeysA);
+      bKeys.push(...symbolKeysB);
+    }
+  }
+
+  // Cheap key test:
+  const keys = {};
+  for (i = 0; i < aKeys.length; i++) {
+    keys[aKeys[i]] = true;
+  }
+  for (i = 0; i < aKeys.length; i++) {
+    if (keys[bKeys[i]] === undefined)
+      return false;
+  }
 
   // Use memos to handle cycles.
   if (memos === undefined) {
@@ -323,25 +350,6 @@ function innerDeepEqual(actual, expected, strict, memos) {
     memos.position++;
   }
 
-  const aKeys = Object.keys(actual);
-  const bKeys = Object.keys(expected);
-  var i;
-
-  // The pair must have the same number of owned properties
-  // (keys incorporates hasOwnProperty).
-  if (aKeys.length !== bKeys.length)
-    return false;
-
-  // Cheap key test:
-  const keys = {};
-  for (i = 0; i < aKeys.length; i++) {
-    keys[aKeys[i]] = true;
-  }
-  for (i = 0; i < aKeys.length; i++) {
-    if (keys[bKeys[i]] === undefined)
-      return false;
-  }
-
   memos.actual.set(actual, memos.position);
   memos.expected.set(expected, memos.position);
 
@@ -353,6 +361,21 @@ function innerDeepEqual(actual, expected, strict, memos) {
   return areEq;
 }
 
+function innerDeepEqual(actual, expected, strict, memos) {
+  // All identical values are equivalent, as determined by ===.
+  if (actual === expected) {
+    if (actual !== 0)
+      return true;
+    return strict ? Object.is(actual, expected) : true;
+  }
+
+  // Check more closely if actual and expected are equal.
+  if (strict === true)
+    return strictDeepEqual(actual, expected, memos);
+
+  return looseDeepEqual(actual, expected, memos);
+}
+
 function setHasEqualElement(set, val1, strict, memo) {
   // Go looking.
   for (const val2 of set) {