diff --git a/lib/internal/util/comparisons.js b/lib/internal/util/comparisons.js index 9f636f31a3d629..729bd1c31efa68 100644 --- a/lib/internal/util/comparisons.js +++ b/lib/internal/util/comparisons.js @@ -4,6 +4,7 @@ const { compare } = process.binding('buffer'); const { isArrayBufferView } = require('internal/util/types'); const { internalBinding } = require('internal/bootstrap/loaders'); const { isDate, isMap, isRegExp, isSet } = internalBinding('types'); +const { getOwnNonIndexProperties } = process.binding('util'); const ReflectApply = Reflect.apply; @@ -106,24 +107,11 @@ function strictDeepEqual(val1, val2, memos) { if (val1.length !== val2.length) { return false; } - const keys = objectKeys(val1); - if (keys.length !== objectKeys(val2).length) { + const keys1 = getOwnNonIndexProperties(val1); + if (keys1.length !== getOwnNonIndexProperties(val2).length) { return false; } - // Fast path for non sparse arrays (no key comparison for indices - // properties). - // See https://tc39.github.io/ecma262/#sec-ordinaryownpropertykeys - if (val1.length === keys.length) { - if (keys.length === 0 || keys[val1.length - 1] === `${val1.length - 1}`) { - return keyCheck(val1, val2, kStrict, memos, kIsArray, []); - } - } else if (keys.length > val1.length && - keys[val1.length - 1] === `${val1.length - 1}`) { - const minimalKeys = keys.slice(val1.length); - return keyCheck(val1, val2, kStrict, memos, kIsArray, minimalKeys); - } - // Only set this to kIsArray in case the array is not sparse! - return keyCheck(val1, val2, kStrict, memos, kNoIterator, keys); + return keyCheck(val1, val2, kStrict, memos, kIsArray, keys1); } if (val1Tag === '[object Object]') { return keyCheck(val1, val2, kStrict, memos, kNoIterator); @@ -148,18 +136,14 @@ function strictDeepEqual(val1, val2, memos) { if (!areSimilarTypedArrays(val1, val2)) { return false; } - // Buffer.compare returns true, so val1.length === val2.length - // if they both only contain numeric keys, we don't need to exam further. - const keys = objectKeys(val1); - if (keys.length !== objectKeys(val2).length) { + // Buffer.compare returns true, so val1.length === val2.length. If they both + // only contain numeric keys, we don't need to exam further than checking + // the symbols. + const keys1 = getOwnNonIndexProperties(val1); + if (keys1.length !== getOwnNonIndexProperties(val2).length) { return false; } - if (keys.length === val1.length) { - return keyCheck(val1, val2, kStrict, memos, kNoIterator, []); - } - // Only compare the special keys. - const minimalKeys = keys.slice(val1.length); - return keyCheck(val1, val2, kStrict, memos, kNoIterator, minimalKeys); + return keyCheck(val1, val2, kStrict, memos, kNoIterator, keys1); } else if (isSet(val1)) { if (!isSet(val2) || val1.size !== val2.size) { return false; @@ -173,21 +157,10 @@ function strictDeepEqual(val1, val2, memos) { // TODO: Make the valueOf checks safe. } else if (typeof val1.valueOf === 'function') { const val1Value = val1.valueOf(); - if (val1Value !== val1) { - if (typeof val2.valueOf !== 'function') { - return false; - } - if (!innerDeepEqual(val1Value, val2.valueOf(), true)) - return false; - // Fast path for boxed primitive strings. - if (typeof val1Value === 'string') { - const keys = objectKeys(val1); - if (keys.length !== objectKeys(val2).length) { - return false; - } - const minimalKeys = keys.slice(val1.length); - return keyCheck(val1, val2, kStrict, memos, kNoIterator, minimalKeys); - } + if (val1Value !== val1 && + (typeof val2.valueOf !== 'function' || + !innerDeepEqual(val1Value, val2.valueOf(), kStrict))) { + return false; } } return keyCheck(val1, val2, kStrict, memos, kNoIterator); @@ -277,7 +250,7 @@ function keyCheck(val1, val2, strict, memos, iterationType, aKeys) { } } - if (strict) { + if (strict && arguments.length === 5) { const symbolKeysA = getOwnPropertySymbols(val1); if (symbolKeysA.length !== 0) { let count = 0; @@ -310,7 +283,7 @@ function keyCheck(val1, val2, strict, memos, iterationType, aKeys) { if (aKeys.length === 0 && (iterationType === kNoIterator || iterationType === kIsArray && val1.length === 0 || - val1.size === 0)) { + val1.size === 0)) { return true; } @@ -578,8 +551,28 @@ function objEquiv(a, b, strict, keys, memos, iterationType) { } } else if (iterationType === kIsArray) { for (; i < a.length; i++) { - if (!innerDeepEqual(a[i], b[i], strict, memos)) { + if (hasOwnProperty(a, i)) { + if (!hasOwnProperty(b, i) || + !innerDeepEqual(a[i], b[i], strict, memos)) { + return false; + } + } else if (hasOwnProperty(b, i)) { return false; + } else { + // Array is sparse. + const keysA = objectKeys(a); + i++; + for (; i < keysA.length; i++) { + const key = keysA[i]; + if (!hasOwnProperty(b, key) || + !innerDeepEqual(a[key], b[i], strict, memos)) { + return false; + } + } + if (keysA.length !== objectKeys(b).length) { + return false; + } + return true; } } } diff --git a/src/node_util.cc b/src/node_util.cc index 41b1307bb4912c..591f3e3b5eb91f 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -16,6 +16,29 @@ using v8::Proxy; using v8::String; using v8::Value; +static void GetOwnNonIndexProperties( + const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + Local context = env->context(); + + if (!args[0]->IsObject()) + return; + + v8::Local object = args[0].As(); + + // Return only non-enumerable properties by default. + v8::Local properties; + + if (!object->GetPropertyNames( + context, v8::KeyCollectionMode::kOwnOnly, + v8::ONLY_ENUMERABLE, + v8::IndexFilter::kSkipIndices) + .ToLocal(&properties)) { + return; + } + args.GetReturnValue().Set(properties); +} + static void GetPromiseDetails(const FunctionCallbackInfo& args) { // Return undefined if it's not a Promise. if (!args[0]->IsPromise()) @@ -177,6 +200,8 @@ void Initialize(Local target, env->SetMethodNoSideEffect(target, "getProxyDetails", GetProxyDetails); env->SetMethodNoSideEffect(target, "safeToString", SafeToString); env->SetMethodNoSideEffect(target, "previewEntries", PreviewEntries); + env->SetMethodNoSideEffect(target, "getOwnNonIndexProperties", + GetOwnNonIndexProperties); env->SetMethod(target, "startSigintWatchdog", StartSigintWatchdog); env->SetMethod(target, "stopSigintWatchdog", StopSigintWatchdog); diff --git a/test/parallel/test-assert-deep.js b/test/parallel/test-assert-deep.js index 04587f3e40b654..ac1e8fa02071f3 100644 --- a/test/parallel/test-assert-deep.js +++ b/test/parallel/test-assert-deep.js @@ -934,6 +934,17 @@ assert.throws(() => assert.deepStrictEqual(new Boolean(true), {}), ' [\n 1,\n- 2\n+ 2,\n+ 3\n ]' } ); util.inspect.defaultOptions = tmp; + + const invalidTrap = new Proxy([1, 2, 3], { + ownKeys() { return []; } + }); + assert.throws( + () => assert.deepStrictEqual(invalidTrap, [1, 2, 3]), + { + name: 'TypeError', + message: "'ownKeys' on proxy: trap result did not include 'length'" + } + ); } // Basic valueOf check.