From b556d3221dbebef26f50bd7898255b1f40ff5702 Mon Sep 17 00:00:00 2001 From: Pierre Tachoire Date: Wed, 17 Jun 2026 21:27:56 +0200 Subject: [PATCH] webapi: structured-clone BroadcastChannel messages per receiver postMessage handed the same js.Value.Temp to every receiver's MessageEvent, which broke two things: 1. No structured clone. Receivers got the literal object the sender posted, so a mutation in one receiver was visible to the others and to the sender (received === payload). The spec requires the message to be cloned for each destination. 2. Shared temp freed after the first delivery (latent use-after-free). Events are refcounted and deinit'd synchronously at the end of each dispatch, and MessageEvent.deinit release()s its data value. With a shared temp, the first receiver's teardown reset the v8 persistent slot, leaving later receivers reading a dead handle. Fix, following the Worker.postMessage precedent: StructuredSerialize the message once, synchronously, in postMessage (so an unserializable value throws a DataCloneError to the caller per spec, and cloning stays off the scheduler tick). A TryCatch contains any v8 exception from a failed serialize so it is re-raised as a clean DataCloneError. Delivery then re-clones the sanitized snapshot per receiver, giving every MessageEvent its own independently owned temp. The snapshot is released by the callback (cancelled finalizer covers the dropped-task path). --- src/browser/tests/broadcast_channel.html | 48 +++++++++++++++++ src/browser/webapi/BroadcastChannel.zig | 67 ++++++++++++++++++++++-- 2 files changed, 112 insertions(+), 3 deletions(-) diff --git a/src/browser/tests/broadcast_channel.html b/src/browser/tests/broadcast_channel.html index 7f2e77681..73315ba6d 100644 --- a/src/browser/tests/broadcast_channel.html +++ b/src/browser/tests/broadcast_channel.html @@ -83,6 +83,54 @@ testing.async(async () => { receiver.close(); }); +// The message is structured-cloned: receivers get an independent copy, not the +// sender's literal object, and one receiver mutating it does not affect others. +testing.async(async () => { + const sender = new BroadcastChannel("clone"); + const r1 = new BroadcastChannel("clone"); + const r2 = new BroadcastChannel("clone"); + + const payload = { n: 1 }; + let got1 = null; + let got2 = null; + + await new Promise((resolve) => { + let pending = 2; + const done = () => { if (--pending === 0) resolve(); }; + r1.onmessage = (e) => { got1 = e.data; got1.n = 999; done(); }; + r2.onmessage = (e) => { got2 = e.data; done(); }; + sender.postMessage(payload); + }); + + // Not the same object the sender posted. + testing.expectEqual(false, got1 === payload); + testing.expectEqual(false, got2 === payload); + // Each receiver got its own independent copy. + testing.expectEqual(false, got1 === got2); + // Sender's object is untouched, and r1's mutation didn't leak into r2. + testing.expectEqual(1, payload.n); + testing.expectEqual(1, got2.n); + testing.expectEqual(999, got1.n); + + sender.close(); + r1.close(); + r2.close(); +}); + +// A value that cannot be structured-cloned throws synchronously (DataCloneError). +{ + const bc = new BroadcastChannel("uncloneable"); + let threw = false; + try { + bc.postMessage(function () {}); + } catch (e) { + threw = true; + testing.expectEqual("DataCloneError", e.name); + } + testing.expectEqual(true, threw); + bc.close(); +} + // After close(), a channel no longer receives messages, and posting on a // closed channel throws InvalidStateError. testing.async(async () => { diff --git a/src/browser/webapi/BroadcastChannel.zig b/src/browser/webapi/BroadcastChannel.zig index beb572727..5f2eb3bef 100644 --- a/src/browser/webapi/BroadcastChannel.zig +++ b/src/browser/webapi/BroadcastChannel.zig @@ -71,15 +71,42 @@ pub fn postMessage(self: *BroadcastChannel, message: js.Value.Temp, frame: *Fram return error.InvalidStateError; } + // StructuredSerialize runs synchronously (per spec): clone the message once + // now so an unserializable value throws a DataCloneError to the caller, and + // so the async delivery has a self-owned snapshot to copy from. Each receiver + // gets its own re-clone of this snapshot in `run()` — that gives every + // MessageEvent an independently owned handle (a shared temp would be freed by + // the first receiver's event teardown, leaving the rest reading a dead slot) + // and honours the spec requirement that receivers don't share one object. + const snapshot = blk: { + var ls: js.Local.Scope = undefined; + frame.js.localScope(&ls); + defer ls.deinit(); + + // Contain any V8 exception raised by a failed serialization so we can + // re-raise it as a clean DataCloneError (per spec) instead of leaking a + // stale pending exception into the caller. deinit() (no rethrow) clears it. + var try_catch: js.TryCatch = undefined; + try_catch.init(&ls.local); + defer try_catch.deinit(); + + const cloned = message.local(&ls.local).structuredCloneTo(&ls.local) catch { + return error.DataClone; + }; + break :blk try cloned.temp(); + }; + errdefer snapshot.release(); + const callback = try frame._factory.create(PostMessageCallback{ .frame = frame, .sender = self, - .message = message, + .message = snapshot, }); try frame.js.scheduler.add(callback, PostMessageCallback.run, 0, .{ .name = "BroadcastChannel.postMessage", .low_priority = false, + .finalizer = PostMessageCallback.cancelled, }); } @@ -109,19 +136,39 @@ pub fn setOnMessageError(self: *BroadcastChannel, cb: ?js.Function.Global) !void const PostMessageCallback = struct { sender: *BroadcastChannel, + // A self-owned structured-clone snapshot of the posted message. Re-cloned + // (never shared) into each receiver's MessageEvent, then released. message: js.Value.Temp, frame: *Frame, + // Called by the scheduler if the task is dropped before it runs (e.g. page + // teardown). `run` and `cancelled` are mutually exclusive, so the snapshot + // is released exactly once. + fn cancelled(ctx: *anyopaque) void { + const self: *PostMessageCallback = @ptrCast(@alignCast(ctx)); + self.message.release(); + self.deinit(); + } + fn deinit(self: *PostMessageCallback) void { self.frame._factory.destroy(self); } fn run(ctx: *anyopaque) !?u32 { const self: *PostMessageCallback = @ptrCast(@alignCast(ctx)); + // LIFO: deinit() frees `self`, so it must run last; the snapshot is + // released after delivery (no MessageEvent owns it) but before deinit. defer self.deinit(); + defer self.message.release(); + const frame = self.frame; const sender = self.sender; + var ls: js.Local.Scope = undefined; + frame.js.localScope(&ls); + defer ls.deinit(); + const snapshot = self.message.local(&ls.local); + var it = frame._broadcast_channels.first; while (it) |node| : (it = node.next) { const channel = BroadcastChannel.fromNode(node); @@ -139,13 +186,27 @@ const PostMessageCallback = struct { continue; } + // Independent clone per receiver. The snapshot is already plain, + // serializable data, so this cannot raise a DataCloneError. The + // resulting temp is owned by the MessageEvent, which releases it on + // teardown — so each receiver frees only its own handle. + const cloned = snapshot.structuredCloneTo(&ls.local) catch |err| { + log.err(.dom, "BroadcastChannel.postMessage", .{ .err = err }); + continue; + }; + const cloned_temp = cloned.temp() catch |err| { + log.err(.dom, "BroadcastChannel.postMessage", .{ .err = err }); + continue; + }; + const event = (MessageEvent.initTrusted(comptime .wrap("message"), .{ - .data = .{ .value = self.message }, + .data = .{ .value = cloned_temp }, .origin = "", .source = null, }, frame._page) catch |err| { + cloned_temp.release(); log.err(.dom, "BroadcastChannel.postMessage", .{ .err = err }); - return null; + continue; }).asEvent(); frame._event_manager.dispatchDirect(target, event, channel._on_message, .{ .context = "BroadcastChannel message" }) catch |err| {