Fix Proxy [[OwnPropertyKeys]] check for integer-index keys (#1609)

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<PseudoHandle<JSArray>> 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<PseudoHandle<StringPrimitive>> 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

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)