From adebe72b3bbbbbeea37d64e5270a3a7cdf79f274 Mon Sep 17 00:00:00 2001
From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com>
Date: Tue, 9 Dec 2025 16:55:58 +0200
Subject: [PATCH] Fixed URLSearchParams.forEach() crash and spec compliance

- Added error handling for callback->Call() return value
- Changed argument order from (key, value) to (value, key, searchParams)
- Use get_entries() iterator to correctly handle duplicate keys
- Added support for optional thisArg parameter
- Added unit tests for forEach
---
 .../app/tests/testURLSearchParamsImpl.js      | 53 +++++++++++++++++++
 .../src/main/cpp/URLSearchParamsImpl.cpp      | 28 ++++++----
 2 files changed, 71 insertions(+), 10 deletions(-)

diff --git a/test-app/app/src/main/assets/app/tests/testURLSearchParamsImpl.js b/test-app/app/src/main/assets/app/tests/testURLSearchParamsImpl.js
index 5437250d5..3d0ca293f 100644
--- a/test-app/app/src/main/assets/app/tests/testURLSearchParamsImpl.js
+++ b/test-app/app/src/main/assets/app/tests/testURLSearchParamsImpl.js
@@ -61,4 +61,57 @@ describe("Test URLSearchParams ", function () {
         expect(url.toString()).toBe(toBe);
     });
 
+    it("Test URLSearchParams forEach", function(){
+        const params = new URLSearchParams(fooBar);
+        const results = [];
+        params.forEach((value, key, searchParams) => {
+            results.push({ key, value });
+            expect(searchParams).toBe(params);
+        });
+        expect(results.length).toBe(2);
+        expect(results[0].key).toBe("foo");
+        expect(results[0].value).toBe("1");
+        expect(results[1].key).toBe("bar");
+        expect(results[1].value).toBe("2");
+    });
+
+    it("Test URLSearchParams forEach with URL", function(){
+        const url = new URL('https://example.com?si=abc123&name=test');
+        const results = [];
+        url.searchParams.forEach((value, key) => {
+            results.push({ key, value });
+        });
+        expect(results.length).toBe(2);
+        expect(results[0].key).toBe("si");
+        expect(results[0].value).toBe("abc123");
+        expect(results[1].key).toBe("name");
+        expect(results[1].value).toBe("test");
+    });
+
+    it("Test URLSearchParams forEach with thisArg", function(){
+        const params = new URLSearchParams(fooBar);
+        const context = { results: [] };
+        params.forEach(function(value, key) {
+            this.results.push({ key, value });
+        }, context);
+        expect(context.results.length).toBe(2);
+        expect(context.results[0].key).toBe("foo");
+        expect(context.results[0].value).toBe("1");
+    });
+
+    it("Test URLSearchParams forEach with duplicate keys", function(){
+        const params = new URLSearchParams("foo=1&foo=2&bar=3");
+        const results = [];
+        params.forEach((value, key) => {
+            results.push({ key, value });
+        });
+        expect(results.length).toBe(3);
+        expect(results[0].key).toBe("foo");
+        expect(results[0].value).toBe("1");
+        expect(results[1].key).toBe("foo");
+        expect(results[1].value).toBe("2");
+        expect(results[2].key).toBe("bar");
+        expect(results[2].value).toBe("3");
+    });
+
 });
diff --git a/test-app/runtime/src/main/cpp/URLSearchParamsImpl.cpp b/test-app/runtime/src/main/cpp/URLSearchParamsImpl.cpp
index f8663e526..710079bab 100644
--- a/test-app/runtime/src/main/cpp/URLSearchParamsImpl.cpp
+++ b/test-app/runtime/src/main/cpp/URLSearchParamsImpl.cpp
@@ -160,19 +160,27 @@ namespace tns {
             return;
         }
         auto callback = args[0].As<v8::Function>();
-        auto keys = ptr->GetURLSearchParams()->get_keys();
-        while (keys.has_next()) {
-            auto key = keys.next();
-            if (key) {
-                auto keyValue = key.value();
-                auto value = ptr->GetURLSearchParams()->get(keyValue).value();
-                v8::Local<v8::Value> values[] = {
-                        ArgConverter::ConvertToV8String(isolate, keyValue.data()),
+        auto searchParams = args.This();
+        // Use thisArg if provided, otherwise undefined
+        auto thisArg = args.Length() > 1 ? args[1] : v8::Undefined(isolate).As<v8::Value>();
+        // Use get_entries() to correctly handle duplicate keys
+        auto entries = ptr->GetURLSearchParams()->get_entries();
+        while (entries.has_next()) {
+            auto entry = entries.next();
+            if (entry) {
+                auto& [key, value] = entry.value();
+                // Per spec, forEach callback receives (value, key, searchParams)
+                v8::Local<v8::Value> callbackArgs[] = {
                         ArgConverter::ConvertToV8String(isolate, value.data()),
+                        ArgConverter::ConvertToV8String(isolate, key.data()),
+                        searchParams,
                 };
-                callback->Call(context, v8::Local<v8::Value>(), 2, values);
+                v8::Local<v8::Value> result;
+                if (!callback->Call(context, thisArg, 3, callbackArgs).ToLocal(&result)) {
+                    // If the callback throws an exception, stop iteration
+                    return;
+                }
             }
-
         }
     }