mirror/browser
1
0
Fork 0
mirror of https://github.com/lightpanda-io/browser.git synced 2026-06-11 17:46:32 -04:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #2526 from lightpanda-io/reset-node-registry

Browse Source 
browser: reset node registry on navigation
This commit is contained in:
Adrià Arrufat
2026-05-22 18:18:58 +02:00
committed by GitHub
parent 9c3d470ad2 3b285df432
commit 033df65c53
3 changed files with 49 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/browser/tests/mcp_nav.html Normal file
View File

@@ -0,0 +1,6 @@
<!DOCTYPE html>
<html>
<body>
<a id="navlink" href="about:blank">Navigate</a>
</body>
</html>

20
src/browser/tools.zig
View File

@@ -884,9 +884,15 @@ fn formatActionResult(
/// Finish a state-changing action: drain any queued navigation triggered by
/// the action, then tag `body` with the resulting page URL and title so the
/// caller (LLM, MCP client) can see whether the action triggered navigation.
fn finalizeAction(arena: std.mem.Allocator, session: *lp.Session, body: []const u8) ToolError![]const u8 {
fn finalizeAction(arena: std.mem.Allocator, session: *lp.Session, registry: *CDPNode.Registry, body: []const u8) ToolError![]const u8 {
const before = session.currentFrame();
try awaitQueuedNavigation(session);
const page = try requireFrame(session);
// A queued navigation that swaps the root frame tears down the previous
// Page (`Session.replaceRootImmediate` / `commitPendingPage`), so every
// DOMNode pointer in the registry now dangles. Drop the registry so the
// next action can't dereference freed memory.
if (before != null and before.? != page) registry.reset();
const page_title = page.getTitle() catch null;
return std.fmt.allocPrint(arena, "{s}. Page url: {s}, title: {s}", .{
body, page.url, page_title orelse "(none)",
@@ -904,7 +910,7 @@ fn execClick(arena: std.mem.Allocator, session: *lp.Session, registry: *CDPNode.
lp.actions.click(resolved.node, resolved.page) catch |err| return mapActionError(err);
const body = try formatActionResult(arena, "Clicked element", resolved.target, "");
return finalizeAction(arena, session, body);
return finalizeAction(arena, session, registry, body);
}
fn execFill(arena: std.mem.Allocator, session: *lp.Session, registry: *CDPNode.Registry, arguments: ?std.json.Value) ToolError![]const u8 {
@@ -923,7 +929,7 @@ fn execFill(arena: std.mem.Allocator, session: *lp.Session, registry: *CDPNode.R
// Show the original reference (e.g. $LP_PASSWORD) in the result, not the resolved value
const suffix = std.fmt.allocPrint(arena, " with \"{s}\"", .{raw_text}) catch return ToolError.InternalError;
const body = try formatActionResult(arena, "Filled element", resolved.target, suffix);
return finalizeAction(arena, session, body);
return finalizeAction(arena, session, registry, body);
}
fn execScroll(arena: std.mem.Allocator, session: *lp.Session, registry: *CDPNode.Registry, arguments: ?std.json.Value) ToolError![]const u8 {
@@ -975,7 +981,7 @@ fn execHover(arena: std.mem.Allocator, session: *lp.Session, registry: *CDPNode.
lp.actions.hover(resolved.node, resolved.page) catch |err| return mapActionError(err);
const body = try formatActionResult(arena, "Hovered element", resolved.target, "");
return finalizeAction(arena, session, body);
return finalizeAction(arena, session, registry, body);
}
fn execPress(arena: std.mem.Allocator, session: *lp.Session, registry: *CDPNode.Registry, arguments: ?std.json.Value) ToolError![]const u8 {
@@ -993,7 +999,7 @@ fn execPress(arena: std.mem.Allocator, session: *lp.Session, registry: *CDPNode.
// Pressing Enter on a form input triggers implicit form submission;
// `finalizeAction` drains the queued navigation before tagging the body.
const body = std.fmt.allocPrint(arena, "Pressed key '{s}'", .{args.key}) catch return ToolError.InternalError;
return finalizeAction(arena, session, body);
return finalizeAction(arena, session, registry, body);
}
fn execSelectOption(arena: std.mem.Allocator, session: *lp.Session, registry: *CDPNode.Registry, arguments: ?std.json.Value) ToolError![]const u8 {
@@ -1009,7 +1015,7 @@ fn execSelectOption(arena: std.mem.Allocator, session: *lp.Session, registry: *C
const prefix = std.fmt.allocPrint(arena, "Selected option '{s}'", .{args.value}) catch return ToolError.InternalError;
const body = try formatActionResult(arena, prefix, resolved.target, "");
return finalizeAction(arena, session, body);
return finalizeAction(arena, session, registry, body);
}
fn execSetChecked(arena: std.mem.Allocator, session: *lp.Session, registry: *CDPNode.Registry, arguments: ?std.json.Value) ToolError![]const u8 {
@@ -1026,7 +1032,7 @@ fn execSetChecked(arena: std.mem.Allocator, session: *lp.Session, registry: *CDP
const state_str: []const u8 = if (args.checked) "checked" else "unchecked";
const suffix = std.fmt.allocPrint(arena, " to {s}", .{state_str}) catch return ToolError.InternalError;
const body = try formatActionResult(arena, "Set element", resolved.target, suffix);
return finalizeAction(arena, session, body);
return finalizeAction(arena, session, registry, body);
}
fn execFindElement(arena: std.mem.Allocator, session: *lp.Session, registry: *CDPNode.Registry, arguments: ?std.json.Value) ToolError![]const u8 {

30
src/mcp/tools.zig
View File

@@ -689,6 +689,36 @@ test "MCP - Actions: click, fill, scroll, hover, press, selectOption, setChecked
try testing.expect(result.isTrue());
}
// Regression for the segfault Karl hit on PR #2520: clicking a link via
// `backendNodeId` queued a navigation, `finalizeAction` swapped pages but
// left the registry intact, and a second click on the same id dereferenced
// a freed DOMNode.
test "MCP - click that navigates clears node registry" {
defer testing.reset();
const aa = testing.arena_allocator;
var out: std.io.Writer.Allocating = .init(aa);
const server = try testLoadPage("http://localhost:9582/src/browser/tests/mcp_nav.html", &out.writer);
defer server.deinit();
const before_frame = server.session.currentFrame().?;
const link = before_frame.document.getElementById("navlink", before_frame).?.asNode();
const link_id = (try server.node_registry.register(link)).id;
try testing.expect(server.node_registry.lookup_by_id.contains(link_id));
var id_buf: [12]u8 = undefined;
const id_str = std.fmt.bufPrint(&id_buf, "{d}", .{link_id}) catch unreachable;
const click_msg = try std.mem.concat(aa, u8, &.{
"{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"tools/call\",\"params\":{\"name\":\"click\",\"arguments\":{\"backendNodeId\":",
id_str,
"}}}",
});
try router.handleMessage(server, aa, click_msg);
try testing.expect(server.session.currentFrame().? != before_frame);
try testing.expect(!server.node_registry.lookup_by_id.contains(link_id));
}
test "MCP - Actions by selector: hover, selectOption, setChecked" {
defer testing.reset();
const aa = testing.arena_allocator;