Skip to content

Commit 975ff4d

Browse files
debadree25V8 LUCI CQ
debadree25
authored and
V8 LUCI CQ
committed
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}
1 parent 99071b0 commit 975ff4d

3 files changed

Lines changed: 113 additions & 2 deletions

File tree

‎AUTHORS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ Darshan Sen <raisinten@gmail.com>
100100
David Carlier <devnexen@gmail.com>
101101
David Manouchehri <david@davidmanouchehri.com>
102102
David Sanders <dsanders11@ucsbalum.com>
103+
Debadree Chatterjee <debadree333@gmail.com>
103104
Deepak Mohan <hop2deep@gmail.com>
104105
Deon Dior <diaoyuanjie@gmail.com>
105106
Derek Tu <derek.t@rioslab.org>

‎src/objects/keys.cc‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,8 @@ ExceptionStatus KeyAccumulator::AddKeys(Handle<JSObject> array_like,
184184
MaybeHandle<FixedArray> FilterProxyKeys(KeyAccumulator* accumulator,
185185
Handle<JSProxy> owner,
186186
Handle<FixedArray> keys,
187-
PropertyFilter filter) {
187+
PropertyFilter filter,
188+
bool skip_indices) {
188189
if (filter == ALL_PROPERTIES) {
189190
// Nothing to do.
190191
return keys;
@@ -194,6 +195,10 @@ MaybeHandle<FixedArray> FilterProxyKeys(KeyAccumulator* accumulator,
194195
for (int i = 0; i < keys->length(); ++i) {
195196
Handle<Name> key(Name::cast(keys->get(i)), isolate);
196197
if (key->FilterKey(filter)) continue; // Skip this key.
198+
if (skip_indices) {
199+
uint32_t index;
200+
if (key->AsArrayIndex(&index)) continue; // Skip this key.
201+
}
197202
if (filter & ONLY_ENUMERABLE) {
198203
PropertyDescriptor desc;
199204
Maybe<bool> found =
@@ -220,7 +225,8 @@ Maybe<bool> KeyAccumulator::AddKeysFromJSProxy(Handle<JSProxy> proxy,
220225
// Postpone the enumerable check for for-in to the ForInFilter step.
221226
if (!is_for_in_) {
222227
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
223-
isolate_, keys, FilterProxyKeys(this, proxy, keys, filter_),
228+
isolate_, keys,
229+
FilterProxyKeys(this, proxy, keys, filter_, skip_indices_),
224230
Nothing<bool>());
225231
}
226232
// https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-ownpropertykeys

‎test/cctest/test-api.cc‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14487,6 +14487,110 @@ THREADED_TEST(ProxyGetPropertyNames) {
1448714487
CheckIsSymbolAt(isolate, properties, 4, "symbol");
1448814488
}
1448914489

14490+
THREADED_TEST(ProxyGetPropertyNamesWithOwnKeysTrap) {
14491+
LocalContext context;
14492+
v8::Isolate* isolate = context->GetIsolate();
14493+
v8::HandleScope scope(isolate);
14494+
v8::Local<v8::Value> result = CompileRun(
14495+
"var target = {0: 0, 1: 1, a: 2, b: 3};"
14496+
"target[2**32] = '4294967296';"
14497+
"target[2**32-1] = '4294967295';"
14498+
"target[2**32-2] = '4294967294';"
14499+
"target[Symbol('symbol')] = true;"
14500+
"target.__proto__ = {__proto__:null, 2: 4, 3: 5, c: 6, d: 7};"
14501+
"var result = new Proxy(target, { ownKeys: (t) => Reflect.ownKeys(t) });"
14502+
"result;");
14503+
v8::Local<v8::Object> object = result.As<v8::Object>();
14504+
v8::PropertyFilter default_filter =
14505+
static_cast<v8::PropertyFilter>(v8::ONLY_ENUMERABLE | v8::SKIP_SYMBOLS);
14506+
v8::PropertyFilter include_symbols_filter = v8::ONLY_ENUMERABLE;
14507+
14508+
v8::Local<v8::Array> properties =
14509+
object->GetPropertyNames(context.local()).ToLocalChecked();
14510+
const char* expected_properties1[] = {"0", "1", "4294967294", "a",
14511+
"b", "4294967296", "4294967295", "2",
14512+
"3", "c", "d"};
14513+
CheckStringArray(isolate, properties, 11, expected_properties1);
14514+
14515+
properties =
14516+
object
14517+
->GetPropertyNames(context.local(),
14518+
v8::KeyCollectionMode::kIncludePrototypes,
14519+
default_filter, v8::IndexFilter::kIncludeIndices)
14520+
.ToLocalChecked();
14521+
CheckStringArray(isolate, properties, 11, expected_properties1);
14522+
14523+
properties = object
14524+
->GetPropertyNames(context.local(),
14525+
v8::KeyCollectionMode::kIncludePrototypes,
14526+
include_symbols_filter,
14527+
v8::IndexFilter::kIncludeIndices)
14528+
.ToLocalChecked();
14529+
const char* expected_properties1_1[] = {
14530+
"0", "1", "4294967294", "a", "b", "4294967296",
14531+
"4294967295", nullptr, "2", "3", "c", "d"};
14532+
CheckStringArray(isolate, properties, 12, expected_properties1_1);
14533+
CheckIsSymbolAt(isolate, properties, 7, "symbol");
14534+
14535+
properties =
14536+
object
14537+
->GetPropertyNames(context.local(),
14538+
v8::KeyCollectionMode::kIncludePrototypes,
14539+
default_filter, v8::IndexFilter::kSkipIndices)
14540+
.ToLocalChecked();
14541+
const char* expected_properties2[] = {"a", "b", "4294967296",
14542+
"4294967295", "c", "d"};
14543+
CheckStringArray(isolate, properties, 6, expected_properties2);
14544+
14545+
properties = object
14546+
->GetPropertyNames(context.local(),
14547+
v8::KeyCollectionMode::kIncludePrototypes,
14548+
include_symbols_filter,
14549+
v8::IndexFilter::kSkipIndices)
14550+
.ToLocalChecked();
14551+
const char* expected_properties2_1[] = {
14552+
"a", "b", "4294967296", "4294967295", nullptr, "c", "d"};
14553+
CheckStringArray(isolate, properties, 7, expected_properties2_1);
14554+
CheckIsSymbolAt(isolate, properties, 4, "symbol");
14555+
14556+
properties =
14557+
object
14558+
->GetPropertyNames(context.local(), v8::KeyCollectionMode::kOwnOnly,
14559+
default_filter, v8::IndexFilter::kIncludeIndices)
14560+
.ToLocalChecked();
14561+
const char* expected_properties3[] = {"0", "1", "4294967294", "a",
14562+
"b", "4294967296", "4294967295"};
14563+
CheckStringArray(isolate, properties, 7, expected_properties3);
14564+
14565+
properties = object
14566+
->GetPropertyNames(
14567+
context.local(), v8::KeyCollectionMode::kOwnOnly,
14568+
include_symbols_filter, v8::IndexFilter::kIncludeIndices)
14569+
.ToLocalChecked();
14570+
const char* expected_properties3_1[] = {
14571+
"0", "1", "4294967294", "a", "b", "4294967296", "4294967295", nullptr};
14572+
CheckStringArray(isolate, properties, 8, expected_properties3_1);
14573+
CheckIsSymbolAt(isolate, properties, 7, "symbol");
14574+
14575+
properties =
14576+
object
14577+
->GetPropertyNames(context.local(), v8::KeyCollectionMode::kOwnOnly,
14578+
default_filter, v8::IndexFilter::kSkipIndices)
14579+
.ToLocalChecked();
14580+
const char* expected_properties4[] = {"a", "b", "4294967296", "4294967295"};
14581+
CheckStringArray(isolate, properties, 4, expected_properties4);
14582+
14583+
properties = object
14584+
->GetPropertyNames(
14585+
context.local(), v8::KeyCollectionMode::kOwnOnly,
14586+
include_symbols_filter, v8::IndexFilter::kSkipIndices)
14587+
.ToLocalChecked();
14588+
const char* expected_properties4_1[] = {"a", "b", "4294967296", "4294967295",
14589+
nullptr};
14590+
CheckStringArray(isolate, properties, 5, expected_properties4_1);
14591+
CheckIsSymbolAt(isolate, properties, 4, "symbol");
14592+
}
14593+
1449014594
THREADED_TEST(AccessChecksReenabledCorrectly) {
1449114595
LocalContext context;
1449214596
v8::Isolate* isolate = context->GetIsolate();

0 commit comments

Comments
 (0)