Skip to content

Commit adebe72

Browse files
adrian-niculescuAdrian Niculescu
authored andcommitted
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
1 parent 3ecd707 commit adebe72

File tree

2 files changed

+71
-10
lines changed

2 files changed

+71
-10
lines changed

test-app/app/src/main/assets/app/tests/testURLSearchParamsImpl.js

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,57 @@ describe("Test URLSearchParams ", function () {
6161
expect(url.toString()).toBe(toBe);
6262
});
6363

64+
it("Test URLSearchParams forEach", function(){
65+
const params = new URLSearchParams(fooBar);
66+
const results = [];
67+
params.forEach((value, key, searchParams) => {
68+
results.push({ key, value });
69+
expect(searchParams).toBe(params);
70+
});
71+
expect(results.length).toBe(2);
72+
expect(results[0].key).toBe("foo");
73+
expect(results[0].value).toBe("1");
74+
expect(results[1].key).toBe("bar");
75+
expect(results[1].value).toBe("2");
76+
});
77+
78+
it("Test URLSearchParams forEach with URL", function(){
79+
const url = new URL('https://example.com?si=abc123&name=test');
80+
const results = [];
81+
url.searchParams.forEach((value, key) => {
82+
results.push({ key, value });
83+
});
84+
expect(results.length).toBe(2);
85+
expect(results[0].key).toBe("si");
86+
expect(results[0].value).toBe("abc123");
87+
expect(results[1].key).toBe("name");
88+
expect(results[1].value).toBe("test");
89+
});
90+
91+
it("Test URLSearchParams forEach with thisArg", function(){
92+
const params = new URLSearchParams(fooBar);
93+
const context = { results: [] };
94+
params.forEach(function(value, key) {
95+
this.results.push({ key, value });
96+
}, context);
97+
expect(context.results.length).toBe(2);
98+
expect(context.results[0].key).toBe("foo");
99+
expect(context.results[0].value).toBe("1");
100+
});
101+
102+
it("Test URLSearchParams forEach with duplicate keys", function(){
103+
const params = new URLSearchParams("foo=1&foo=2&bar=3");
104+
const results = [];
105+
params.forEach((value, key) => {
106+
results.push({ key, value });
107+
});
108+
expect(results.length).toBe(3);
109+
expect(results[0].key).toBe("foo");
110+
expect(results[0].value).toBe("1");
111+
expect(results[1].key).toBe("foo");
112+
expect(results[1].value).toBe("2");
113+
expect(results[2].key).toBe("bar");
114+
expect(results[2].value).toBe("3");
115+
});
116+
64117
});

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

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -160,19 +160,27 @@ namespace tns {
160160
return;
161161
}
162162
auto callback = args[0].As<v8::Function>();
163-
auto keys = ptr->GetURLSearchParams()->get_keys();
164-
while (keys.has_next()) {
165-
auto key = keys.next();
166-
if (key) {
167-
auto keyValue = key.value();
168-
auto value = ptr->GetURLSearchParams()->get(keyValue).value();
169-
v8::Local<v8::Value> values[] = {
170-
ArgConverter::ConvertToV8String(isolate, keyValue.data()),
163+
auto searchParams = args.This();
164+
// Use thisArg if provided, otherwise undefined
165+
auto thisArg = args.Length() > 1 ? args[1] : v8::Undefined(isolate).As<v8::Value>();
166+
// Use get_entries() to correctly handle duplicate keys
167+
auto entries = ptr->GetURLSearchParams()->get_entries();
168+
while (entries.has_next()) {
169+
auto entry = entries.next();
170+
if (entry) {
171+
auto& [key, value] = entry.value();
172+
// Per spec, forEach callback receives (value, key, searchParams)
173+
v8::Local<v8::Value> callbackArgs[] = {
171174
ArgConverter::ConvertToV8String(isolate, value.data()),
175+
ArgConverter::ConvertToV8String(isolate, key.data()),
176+
searchParams,
172177
};
173-
callback->Call(context, v8::Local<v8::Value>(), 2, values);
178+
v8::Local<v8::Value> result;
179+
if (!callback->Call(context, thisArg, 3, callbackArgs).ToLocal(&result)) {
180+
// If the callback throws an exception, stop iteration
181+
return;
182+
}
174183
}
175-
176184
}
177185
}
178186

0 commit comments

Comments
 (0)