Skip to content

Commit

Permalink
deps: V8: cherry-pick 975ff4dbfd1b
Browse files Browse the repository at this point in the history
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 <cbruni@chromium.org>
    Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#86548}

Refs: v8/v8@975ff4d
PR-URL: #47209
Fixes: #41714
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
  • Loading branch information
debadree25 authored and RafaelGSS committed Apr 7, 2023
1 parent b8c6ced commit 8a69929
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 3 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Expand Up @@ -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 #####

Expand Down
1 change: 1 addition & 0 deletions deps/v8/AUTHORS
Expand Up @@ -98,6 +98,7 @@ Darshan Sen <raisinten@gmail.com>
David Carlier <devnexen@gmail.com>
David Manouchehri <david@davidmanouchehri.com>
David Sanders <dsanders11@ucsbalum.com>
Debadree Chatterjee <debadree333@gmail.com>
Deepak Mohan <hop2deep@gmail.com>
Deon Dior <diaoyuanjie@gmail.com>
Derek Tu <derek.t@rioslab.org>
Expand Down
10 changes: 8 additions & 2 deletions deps/v8/src/objects/keys.cc
Expand Up @@ -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;
Expand All @@ -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 =
Expand All @@ -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
Expand Down
104 changes: 104 additions & 0 deletions deps/v8/test/cctest/test-api.cc
Expand Up @@ -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();
Expand Down

0 comments on commit 8a69929

Please sign in to comment.