From 761ed9eec009cd4ba27441980704d8b4da77b4ae Mon Sep 17 00:00:00 2001 From: Daijiro Wachi Date: Sun, 28 Jun 2026 02:56:39 +0900 Subject: [PATCH] fix(url): validate name in URLSearchParams.append() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit URLSearchParams.append() never checked whether stringifying the name argument succeeded. For a non-stringifiable name (a Symbol, or an object whose toString throws), core::encode_spec_string returns a SpecString with a null data pointer and a pending exception, which append handed straight to the Rust params_append — crashing the process in slice::from_raw_parts (UB in release) instead of throwing. Null-check the encoded name and propagate the pending exception, exactly as delete()/has()/get()/set() already do. Add an integration test. Signed-off-by: Daijiro Wachi --- builtins/web/text-codec/text-decoder.cpp | 4 ++- builtins/web/url.cpp | 7 ++-- tests/integration/handlers.js | 2 ++ tests/integration/textdecoder/textdecoder.js | 30 ++++++++++++++++ .../urlsearchparams/urlsearchparams.js | 36 +++++++++++++++++++ tests/tests.cmake | 2 ++ 6 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 tests/integration/textdecoder/textdecoder.js create mode 100644 tests/integration/urlsearchparams/urlsearchparams.js diff --git a/builtins/web/text-codec/text-decoder.cpp b/builtins/web/text-codec/text-decoder.cpp index a5bd84e6..610831fd 100644 --- a/builtins/web/text-codec/text-decoder.cpp +++ b/builtins/web/text-codec/text-decoder.cpp @@ -39,7 +39,9 @@ bool TextDecoder::decode(JSContext *cx, unsigned argc, JS::Value *vp) { } bool stream = false; - if (args.hasDefined(1)) { + // `options` is a WebIDL dictionary: `null` (like `undefined`) means use the + // defaults, matching how the constructor treats it. + if (args.hasDefined(1) && !args.get(1).isNull()) { auto options_value = args.get(1); if (!options_value.isObject()) { return api::throw_error(cx, api::Errors::TypeError, "TextDecoder.decode", diff --git a/builtins/web/url.cpp b/builtins/web/url.cpp index e240ea50..9298be27 100644 --- a/builtins/web/url.cpp +++ b/builtins/web/url.cpp @@ -219,8 +219,11 @@ jsurl::SpecSlice URLSearchParams::serialize(JSContext *cx, JS::HandleObject self bool URLSearchParams::append(JSContext *cx, unsigned argc, JS::Value *vp) { METHOD_HEADER(2) - auto value = validate(cx, args[0], "append"); - if (!append_impl(cx, self, value, args[1], "append")) { + auto name = validate(cx, args[0], "append"); + if (!name.data) { + return false; + } + if (!append_impl(cx, self, name, args[1], "append")) { return false; } diff --git a/tests/integration/handlers.js b/tests/integration/handlers.js index fc07035a..0e272b6e 100644 --- a/tests/integration/handlers.js +++ b/tests/integration/handlers.js @@ -5,3 +5,5 @@ export { handler as crypto } from './crypto/crypto.js'; export { handler as timers } from './timers/timers.js'; export { handler as fetch } from './fetch/fetch.js'; export { handler as event } from './event/event.js'; +export { handler as urlsearchparams } from './urlsearchparams/urlsearchparams.js'; +export { handler as textdecoder } from './textdecoder/textdecoder.js'; diff --git a/tests/integration/textdecoder/textdecoder.js b/tests/integration/textdecoder/textdecoder.js new file mode 100644 index 00000000..23cd0fb6 --- /dev/null +++ b/tests/integration/textdecoder/textdecoder.js @@ -0,0 +1,30 @@ +import { serveTest } from "../test-server.js"; +import { strictEqual, throws } from "../../assert.js"; + +const AB = new Uint8Array([0x61, 0x62]); // "ab" + +export const handler = serveTest(async (t) => { + await t.test("decode-options-null-uses-defaults", () => { + // `options` is a WebIDL dictionary: null means defaults, not a type error. + strictEqual(new TextDecoder().decode(AB, null), "ab", "null options decode"); + }); + + await t.test("decode-options-undefined-and-omitted", () => { + strictEqual(new TextDecoder().decode(AB, undefined), "ab", "undefined options"); + strictEqual(new TextDecoder().decode(AB), "ab", "omitted options"); + }); + + await t.test("decode-options-object", () => { + strictEqual(new TextDecoder().decode(AB, { stream: false }), "ab", "object options"); + }); + + await t.test("decode-options-non-object-still-throws", () => { + // A defined, non-null, non-object value is still a TypeError. + throws(() => new TextDecoder().decode(AB, "nope"), TypeError); + throws(() => new TextDecoder().decode(AB, 42), TypeError); + }); + + await t.test("constructor-options-null-uses-defaults", () => { + strictEqual(new TextDecoder("utf-8", null).decode(AB), "ab", "ctor null options"); + }); +}); diff --git a/tests/integration/urlsearchparams/urlsearchparams.js b/tests/integration/urlsearchparams/urlsearchparams.js new file mode 100644 index 00000000..476ebd15 --- /dev/null +++ b/tests/integration/urlsearchparams/urlsearchparams.js @@ -0,0 +1,36 @@ +import { serveTest } from "../test-server.js"; +import { strictEqual, throws } from "../../assert.js"; + +export const handler = serveTest(async (t) => { + await t.test("append-symbol-name-throws-and-does-not-mutate", () => { + const p = new URLSearchParams("a=1"); + // A Symbol cannot be converted to a string: append must throw a TypeError + // and leave the params untouched (not append a bogus empty-name entry). + throws(() => p.append(Symbol("s"), "x"), TypeError); + strictEqual(p.toString(), "a=1", "params unchanged after throwing append"); + }); + + await t.test("append-throwing-tostring-name-propagates", () => { + const p = new URLSearchParams(); + const bad = { + toString() { + throw new Error("boom"); + }, + }; + throws(() => p.append(bad, "v"), Error, "boom"); + strictEqual(p.toString(), "", "params unchanged after throwing toString"); + }); + + await t.test("append-symbol-name-does-not-corrupt-attached-url", () => { + const u = new URL("http://example.com/?a=1"); + throws(() => u.searchParams.append(Symbol("s"), "x"), TypeError); + strictEqual(u.href, "http://example.com/?a=1", "url href unchanged"); + }); + + await t.test("append-normal-still-works", () => { + const p = new URLSearchParams("a=1"); + p.append("b", "2"); + p.append(3, 4); // non-string coercible values are fine + strictEqual(p.toString(), "a=1&b=2&3=4", "valid appends work"); + }); +}); diff --git a/tests/tests.cmake b/tests/tests.cmake index 7b7840f3..772fcb0d 100644 --- a/tests/tests.cmake +++ b/tests/tests.cmake @@ -68,5 +68,7 @@ integration_tests( event fetch performance + textdecoder timers + urlsearchparams )