diff --git a/lib/VM/JSProxy.cpp b/lib/VM/JSProxy.cpp index ab0ec4fb0f8..0bd1a7f1dc7 100644 --- a/lib/VM/JSProxy.cpp +++ b/lib/VM/JSProxy.cpp @@ -12,6 +12,7 @@ #include "hermes/VM/JSArray.h" #include "hermes/VM/JSCallableProxy.h" #include "hermes/VM/JSMapImpl.h" +#include "hermes/VM/Operations.h" #include "hermes/VM/PropertyAccessor.h" #include "llvh/ADT/SetVector.h" @@ -1496,6 +1497,40 @@ CallResult> JSProxy::ownPropertyKeys( // a. Return trapResult. return filterKeys(selfHandle, trapResult, runtime, okFlags); } + // Hermes represents integer-index own-property keys internally as numbers, + // but [[OwnPropertyKeys]] is specified to return only Strings and Symbols + // (see the assert on step 12 above), and the trap result was already + // validated to contain only Strings and Symbols. Convert any numeric index + // keys in targetKeys to their canonical string form so the SameValue + // comparisons below behave correctly; otherwise an integer-index key such as + // "12345" on a non-extensible target would never match its string form in + // the trap result. See https://github.com/facebook/hermes/issues/1609. + { + GCScopeMarkerRAII normalizeMarker{runtime}; + for (uint32_t i = 0, len = JSArray::getLength(*targetKeys, runtime); + i < len; + ++i) { + HermesValue key = targetKeys->at(runtime, i).unboxToHV(runtime); + if (!key.isNumber()) { + continue; + } + normalizeMarker.flush(); + CallResult> keyStrRes = + numberToStringPrimitive(runtime, key.getNumber()); + if (LLVM_UNLIKELY(keyStrRes == ExecutionStatus::EXCEPTION)) { + return ExecutionStatus::EXCEPTION; + } + if (LLVM_UNLIKELY( + JSArray::setElementAt( + targetKeys, + runtime, + i, + runtime.makeHandle(std::move(*keyStrRes))) == + ExecutionStatus::EXCEPTION)) { + return ExecutionStatus::EXCEPTION; + } + } + } // 18. Let uncheckedResultKeys be a new List which is a copy of trapResult. // 19. For each key that is an element of targetNonconfigurableKeys, do // a. If key is not an element of uncheckedResultKeys, throw a TypeError diff --git a/test/hermes/regress-proxy-ownkeys-numeric-key.js b/test/hermes/regress-proxy-ownkeys-numeric-key.js new file mode 100644 index 00000000000..7d649c2cb55 --- /dev/null +++ b/test/hermes/regress-proxy-ownkeys-numeric-key.js @@ -0,0 +1,58 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +// RUN: %hermes -Xes6-proxy -non-strict -O -target=HBC %s | %FileCheck --match-full-lines %s + +// Regression test for https://github.com/facebook/hermes/issues/1609 +// +// A Proxy [[OwnPropertyKeys]] over a non-extensible target used to throw +// "ownKeys target is non-extensible but key is missing from trap result" +// whenever the target had an integer-index own key (e.g. "12345"). Hermes +// represents integer-index keys internally as numbers, but the ownKeys trap +// result holds them (per spec) as strings, so the SameValue comparison in the +// non-extensible invariant check never matched. + +function ownKeysTrap(t) { + return Reflect.ownKeys(t); +} + +// Case 1: preventExtensions with a configurable integer-index key. +// Exercises the "target configurable keys" invariant loop. +var t1 = {}; +Object.defineProperty(t1, '12345', { + value: 1, + enumerable: true, + configurable: true, + writable: true, +}); +Object.preventExtensions(t1); +var p1 = new Proxy(t1, {ownKeys: ownKeysTrap}); +print('case1:', Object.keys(p1).join(',')); +// CHECK: case1: 12345 + +// Case 2: frozen (non-configurable) integer-index keys. +// Exercises the "target non-configurable keys" invariant loop. +var t2 = Object.freeze({0: 'a', 42: 'b'}); +var p2 = new Proxy(t2, {ownKeys: ownKeysTrap}); +print('case2:', Object.getOwnPropertyNames(p2).join(',')); +// CHECK-NEXT: case2: 0,42 + +// Case 3: mixed integer-index and string keys, frozen; spec ordering is +// ascending integer indices first, then string keys in insertion order. +var t3 = Object.freeze({b: 1, 2: 2, a: 3, 1: 4}); +var p3 = new Proxy(t3, {ownKeys: ownKeysTrap}); +print('case3:', Reflect.ownKeys(p3).join(',')); +// CHECK-NEXT: case3: 1,2,b,a + +// Case 4: integer-index keys alongside symbols must still work. +var s = Symbol('s'); +var t4 = {7: 'x'}; +t4[s] = 'y'; +Object.preventExtensions(t4); +var p4 = new Proxy(t4, {ownKeys: ownKeysTrap}); +print('case4:', Reflect.ownKeys(p4).map(String).join(',')); +// CHECK-NEXT: case4: 7,Symbol(s)