Non-extensible holey array: a failed `a[i] = v`, `push()` or `defineProperty` past the end still sets `length` (regression for assignment since fce112d)

Cerrado Apto para principiantes
  • ¿Algo no está bien?
#593 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 3 días

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
2/5
Tiempo estimado
1-3 horas
Aptitud para principiantes
74/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
c
Área
backend

Línea de trabajo

Empieza en quickjs.c, en JS_CreateProperty() y la etiqueta generic_array:; sigue cuándo se actualiza la longitud del array con respecto a la comprobación de extensibilidad. Añade un caso de regresión a tests/test_builtin.js para el intento fallido de añadir una propiedad más allá del final de un array disperso no extensible y luego ejecuta make test. La tarea estará terminada cuando la operación falle sin cambiar length y la suite de pruebas pase.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

Environment
  • QuickJS master 712e8c0 (2026-11-02): every case below
  • QuickJS 2026-06-04 (release): only the defineProperty / Reflect.defineProperty case
  • The assignment and push cases bisect to fce112d ("fix super property assignment on non-extensible receiver with existing property", 2026-09-29)
  • make qjs, Debian 12 x86-64, gcc 12.2. Reference: node v26.13.0
  • Checked on 2026-11-03
Reproduction
"use strict";
var b = [1, , 3];                    // a hole: not a fast array
Object.preventExtensions(b);
try { b[3] = 9; } catch (e) { console.log(e.name); }
console.log(b.length);               // expected: TypeError, 3     master: TypeError, 4

var c = [1, , 3]; Object.preventExtensions(c);
try { c.push(9); } catch (e) { console.log(e.name); }
console.log(c.length);               // expected: TypeError, 3     master: TypeError, 4

var s = [1, , 3]; Object.seal(s);
try { s[5] = 1; } catch (e) { console.log(e.name); }
console.log(s.length);               // expected: TypeError, 3     master: TypeError, 6

var o = [1, , 3]; Object.preventExtensions(o);
console.log(Reflect.defineProperty(o, 7, { value: 1 }), o.length);
                                     // expected: false 3          master and 2026-06-04: false 8

The element itself is not created (Object.getOwnPropertyNames(b) stays 0,2,length); only length changes. Arrays
without holes are not affected.

Expected (spec)

Array [[DefineOwnProperty]] for an array
index: Let succeeded be ! OrdinaryDefineOwnProperty(A, P, Desc). If succeeded is false, return false. Only after that:
If index ≥ length, then Set lengthDesc.[[Value]] to index + 1 …. A new property on a non-extensible object is rejected,
so length must not change. b[3] = 9 and push reach the same method through OrdinarySet / CreateDataProperty.

Cause and patch

JS_CreateProperty(), Array case, label generic_array:, updates length (XXX: should update the length after defining the property) before the if (!p->extensible) goto not_extensible check further down. Before fce112d,
JS_SetPropertyInternal() rejected non-extensible receivers before calling JS_CreateProperty(), so only the define path
showed the bug. fce112d moved that test ("the extensibility test is included in JS_CreateProperty()"), which exposes the
early length update to ordinary assignment and push.

diff --git a/quickjs.c b/quickjs.c
--- a/quickjs.c
+++ b/quickjs.c
@@ -10374,6 +10374,9 @@ static int JS_CreateProperty(JSContext *ctx, JSObject *p,
                 JSProperty *plen;
                 JSShapeProperty *pslen;
             generic_array:
+                /* the length must not change if the property cannot be added */
+                if (!p->extensible)
+                    goto not_extensible;
                 /* update the length field */
                 plen = &p->prop[0];
                 JS_ToUint32(ctx, &len, plen->u.value);

With the patch the cases above match node, make test passes (a test is added to tests/test_builtin.js), and test262
shows no new failures against test262_errors.txt.

Related
  • quickjs-ng has the defineProperty half of this: quickjs-ng/quickjs#1882
Lenguaje dominante
C
Estrellas
11.1k
Forks
1.2k
Merge medio
11 h 20 min
PR fusionados (30 d)
1

Preparar el entorno

Este proyecto no incluye contenedor de desarrollo, Dockerfile ni guía de contribución, así que la configuración corre por tu cuenta: empieza por su README y consulta nuestra guía para la primera contribución para los pasos generales.

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de bellard/quickjs

Todos los issues de bellard/quickjs

Issues similares

Más issues de C

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.