Merge pull request #2051 from lightpanda-io/structureClone_host_object

Provide a failing callback to ValueSerializer for host objects

src/browser/js/Value.zig

@@ -245,15 +245,37 @@ pub fn toJson(self: Value, allocator: Allocator) ![]u8 {
     return js.String.toSliceWithAlloc(.{ .local = local, .handle = str_handle }, allocator);
 }
 
-// Currently does not support host objects (Blob, File, etc.) or transferables
-// which require delegate callbacks to be implemented.
+// Throws a DataCloneError for host objects (Blob, File, etc.) that cannot be serialized.
+// Does not support transferables which require additional delegate callbacks.
 pub fn structuredClone(self: Value) !Value {
     const local = self.local;
     const v8_context = local.handle;
     const v8_isolate = local.isolate.handle;
 
+    const SerializerDelegate = struct {
+        // Called when V8 encounters a host object it doesn't know how to serialize.
+        // Returns false to indicate the object cannot be cloned, and throws a DataCloneError.
+        // V8 asserts has_exception() after this returns false, so we must throw here.
+        fn writeHostObject(_: ?*anyopaque, isolate: ?*v8.Isolate, _: ?*const v8.Object) callconv(.c) v8.MaybeBool {
+            const iso = isolate orelse return .{ .has_value = true, .value = false };
+            const message = v8.v8__String__NewFromUtf8(iso, "The object cannot be cloned.", v8.kNormal, -1);
+            const error_value = v8.v8__Exception__Error(message) orelse return .{ .has_value = true, .value = false };
+            _ = v8.v8__Isolate__ThrowException(iso, error_value);
+            return .{ .has_value = true, .value = false };
+        }
+
+        // Called by V8 to report serialization errors. The exception should already be thrown.
+        fn throwDataCloneError(_: ?*anyopaque, _: ?*const v8.String) callconv(.c) void {}
+    };
+
     const size, const data = blk: {
-        const serializer = v8.v8__ValueSerializer__New(v8_isolate, null) orelse return error.JsException;
+        const serializer = v8.v8__ValueSerializer__New(v8_isolate, &.{
+            .data = null,
+            .get_shared_array_buffer_id = null,
+            .write_host_object = SerializerDelegate.writeHostObject,
+            .throw_data_clone_error = SerializerDelegate.throwDataCloneError,
+        }) orelse return error.JsException;
+
         defer v8.v8__ValueSerializer__DELETE(serializer);
 
         var write_result: v8.MaybeBool = undefined;

src/browser/tests/net/url_search_params.html

@@ -20,8 +20,8 @@
 
 <script id=urlSearchParams>
   const inputs = [
-    [["over", "9000!!"], ["abc", "123"], ["key1", ""], ["key2", ""]],
-    {over: "9000!!", abc: 123, key1: "", key2: ""},
+   // @ZIGDOM [["over", "9000!!"], ["abc", 123], ["key1", ""], ["key2", ""]],
+   {over: "9000!!", abc: 123, key1: "", key2: ""},
     "over=9000!!&abc=123&key1&key2=",
     "?over=9000!!&abc=123&key1&key2=",
   ]
@@ -367,49 +367,3 @@
     testing.expectEqual(['3'], ups.getAll('b'));
   }
 </script>
-
-<script id=arrayOfArrays>
-{
-  const usp = new URLSearchParams([["a", "1"], ["b", "2"], ["a", "3"]]);
-  testing.expectEqual(3, usp.size);
-  testing.expectEqual('1', usp.get('a'));
-  testing.expectEqual(['1', '3'], usp.getAll('a'));
-  testing.expectEqual('2', usp.get('b'));
-  testing.expectEqual('a=1&b=2&a=3', usp.toString());
-}
-</script>
-
-<script id=arrayOfArraysEmpty>
-{
-  const usp = new URLSearchParams([]);
-  testing.expectEqual(0, usp.size);
-  testing.expectEqual('', usp.toString());
-}
-</script>
-
-<script id=arrayOfArraysDuplicateKeys>
-{
-  const usp = new URLSearchParams([["key", "first"], ["key", "second"], ["key", "third"]]);
-  testing.expectEqual(3, usp.size);
-  testing.expectEqual('first', usp.get('key'));
-  testing.expectEqual(['first', 'second', 'third'], usp.getAll('key'));
-}
-</script>
-
-<script id=arrayOfArraysSpecialChars>
-{
-  const usp = new URLSearchParams([["q", "hello world"], ["url", "https://example.com/?a=1&b=2"]]);
-  testing.expectEqual(2, usp.size);
-  testing.expectEqual('hello world', usp.get('q'));
-  testing.expectEqual('https://example.com/?a=1&b=2', usp.get('url'));
-}
-</script>
-
-<script id=arrayOfArraysNumericValues>
-{
-  const usp = new URLSearchParams([["count", 42], ["pi", 3.14]]);
-  testing.expectEqual(2, usp.size);
-  testing.expectEqual('42', usp.get('count'));
-  testing.expectEqual('3.14', usp.get('pi'));
-}
-</script>

src/browser/tests/window/window.html

@@ -1,5 +1,6 @@
 <!DOCTYPE html>
 <script src="../testing.js"></script>
+<body></body>
 
 <script id=window>
   testing.expectEqual(window, globalThis);
@@ -260,6 +261,28 @@
     }
     testing.expectEqual(true, threw);
   }
