Merge pull request #2443 from lightpanda-io/url_fixes

Fix URLSearchParams constructor

src/browser/js/Object.zig

@@ -152,7 +152,7 @@ pub fn getPropertyNames(self: Object) js.Array {
 }
 
 pub fn nameIterator(self: Object) !NameIterator {
-    const handle = v8.v8__Object__GetPropertyNames(self.handle, self.local.handle) orelse {
+    const handle = v8.v8__Object__GetOwnPropertyNames(self.handle, self.local.handle) orelse {
         // see getOwnPropertyNames above
         return error.TypeError;
     };

src/browser/js/Snapshot.zig

@@ -670,11 +670,12 @@ fn attachClass(comptime JsApi: type, isolate: *v8.Isolate, template: *const v8.F
                 if (value.static) {
                     v8.v8__Template__SetAccessorProperty(@ptrCast(template), js_name, getter_callback, setter_callback, attribute);
                 } else {
+                    const accessor_attr = if (own_properties) attribute else attribute | v8.DontEnum;
                     v8.v8__ObjectTemplate__SetAccessorProperty__Config(prototype, &.{
                         .key = js_name,
                         .getter = getter_callback,
                         .setter = setter_callback,
-                        .attribute = attribute,
+                        .attribute = accessor_attr,
                     });
                 }
             },
@@ -695,10 +696,8 @@ fn attachClass(comptime JsApi: type, isolate: *v8.Isolate, template: *const v8.F
                 if (value.static and !own_properties) {
                     v8.v8__Template__Set(@ptrCast(template), js_name, @ptrCast(function_template), v8.None);
                 } else {
-                    // For own_properties namespaces, static methods still belong
-                    // on the instance — `CSS` is exposed as an instance via
-                    // `window.CSS`, not as a constructor.
-                    v8.v8__Template__Set(@ptrCast(member_template), js_name, @ptrCast(function_template), v8.None);
+                    const fn_attr: v8.PropertyAttribute = if (own_properties) v8.None else v8.DontEnum;
+                    v8.v8__Template__Set(@ptrCast(member_template), js_name, @ptrCast(function_template), fn_attr);
                 }
             },
             bridge.Indexed => {

src/browser/tests/net/url_search_params.html

@@ -416,6 +416,99 @@
 }
 </script>
 
+<script id=fromURLSearchParams>
+{
+  const original = new URLSearchParams();
+  original.append('operationId', 'abc123');
+  original.append('variables', '{"x":1}');
+  original.append('locale', 'en');
+
+  const cloned = new URLSearchParams(original);
+  testing.expectEqual(3, cloned.size);
+  testing.expectEqual('abc123', cloned.get('operationId'));
+  testing.expectEqual('{"x":1}', cloned.get('variables'));
+  testing.expectEqual('en', cloned.get('locale'));
+  testing.expectEqual('operationId=abc123&variables=%7B%22x%22%3A1%7D&locale=en', cloned.toString());
+
+  // Regression: prototype methods (has, get, size, ...) must not leak in as entries.
+  testing.expectEqual(false, cloned.has('has'));
+  testing.expectEqual(false, cloned.has('get'));
+  testing.expectEqual(false, cloned.has('size'));
+  testing.expectEqual(false, cloned.has('toString'));
+}
+</script>
+
+<script id=fromURLSearchParamsDuplicateKeys>
+{
+  const original = new URLSearchParams('a=1&b=2&a=3');
+  const cloned = new URLSearchParams(original);
+  testing.expectEqual(3, cloned.size);
+  testing.expectEqual(['1', '3'], cloned.getAll('a'));
+  testing.expectEqual('a=1&b=2&a=3', cloned.toString());
+}
+</script>
+
+<script id=fromURLSearchParamsEmpty>
+{
+  const cloned = new URLSearchParams(new URLSearchParams());
+  testing.expectEqual(0, cloned.size);
+  testing.expectEqual('', cloned.toString());
+}
+</script>
+
+<script id=fromURLSearchParamsIndependent>
+{
+  // Mutating the clone must not affect the original, and vice versa.
+  const original = new URLSearchParams('a=1&b=2');
+  const cloned = new URLSearchParams(original);
+
+  cloned.append('c', '3');
+  testing.expectEqual(2, original.size);
+  testing.expectEqual(false, original.has('c'));
+  testing.expectEqual(3, cloned.size);
+
+  original.set('a', 'changed');
+  testing.expectEqual('1', cloned.get('a'));
+}
+</script>
+
+<script id=fromObjectIgnoresInheritedProperties>
+{
+  // Per WebIDL "record" semantics, only own enumerable string properties of
+  // the init object contribute to the URLSearchParams. Properties reached
+  // via the prototype chain (including bridged WebAPI prototype methods)
+  // must be ignored.
+  const proto = {inherited: 'no'};
+  const target = Object.create(proto);
+  target.own = 'yes';
+
+  const usp = new URLSearchParams(target);
+  testing.expectEqual(1, usp.size);
+  testing.expectEqual('yes', usp.get('own'));
+  testing.expectEqual(false, usp.has('inherited'));
+}
+</script>
+
+<script id=prototypeMembersAreNonEnumerable>
+{
+  // Per WebIDL, interface prototype members are non-enumerable. Several
+  // libraries iterate caller-supplied objects with `for..in` or
+  // `Object.keys` to build querystrings; if prototype methods leak in,
+  // they end up serialized as URL params with values like
+  // "function has() { [native code] }". Real Chrome/Firefox return [].
+  testing.expectEqual([], Object.keys(URLSearchParams.prototype));
+
+  const usp = new URLSearchParams('a=1&b=2');
+  const keys = [];
+  for (const k in usp) keys.push(k);
+  testing.expectEqual([], keys);
+
+  // Sanity: instance own enumerable keys are still empty (entries live in
+  // the C++ side, exposed only via the iterator protocol).
+  testing.expectEqual([], Object.keys(usp));
+}
+</script>
+
 <script id=utf8Encoding>
 {
   // Test that UTF-8 characters are properly percent-encoded (not double-encoded)

src/browser/webapi/net/URLSearchParams.zig

@@ -53,6 +53,15 @@ pub fn init(opts_: ?InitOpts, exec: *const Execution) !*URLSearchParams {
                     break :blk try paramsFromArray(arena, js_val.toArray());
                 }
                 if (js_val.isObject()) {
+                    // Per the URL spec, an iterable init (URLSearchParams,
+                    // Map, ...) should be walked via its @@iterator. We
+                    // don't have a generic iterable path yet; cover the
+                    // common case of `new URLSearchParams(otherUSP)` so
+                    // the prototype-method-leak doesn't just turn into a
+                    // silent empty querystring.
+                    if (js_val.toZig(*URLSearchParams)) |other| {
+                        break :blk try KeyValueList.copy(arena, other._params);
+                    } else |_| {}
                     // normalizer is null, so frame won't be used
                     break :blk try KeyValueList.fromJsObject(arena, js_val.toObject(), null, exec.buf);
                 }