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

Aperta Adatta ai principianti
  • Qualcosa non va?
#593 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 2 giorni

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
2/5
Tempo stimato
1-3 ore
Idoneità per principianti
74/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
c
Ambito
backend

Direzione di ricerca

Inizia in quickjs.c, in corrispondenza di JS_CreateProperty() e dell’etichetta generic_array:; segui quando viene aggiornata la lunghezza dell’array rispetto al controllo di estensibilità. Aggiungi un caso di regressione a tests/test_builtin.js per il tentativo non riuscito di aggiungere una proprietà oltre la fine di un array sparso non estensibile, quindi esegui make test. Il lavoro è completato quando l’operazione fallisce senza modificare length e la suite di test viene superata.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

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
Lingua principale
C
Stelle
11.1k
Fork
1.2k
Merge medio
11h 20m
PR unite (30g)
1

Preparare l'ambiente

Questo progetto non fornisce container di sviluppo, Dockerfile né guida per i contributori, quindi l'ambiente è a tuo carico: parti dal suo README e consulta la nostra guida al primo contributo per i passaggi generali.

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di bellard/quickjs

Tutte le issue di bellard/quickjs

Issue simili

Altre issue su C

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.