+
+  // Host objects (DOM elements) cannot be cloned - should throw, not crash
+  {
+    let threw = false;
+    try {
+      structuredClone(document.body);
+    } catch (err) {
+      threw = true;
+    }
+    testing.expectEqual(true, threw);
+  }
+
+  // Objects containing host objects cannot be cloned - should throw, not crash
+  {
+    let threw = false;
+    try {
+      structuredClone({ element: document.body });
+    } catch (err) {
+      threw = true;
+    }
+    testing.expectEqual(true, threw);
+  }
 </script>
 
 <script id=cached_getter_wrong_this>

src/browser/webapi/net/URLSearchParams.zig

@@ -46,10 +46,6 @@ pub fn init(opts_: ?InitOpts, page: *Page) !*URLSearchParams {
             .query_string => |qs| break :blk try paramsFromString(arena, qs, &page.buf),
             .form_data => |fd| break :blk try KeyValueList.copy(arena, fd._list),
             .value => |js_val| {
-                // Order matters here; Array is also an Object.
-                if (js_val.isArray()) {
-                    break :blk try paramsFromArray(arena, js_val.toArray());
-                }
                 if (js_val.isObject()) {
                     break :blk try KeyValueList.fromJsObject(arena, js_val.toObject(), null, page);
                 }
@@ -138,37 +134,6 @@ pub fn sort(self: *URLSearchParams) void {
     }.cmp);
 }
 
-fn paramsFromArray(allocator: Allocator, array: js.Array) !KeyValueList {
-    const array_len = array.len();
-    if (array_len == 0) {
-        return .empty;
-    }
-
-    var params = KeyValueList.init();
-    try params.ensureTotalCapacity(allocator, array_len);
-    // TODO: Release `params` on error.
-
-    var i: u32 = 0;
-    while (i < array_len) : (i += 1) {
-        const item = try array.get(i);
-        if (!item.isArray()) return error.InvalidArgument;
-
-        const as_array = item.toArray();
-        // Need 2 items for KV.
-        if (as_array.len() != 2) return error.InvalidArgument;
-
-        const name_val = try as_array.get(0);
-        const value_val = try as_array.get(1);
-
-        params._entries.appendAssumeCapacity(.{
-            .name = try name_val.toSSOWithAlloc(allocator),
-            .value = try value_val.toSSOWithAlloc(allocator),
-        });
-    }
-
-    return params;
-}
-
 fn paramsFromString(allocator: Allocator, input_: []const u8, buf: []u8) !KeyValueList {
     if (input_.len == 0) {
         return .empty;