diff --git a/build.zig.zon b/build.zig.zon index d058875ad..a81b4011c 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -5,8 +5,8 @@ .minimum_zig_version = "0.15.2", .dependencies = .{ .v8 = .{ - .url = "https://github.com/lightpanda-io/zig-v8-fork/archive/refs/tags/v0.4.7.tar.gz", - .hash = "v8-0.0.0-xddH63edBADvfTFAnAlB6WutVta1yhsLuyPdXXt_BnKK", + .url = "https://github.com/lightpanda-io/zig-v8-fork/archive/a7556bba1f2e49cd179ab4d5eb8a10cfd73660bc.tar.gz", + .hash = "v8-0.0.0-xddH65qdBAA1hmB-gqEmhEzO0-aXKyEsmo1s34mssemb", }, // .v8 = .{ .path = "../zig-v8-fork" }, .brotli = .{ diff --git a/src/browser/Frame.zig b/src/browser/Frame.zig index b547fa995..fb9ff1f64 100644 --- a/src/browser/Frame.zig +++ b/src/browser/Frame.zig @@ -293,9 +293,8 @@ pub const InitOpts = struct { // When a frame/popup re-navigates, we should preserve the same window. // There are a couple reasons for this. First, iframe.contentWindow should - // maintain the same identity (which we don't correctly do now, but this - // is a first step). Secondly, a reference to the window can be acquired - // prior to navigation, and then used after. So it should remain valid. + // maintain the same identity. Secondly, a reference to the window can be + // acquired prior to navigation, and then used after. So it should remain valid. reuse_window: ?*Window = null, }; diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index 34a47da5f..ea121e64d 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -221,20 +221,6 @@ pub fn deinit(self: *Context) void { // have a dangling pointer to our freed Context struct. v8.v8__Context__SetAlignedPointerInEmbedderData(entered.handle, 1, null); - // Evict the window's entry from the identity map. The window pointer is the - // identity-map key (see Env.createContext), and an in-place navigation - // reuses the same Window address — a lingering entry would make the next - // context's getOrPut see `found_existing` and skip registering its global. - switch (self.global) { - .frame => |frame| { - if (self.identity.identity_map.fetchRemove(@intFromPtr(frame.window))) |kv| { - var global = kv.value; - v8.v8__Global__Reset(&global); - } - }, - .worker => {}, - } - v8.v8__Global__Reset(&self.handle); env.isolate.notifyContextDisposed(); // There can be other tasks associated with this context that we need to diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index 63026f8e1..ae5550f28 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -254,11 +254,20 @@ fn _createContext(self: *Env, global: anytype, params: ContextParams) !*Context const microtask_queue = v8.v8__MicrotaskQueue__New(isolate.handle, v8.kExplicit).?; errdefer v8.v8__MicrotaskQueue__DELETE(microtask_queue); + // Reuse the window's existing global proxy across an in-place navigation. + // The same *Window keeps its identity_map entry so reattaching it to the + // new context preserves WindowProxy identity + const reuse_global_object: ?*const v8.Value = blk: { + if (comptime !is_frame) break :blk null; + const existing = params.identity.identity_map.getPtr(@intFromPtr(global.window)) orelse break :blk null; + break :blk @ptrCast(v8.v8__Global__Get(existing, isolate.handle)); + }; + // Restore the context from the snapshot (0 = Page, 1 = Worker) const snapshot_index: u32 = if (comptime is_frame) 0 else 1; const v8_context = v8.v8__Context__FromSnapshot__Config(isolate.handle, snapshot_index, &.{ .global_template = null, - .global_object = null, + .global_object = reuse_global_object, .microtask_queue = microtask_queue, }).?; diff --git a/src/browser/tests/frames/window_reuse.html b/src/browser/tests/frames/window_reuse.html index 0128fac4c..09813f832 100644 --- a/src/browser/tests/frames/window_reuse.html +++ b/src/browser/tests/frames/window_reuse.html @@ -38,7 +38,10 @@ reuse and is tracked separately. This test guards the UAF, not the identity. state.resolve(); await state.done(() => { - // Reading the cached window's location returns the new, valid location + // Same Window object across the navigation: the v8 global proxy is reused + // (WindowProxy identity). + testing.expectEqual(true, w === iframe.contentWindow); + // And reading the cached window's location returns the new, valid location // (previously a UAF double-free on the freed Location). testing.expectEqual(true, w.location.href.endsWith('/support/win_b.html')); });