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.

Author: adrian-niculescu

test-app/app/src/main/assets/app/tests/testURLSearchParamsImpl.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");
+    });
+
 });

test-app/runtime/src/main/cpp/URLSearchParamsImpl.cpp

@@ -10,6 +10,12 @@ namespace tns {
     URLSearchParamsImpl::URLSearchParamsImpl(ada::url_search_params params) : params_(params) {}
 
     URLSearchParamsImpl *URLSearchParamsImpl::GetPointer(v8::Local<v8::Object> 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;
@@ -115,6 +121,7 @@ namespace tns {
         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<v8::Value> &args) {
             // An iterator is itself iterable (iterator[@@iterator]() === iterator),
@@ -127,6 +134,17 @@ namespace tns {
             auto context = isolate->GetCurrentContext();
             auto state = args.Data().As<v8::Object>();
 
+            // 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<v8::Value> iteratorValue;
+            if (!state->Get(context, ArgConverter::ConvertToV8String(isolate, kStateIterator))
+                    .ToLocal(&iteratorValue) ||
+                !iteratorValue->StrictEquals(args.This())) {
+                ThrowTypeError(isolate, "Illegal invocation");
+                return;
+            }
+
             auto indexKey = ArgConverter::ConvertToV8String(isolate, kStateIndex);
             v8::Local<v8::Value> sourceValue;
             v8::Local<v8::Value> indexValue;
@@ -180,6 +198,13 @@ namespace tns {
             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, ArgConverter::ConvertToV8String(isolate, kStateSource),
                        args.This()).Check();
@@ -196,6 +221,9 @@ namespace tns {
             }
 
             auto iterator = v8::Object::New(isolate);
+            // Recorded in the hidden state so next() can brand-check its receiver.
+            state->Set(context, ArgConverter::ConvertToV8String(isolate, kStateIterator),
+                       iterator).Check();
             iterator->Set(context, ArgConverter::ConvertToV8String(isolate, "next"), nextFn).Check();
             iterator->Set(context, v8::Symbol::GetIterator(isolate), selfFn).Check();
             iterator->DefineOwnProperty(context, v8::Symbol::GetToStringTag(isolate),
@@ -525,7 +553,13 @@ namespace tns {
         if (ptr == nullptr) {
             return;
         }
-        auto key = ArgConverter::ConvertToString(args[0].As<v8::String>());
+        // 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(args.GetIsolate()->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
@@ -590,8 +624,13 @@ namespace tns {
             args.GetReturnValue().SetUndefined();
             return;
         }
-        auto key = args[0].As<v8::String>();
-        auto value = ptr->GetURLSearchParams()->get(ArgConverter::ConvertToString(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 = ArgConverter::ConvertToV8String(isolate, std::string(value.value()));
             args.GetReturnValue().Set(ret);
@@ -610,8 +649,13 @@ namespace tns {
             args.GetReturnValue().Set(v8::Array::New(isolate));
             return;
         }
-        auto key = args[0].As<v8::String>();
-        auto values = ptr->GetURLSearchParams()->get_all(ArgConverter::ConvertToString(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, values.size());
         size_t i = 0;
         for (auto item: values) {
@@ -626,7 +670,13 @@ namespace tns {
             args.GetReturnValue().Set(false);
             return;
         }
-        auto key = args[0].As<v8::String>();
+        // 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(args.GetIsolate()->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;
@@ -640,9 +690,9 @@ namespace tns {
             if (!ValueToString(args.GetIsolate()->GetCurrentContext(), args[1], filter)) {
                 return;  // coercion threw (Symbol / throwing toString)
             }
-            value = ptr->GetURLSearchParams()->has(ArgConverter::ConvertToString(key), filter);
+            value = ptr->GetURLSearchParams()->has(key, filter);
         } else {
-            value = ptr->GetURLSearchParams()->has(ArgConverter::ConvertToString(key));
+            value = ptr->GetURLSearchParams()->has(key);
         }
 
         args.GetReturnValue().Set(value);