From 8a69929f23de27a931414e43634ab4374b3a5a6a Mon Sep 17 00:00:00 2001 From: Debadree Chatterjee Date: Fri, 24 Mar 2023 12:50:31 +0530 Subject: [PATCH] deps: V8: cherry-pick 975ff4dbfd1b MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit message: fix GetPropertyNames for proxys with ownKeys trap Added checks to FilterProxyKeys function for when skip_indices is enabled. Bug: v8:13728 Change-Id: Id096e32ef8e6c2344be9682e8222aea8790bd66d Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4333698 Reviewed-by: Camillo Bruni Commit-Queue: Jakob Kummerow Cr-Commit-Position: refs/heads/main@{#86548} Refs: https://github.com/v8/v8/commit/975ff4dbfd1be3a7395e26d412774bc955b47341 PR-URL: https://github.com/nodejs/node/pull/47209 Fixes: https://github.com/nodejs/node/issues/41714 Reviewed-By: Michaël Zasso Reviewed-By: Jiawen Geng Reviewed-By: Richard Lau Reviewed-By: Erick Wendel --- common.gypi | 2 +- deps/v8/AUTHORS | 1 + deps/v8/src/objects/keys.cc | 10 ++- deps/v8/test/cctest/test-api.cc | 104 ++++++++++++++++++++++++++++++++ 4 files changed, 114 insertions(+), 3 deletions(-) diff --git a/common.gypi b/common.gypi index 6f8518cc8e23a7..70569b3e3fe785 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.13', + 'v8_embedder_string': '-node.14', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/AUTHORS b/deps/v8/AUTHORS index af37f8db25121e..38e5cd01b2d41d 100644 --- a/deps/v8/AUTHORS +++ b/deps/v8/AUTHORS @@ -98,6 +98,7 @@ Darshan Sen David Carlier David Manouchehri David Sanders +Debadree Chatterjee Deepak Mohan Deon Dior Derek Tu diff --git a/deps/v8/src/objects/keys.cc b/deps/v8/src/objects/keys.cc index ac21fbf9c3d2d2..001008f3b9d268 100644 --- a/deps/v8/src/objects/keys.cc +++ b/deps/v8/src/objects/keys.cc @@ -182,7 +182,8 @@ ExceptionStatus KeyAccumulator::AddKeys(Handle array_like, MaybeHandle FilterProxyKeys(KeyAccumulator* accumulator, Handle owner, Handle keys, - PropertyFilter filter) { + PropertyFilter filter, + bool skip_indices) { if (filter == ALL_PROPERTIES) { // Nothing to do. return keys; @@ -192,6 +193,10 @@ MaybeHandle FilterProxyKeys(KeyAccumulator* accumulator, for (int i = 0; i < keys->length(); ++i) { Handle key(Name::cast(keys->get(i)), isolate); if (key->FilterKey(filter)) continue; // Skip this key. + if (skip_indices) { + uint32_t index; + if (key->AsArrayIndex(&index)) continue; // Skip this key. + } if (filter & ONLY_ENUMERABLE) { PropertyDescriptor desc; Maybe found = @@ -218,7 +223,8 @@ Maybe KeyAccumulator::AddKeysFromJSProxy(Handle proxy, // Postpone the enumerable check for for-in to the ForInFilter step. if (!is_for_in_) { ASSIGN_RETURN_ON_EXCEPTION_VALUE( - isolate_, keys, FilterProxyKeys(this, proxy, keys, filter_), + isolate_, keys, + FilterProxyKeys(this, proxy, keys, filter_, skip_indices_), Nothing()); } // https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-ownpropertykeys diff --git a/deps/v8/test/cctest/test-api.cc b/deps/v8/test/cctest/test-api.cc index 5e13f3b4bae67f..755e12bb3d51fa 100644 --- a/deps/v8/test/cctest/test-api.cc +++ b/deps/v8/test/cctest/test-api.cc @@ -14429,6 +14429,110 @@ THREADED_TEST(ProxyGetPropertyNames) { CheckIsSymbolAt(isolate, properties, 4, "symbol"); } +THREADED_TEST(ProxyGetPropertyNamesWithOwnKeysTrap) { + LocalContext context; + v8::Isolate* isolate = context->GetIsolate(); + v8::HandleScope scope(isolate); + v8::Local result = CompileRun( + "var target = {0: 0, 1: 1, a: 2, b: 3};" + "target[2**32] = '4294967296';" + "target[2**32-1] = '4294967295';" + "target[2**32-2] = '4294967294';" + "target[Symbol('symbol')] = true;" + "target.__proto__ = {__proto__:null, 2: 4, 3: 5, c: 6, d: 7};" + "var result = new Proxy(target, { ownKeys: (t) => Reflect.ownKeys(t) });" + "result;"); + v8::Local object = result.As(); + v8::PropertyFilter default_filter = + static_cast(v8::ONLY_ENUMERABLE | v8::SKIP_SYMBOLS); + v8::PropertyFilter include_symbols_filter = v8::ONLY_ENUMERABLE; + + v8::Local properties = + object->GetPropertyNames(context.local()).ToLocalChecked(); + const char* expected_properties1[] = {"0", "1", "4294967294", "a", + "b", "4294967296", "4294967295", "2", + "3", "c", "d"}; + CheckStringArray(isolate, properties, 11, expected_properties1); + + properties = + object + ->GetPropertyNames(context.local(), + v8::KeyCollectionMode::kIncludePrototypes, + default_filter, v8::IndexFilter::kIncludeIndices) + .ToLocalChecked(); + CheckStringArray(isolate, properties, 11, expected_properties1); + + properties = object + ->GetPropertyNames(context.local(), + v8::KeyCollectionMode::kIncludePrototypes, + include_symbols_filter, + v8::IndexFilter::kIncludeIndices) + .ToLocalChecked(); + const char* expected_properties1_1[] = { + "0", "1", "4294967294", "a", "b", "4294967296", + "4294967295", nullptr, "2", "3", "c", "d"}; + CheckStringArray(isolate, properties, 12, expected_properties1_1); + CheckIsSymbolAt(isolate, properties, 7, "symbol"); + + properties = + object + ->GetPropertyNames(context.local(), + v8::KeyCollectionMode::kIncludePrototypes, + default_filter, v8::IndexFilter::kSkipIndices) + .ToLocalChecked(); + const char* expected_properties2[] = {"a", "b", "4294967296", + "4294967295", "c", "d"}; + CheckStringArray(isolate, properties, 6, expected_properties2); + + properties = object + ->GetPropertyNames(context.local(), + v8::KeyCollectionMode::kIncludePrototypes, + include_symbols_filter, + v8::IndexFilter::kSkipIndices) + .ToLocalChecked(); + const char* expected_properties2_1[] = { + "a", "b", "4294967296", "4294967295", nullptr, "c", "d"}; + CheckStringArray(isolate, properties, 7, expected_properties2_1); + CheckIsSymbolAt(isolate, properties, 4, "symbol"); + + properties = + object + ->GetPropertyNames(context.local(), v8::KeyCollectionMode::kOwnOnly, + default_filter, v8::IndexFilter::kIncludeIndices) + .ToLocalChecked(); + const char* expected_properties3[] = {"0", "1", "4294967294", "a", + "b", "4294967296", "4294967295"}; + CheckStringArray(isolate, properties, 7, expected_properties3); + + properties = object + ->GetPropertyNames( + context.local(), v8::KeyCollectionMode::kOwnOnly, + include_symbols_filter, v8::IndexFilter::kIncludeIndices) + .ToLocalChecked(); + const char* expected_properties3_1[] = { + "0", "1", "4294967294", "a", "b", "4294967296", "4294967295", nullptr}; + CheckStringArray(isolate, properties, 8, expected_properties3_1); + CheckIsSymbolAt(isolate, properties, 7, "symbol"); + + properties = + object + ->GetPropertyNames(context.local(), v8::KeyCollectionMode::kOwnOnly, + default_filter, v8::IndexFilter::kSkipIndices) + .ToLocalChecked(); + const char* expected_properties4[] = {"a", "b", "4294967296", "4294967295"}; + CheckStringArray(isolate, properties, 4, expected_properties4); + + properties = object + ->GetPropertyNames( + context.local(), v8::KeyCollectionMode::kOwnOnly, + include_symbols_filter, v8::IndexFilter::kSkipIndices) + .ToLocalChecked(); + const char* expected_properties4_1[] = {"a", "b", "4294967296", "4294967295", + nullptr}; + CheckStringArray(isolate, properties, 5, expected_properties4_1); + CheckIsSymbolAt(isolate, properties, 4, "symbol"); +} + THREADED_TEST(AccessChecksReenabledCorrectly) { LocalContext context; v8::Isolate* isolate = context->GetIsolate();