From 4a98a003d9ba5af9768b29a4bc2c286dc249b410 Mon Sep 17 00:00:00 2001 From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com> Date: Fri, 12 Jun 2026 16:23:27 +0300 Subject: [PATCH 1/5] fix: URLSearchParams construction and iteration spec compliance The constructor previously handled only the string init form; any object init was stringified to "[object Object]", collapsing every query parameter into a single bogus key. It now follows the WHATWG/WebIDL init dispatch (https://url.spec.whatwg.org/#interface-urlsearchparams): record, sequence (any iterable of pairs: Array, Map, Set, another URLSearchParams, generators - with IteratorClose on abrupt completion), other primitives (USVString coercion; Symbol throws), and null/undefined (empty). entries()/keys()/values() returned plain v8::Arrays and the type exposed no @@iterator, so for..of, spread, and the copy-constructor form did not work; the array path also returned the first value for every occurrence of a repeated key. They now return genuine live ES iterators (reflecting mutations mid-iteration, per spec), and @@iterator aliases entries(). Also aligned to spec: get() returns null (not undefined) for a missing name; delete(name, value) and has(name, value) honor the optional value argument (coerced to USVString; an explicit undefined is treated as omitted, matching WPT). --- NativeScript/runtime/URLSearchParamsImpl.cpp | 574 ++++++++++++++++--- TestRunner/app/tests/URLSearchParams.js | 331 ++++++++++- 2 files changed, 804 insertions(+), 101 deletions(-) diff --git a/NativeScript/runtime/URLSearchParamsImpl.cpp b/NativeScript/runtime/URLSearchParamsImpl.cpp index fd39addd..9aacb58c 100644 --- a/NativeScript/runtime/URLSearchParamsImpl.cpp +++ b/NativeScript/runtime/URLSearchParamsImpl.cpp @@ -44,8 +44,11 @@ v8::Local URLSearchParamsImpl::GetCtor( tmpl->Set(ToV8String(isolate, "delete"), v8::FunctionTemplate::New(isolate, Delete)); - tmpl->Set(ToV8String(isolate, "entries"), - v8::FunctionTemplate::New(isolate, Entries)); + // Capture the entries template so it can double as the default @@iterator + // (registered below), matching the browser identity + // URLSearchParams.prototype[Symbol.iterator] === entries. + auto entriesTmpl = v8::FunctionTemplate::New(isolate, Entries); + tmpl->Set(ToV8String(isolate, "entries"), entriesTmpl); tmpl->Set(ToV8String(isolate, "forEach"), v8::FunctionTemplate::New(isolate, ForEach)); @@ -76,9 +79,412 @@ v8::Local URLSearchParamsImpl::GetCtor( tmpl->Set(ToV8String(isolate, "values"), v8::FunctionTemplate::New(isolate, &Values)); + // Declare the type as iterable by exposing + // @@iterator. It aliases entries() (which returns a genuine ES iterator), + // so `for..of`, spread, and `new URLSearchParams(otherParams)` (the spec + // sequence/copy form) all work. Non-enumerable, to match the browser + // prototype shape. + tmpl->Set(v8::Symbol::GetIterator(isolate), entriesTmpl, + v8::PropertyAttribute::DontEnum); + return ctorTmpl; } +// Spec-compliant URLSearchParams init (record / sequence / primitive) and the +// live iterators backing entries()/keys()/values()/@@iterator. The dispatch +// in Ctor mirrors the WebIDL union conversion to +// `(sequence> or record or +// USVString)` that browsers/Node apply +// (https://url.spec.whatwg.org/#interface-urlsearchparams): +// object with @@iterator -> sequence (Array, Map, Set, URLSearchParams, +// generator) +// object without @@iterator -> record (own enumerable string keys, in +// order) +// other primitive -> USVString (number/boolean/bigint -> string; +// Symbol throws) +// null / undefined -> empty +namespace { +void ThrowTypeError(v8::Isolate* isolate, const char* message) { + isolate->ThrowException( + v8::Exception::TypeError(ToV8String(isolate, message))); +} + +// --- Live iterators for entries()/keys()/values()/@@iterator. --- +// Spec URLSearchParams iterators are *live*: they reflect append()/delete() +// performed after the iterator is created (and mid-iteration). Each is backed +// by a small hidden state object — { src: the URLSearchParams, idx: cursor, +// knd: which projection } — captured as next()'s Data. No snapshot, and no +// bespoke iterator class/finalizer: the source is kept alive by the state +// reference and collected normally with the iterator. next() re-reads the +// params on every call, so duplicate keys keep their own values too. +enum IteratorKind { ITER_KEYS = 0, ITER_VALUES = 1, ITER_ENTRIES = 2 }; + +const char* const kStateSource = "src"; +const char* const kStateIndex = "idx"; +const char* const kStateKind = "knd"; + +void IteratorSelf(const v8::FunctionCallbackInfo& args) { + // An iterator is itself iterable (iterator[@@iterator]() === iterator), + // so `for..of params.entries()` works. + args.GetReturnValue().Set(args.This()); +} + +void IteratorNext(const v8::FunctionCallbackInfo& args) { + auto isolate = args.GetIsolate(); + auto context = isolate->GetCurrentContext(); + auto state = args.Data().As(); + + auto indexKey = ToV8String(isolate, kStateIndex); + v8::Local sourceValue; + v8::Local indexValue; + v8::Local kindValue; + if (!state->Get(context, ToV8String(isolate, kStateSource)) + .ToLocal(&sourceValue) || + !state->Get(context, indexKey).ToLocal(&indexValue) || + !state->Get(context, ToV8String(isolate, kStateKind)) + .ToLocal(&kindValue)) { + return; + } + auto index = indexValue->Uint32Value(context).FromMaybe(0u); + auto kind = kindValue->Int32Value(context).FromMaybe(ITER_ENTRIES); + + auto result = v8::Object::New(isolate); + auto valueKey = ToV8String(isolate, "value"); + auto doneKey = ToV8String(isolate, "done"); + + URLSearchParamsImpl* ptr = + sourceValue->IsObject() + ? URLSearchParamsImpl::GetPointer(sourceValue.As()) + : nullptr; + if (ptr == nullptr || index >= ptr->GetURLSearchParams()->size()) { + result->Set(context, valueKey, v8::Undefined(isolate)).Check(); + result->Set(context, doneKey, v8::True(isolate)).Check(); + args.GetReturnValue().Set(result); + return; + } + + auto pair = (*ptr->GetURLSearchParams())[index]; + v8::Local value; + if (kind == ITER_KEYS) { + value = ToV8String(isolate, pair.first); + } else if (kind == ITER_VALUES) { + value = ToV8String(isolate, pair.second); + } else { + v8::Local kv[] = { + ToV8String(isolate, pair.first), + ToV8String(isolate, pair.second), + }; + value = v8::Array::New(isolate, kv, 2); + } + state + ->Set(context, indexKey, v8::Integer::NewFromUnsigned(isolate, index + 1)) + .Check(); + result->Set(context, valueKey, value).Check(); + result->Set(context, doneKey, v8::False(isolate)).Check(); + args.GetReturnValue().Set(result); +} + +// Sets `args`' return value to a fresh live iterator of `kind` over +// `args.This()` (the URLSearchParams). The state object is never exposed to +// JS — only the iterator (with next() and @@iterator) is returned. +void ReturnLiveIterator(const v8::FunctionCallbackInfo& args, + int kind) { + auto isolate = args.GetIsolate(); + auto context = isolate->GetCurrentContext(); + + auto state = v8::Object::New(isolate); + state->Set(context, ToV8String(isolate, kStateSource), args.This()).Check(); + state + ->Set(context, ToV8String(isolate, kStateIndex), + v8::Integer::NewFromUnsigned(isolate, 0)) + .Check(); + state + ->Set(context, ToV8String(isolate, kStateKind), + v8::Integer::New(isolate, kind)) + .Check(); + + v8::Local nextFn; + v8::Local selfFn; + if (!v8::Function::New(context, IteratorNext, state).ToLocal(&nextFn) || + !v8::Function::New(context, IteratorSelf).ToLocal(&selfFn)) { + return; + } + + auto iterator = v8::Object::New(isolate); + iterator->Set(context, ToV8String(isolate, "next"), nextFn).Check(); + iterator->Set(context, v8::Symbol::GetIterator(isolate), selfFn).Check(); + iterator + ->DefineOwnProperty(context, v8::Symbol::GetToStringTag(isolate), + ToV8String(isolate, "URLSearchParams Iterator"), + v8::PropertyAttribute::DontEnum) + .Check(); + args.GetReturnValue().Set(iterator); +} + +// Coerce a JS value to a UTF-8 string via the USVString conversion the +// URLSearchParams init algorithm applies to every name/value. Returns +// false if the conversion threw — a Symbol value, or a user toString that +// throws — leaving the JS exception pending so the caller can stop and +// let it propagate (the spec throws here rather than dropping the entry). +// No further V8 API may be called once that happens. +bool ValueToString(v8::Local context, v8::Local value, + std::string& out) { + auto isolate = context->GetIsolate(); + v8::Local str; + if (!value->ToString(context).ToLocal(&str)) { + return false; + } + out = tns::ToString(isolate, str); + return true; +} + +// Opens a JS sync iterator: invokes `iterMethod` (the resolved @@iterator) +// with `receiver` as the `this` value, requires the result to be an object, +// and reads its callable `next` method. On any deviation the matching JS +// TypeError is thrown (or left pending, if a user callback threw) and the +// function returns false. +bool OpenIterator(v8::Local context, v8::Local receiver, + v8::Local iterMethod, + v8::Local& outIterator, + v8::Local& outNext) { + auto isolate = context->GetIsolate(); + v8::Local iteratorValue; + if (!iterMethod->Call(context, receiver, 0, nullptr) + .ToLocal(&iteratorValue)) { + return false; + } + if (!iteratorValue->IsObject()) { + ThrowTypeError(isolate, + "Result of the Symbol.iterator method is not an object"); + return false; + } + outIterator = iteratorValue.As(); + v8::Local nextValue; + if (!outIterator->Get(context, ToV8String(isolate, "next")) + .ToLocal(&nextValue)) { + return false; + } + if (!nextValue->IsFunction()) { + ThrowTypeError(isolate, "Iterator has no 'next' method"); + return false; + } + outNext = nextValue.As(); + return true; +} + +// Advances `iterator` once. On a normal step writes the yielded value to +// `outValue` and leaves `outDone` false; at completion sets `outDone` true. +// Returns false (exception pending) if next() threw or yielded a non-object +// result. `next` is captured once by the caller, per the iterator protocol. +bool StepIterator(v8::Local context, + v8::Local iterator, v8::Local next, + v8::Local& outValue, bool& outDone) { + auto isolate = context->GetIsolate(); + v8::Local resultValue; + if (!next->Call(context, iterator, 0, nullptr).ToLocal(&resultValue)) { + return false; + } + if (!resultValue->IsObject()) { + ThrowTypeError(isolate, "Iterator result is not an object"); + return false; + } + auto result = resultValue.As(); + v8::Local doneValue; + if (!result->Get(context, ToV8String(isolate, "done")).ToLocal(&doneValue)) { + return false; + } + outDone = doneValue->BooleanValue(isolate); + if (outDone) { + return true; + } + return result->Get(context, ToV8String(isolate, "value")).ToLocal(&outValue); +} + +// ES IteratorClose for an abrupt (error) completion: call the iterator's +// `return` method, if it has one, and discard both its result and any error +// it raises — the triggering exception wins (spec discards `return`'s +// completion on a throw). Generators with `finally` and resource-backed +// iterables rely on this to release state when conversion fails partway. +// Must be called with no pending JS exception: the caller captures and +// re-throws the triggering exception around this call. +void CloseIterator(v8::Local context, + v8::Local iterator) { + auto isolate = context->GetIsolate(); + v8::TryCatch tryCatch(isolate); + v8::Local returnMethod; + if (iterator->Get(context, ToV8String(isolate, "return")) + .ToLocal(&returnMethod) && + returnMethod->IsFunction()) { + v8::Local ignored; + (void)returnMethod.As() + ->Call(context, iterator, 0, nullptr) + .ToLocal(&ignored); + } + // tryCatch swallows anything return() throws when it goes out of scope. +} + +// Materializes one inner element of a sequence init into its strings. Per +// spec each element is itself converted to `sequence`, whose first +// step ("convert to sequence") throws a TypeError when Type(V) is not +// Object. So a non-Object (primitive) element — including a primitive string — +// throws; it is NOT boxed (boxing would wrongly accept ["ab"] as the pair +// ("a","b")). Iterable objects (Array, Set, a String *object*) are materialized +// by driving their @@iterator. If an element fails to coerce, the inner +// iterator is closed before the exception propagates. +bool MaterializeInnerSequence(v8::Local context, + v8::Local value, + std::vector& out) { + auto isolate = context->GetIsolate(); + // Per WebIDL "convert to sequence": if Type(V) is not Object, throw a + // TypeError. Primitives (including strings) are NOT boxed here — boxing + // would wrongly accept e.g. ["ab"] as the pair ("a","b") instead of + // throwing. A String/Set/Array *object* element is fine (it is an Object). + if (!value->IsObject()) { + ThrowTypeError(isolate, + "URLSearchParams init sequence element is not iterable"); + return false; + } + v8::Local object = value.As(); + v8::Local iterMethod; + if (!object->Get(context, v8::Symbol::GetIterator(isolate)) + .ToLocal(&iterMethod)) { + return false; + } + if (!iterMethod->IsFunction()) { + ThrowTypeError(isolate, + "URLSearchParams init sequence element is not iterable"); + return false; + } + v8::Local iterator; + v8::Local next; + if (!OpenIterator(context, object, iterMethod.As(), iterator, + next)) { + return false; + } + while (true) { + v8::Local element; + bool done = false; + if (!StepIterator(context, iterator, next, element, done)) { + return false; // next() threw: the iterator is already finished + } + if (done) { + break; + } + // Capture a conversion failure, then close the iterator outside the + // TryCatch's scope so the close runs with a clean isolate and the + // original exception is re-thrown (not swallowed by this handler). + v8::Local caught; + { + v8::TryCatch tryCatch(isolate); + std::string str; + if (ValueToString(context, element, str)) { + out.push_back(std::move(str)); + } else { + caught = tryCatch.Exception(); + tryCatch.Reset(); + } + } + if (!caught.IsEmpty()) { + CloseIterator(context, iterator); + isolate->ThrowException(caught); + return false; + } + } + return true; +} + +// Sequence init form: iterate the value via its @@iterator (already resolved +// by the caller) and append each [name, value] pair. Covers Array, Map, Set, +// another URLSearchParams and generators uniformly. A pair whose length is +// not exactly 2 throws a TypeError; a name/value that cannot be coerced to a +// string aborts with the JS exception left pending. On either abrupt +// completion the source iterator is closed first (ES IteratorClose). +bool BuildFromSequence(v8::Local context, + v8::Local object, + v8::Local iterMethod, + ada::url_search_params& params) { + auto isolate = context->GetIsolate(); + v8::Local iterator; + v8::Local next; + if (!OpenIterator(context, object, iterMethod, iterator, next)) { + return false; + } + while (true) { + v8::Local entry; + bool done = false; + if (!StepIterator(context, iterator, next, entry, done)) { + return false; // next() threw: the iterator is already finished + } + if (done) { + break; + } + // Convert and append under a TryCatch; on failure capture the + // exception, then close the source iterator outside the handler's + // scope and re-throw (so the close runs clean and the error survives). + v8::Local caught; + { + v8::TryCatch tryCatch(isolate); + std::vector pair; + bool built = MaterializeInnerSequence(context, entry, pair); + if (built && pair.size() != 2) { + ThrowTypeError( + isolate, + "Failed to construct 'URLSearchParams': Sequence initializer " + "must only contain pair elements"); + } + if (tryCatch.HasCaught()) { + caught = tryCatch.Exception(); + tryCatch.Reset(); + } else { + params.append(pair[0], pair[1]); + } + } + if (!caught.IsEmpty()) { + CloseIterator(context, iterator); + isolate->ThrowException(caught); + return false; + } + } + return true; +} + +// Record init form: a plain object of name -> value, iterated in own +// enumerable string-key order. A value that cannot be coerced to a string +// aborts with the JS exception left pending. +bool BuildFromRecord(v8::Local context, + v8::Local object, + ada::url_search_params& params) { + auto filter = static_cast( + v8::PropertyFilter::ONLY_ENUMERABLE | v8::PropertyFilter::SKIP_SYMBOLS); + v8::Local keys; + if (!object + ->GetOwnPropertyNames(context, filter, + v8::KeyConversionMode::kConvertToString) + .ToLocal(&keys)) { + return false; + } + auto length = keys->Length(); + for (uint32_t i = 0; i < length; i++) { + v8::Local key; + if (!keys->Get(context, i).ToLocal(&key)) { + return false; + } + v8::Local value; + if (!object->Get(context, key).ToLocal(&value)) { + return false; + } + std::string keyStr; + std::string valueStr; + if (!ValueToString(context, key, keyStr) || + !ValueToString(context, value, valueStr)) { + return false; + } + params.append(keyStr, valueStr); + } + return true; +} +} // namespace + void URLSearchParamsImpl::Ctor( const v8::FunctionCallbackInfo& args) { auto value = args[0]; @@ -89,12 +495,47 @@ void URLSearchParamsImpl::Ctor( ada::url_search_params params; if (value->IsString()) { + // ada strips a single leading '?' (constructor step), then runs the + // urlencoded string parser. params = ada::url_search_params(tns::ToString(isolate, value.As())); - } else if (value->IsObject()) { - params = ada::url_search_params( - tns::ToString(isolate, value->ToString(context).ToLocalChecked())); } + // Dispatch the record/sequence/primitive init forms via the helpers above. + // A failed coercion leaves a pending exception; return so V8 throws it. + else if (value->IsObject()) { + auto object = value.As(); + v8::Local iterMethod; + if (!object->Get(context, v8::Symbol::GetIterator(isolate)) + .ToLocal(&iterMethod)) { + return; // the @@iterator getter threw + } + if (iterMethod->IsNullOrUndefined()) { + if (!BuildFromRecord(context, object, params)) { + return; + } + } else if (iterMethod->IsFunction()) { + if (!BuildFromSequence(context, object, iterMethod.As(), + params)) { + return; + } + } else { + // Per WebIDL GetMethod, a present-but-non-callable @@iterator is a + // TypeError rather than a silent fall-back to the record form. + ThrowTypeError(isolate, + "URLSearchParams init Symbol.iterator is not a function"); + return; + } + } else if (!value->IsNullOrUndefined()) { + // Other primitive (number / boolean / bigint / symbol): coerce to a + // USVString and run the urlencoded string parser. A Symbol cannot be + // converted and throws here, matching the spec. + std::string init; + if (!ValueToString(context, value, init)) { + return; + } + params = ada::url_search_params(init); + } + // null / undefined -> leave params empty auto searchParams = new URLSearchParamsImpl(params); @@ -125,38 +566,30 @@ void URLSearchParamsImpl::Delete( } auto isolate = args.GetIsolate(); auto key = tns::ToString(isolate, args[0].As()); - ptr->GetURLSearchParams()->remove(key.c_str()); + // Spec delete(name, value): when the value argument is given, remove only + // tuples matching BOTH name and value (url.bs:4000). The value is a + // USVString, so coerce via ValueToString (number/boolean -> string; a Symbol + // or throwing toString leaves the pending exception and aborts). + // An explicit `undefined` value is treated as omitted (name-only), matching + // WPT "Two-argument delete() respects undefined as second arg"; only a + // present, non-undefined value takes the value-filtered path. + if (args.Length() > 1 && !args[1]->IsUndefined()) { + std::string value; + if (!ValueToString(isolate->GetCurrentContext(), args[1], value)) { + return; // coercion threw (Symbol / throwing toString) + } + ptr->GetURLSearchParams()->remove(key, value); + } else { + ptr->GetURLSearchParams()->remove(key.c_str()); + } } +// entries()/keys()/values() return live ES iterators (per spec they are +// iterators, not arrays). Reading the params by index per next() also keeps +// each occurrence of a repeated key paired with its own value. void URLSearchParamsImpl::Entries( const v8::FunctionCallbackInfo& args) { - URLSearchParamsImpl* ptr = GetPointer(args.This()); - auto isolate = args.GetIsolate(); - auto context = isolate->GetCurrentContext(); - if (ptr == nullptr) { - args.GetReturnValue().Set(v8::Array::New(isolate)); - return; - } - - auto keys = ptr->GetURLSearchParams()->get_keys(); - auto len = ptr->GetURLSearchParams()->size(); - auto ret = v8::Array::New(isolate, (int)len); - int i = 0; - while (keys.has_next()) { - auto key = keys.next(); - if (key) { - auto keyValue = key.value(); - auto value = ptr->GetURLSearchParams()->get(keyValue).value(); - v8::Local values[] = { - ToV8String(isolate, keyValue.data()), - ToV8String(isolate, value.data()), - }; - auto success = ret->Set(context, i++, v8::Array::New(isolate, values, 2)) - .FromMaybe(false); - tns::Assert(success, isolate); - } - } - args.GetReturnValue().Set(ret); + ReturnLiveIterator(args, ITER_ENTRIES); } void URLSearchParamsImpl::ForEach( @@ -206,7 +639,9 @@ void URLSearchParamsImpl::Get(const v8::FunctionCallbackInfo& args) { auto ret = ToV8String(isolate, std::string(value.value())); args.GetReturnValue().Set(ret); } else { - args.GetReturnValue().SetUndefined(); + // Spec: get() returns null for a missing name (url.bs:4016), not + // undefined. + args.GetReturnValue().SetNull(); } } @@ -239,36 +674,31 @@ void URLSearchParamsImpl::Has(const v8::FunctionCallbackInfo& args) { } auto isolate = args.GetIsolate(); auto key = args[0].As(); - auto value = ptr->GetURLSearchParams()->has(tns::ToString(isolate, key)); + // Spec has(name, value): when the value argument is given, return true + // only for a tuple matching BOTH name and value (url.bs:4028). The value + // is a USVString, so coerce via ValueToString (number/boolean -> string; + // a Symbol or throwing toString leaves the pending exception and aborts). + bool value; + // An explicit `undefined` value is treated as omitted (name-only), matching + // WPT "Two-argument has() respects undefined as second arg"; only a present, + // non-undefined value takes the value-filtered path. + if (args.Length() > 1 && !args[1]->IsUndefined()) { + std::string filter; + if (!ValueToString(isolate->GetCurrentContext(), args[1], filter)) { + return; // coercion threw (Symbol / throwing toString) + } + value = ptr->GetURLSearchParams()->has(tns::ToString(isolate, key), filter); + } else { + value = ptr->GetURLSearchParams()->has(tns::ToString(isolate, key)); + } args.GetReturnValue().Set(value); } +// Live ES iterator (see Entries above). void URLSearchParamsImpl::Keys( const v8::FunctionCallbackInfo& args) { - URLSearchParamsImpl* ptr = GetPointer(args.This()); - auto isolate = args.GetIsolate(); - auto context = isolate->GetCurrentContext(); - if (ptr == nullptr) { - args.GetReturnValue().Set(v8::Array::New(isolate)); - return; - } - - auto keys = ptr->GetURLSearchParams()->get_keys(); - - auto len = ptr->GetURLSearchParams()->size(); - auto ret = v8::Array::New(isolate, (int)len); - int i = 0; - while (keys.has_next()) { - auto key = keys.next(); - if (key) { - auto keyValue = key.value(); - tns::Assert(ret->Set(context, i++, ToV8String(isolate, keyValue.data())) - .FromMaybe(false), - isolate); - } - } - args.GetReturnValue().Set(ret); + ReturnLiveIterator(args, ITER_KEYS); } void URLSearchParamsImpl::Set(const v8::FunctionCallbackInfo& args) { @@ -322,34 +752,10 @@ void URLSearchParamsImpl::ToString( args.GetReturnValue().Set(ret); } +// Live ES iterator (see Entries above). void URLSearchParamsImpl::Values( const v8::FunctionCallbackInfo& args) { - URLSearchParamsImpl* ptr = GetPointer(args.This()); - auto isolate = args.GetIsolate(); - auto context = isolate->GetCurrentContext(); - if (ptr == nullptr) { - args.GetReturnValue().Set(v8::Array::New(isolate)); - return; - } - - auto keys = ptr->GetURLSearchParams()->get_keys(); - - auto len = ptr->GetURLSearchParams()->size(); - auto ret = v8::Array::New(isolate, (int)len); - int i = 0; - while (keys.has_next()) { - auto key = keys.next(); - if (key) { - auto value = ptr->GetURLSearchParams()->get(key.value()); - if (value.has_value()) { - tns::Assert(ret->Set(context, i++, - ToV8String(isolate, std::string(value.value()))) - .FromMaybe(false), - isolate); - } - } - } - args.GetReturnValue().Set(ret); + ReturnLiveIterator(args, ITER_VALUES); } url_search_params* URLSearchParamsImpl::GetURLSearchParams() { diff --git a/TestRunner/app/tests/URLSearchParams.js b/TestRunner/app/tests/URLSearchParams.js index 2da2ef20..7b7f6760 100644 --- a/TestRunner/app/tests/URLSearchParams.js +++ b/TestRunner/app/tests/URLSearchParams.js @@ -1,57 +1,187 @@ describe("Test URLSearchParams ", function () { const fooBar = "foo=1&bar=2"; it("Test URLSearchParams keys", function(){ + // keys() returns a spec iterator, not an array — consume it via spread. const params = new URLSearchParams(fooBar); - const keys = params.keys(); + const keys = [...params.keys()]; expect(keys[0]).toBe("foo"); expect(keys[1]).toBe("bar"); }); - + it("Test URLSearchParams values", function(){ const params = new URLSearchParams(fooBar); - const values = params.values(); + const values = [...params.values()]; expect(values[0]).toBe("1"); expect(values[1]).toBe("2"); }); - - + + it("Test URLSearchParams entries", function(){ const params = new URLSearchParams(fooBar); - const entries = params.entries(); + const entries = [...params.entries()]; expect(entries[0][0]).toBe("foo"); expect(entries[0][1]).toBe("1"); - + expect(entries[1][0]).toBe("bar"); expect(entries[1][1]).toBe("2"); - + }); - - + + it("Test URLSearchParams keys/values/entries return spec iterators", function(){ + const params = new URLSearchParams(fooBar); + // A spec iterator has a next() and is itself iterable. + expect(typeof params.entries().next).toBe("function"); + expect(typeof params.keys().next).toBe("function"); + expect(typeof params.values().next).toBe("function"); + const it = params.entries(); + const first = it.next(); + expect(first.done).toBe(false); + expect(first.value[0]).toBe("foo"); + expect(first.value[1]).toBe("1"); + }); + + it("Test URLSearchParams entries preserves duplicate keys", function(){ + // Regression: the old get_keys()+get() path returned the first value for + // every occurrence of a repeated key. + const params = new URLSearchParams("a=1&a=2&b=3"); + const entries = [...params.entries()]; + expect(entries.length).toBe(3); + expect(entries[0][1]).toBe("1"); + expect(entries[1][1]).toBe("2"); + expect(entries[2][1]).toBe("3"); + expect([...params.values()].join(",")).toBe("1,2,3"); + }); + + it("Test URLSearchParams default iterator aliases entries", function(){ + // The default @@iterator IS the entries method (browser identity). This binding + // installs members per-instance (not on the prototype), so assert on an instance + // AND assert it is actually a function — not the vacuous undefined === undefined. + const params = new URLSearchParams(fooBar); + expect(typeof params[Symbol.iterator]).toBe("function"); + expect(params[Symbol.iterator]).toBe(params.entries); + }); + + it("Test URLSearchParams iterator carries the spec brand", function(){ + const params = new URLSearchParams(fooBar); + expect(Object.prototype.toString.call(params.entries())).toBe("[object URLSearchParams Iterator]"); + }); + + it("Test URLSearchParams iterator is live", function(){ + // Spec iterators reflect mutations made after they are created. + const params = new URLSearchParams("a=1&b=2"); + const it = params.entries(); + expect(it.next().value[0]).toBe("a"); // consume "a" + params.append("c", "3"); // mutate mid-iteration + const rest = []; + let r; + while (!(r = it.next()).done) { + rest.push(r.value[0]); + } + expect(rest.join(",")).toBe("b,c"); // sees the appended "c" + }); + + it("Test URLSearchParams closes the source iterator on a bad pair", function(){ + // On an abrupt completion (a too-long pair) the source iterator must be + // closed, so a generator's finally runs and resource-backed iterables free. + let closed = false; + function* gen() { + try { + yield ["a", "1", "2"]; // 3-element pair → TypeError + } finally { + closed = true; + } + } + expect(function(){ new URLSearchParams(gen()); }).toThrow(); + expect(closed).toBe(true); + }); + + it("Test URLSearchParams size", function(){ const params = new URLSearchParams(fooBar); expect(params.size).toBe(2); }); - + it("Test URLSearchParams append", function(){ const params = new URLSearchParams(fooBar); params.append("first", "Osei"); expect(params.get("first")).toBe("Osei"); }); - - + + it("Test URLSearchParams delete", function(){ const params = new URLSearchParams(fooBar); params.append("first", "Osei"); params.delete("first"); - expect(params.get("first")).toBe(undefined); + // Spec: get() returns null for a missing name (url.bs:4016). + expect(params.get("first")).toBe(null); + }); + + it("Test URLSearchParams get returns null for a missing name", function(){ + // Spec get(name): "...otherwise null" (url.bs:4016). + const params = new URLSearchParams("a=1"); + expect(params.get("missing")).toBe(null); }); - - + + it("Test URLSearchParams delete with value removes only matching pairs", function(){ + // Spec delete(name, value): when value is given, remove only tuples + // matching BOTH name and value (url.bs:4000). The value is a USVString, + // so a non-string (the number 1) coerces to "1". + const params = new URLSearchParams("a=1&a=2&a=1&b=1"); + params.delete("a", 1); + expect(params.getAll("a").join(",")).toBe("2"); + expect(params.getAll("b").join(",")).toBe("1"); + // Single-arg delete still removes every pair with that name. + params.delete("a"); + expect(params.has("a")).toBe(false); + }); + + it("Test URLSearchParams has", function(){ const params = new URLSearchParams(fooBar); expect(params.has("foo")).toBe(true); }); - + + it("Test URLSearchParams has with value matches name and value", function(){ + // Spec has(name, value): true only for a tuple matching BOTH (url.bs:4028). + // The value is a USVString, so non-strings (number, boolean) coerce. + const params = new URLSearchParams("a=1&a=2&flag=true"); + expect(params.has("a", "1")).toBe(true); + expect(params.has("a", 2)).toBe(true); // number coerces to "2" + expect(params.has("a", "3")).toBe(false); + expect(params.has("flag", true)).toBe(true); // boolean coerces to "true" + expect(params.has("missing", "1")).toBe(false); + // Single-arg has still matches by name only. + expect(params.has("a")).toBe(true); + }); + + it("Test URLSearchParams has/delete throw when the value cannot be coerced", function(){ + // The value argument is a USVString; a Symbol (or a throwing toString) + // cannot convert, so the call must throw rather than silently matching "" + // (WebIDL USVString conversion, url.bs:4000 / 4028). + const params = new URLSearchParams("a=1"); + expect(function(){ params.has("a", Symbol("x")); }).toThrow(); + expect(function(){ params.delete("a", Symbol("x")); }).toThrow(); + }); + + it("Test URLSearchParams has/delete treat an explicit undefined value as omitted", function(){ + // Per WPT (urlsearchparams-has / -delete "respects undefined as second + // arg"), an explicit `undefined` second argument is treated as omitted + // (name-only), NOT as the string "undefined". + const params = new URLSearchParams("a=b&a=d&c&e&"); + expect(params.has("a", "b")).toBe(true); + expect(params.has("a", "c")).toBe(false); + expect(params.has("a", undefined)).toBe(true); // undefined -> name-only + + const del = new URLSearchParams(); + del.append("a", "b"); + del.append("a", "c"); + del.append("b", "c"); + del.append("b", "d"); + del.delete("b", "c"); + del.delete("a", undefined); // undefined -> delete by name + expect(del.toString()).toBe("b=d"); + }); + it("Test URLSearchParams changes propagates to URL parent", function(){ const toBe = 'https://github.com/triniwiz?first=Osei'; const url = new URL('https://github.com/triniwiz'); @@ -114,4 +244,171 @@ describe("Test URLSearchParams ", function () { expect(results[2].value).toBe("3"); }); + it("Test URLSearchParams from record object", function(){ + const params = new URLSearchParams({ foo: "1", bar: "2" }); + expect(params.get("foo")).toBe("1"); + expect(params.get("bar")).toBe("2"); + expect(params.size).toBe(2); + // A plain object must expand to its entries, not collapse into a + // single "[object Object]" key. + expect(params.has("[object Object]")).toBe(false); + }); + + it("Test URLSearchParams from record serializes every pair in toString", function(){ + const params = new URLSearchParams({ one: "1", two: "2" }); + expect(params.toString()).toBe("one=1&two=2"); + }); + + it("Test URLSearchParams from record coerces values to strings", function(){ + const params = new URLSearchParams({ a: 1, b: true }); + expect(params.get("a")).toBe("1"); + expect(params.get("b")).toBe("true"); + }); + + it("Test URLSearchParams from record encodes special characters", function(){ + const params = new URLSearchParams({ q: "a b&c" }); + expect(params.get("q")).toBe("a b&c"); + expect(params.toString()).toBe("q=a+b%26c"); + }); + + it("Test URLSearchParams from array of pairs", function(){ + const params = new URLSearchParams([["foo", "1"], ["bar", "2"], ["foo", "3"]]); + expect(params.get("foo")).toBe("1"); + expect(params.getAll("foo").length).toBe(2); + expect(params.get("bar")).toBe("2"); + expect(params.size).toBe(3); + }); + + it("Test URLSearchParams empty record and no-arg produce empty query", function(){ + expect(new URLSearchParams().toString()).toBe(""); + expect(new URLSearchParams({}).toString()).toBe(""); + }); + + it("Test URLSearchParams from record throws when a value cannot be coerced to a string", function(){ + // Per spec the record/sequence init coerces every value to a USVString; + // a value that cannot convert (a Symbol here) must throw rather than + // silently dropping or emptying the entry. + expect(function(){ new URLSearchParams({ bad: Symbol("x") }); }).toThrow(); + }); + + // --- Iterable (sequence) init: any iterable of pairs, not only arrays. --- + + it("Test URLSearchParams from a Map", function(){ + const params = new URLSearchParams(new Map([["a", "1"], ["b", "2"]])); + expect(params.toString()).toBe("a=1&b=2"); + }); + + it("Test URLSearchParams from a Set of pairs", function(){ + const params = new URLSearchParams(new Set([["x", "1"], ["y", "2"]])); + expect(params.toString()).toBe("x=1&y=2"); + }); + + it("Test URLSearchParams copy-constructs from another URLSearchParams", function(){ + // A URLSearchParams is iterable, so per spec it resolves to the sequence + // (copy) form — including duplicate keys, which proves the @@iterator walks + // entries rather than collapsing them. + const source = new URLSearchParams("a=1&a=2&b=3"); + const copy = new URLSearchParams(source); + expect(copy.toString()).toBe("a=1&a=2&b=3"); + expect(copy.getAll("a").length).toBe(2); + }); + + it("Test URLSearchParams from a generator of pairs", function(){ + function* pairs() { + yield ["a", "1"]; + yield ["b", "2"]; + } + const params = new URLSearchParams(pairs()); + expect(params.toString()).toBe("a=1&b=2"); + }); + + it("Test URLSearchParams from sequence with non-array inner pairs", function(){ + // Each pair need only be a 2-element iterable, not specifically an array. + // A Set is iterable but not an Array, so it exercises the inner iterator path. + const params = new URLSearchParams([new Set(["k", "v"])]); + expect(params.get("k")).toBe("v"); + }); + + it("Test URLSearchParams sequence init throws on a too-long pair", function(){ + expect(function(){ new URLSearchParams([["a", "1", "2"]]); }).toThrow(); + }); + + it("Test URLSearchParams sequence init throws on a too-short pair", function(){ + expect(function(){ new URLSearchParams([["a"]]); }).toThrow(); + }); + + it("Test URLSearchParams sequence init throws on a non-iterable element", function(){ + expect(function(){ new URLSearchParams([null]); }).toThrow(); + expect(function(){ new URLSearchParams([1]); }).toThrow(); + }); + + it("Test URLSearchParams sequence init throws on a primitive string element", function(){ + // WebIDL converts each element to sequence, whose first step throws + // when the element is not an Object. A 2-code-point string must NOT be accepted + // as the pair ("a","b"). + expect(function(){ new URLSearchParams(["ab"]); }).toThrow(); + }); + + it("Test URLSearchParams sequence init accepts a String-object element", function(){ + // A String *object* IS an Object and is iterable, so it is a valid 2-char pair. + const params = new URLSearchParams([new String("ab")]); + expect(params.get("a")).toBe("b"); + }); + + it("Test URLSearchParams throws when @@iterator is present but not callable", function(){ + // Per WebIDL GetMethod, a non-callable @@iterator is a TypeError, not a + // silent fall-back to the record form. + expect(function(){ new URLSearchParams({ [Symbol.iterator]: 5 }); }).toThrow(); + }); + + // --- The type itself is iterable. --- + + it("Test URLSearchParams is spread-iterable via Symbol.iterator", function(){ + const params = new URLSearchParams("a=1&b=2"); + const entries = [...params]; + expect(entries.length).toBe(2); + expect(entries[0][0]).toBe("a"); + expect(entries[0][1]).toBe("1"); + expect(entries[1][0]).toBe("b"); + expect(entries[1][1]).toBe("2"); + }); + + it("Test URLSearchParams works in a for..of loop", function(){ + const params = new URLSearchParams("a=1&a=2"); + const seen = []; + for (const [key, value] of params) { + seen.push(key + "=" + value); + } + expect(seen.length).toBe(2); + expect(seen[0]).toBe("a=1"); + expect(seen[1]).toBe("a=2"); + }); + + // --- Primitive init: coerced to USVString, then parsed. --- + + it("Test URLSearchParams from a number", function(){ + expect(new URLSearchParams(123).toString()).toBe("123="); + }); + + it("Test URLSearchParams from a boolean", function(){ + expect(new URLSearchParams(true).toString()).toBe("true="); + }); + + it("Test URLSearchParams from a bigint", function(){ + expect(new URLSearchParams(10n).toString()).toBe("10="); + }); + + it("Test URLSearchParams strips a single leading question mark", function(){ + expect(new URLSearchParams("?a=1").get("a")).toBe("1"); + }); + + it("Test URLSearchParams throws when init is a Symbol", function(){ + expect(function(){ new URLSearchParams(Symbol("x")); }).toThrow(); + }); + + it("Test URLSearchParams from null or undefined is empty", function(){ + expect(new URLSearchParams(null).toString()).toBe(""); + expect(new URLSearchParams(undefined).toString()).toBe(""); + }); + }); From 0efba58ad9069a713db7128bb5c64c2359799b99 Mon Sep 17 00:00:00 2001 From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com> Date: Fri, 12 Jun 2026 16:57:54 +0300 Subject: [PATCH 2/5] fix: coerce names to USVString and brand-check iterator receivers Review follow-up: - get()/getAll()/has()/delete() now coerce the name argument through the same USVString conversion as the optional value argument instead of casting it to a string blindly, so a number/boolean/object name works and a Symbol (or a throwing toString) raises a TypeError. - entries()/keys()/values() and the live iterator's next() brand-check their receivers and throw "Illegal invocation" for foreign objects, per the WebIDL iterable<> semantics. GetPointer also refuses to read an internal field from an object that has none (e.g. entries.call({})), which was an out-of-bounds read. --- NativeScript/runtime/URLSearchParamsImpl.cpp | 65 +++++++++++++++++--- TestRunner/app/tests/URLSearchParams.js | 61 ++++++++++++++++++ 2 files changed, 118 insertions(+), 8 deletions(-) diff --git a/NativeScript/runtime/URLSearchParamsImpl.cpp b/NativeScript/runtime/URLSearchParamsImpl.cpp index 9aacb58c..e0fc448c 100644 --- a/NativeScript/runtime/URLSearchParamsImpl.cpp +++ b/NativeScript/runtime/URLSearchParamsImpl.cpp @@ -24,6 +24,12 @@ void URLSearchParamsImpl::Init(v8::Isolate* isolate, URLSearchParamsImpl* URLSearchParamsImpl::GetPointer( v8::Local object) { + // A foreign receiver (e.g. entries.call({})) has no internal field, and + // reading one that is not there is undefined behavior, so check first + // and report not-a-URLSearchParams instead. + if (object->InternalFieldCount() < 1) { + return nullptr; + } auto ptr = object->GetAlignedPointerFromInternalField(0); if (ptr == nullptr) { return nullptr; @@ -122,6 +128,7 @@ enum IteratorKind { ITER_KEYS = 0, ITER_VALUES = 1, ITER_ENTRIES = 2 }; const char* const kStateSource = "src"; const char* const kStateIndex = "idx"; const char* const kStateKind = "knd"; +const char* const kStateIterator = "itr"; void IteratorSelf(const v8::FunctionCallbackInfo& args) { // An iterator is itself iterable (iterator[@@iterator]() === iterator), @@ -134,6 +141,17 @@ void IteratorNext(const v8::FunctionCallbackInfo& args) { auto context = isolate->GetCurrentContext(); auto state = args.Data().As(); + // WebIDL brand check: next() must be invoked on the iterator it was + // created on, so a detached next() throws instead of advancing the + // captured state. + v8::Local iteratorValue; + if (!state->Get(context, ToV8String(isolate, kStateIterator)) + .ToLocal(&iteratorValue) || + !iteratorValue->StrictEquals(args.This())) { + ThrowTypeError(isolate, "Illegal invocation"); + return; + } + auto indexKey = ToV8String(isolate, kStateIndex); v8::Local sourceValue; v8::Local indexValue; @@ -192,6 +210,13 @@ void ReturnLiveIterator(const v8::FunctionCallbackInfo& args, auto isolate = args.GetIsolate(); auto context = isolate->GetCurrentContext(); + // WebIDL brand check: entries()/keys()/values() require a genuine + // URLSearchParams receiver, so e.g. entries.call({}) throws. + if (URLSearchParamsImpl::GetPointer(args.This()) == nullptr) { + ThrowTypeError(isolate, "Illegal invocation"); + return; + } + auto state = v8::Object::New(isolate); state->Set(context, ToV8String(isolate, kStateSource), args.This()).Check(); state @@ -211,6 +236,8 @@ void ReturnLiveIterator(const v8::FunctionCallbackInfo& args, } auto iterator = v8::Object::New(isolate); + // Recorded in the hidden state so next() can brand-check its receiver. + state->Set(context, ToV8String(isolate, kStateIterator), iterator).Check(); iterator->Set(context, ToV8String(isolate, "next"), nextFn).Check(); iterator->Set(context, v8::Symbol::GetIterator(isolate), selfFn).Check(); iterator @@ -565,7 +592,13 @@ void URLSearchParamsImpl::Delete( return; } auto isolate = args.GetIsolate(); - auto key = tns::ToString(isolate, args[0].As()); + // The name is a USVString (url.bs:3861): coerce it like the optional + // value below instead of assuming a string (a Symbol or throwing + // toString leaves the pending exception and aborts). + std::string key; + if (!ValueToString(isolate->GetCurrentContext(), args[0], key)) { + return; + } // Spec delete(name, value): when the value argument is given, remove only // tuples matching BOTH name and value (url.bs:4000). The value is a // USVString, so coerce via ValueToString (number/boolean -> string; a Symbol @@ -633,8 +666,13 @@ void URLSearchParamsImpl::Get(const v8::FunctionCallbackInfo& args) { args.GetReturnValue().SetUndefined(); return; } - auto key = args[0].As(); - auto value = ptr->GetURLSearchParams()->get(tns::ToString(isolate, key)); + // The name is a USVString (url.bs:3862); a Symbol or throwing toString + // leaves the pending exception and aborts. + std::string key; + if (!ValueToString(isolate->GetCurrentContext(), args[0], key)) { + return; + } + auto value = ptr->GetURLSearchParams()->get(key); if (value.has_value()) { auto ret = ToV8String(isolate, std::string(value.value())); args.GetReturnValue().Set(ret); @@ -654,8 +692,13 @@ void URLSearchParamsImpl::GetAll( args.GetReturnValue().Set(v8::Array::New(isolate)); return; } - auto key = args[0].As(); - auto values = ptr->GetURLSearchParams()->get_all(tns::ToString(isolate, key)); + // The name is a USVString (url.bs:3863); a Symbol or throwing toString + // leaves the pending exception and aborts. + std::string key; + if (!ValueToString(context, args[0], key)) { + return; + } + auto values = ptr->GetURLSearchParams()->get_all(key); auto ret = v8::Array::New(isolate, (int)values.size()); int i = 0; for (auto item : values) { @@ -673,7 +716,13 @@ void URLSearchParamsImpl::Has(const v8::FunctionCallbackInfo& args) { return; } auto isolate = args.GetIsolate(); - auto key = args[0].As(); + // The name is a USVString (url.bs:3864): coerce it like the optional + // value below instead of assuming a string (a Symbol or throwing + // toString leaves the pending exception and aborts). + std::string key; + if (!ValueToString(isolate->GetCurrentContext(), args[0], key)) { + return; + } // Spec has(name, value): when the value argument is given, return true // only for a tuple matching BOTH name and value (url.bs:4028). The value // is a USVString, so coerce via ValueToString (number/boolean -> string; @@ -687,9 +736,9 @@ void URLSearchParamsImpl::Has(const v8::FunctionCallbackInfo& args) { if (!ValueToString(isolate->GetCurrentContext(), args[1], filter)) { return; // coercion threw (Symbol / throwing toString) } - value = ptr->GetURLSearchParams()->has(tns::ToString(isolate, key), filter); + value = ptr->GetURLSearchParams()->has(key, filter); } else { - value = ptr->GetURLSearchParams()->has(tns::ToString(isolate, key)); + value = ptr->GetURLSearchParams()->has(key); } args.GetReturnValue().Set(value); diff --git a/TestRunner/app/tests/URLSearchParams.js b/TestRunner/app/tests/URLSearchParams.js index 7b7f6760..7e22e4aa 100644 --- a/TestRunner/app/tests/URLSearchParams.js +++ b/TestRunner/app/tests/URLSearchParams.js @@ -411,4 +411,65 @@ describe("Test URLSearchParams ", function () { expect(new URLSearchParams(undefined).toString()).toBe(""); }); + // --- The name argument is a USVString: coerced, not assumed. --- + + it("Test URLSearchParams coerces a non-string name in get/getAll/has/delete", function(){ + const params = new URLSearchParams("1=a&true=b&null=c"); + expect(params.get(1)).toBe("a"); + expect(params.getAll(1).length).toBe(1); + expect(params.getAll(1)[0]).toBe("a"); + expect(params.has(true)).toBe(true); + expect(params.get(null)).toBe("c"); + params.delete(true); + expect(params.has("true")).toBe(false); + expect(params.has("1")).toBe(true); + }); + + it("Test URLSearchParams coerces an object name via toString", function(){ + const params = new URLSearchParams("a=1"); + const name = { toString: function(){ return "a"; } }; + expect(params.get(name)).toBe("1"); + expect(params.has(name)).toBe(true); + params.delete(name); + expect(params.has("a")).toBe(false); + }); + + it("Test URLSearchParams throws when the name is a Symbol", function(){ + const params = new URLSearchParams("a=1"); + expect(function(){ params.get(Symbol("x")); }).toThrow(); + expect(function(){ params.getAll(Symbol("x")); }).toThrow(); + expect(function(){ params.has(Symbol("x")); }).toThrow(); + expect(function(){ params.delete(Symbol("x")); }).toThrow(); + expect(params.get("a")).toBe("1"); + }); + + // --- Brand checks: iterators require a genuine receiver. --- + + it("Test URLSearchParams entries/keys/values throw on a foreign receiver", function(){ + const params = new URLSearchParams("a=1"); + expect(function(){ params.entries.call({}); }).toThrow(); + expect(function(){ params.keys.call({}); }).toThrow(); + expect(function(){ params.values.call({}); }).toThrow(); + }); + + it("Test URLSearchParams iterator next() throws on a foreign receiver", function(){ + const params = new URLSearchParams("a=1"); + const iterator = params.entries(); + const next = iterator.next; + expect(function(){ next.call({}); }).toThrow(); + // The iterator keeps working when invoked correctly afterwards. + const first = iterator.next(); + expect(first.done).toBe(false); + expect(first.value[0]).toBe("a"); + }); + + it("Test URLSearchParams entries retargets to another instance receiver", function(){ + const a = new URLSearchParams("a=1"); + const b = new URLSearchParams("b=2"); + const entries = [...a.entries.call(b)]; + expect(entries.length).toBe(1); + expect(entries[0][0]).toBe("b"); + expect(entries[0][1]).toBe("2"); + }); + }); From 49893eb9317858c52d5f8f37a65ac4ce0f2d87e5 Mon Sep 17 00:00:00 2001 From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com> Date: Fri, 12 Jun 2026 17:12:55 +0300 Subject: [PATCH 3/5] fix: coerce append/set arguments to USVStrings Completes the review follow-up: append() and set() now run both arguments through the same USVString coercion as the rest of the binding, instead of casting them to strings blindly. A number/boolean/object argument coerces via toString; a Symbol (or a throwing toString) raises a TypeError before anything is appended or replaced. --- NativeScript/runtime/URLSearchParamsImpl.cpp | 30 +++++++++++++------- TestRunner/app/tests/URLSearchParams.js | 20 +++++++++++++ 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/NativeScript/runtime/URLSearchParamsImpl.cpp b/NativeScript/runtime/URLSearchParamsImpl.cpp index e0fc448c..66e753a2 100644 --- a/NativeScript/runtime/URLSearchParamsImpl.cpp +++ b/NativeScript/runtime/URLSearchParamsImpl.cpp @@ -579,10 +579,16 @@ void URLSearchParamsImpl::Append( if (ptr == nullptr) { return; } - auto isolate = args.GetIsolate(); - auto key = tns::ToString(isolate, args[0].As()); - auto value = tns::ToString(isolate, args[1].As()); - ptr->GetURLSearchParams()->append(key.c_str(), value.c_str()); + // Both arguments are USVStrings (url.bs:3860); a Symbol or throwing + // toString leaves the pending exception and aborts. + auto context = args.GetIsolate()->GetCurrentContext(); + std::string key; + std::string value; + if (!ValueToString(context, args[0], key) || + !ValueToString(context, args[1], value)) { + return; + } + ptr->GetURLSearchParams()->append(key, value); } void URLSearchParamsImpl::Delete( @@ -755,12 +761,16 @@ void URLSearchParamsImpl::Set(const v8::FunctionCallbackInfo& args) { if (ptr == nullptr) { return; } - auto key = args[0].As(); - auto value = args[1].As(); - - auto isolate = args.GetIsolate(); - ptr->GetURLSearchParams()->set(tns::ToString(isolate, key), - tns::ToString(isolate, value)); + // Both arguments are USVStrings (url.bs:3865); a Symbol or throwing + // toString leaves the pending exception and aborts. + auto context = args.GetIsolate()->GetCurrentContext(); + std::string key; + std::string value; + if (!ValueToString(context, args[0], key) || + !ValueToString(context, args[1], value)) { + return; + } + ptr->GetURLSearchParams()->set(key, value); } void URLSearchParamsImpl::GetSize( diff --git a/TestRunner/app/tests/URLSearchParams.js b/TestRunner/app/tests/URLSearchParams.js index 7e22e4aa..68fc1633 100644 --- a/TestRunner/app/tests/URLSearchParams.js +++ b/TestRunner/app/tests/URLSearchParams.js @@ -443,6 +443,26 @@ describe("Test URLSearchParams ", function () { expect(params.get("a")).toBe("1"); }); + it("Test URLSearchParams coerces non-string arguments in append and set", function(){ + const params = new URLSearchParams(); + params.append(1, 2); + expect(params.get("1")).toBe("2"); + params.set(true, { toString: function(){ return "x"; } }); + expect(params.get("true")).toBe("x"); + params.append("a", null); + expect(params.get("a")).toBe("null"); + }); + + it("Test URLSearchParams append and set throw when an argument is a Symbol", function(){ + const params = new URLSearchParams("a=1"); + expect(function(){ params.append(Symbol("x"), "v"); }).toThrow(); + expect(function(){ params.append("k", Symbol("x")); }).toThrow(); + expect(function(){ params.set(Symbol("x"), "v"); }).toThrow(); + expect(function(){ params.set("k", Symbol("x")); }).toThrow(); + // Nothing was appended or replaced by the failed calls. + expect(params.toString()).toBe("a=1"); + }); + // --- Brand checks: iterators require a genuine receiver. --- it("Test URLSearchParams entries/keys/values throw on a foreign receiver", function(){ From c45030e6b4090d11e72f2282edf8044d038810d7 Mon Sep 17 00:00:00 2001 From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com> Date: Fri, 12 Jun 2026 18:05:11 +0300 Subject: [PATCH 4/5] fix: align null init, record Symbol keys, and receiver checks with WebIDL - new URLSearchParams(null) parses the string "null" (so "null="). The IDL default "" applies only to undefined/missing; null goes through the union conversion, which has no null special case (the union is not nullable and a record is not a dictionary), so it lands in the USVString branch. Matches the whatwg-url reference implementation; Node treats null as empty and is the outlier, and WPT does not cover the case. - An own enumerable Symbol key in a record init throws a TypeError instead of being silently skipped: WebIDL record conversion takes the keys from [[OwnPropertyKeys]] filtered by enumerability only, and converting a Symbol key to a USVString throws. - The remaining members (get/getAll/has/append/set/delete/forEach/ sort/toString/size) brand-check their receiver and throw an "Illegal invocation" TypeError for a foreign object, consistent with entries()/keys()/values(). --- NativeScript/runtime/URLSearchParamsImpl.cpp | 50 ++++++++++++-------- TestRunner/app/tests/URLSearchParams.js | 31 +++++++++++- 2 files changed, 60 insertions(+), 21 deletions(-) diff --git a/NativeScript/runtime/URLSearchParamsImpl.cpp b/NativeScript/runtime/URLSearchParamsImpl.cpp index 66e753a2..53b6159f 100644 --- a/NativeScript/runtime/URLSearchParamsImpl.cpp +++ b/NativeScript/runtime/URLSearchParamsImpl.cpp @@ -104,11 +104,11 @@ v8::Local URLSearchParamsImpl::GetCtor( // (https://url.spec.whatwg.org/#interface-urlsearchparams): // object with @@iterator -> sequence (Array, Map, Set, URLSearchParams, // generator) -// object without @@iterator -> record (own enumerable string keys, in -// order) -// other primitive -> USVString (number/boolean/bigint -> string; -// Symbol throws) -// null / undefined -> empty +// object without @@iterator -> record (own enumerable keys, in order; a +// Symbol key throws) +// other primitive / null -> USVString (number/boolean/bigint/null -> +// string; Symbol throws) +// undefined / missing -> empty (the IDL default "") namespace { void ThrowTypeError(v8::Isolate* isolate, const char* message) { isolate->ThrowException( @@ -476,13 +476,15 @@ bool BuildFromSequence(v8::Local context, } // Record init form: a plain object of name -> value, iterated in own -// enumerable string-key order. A value that cannot be coerced to a string -// aborts with the JS exception left pending. +// enumerable key order ([[OwnPropertyKeys]]: strings, then symbols). Per +// WebIDL record conversion every enumerable key is converted to a USVString, +// so an own enumerable Symbol key throws a TypeError (ValueToString below +// raises it); symbols are NOT silently skipped. A key or value that cannot +// be coerced to a string aborts with the JS exception left pending. bool BuildFromRecord(v8::Local context, v8::Local object, ada::url_search_params& params) { - auto filter = static_cast( - v8::PropertyFilter::ONLY_ENUMERABLE | v8::PropertyFilter::SKIP_SYMBOLS); + auto filter = v8::PropertyFilter::ONLY_ENUMERABLE; v8::Local keys; if (!object ->GetOwnPropertyNames(context, filter, @@ -552,17 +554,20 @@ void URLSearchParamsImpl::Ctor( "URLSearchParams init Symbol.iterator is not a function"); return; } - } else if (!value->IsNullOrUndefined()) { - // Other primitive (number / boolean / bigint / symbol): coerce to a - // USVString and run the urlencoded string parser. A Symbol cannot be - // converted and throws here, matching the spec. + } else if (!value->IsUndefined()) { + // Other primitive (number / boolean / bigint / symbol / null): coerce to + // a USVString and run the urlencoded string parser. The WebIDL union + // conversion has no null special case (the union is not nullable and a + // record is not a dictionary), so new URLSearchParams(null) parses the + // string "null", as the reference implementation does. A Symbol cannot + // be converted and throws here, matching the spec. std::string init; if (!ValueToString(context, value, init)) { return; } params = ada::url_search_params(init); } - // null / undefined -> leave params empty + // undefined / missing -> the IDL default "" -> leave params empty auto searchParams = new URLSearchParamsImpl(params); @@ -577,6 +582,9 @@ void URLSearchParamsImpl::Append( const v8::FunctionCallbackInfo& args) { URLSearchParamsImpl* ptr = GetPointer(args.This()); if (ptr == nullptr) { + // WebIDL brand check: every member requires a genuine URLSearchParams + // receiver (same as entries()/keys()/values()). + ThrowTypeError(args.GetIsolate(), "Illegal invocation"); return; } // Both arguments are USVStrings (url.bs:3860); a Symbol or throwing @@ -595,6 +603,7 @@ void URLSearchParamsImpl::Delete( const v8::FunctionCallbackInfo& args) { URLSearchParamsImpl* ptr = GetPointer(args.This()); if (ptr == nullptr) { + ThrowTypeError(args.GetIsolate(), "Illegal invocation"); return; } auto isolate = args.GetIsolate(); @@ -637,6 +646,7 @@ void URLSearchParamsImpl::ForEach( auto isolate = args.GetIsolate(); auto context = isolate->GetCurrentContext(); if (ptr == nullptr) { + ThrowTypeError(isolate, "Illegal invocation"); return; } auto callback = args[0].As(); @@ -669,7 +679,7 @@ void URLSearchParamsImpl::Get(const v8::FunctionCallbackInfo& args) { URLSearchParamsImpl* ptr = GetPointer(args.This()); auto isolate = args.GetIsolate(); if (ptr == nullptr) { - args.GetReturnValue().SetUndefined(); + ThrowTypeError(isolate, "Illegal invocation"); return; } // The name is a USVString (url.bs:3862); a Symbol or throwing toString @@ -695,7 +705,7 @@ void URLSearchParamsImpl::GetAll( auto isolate = args.GetIsolate(); auto context = isolate->GetCurrentContext(); if (ptr == nullptr) { - args.GetReturnValue().Set(v8::Array::New(isolate)); + ThrowTypeError(isolate, "Illegal invocation"); return; } // The name is a USVString (url.bs:3863); a Symbol or throwing toString @@ -718,7 +728,7 @@ void URLSearchParamsImpl::GetAll( void URLSearchParamsImpl::Has(const v8::FunctionCallbackInfo& args) { URLSearchParamsImpl* ptr = GetPointer(args.This()); if (ptr == nullptr) { - args.GetReturnValue().Set(false); + ThrowTypeError(args.GetIsolate(), "Illegal invocation"); return; } auto isolate = args.GetIsolate(); @@ -759,6 +769,7 @@ void URLSearchParamsImpl::Keys( void URLSearchParamsImpl::Set(const v8::FunctionCallbackInfo& args) { URLSearchParamsImpl* ptr = GetPointer(args.This()); if (ptr == nullptr) { + ThrowTypeError(args.GetIsolate(), "Illegal invocation"); return; } // Both arguments are USVStrings (url.bs:3865); a Symbol or throwing @@ -778,7 +789,7 @@ void URLSearchParamsImpl::GetSize( const v8::PropertyCallbackInfo& info) { URLSearchParamsImpl* ptr = GetPointer(info.This()); if (ptr == nullptr) { - info.GetReturnValue().Set(0); + ThrowTypeError(info.GetIsolate(), "Illegal invocation"); return; } @@ -790,6 +801,7 @@ void URLSearchParamsImpl::Sort( const v8::FunctionCallbackInfo& args) { URLSearchParamsImpl* ptr = GetPointer(args.This()); if (ptr == nullptr) { + ThrowTypeError(args.GetIsolate(), "Illegal invocation"); return; } ptr->GetURLSearchParams()->sort(); @@ -799,7 +811,7 @@ void URLSearchParamsImpl::ToString( const v8::FunctionCallbackInfo& args) { URLSearchParamsImpl* ptr = GetPointer(args.This()); if (ptr == nullptr) { - args.GetReturnValue().SetEmptyString(); + ThrowTypeError(args.GetIsolate(), "Illegal invocation"); return; } auto isolate = args.GetIsolate(); diff --git a/TestRunner/app/tests/URLSearchParams.js b/TestRunner/app/tests/URLSearchParams.js index 68fc1633..b326af9a 100644 --- a/TestRunner/app/tests/URLSearchParams.js +++ b/TestRunner/app/tests/URLSearchParams.js @@ -406,9 +406,23 @@ describe("Test URLSearchParams ", function () { expect(function(){ new URLSearchParams(Symbol("x")); }).toThrow(); }); - it("Test URLSearchParams from null or undefined is empty", function(){ - expect(new URLSearchParams(null).toString()).toBe(""); + it("Test URLSearchParams from undefined or no argument is empty", function(){ expect(new URLSearchParams(undefined).toString()).toBe(""); + expect(new URLSearchParams().toString()).toBe(""); + }); + + it("Test URLSearchParams from null parses as the string null", function(){ + // The IDL union has no null special case (the type is not nullable and + // a record is not a dictionary), so null coerces like any primitive. + const params = new URLSearchParams(null); + expect(params.toString()).toBe("null="); + expect(params.get("null")).toBe(""); + }); + + it("Test URLSearchParams throws when a record key is a Symbol", function(){ + // Per WebIDL record conversion every own enumerable key is converted to + // a USVString, and converting a Symbol throws. + expect(function(){ new URLSearchParams({ a: "1", [Symbol("x")]: "v" }); }).toThrow(); }); // --- The name argument is a USVString: coerced, not assumed. --- @@ -472,6 +486,19 @@ describe("Test URLSearchParams ", function () { expect(function(){ params.values.call({}); }).toThrow(); }); + it("Test URLSearchParams methods throw on a foreign receiver", function(){ + const params = new URLSearchParams("a=1"); + expect(function(){ params.get.call({}, "a"); }).toThrow(); + expect(function(){ params.getAll.call({}, "a"); }).toThrow(); + expect(function(){ params.has.call({}, "a"); }).toThrow(); + expect(function(){ params.append.call({}, "a", "b"); }).toThrow(); + expect(function(){ params.set.call({}, "a", "b"); }).toThrow(); + expect(function(){ params.delete.call({}, "a"); }).toThrow(); + expect(function(){ params.forEach.call({}, function(){}); }).toThrow(); + expect(function(){ params.sort.call({}); }).toThrow(); + expect(function(){ params.toString.call({}); }).toThrow(); + }); + it("Test URLSearchParams iterator next() throws on a foreign receiver", function(){ const params = new URLSearchParams("a=1"); const iterator = params.entries(); From 72c8589b953123480ff058078dcb183455eb9c13 Mon Sep 17 00:00:00 2001 From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com> Date: Fri, 12 Jun 2026 19:06:11 +0300 Subject: [PATCH 5/5] docs: clarify when the sequence init runs IteratorClose Comment-only. The previous wording ("on either abrupt completion") could be read as closing the iterator on any failure, including a throw from next() itself. Spelled out the actual rule: conversion failures and wrong-length pairs close the source iterator, while a throw from the stepping (next(), or reading the result's done/value) does not, because the ES iterator protocol marks the iterator done on such a throw and IteratorClose is skipped for a done iterator. --- NativeScript/runtime/URLSearchParamsImpl.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/NativeScript/runtime/URLSearchParamsImpl.cpp b/NativeScript/runtime/URLSearchParamsImpl.cpp index 53b6159f..7eb325ac 100644 --- a/NativeScript/runtime/URLSearchParamsImpl.cpp +++ b/NativeScript/runtime/URLSearchParamsImpl.cpp @@ -424,8 +424,12 @@ bool MaterializeInnerSequence(v8::Local context, // by the caller) and append each [name, value] pair. Covers Array, Map, Set, // another URLSearchParams and generators uniformly. A pair whose length is // not exactly 2 throws a TypeError; a name/value that cannot be coerced to a -// string aborts with the JS exception left pending. On either abrupt -// completion the source iterator is closed first (ES IteratorClose). +// string aborts with the JS exception left pending. On either of those two +// failures the source iterator is closed first (ES IteratorClose). A throw +// from the stepping itself (next(), or reading the result's done/value) +// intentionally does NOT close the iterator: the ES iterator protocol marks +// the iterator done on such a throw (IteratorStepValue), and IteratorClose +// is skipped for a done iterator, which is also how for..of behaves. bool BuildFromSequence(v8::Local context, v8::Local object, v8::Local iterMethod,