deps: V8: cherry-pick 975ff4dbfd1b

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 <[email protected]> Commit-Queue: Jakob Kummerow <[email protected]> Cr-Commit-Position: refs/heads/main@{#86548} Refs: v8/v8@975ff4d PR-URL: #47209 Fixes: #41714 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Erick Wendel <[email protected]>
nodejs · Apr 7, 2023 · 8a69929 · 8a69929
1 parent b8c6ced
commit 8a69929
Show file tree

Hide file tree

Showing 4 changed files with 114 additions and 3 deletions.
diff --git 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
@@ -98,6 +98,7 @@ Darshan Sen <[email protected]>
 David Carlier <[email protected]>
 David Manouchehri <[email protected]>
 David Sanders <[email protected]>
+Debadree Chatterjee <[email protected]>
 Deepak Mohan <[email protected]>
 Deon Dior <[email protected]>
 Derek Tu <[email protected]>

diff --git a/deps/v8/src/objects/keys.cc b/deps/v8/src/objects/keys.cc
@@ -182,7 +182,8 @@ ExceptionStatus KeyAccumulator::AddKeys(Handle<JSObject> array_like,
 MaybeHandle<FixedArray> FilterProxyKeys(KeyAccumulator* accumulator,
                                         Handle<JSProxy> owner,
                                         Handle<FixedArray> keys,
-                                        PropertyFilter filter) {
+                                        PropertyFilter filter,
+                                        bool skip_indices) {
   if (filter == ALL_PROPERTIES) {
     // Nothing to do.
     return keys;
@@ -192,6 +193,10 @@ MaybeHandle<FixedArray> FilterProxyKeys(KeyAccumulator* accumulator,
   for (int i = 0; i < keys->length(); ++i) {
     Handle<Name> 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<bool> found =
@@ -218,7 +223,8 @@ Maybe<bool> KeyAccumulator::AddKeysFromJSProxy(Handle<JSProxy> 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<bool>());
   }
   // 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
@@ -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<v8::Value> 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<v8::Object> object = result.As<v8::Object>();
+  v8::PropertyFilter default_filter =
+      static_cast<v8::PropertyFilter>(v8::ONLY_ENUMERABLE | v8::SKIP_SYMBOLS);
+  v8::PropertyFilter include_symbols_filter = v8::ONLY_ENUMERABLE;
+
+  v8::Local<v8::Array> 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();