From 5e7de3061b205c701f80322599107b824d87e455 Mon Sep 17 00:00:00 2001 From: Lucy Date: Sat, 24 Oct 2026 10:42:35 +0200 Subject: [PATCH 1/3] Write back every element in Array.prototype.sort sort skipped the write for elements that were already in place. The spec writes every sorted value back with Set(obj, j, v, true), so a read-only receiver such as a String or a frozen array must throw, and setters must see each write. Always write the value back. This made no measurable difference when sorting a large already-sorted array. Fixes: https://github.com/quickjs-ng/quickjs/issues/1736 --- quickjs.c | 10 +++------- tests/array-sort-write-back.js | 35 ++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 7 deletions(-) create mode 100644 tests/array-sort-write-back.js diff --git a/quickjs.c b/quickjs.c index af401ae37..47177e7ad 100644 --- a/quickjs.c +++ b/quickjs.c @@ -44802,13 +44802,9 @@ static JSValue js_array_sort(JSContext *ctx, JSValueConst this_val, while (n < pos) { if (array[n].str) JS_FreeValue(ctx, JS_MKPTR(JS_TAG_STRING, array[n].str)); - if (array[n].pos == n) { - JS_FreeValue(ctx, array[n].val); - } else { - if (JS_SetPropertyInt64(ctx, obj, n, array[n].val) < 0) { - n++; - goto exception; - } + if (JS_SetPropertyInt64(ctx, obj, n, array[n].val) < 0) { + n++; + goto exception; } n++; } diff --git a/tests/array-sort-write-back.js b/tests/array-sort-write-back.js new file mode 100644 index 000000000..058e5d14c --- /dev/null +++ b/tests/array-sort-write-back.js @@ -0,0 +1,35 @@ +import { assert } from "./assert.js"; + +/* Array.prototype.sort writes every sorted value back with Set(..., true), + even ones that are already in place. */ + +function throwsTypeError(f) { + try { + f(); + } catch (e) { + return e instanceof TypeError; + } + return false; +} + +/* a String's indices are read-only */ +assert(throwsTypeError(() => Array.prototype.sort.call("aa")), true); +assert(throwsTypeError(() => Array.prototype.sort.call("ab")), true); + +/* so are a frozen array's, even when it is already sorted */ +assert(throwsTypeError(() => Object.freeze([1, 2]).sort()), true); + +/* setters see every write */ +{ + const writes = []; + const o = { length: 2 }; + for (const i of [0, 1]) { + let v = i; + Object.defineProperty(o, i, { + get: () => v, + set: x => { writes.push(i); v = x; }, + }); + } + Array.prototype.sort.call(o); + assert(writes.join(), "0,1"); +} From 76de57b35e4840b1de1188f50a92b5a5328febcb Mon Sep 17 00:00:00 2001 From: Lucy Date: Sat, 24 Oct 2026 13:48:00 +0200 Subject: [PATCH 2/3] Increment n once per iteration in the sort write-back loop --- quickjs.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/quickjs.c b/quickjs.c index 47177e7ad..26904fd40 100644 --- a/quickjs.c +++ b/quickjs.c @@ -44800,13 +44800,11 @@ static JSValue js_array_sort(JSContext *ctx, JSValueConst this_val, /* XXX: should special case fast arrays */ while (n < pos) { - if (array[n].str) - JS_FreeValue(ctx, JS_MKPTR(JS_TAG_STRING, array[n].str)); - if (JS_SetPropertyInt64(ctx, obj, n, array[n].val) < 0) { - n++; + i = n++; + if (array[i].str) + JS_FreeValue(ctx, JS_MKPTR(JS_TAG_STRING, array[i].str)); + if (JS_SetPropertyInt64(ctx, obj, i, array[i].val) < 0) goto exception; - } - n++; } js_free(ctx, array); for (i = n; undefined_count-- > 0; i++) { From 1431684ead9ad19d88a9ef4ac011ff1b93770ab4 Mon Sep 17 00:00:00 2001 From: Lucy Date: Sat, 24 Oct 2026 13:48:00 +0200 Subject: [PATCH 3/3] Use assertThrows in the sort write-back test --- tests/array-sort-write-back.js | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/tests/array-sort-write-back.js b/tests/array-sort-write-back.js index 058e5d14c..dba0dc669 100644 --- a/tests/array-sort-write-back.js +++ b/tests/array-sort-write-back.js @@ -1,23 +1,14 @@ -import { assert } from "./assert.js"; +import { assert, assertThrows } from "./assert.js"; /* Array.prototype.sort writes every sorted value back with Set(..., true), even ones that are already in place. */ -function throwsTypeError(f) { - try { - f(); - } catch (e) { - return e instanceof TypeError; - } - return false; -} - /* a String's indices are read-only */ -assert(throwsTypeError(() => Array.prototype.sort.call("aa")), true); -assert(throwsTypeError(() => Array.prototype.sort.call("ab")), true); +assertThrows(TypeError, () => Array.prototype.sort.call("aa")); +assertThrows(TypeError, () => Array.prototype.sort.call("ab")); /* so are a frozen array's, even when it is already sorted */ -assert(throwsTypeError(() => Object.freeze([1, 2]).sort()), true); +assertThrows(TypeError, () => Object.freeze([1, 2]).sort()); /* setters see every write */ {