From dab82fa29a8135c907236568cc5d5e5c13686608 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Thu, 28 May 2026 20:17:28 +0200 Subject: [PATCH 1/5] storage: persist localStorage/sessionStorage across navigations, fix quota MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the per-Window storage bucket to an origin-keyed Shed on the Session so localStorage and sessionStorage survive navigation within an origin, matching the Web Storage spec. Also fixes two pre-existing bugs surfaced by this work: - setItem's quota counter was incremented on every call, never decremented on overwrite — five same-key overwrites tripped the cap spuriously. Now subtracts the old value's length first. - Shed.getOrPut used allocator.free on a single-pointer allocation where allocator.destroy was required, and inserted into _origins before its dependent allocations could fail. Reordered so the entry is only put once both key dupe and bucket creation have succeeded. Adds an MCP test that round-trips localStorage between two origins via the eval tool to lock in the persistence + isolation contract. --- src/browser/tests/storage.html | 4 +- src/browser/webapi/Window.zig | 12 +++- src/browser/webapi/storage/storage.zig | 80 ++++++++++++++++------- src/mcp/tools.zig | 90 ++++++++++++++++++++++++++ 4 files changed, 159 insertions(+), 27 deletions(-) diff --git a/src/browser/tests/storage.html b/src/browser/tests/storage.html index cb41bb3d..53aa2f92 100644 --- a/src/browser/tests/storage.html +++ b/src/browser/tests/storage.html @@ -91,9 +91,9 @@ diff --git a/src/browser/webapi/Window.zig b/src/browser/webapi/Window.zig index f6bef76c..751df5d3 100644 --- a/src/browser/webapi/Window.zig +++ b/src/browser/webapi/Window.zig @@ -71,7 +71,6 @@ _model_context: ModelContext = .init, _screen: *Screen, _visual_viewport: *VisualViewport, _performance: Performance, -_storage_bucket: storage.Bucket = .{}, _on_load: ?js.Function.Global = null, _on_pageshow: ?js.Function.Global = null, _on_popstate: ?js.Function.Global = null, @@ -233,12 +232,19 @@ pub fn getPerformance(self: *Window) *Performance { return &self._performance; } +fn bucketForOrigin(self: *Window) *storage.Bucket { + return self._frame._session.storage_shed.getOrPut( + self._frame._session.browser.app.allocator, + self.getOrigin(), + ) catch @panic("OOM"); +} + pub fn getLocalStorage(self: *Window) *storage.Lookup { - return &self._storage_bucket.local; + return &self.bucketForOrigin().local; } pub fn getSessionStorage(self: *Window) *storage.Lookup { - return &self._storage_bucket.session; + return &self.bucketForOrigin().session; } pub fn getOrigin(self: *const Window) []const u8 { diff --git a/src/browser/webapi/storage/storage.zig b/src/browser/webapi/storage/storage.zig index 396c81b9..3261263b 100644 --- a/src/browser/webapi/storage/storage.zig +++ b/src/browser/webapi/storage/storage.zig @@ -35,28 +35,35 @@ pub const Shed = struct { var it = self._origins.iterator(); while (it.next()) |kv| { allocator.free(kv.key_ptr.*); + kv.value_ptr.*.deinit(allocator); allocator.destroy(kv.value_ptr.*); } self._origins.deinit(allocator); } pub fn getOrPut(self: *Shed, allocator: Allocator, origin: []const u8) !*Bucket { - const gop = try self._origins.getOrPut(allocator, origin); - if (gop.found_existing) { - return gop.value_ptr.*; - } + if (self._origins.get(origin)) |existing| return existing; + const key_owned = try allocator.dupe(u8, origin); + errdefer allocator.free(key_owned); const bucket = try allocator.create(Bucket); - errdefer allocator.free(bucket); + errdefer allocator.destroy(bucket); bucket.* = .{}; - gop.key_ptr.* = try allocator.dupe(u8, origin); - gop.value_ptr.* = bucket; + try self._origins.put(allocator, key_owned, bucket); return bucket; } }; -pub const Bucket = struct { local: Lookup = .{}, session: Lookup = .{} }; +pub const Bucket = struct { + local: Lookup = .{}, + session: Lookup = .{}, + + pub fn deinit(self: *Bucket, allocator: Allocator) void { + self.local.deinit(allocator); + self.session.deinit(allocator); + } +}; pub const Lookup = struct { _data: std.StringHashMapUnmanaged([]const u8) = .empty, @@ -64,6 +71,16 @@ pub const Lookup = struct { const max_size = 5 * 1024 * 1024; + pub fn deinit(self: *Lookup, allocator: Allocator) void { + var it = self._data.iterator(); + while (it.next()) |entry| { + allocator.free(entry.key_ptr.*); + allocator.free(entry.value_ptr.*); + } + self._data.deinit(allocator); + self._size = 0; + } + pub fn getItem(self: *const Lookup, key_: ?[]const u8) ?[]const u8 { const k = key_ orelse return null; return self._data.get(k); @@ -71,28 +88,47 @@ pub const Lookup = struct { pub fn setItem(self: *Lookup, key_: ?[]const u8, value: []const u8, frame: *Frame) !void { const k = key_ orelse return; + const allocator = frame._session.browser.app.allocator; - if (self._size + value.len > max_size) { + const old_len = if (self._data.get(k)) |old| old.len else 0; + std.debug.assert(old_len <= self._size); + if (self._size - old_len + value.len > max_size) { return error.QuotaExceeded; } - defer self._size += value.len; - const key_owned = try frame.dupeString(k); - const value_owned = try frame.dupeString(value); + if (self._data.getPtr(k)) |value_ptr| { + const value_owned = try allocator.dupe(u8, value); + self._size -= value_ptr.*.len; + allocator.free(value_ptr.*); + value_ptr.* = value_owned; + self._size += value.len; + } else { + const key_owned = try allocator.dupe(u8, k); + errdefer allocator.free(key_owned); + const value_owned = try allocator.dupe(u8, value); + errdefer allocator.free(value_owned); - const gop = try self._data.getOrPut(frame.arena, key_owned); - gop.value_ptr.* = value_owned; - } - - pub fn removeItem(self: *Lookup, key_: ?[]const u8) void { - const k = key_ orelse return; - if (self._data.get(k)) |value| { - self._size -= value.len; - _ = self._data.remove(k); + try self._data.put(allocator, key_owned, value_owned); + self._size += value.len; } } - pub fn clear(self: *Lookup) void { + pub fn removeItem(self: *Lookup, key_: ?[]const u8, frame: *Frame) void { + const k = key_ orelse return; + const allocator = frame._session.browser.app.allocator; + const kv = self._data.fetchRemove(k) orelse return; + self._size -= kv.value.len; + allocator.free(kv.key); + allocator.free(kv.value); + } + + pub fn clear(self: *Lookup, frame: *Frame) void { + const allocator = frame._session.browser.app.allocator; + var it = self._data.iterator(); + while (it.next()) |entry| { + allocator.free(entry.key_ptr.*); + allocator.free(entry.value_ptr.*); + } self._data.clearRetainingCapacity(); self._size = 0; } diff --git a/src/mcp/tools.zig b/src/mcp/tools.zig index 053957d8..10d8879e 100644 --- a/src/mcp/tools.zig +++ b/src/mcp/tools.zig @@ -980,6 +980,96 @@ test "MCP - evaluate error reporting" { } }, out.written()); } +test "MCP - eval: localStorage persists across navigations and is origin-scoped" { + defer testing.reset(); + var out: std.io.Writer.Allocating = .init(testing.arena_allocator); + const server = try testLoadPage("http://localhost:9582/src/browser/tests/mcp_actions.html", &out.writer); + defer server.deinit(); + + // 1. Set a value in localStorage on localhost + const first = + \\{ + \\ "jsonrpc": "2.0", + \\ "id": 1, + \\ "method": "tools/call", + \\ "params": { + \\ "name": "eval", + \\ "arguments": { "script": "localStorage.setItem('foo', 'bar'); localStorage.getItem('foo')" } + \\ } + \\} + ; + try router.handleMessage(server, testing.arena_allocator, first); + try testing.expectJson(.{ .id = 1, .result = .{ + .content = &.{.{ .type = "text", .text = "bar" }}, + } }, out.written()); + + // 2. Navigate to another origin (127.0.0.1) + out.clearRetainingCapacity(); + const navigate_other = + \\{ + \\ "jsonrpc": "2.0", + \\ "id": 2, + \\ "method": "tools/call", + \\ "params": { + \\ "name": "goto", + \\ "arguments": { "url": "http://127.0.0.1:9582/src/browser/tests/mcp_actions.html" } + \\ } + \\} + ; + try router.handleMessage(server, testing.arena_allocator, navigate_other); + + // 3. Get the value on 127.0.0.1, verify it is null (isolated origin storage) + out.clearRetainingCapacity(); + const second = + \\{ + \\ "jsonrpc": "2.0", + \\ "id": 3, + \\ "method": "tools/call", + \\ "params": { + \\ "name": "eval", + \\ "arguments": { "script": "localStorage.getItem('foo')" } + \\ } + \\} + ; + try router.handleMessage(server, testing.arena_allocator, second); + try testing.expectJson(.{ .id = 3, .result = .{ + .content = &.{.{ .type = "text", .text = "null" }}, + } }, out.written()); + + // 4. Navigate back to localhost + out.clearRetainingCapacity(); + const navigate_back = + \\{ + \\ "jsonrpc": "2.0", + \\ "id": 4, + \\ "method": "tools/call", + \\ "params": { + \\ "name": "goto", + \\ "arguments": { "url": "http://localhost:9582/src/browser/tests/mcp_actions.html" } + \\ } + \\} + ; + try router.handleMessage(server, testing.arena_allocator, navigate_back); + + // 5. Get the value on localhost, verify it is still 'bar' + out.clearRetainingCapacity(); + const third = + \\{ + \\ "jsonrpc": "2.0", + \\ "id": 5, + \\ "method": "tools/call", + \\ "params": { + \\ "name": "eval", + \\ "arguments": { "script": "localStorage.getItem('foo')" } + \\ } + \\} + ; + try router.handleMessage(server, testing.arena_allocator, third); + try testing.expectJson(.{ .id = 5, .result = .{ + .content = &.{.{ .type = "text", .text = "bar" }}, + } }, out.written()); +} + test "MCP - Actions: click, fill, scroll, hover, press, selectOption, setChecked" { defer testing.reset(); const aa = testing.arena_allocator; From ca9911c641cb342f43355cd63209ac3b698823e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Fri, 29 May 2026 07:51:33 +0200 Subject: [PATCH 2/5] storage: key bucket by JS Origin key so opaque origins don't collide --- src/browser/webapi/Window.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/browser/webapi/Window.zig b/src/browser/webapi/Window.zig index 751df5d3..2a7e1603 100644 --- a/src/browser/webapi/Window.zig +++ b/src/browser/webapi/Window.zig @@ -235,7 +235,7 @@ pub fn getPerformance(self: *Window) *Performance { fn bucketForOrigin(self: *Window) *storage.Bucket { return self._frame._session.storage_shed.getOrPut( self._frame._session.browser.app.allocator, - self.getOrigin(), + self._frame.js.origin.key, ) catch @panic("OOM"); } From 78312768ced984f8447a00ac650667136bb58995 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Fri, 29 May 2026 07:55:54 +0200 Subject: [PATCH 3/5] storage: use getOrPut in Shed.getOrPut --- src/browser/webapi/storage/storage.zig | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/browser/webapi/storage/storage.zig b/src/browser/webapi/storage/storage.zig index 3261263b..d46a57dc 100644 --- a/src/browser/webapi/storage/storage.zig +++ b/src/browser/webapi/storage/storage.zig @@ -42,15 +42,16 @@ pub const Shed = struct { } pub fn getOrPut(self: *Shed, allocator: Allocator, origin: []const u8) !*Bucket { - if (self._origins.get(origin)) |existing| return existing; + const gop = try self._origins.getOrPut(allocator, origin); + if (gop.found_existing) return gop.value_ptr.*; + errdefer std.debug.assert(self._origins.remove(origin)); - const key_owned = try allocator.dupe(u8, origin); - errdefer allocator.free(key_owned); const bucket = try allocator.create(Bucket); errdefer allocator.destroy(bucket); bucket.* = .{}; - try self._origins.put(allocator, key_owned, bucket); + gop.key_ptr.* = try allocator.dupe(u8, origin); + gop.value_ptr.* = bucket; return bucket; } }; From f62f9b42cea4c99b5dcbc86f849bd32862e1a1a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Fri, 29 May 2026 08:00:15 +0200 Subject: [PATCH 4/5] storage: store allocator in Lookup --- src/browser/webapi/storage/storage.zig | 62 ++++++++++++++------------ 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/src/browser/webapi/storage/storage.zig b/src/browser/webapi/storage/storage.zig index d46a57dc..1e9846c9 100644 --- a/src/browser/webapi/storage/storage.zig +++ b/src/browser/webapi/storage/storage.zig @@ -18,7 +18,6 @@ const std = @import("std"); const js = @import("../../js/js.zig"); -const Frame = @import("../../Frame.zig"); const Allocator = std.mem.Allocator; @@ -35,7 +34,7 @@ pub const Shed = struct { var it = self._origins.iterator(); while (it.next()) |kv| { allocator.free(kv.key_ptr.*); - kv.value_ptr.*.deinit(allocator); + kv.value_ptr.*.deinit(); allocator.destroy(kv.value_ptr.*); } self._origins.deinit(allocator); @@ -48,7 +47,7 @@ pub const Shed = struct { const bucket = try allocator.create(Bucket); errdefer allocator.destroy(bucket); - bucket.* = .{}; + bucket.* = .init(allocator); gop.key_ptr.* = try allocator.dupe(u8, origin); gop.value_ptr.* = bucket; @@ -57,28 +56,36 @@ pub const Shed = struct { }; pub const Bucket = struct { - local: Lookup = .{}, - session: Lookup = .{}, + local: Lookup, + session: Lookup, - pub fn deinit(self: *Bucket, allocator: Allocator) void { - self.local.deinit(allocator); - self.session.deinit(allocator); + pub fn init(allocator: Allocator) Bucket { + return .{ + .local = .{ .allocator = allocator }, + .session = .{ .allocator = allocator }, + }; + } + + pub fn deinit(self: *Bucket) void { + self.local.deinit(); + self.session.deinit(); } }; pub const Lookup = struct { _data: std.StringHashMapUnmanaged([]const u8) = .empty, _size: usize = 0, + allocator: Allocator, const max_size = 5 * 1024 * 1024; - pub fn deinit(self: *Lookup, allocator: Allocator) void { + pub fn deinit(self: *Lookup) void { var it = self._data.iterator(); while (it.next()) |entry| { - allocator.free(entry.key_ptr.*); - allocator.free(entry.value_ptr.*); + self.allocator.free(entry.key_ptr.*); + self.allocator.free(entry.value_ptr.*); } - self._data.deinit(allocator); + self._data.deinit(self.allocator); self._size = 0; } @@ -87,9 +94,8 @@ pub const Lookup = struct { return self._data.get(k); } - pub fn setItem(self: *Lookup, key_: ?[]const u8, value: []const u8, frame: *Frame) !void { + pub fn setItem(self: *Lookup, key_: ?[]const u8, value: []const u8) !void { const k = key_ orelse return; - const allocator = frame._session.browser.app.allocator; const old_len = if (self._data.get(k)) |old| old.len else 0; std.debug.assert(old_len <= self._size); @@ -98,37 +104,35 @@ pub const Lookup = struct { } if (self._data.getPtr(k)) |value_ptr| { - const value_owned = try allocator.dupe(u8, value); + const value_owned = try self.allocator.dupe(u8, value); self._size -= value_ptr.*.len; - allocator.free(value_ptr.*); + self.allocator.free(value_ptr.*); value_ptr.* = value_owned; self._size += value.len; } else { - const key_owned = try allocator.dupe(u8, k); - errdefer allocator.free(key_owned); - const value_owned = try allocator.dupe(u8, value); - errdefer allocator.free(value_owned); + const key_owned = try self.allocator.dupe(u8, k); + errdefer self.allocator.free(key_owned); + const value_owned = try self.allocator.dupe(u8, value); + errdefer self.allocator.free(value_owned); - try self._data.put(allocator, key_owned, value_owned); + try self._data.put(self.allocator, key_owned, value_owned); self._size += value.len; } } - pub fn removeItem(self: *Lookup, key_: ?[]const u8, frame: *Frame) void { + pub fn removeItem(self: *Lookup, key_: ?[]const u8) void { const k = key_ orelse return; - const allocator = frame._session.browser.app.allocator; const kv = self._data.fetchRemove(k) orelse return; self._size -= kv.value.len; - allocator.free(kv.key); - allocator.free(kv.value); + self.allocator.free(kv.key); + self.allocator.free(kv.value); } - pub fn clear(self: *Lookup, frame: *Frame) void { - const allocator = frame._session.browser.app.allocator; + pub fn clear(self: *Lookup) void { var it = self._data.iterator(); while (it.next()) |entry| { - allocator.free(entry.key_ptr.*); - allocator.free(entry.value_ptr.*); + self.allocator.free(entry.key_ptr.*); + self.allocator.free(entry.value_ptr.*); } self._data.clearRetainingCapacity(); self._size = 0; From f3c8595b3923c0ce2610ec2fc0fa02cc9b7198d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Fri, 29 May 2026 11:04:19 +0200 Subject: [PATCH 5/5] storage: rename allocator to _allocator in Lookup --- src/browser/webapi/storage/storage.zig | 34 +++++++++++++------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/browser/webapi/storage/storage.zig b/src/browser/webapi/storage/storage.zig index 1e9846c9..127685fe 100644 --- a/src/browser/webapi/storage/storage.zig +++ b/src/browser/webapi/storage/storage.zig @@ -61,8 +61,8 @@ pub const Bucket = struct { pub fn init(allocator: Allocator) Bucket { return .{ - .local = .{ .allocator = allocator }, - .session = .{ .allocator = allocator }, + .local = .{ ._allocator = allocator }, + .session = .{ ._allocator = allocator }, }; } @@ -75,17 +75,17 @@ pub const Bucket = struct { pub const Lookup = struct { _data: std.StringHashMapUnmanaged([]const u8) = .empty, _size: usize = 0, - allocator: Allocator, + _allocator: Allocator, const max_size = 5 * 1024 * 1024; pub fn deinit(self: *Lookup) void { var it = self._data.iterator(); while (it.next()) |entry| { - self.allocator.free(entry.key_ptr.*); - self.allocator.free(entry.value_ptr.*); + self._allocator.free(entry.key_ptr.*); + self._allocator.free(entry.value_ptr.*); } - self._data.deinit(self.allocator); + self._data.deinit(self._allocator); self._size = 0; } @@ -104,18 +104,18 @@ pub const Lookup = struct { } if (self._data.getPtr(k)) |value_ptr| { - const value_owned = try self.allocator.dupe(u8, value); + const value_owned = try self._allocator.dupe(u8, value); self._size -= value_ptr.*.len; - self.allocator.free(value_ptr.*); + self._allocator.free(value_ptr.*); value_ptr.* = value_owned; self._size += value.len; } else { - const key_owned = try self.allocator.dupe(u8, k); - errdefer self.allocator.free(key_owned); - const value_owned = try self.allocator.dupe(u8, value); - errdefer self.allocator.free(value_owned); + const key_owned = try self._allocator.dupe(u8, k); + errdefer self._allocator.free(key_owned); + const value_owned = try self._allocator.dupe(u8, value); + errdefer self._allocator.free(value_owned); - try self._data.put(self.allocator, key_owned, value_owned); + try self._data.put(self._allocator, key_owned, value_owned); self._size += value.len; } } @@ -124,15 +124,15 @@ pub const Lookup = struct { const k = key_ orelse return; const kv = self._data.fetchRemove(k) orelse return; self._size -= kv.value.len; - self.allocator.free(kv.key); - self.allocator.free(kv.value); + self._allocator.free(kv.key); + self._allocator.free(kv.value); } pub fn clear(self: *Lookup) void { var it = self._data.iterator(); while (it.next()) |entry| { - self.allocator.free(entry.key_ptr.*); - self.allocator.free(entry.value_ptr.*); + self._allocator.free(entry.key_ptr.*); + self._allocator.free(entry.value_ptr.*); } self._data.clearRetainingCapacity(); self._size = 0;