fix(napi/parser): NodeArray allow setting large integer properties (#11883)

overlookmotel · overlookmotel · commit cf0e18ae6d96 · 2025-06-24T15:35:07.000Z
Integers `&gt;= 4294967295` are treated as string properties, not numerical array indexes. Make `NodeArray` behave the same as `Array` for such properties, and allow user to set them.
diff --git a/napi/parser/raw-transfer/node-array.js b/napi/parser/raw-transfer/node-array.js
@@ -258,7 +258,8 @@ const PROXY_HANDLERS = {
   // Return `true` for indexes which are in bounds.
   // e.g. `'0' in arr`.
   has(arr, key) {
-    if (isIndex(key)) return key * 1 < getLength(arr);
+    const index = toIndex(key);
+    if (index !== null) return index < getLength(arr);
     return Reflect.has(arr, key);
   },
 
@@ -268,7 +269,8 @@ const PROXY_HANDLERS = {
     // They can "unwrap" the proxy by getting `this[ARRAY]`.
     if (key === ARRAY) return arr;
     if (key === 'length') return getLength(arr);
-    if (isIndex(key)) return getElement(arr, key * 1);
+    const index = toIndex(key);
+    if (index !== null) return getElement(arr, index);
 
     return Reflect.get(arr, key);
   },
@@ -280,8 +282,9 @@ const PROXY_HANDLERS = {
       return { value: getLength(arr), writable: true, enumerable: false, configurable: false };
     }
 
-    if (isIndex(key)) {
-      const value = getElement(arr, key * 1);
+    const index = toIndex(key);
+    if (index !== null) {
+      const value = getElement(arr, index);
       if (value === void 0) return void 0;
       // Cannot return `configurable: false` unfortunately
       return { value, writable: false, enumerable: true, configurable: true };
@@ -298,14 +301,14 @@ const PROXY_HANDLERS = {
   // * `Object.defineProperty(arr, 'length', {value: 0})`.
   // * Other operations which mutate entries e.g. `arr.push(123)`.
   defineProperty(arr, key, descriptor) {
-    if (key === 'length' || isIndex(key)) return false;
+    if (key === 'length' || toIndex(key) !== null) return false;
     return Reflect.defineProperty(arr, key, descriptor);
   },
 
   // Prevent deleting entries.
   deleteProperty(arr, key) {
     // Note: `Reflect.deleteProperty(arr, 'length')` already returns `false`
-    if (isIndex(key)) return false;
+    if (toIndex(key) !== null) return false;
     return Reflect.deleteProperty(arr, key);
   },
 
@@ -322,16 +325,24 @@ const PROXY_HANDLERS = {
 };
 
 /**
- * Check if a key is a valid array index.
+ * Convert key to array index, if it is a valid array index.
+ *
  * Only strings comprising a plain integer are valid indexes.
  * e.g. `"-1"`, `"01"`, `"0xFF"`, `"1e1"`, `"1 "` are not valid indexes.
+ * Integers >= 4294967295 are not valid indexes.
  *
- * @param {*} - Key used for property lookup.
- * @returns {boolean} - `true` if `key` is a valid array index.
+ * @param {string|Symbol} - Key used for property lookup.
+ * @returns {number|null} - `key` converted to integer, if it's a valid array index, otherwise `null`.
  */
-function isIndex(key) {
-  // TODO: Any way to do this without a regex?
-  return typeof key === 'string' && (key === '0' || INDEX_REGEX.test(key));
+function toIndex(key) {
+  if (typeof key === 'string') {
+    if (key === '0') return 0;
+    if (INDEX_REGEX.test(key)) {
+      const index = +key;
+      if (index < 4294967295) return index;
+    }
+  }
+  return null;
 }
 
 const INDEX_REGEX = /^[1-9]\d*$/;
diff --git a/napi/parser/test/lazy-deserialization.test.ts b/napi/parser/test/lazy-deserialization.test.ts
@@ -530,6 +530,8 @@ describe('NodeArray', () => {
     expect(() => body[1] = {}).toThrow(new TypeError('Cannot redefine property: 1'));
     expect(() => body[2] = {})
       .toThrow(new TypeError("'defineProperty' on proxy: trap returned falsish for property '2'"));
+    expect(() => body[4294967294] = {})
+      .toThrow(new TypeError("'defineProperty' on proxy: trap returned falsish for property '4294967294'"));
   });
 
   it('set element via `defineProperty` (throws)', () => {
@@ -540,28 +542,45 @@ describe('NodeArray', () => {
       .toThrow(new TypeError("'defineProperty' on proxy: trap returned falsish for property '1'"));
     expect(() => Object.defineProperty(body, 2, { value: {} }))
       .toThrow(new TypeError("'defineProperty' on proxy: trap returned falsish for property '2'"));
+    expect(() => Object.defineProperty(body, 4294967294, { value: {} }))
+      .toThrow(new TypeError("'defineProperty' on proxy: trap returned falsish for property '4294967294'"));
   });
 
+  const propertyKeys = [
+    'foo',
+    'bar',
+    Symbol('yeah'),
+    -1,
+    '01',
+    '0x1',
+    '1 ',
+    ' 1',
+    '1e1',
+    '4294967295',
+    '4294967296',
+    '10000000000000',
+  ];
+
   it('set properties', () => {
     const { body } = parseSyncLazy('test.js', 'let x = 1; x = 2;').program;
 
-    const keys = [
-      'foo',
-      'bar',
-      Symbol('yeah'),
-      -1,
-      '01',
-      '0x1',
-      '1 ',
-      ' 1',
-      '1e1',
-    ];
-
-    for (const [i, key] of keys.entries()) {
+    for (const [i, key] of propertyKeys.entries()) {
       body[key] = i + 100;
     }
 
-    for (const [i, key] of keys.entries()) {
+    for (const [i, key] of propertyKeys.entries()) {
+      expect(body[key]).toBe(i + 100);
+    }
+  });
+
+  it('set properties via `defineProperty`', () => {
+    const { body } = parseSyncLazy('test.js', 'let x = 1; x = 2;').program;
+
+    for (const [i, key] of propertyKeys.entries()) {
+      Object.defineProperty(body, key, { value: i + 100 });
+    }
+
+    for (const [i, key] of propertyKeys.entries()) {
       expect(body[key]).toBe(i + 100);
     }
   });
