Make CDP server more authoritative with respect to IDs

The TL;DR is that this commit enforces the use of correct IDs, introduces a
BrowserContext, and adds some CDP tests.

These are the ids we need to be aware of when talking about CDP:
- id
- browserContextId
- targetId
- sessionId
- loaderId
- frameId

The `id` is the only one that _should_ originate from the driver. It's attached
to most messages and it's how we maintain a request -> response flow: when
the server responds to a specific message, it echo's back the id from the
requested message. (As opposed to out-of-band events sent from the server which
won't have an `id`). When I say "id" from this point forward, I mean every id
except for this req->res id.

Every other id is created by the browser.

Prior to this commit, we didn't really check incoming ids from the driver. If
the driver said "attachToTarget" and included a targetId, we just assumed that
this was the current targetId. This was aided by the fact that we only used
hard-coded IDS. If _we_ only "create" a frameId of "FRAME-1", then it's tempting
to think the driver will only ever send a frameId of "FRAME-1".

The issue with this approach is that _if_ the browser and driver fall out of sync
and there's only ever 1 browserContextId, 1 sessionId and 1 frameId, it's not
impossible to imagine cases where we behave on the thing.

Imagine this flow:
- Driver asks for a new BrowserContext
- Browser says OK, your browserContextId is 1
- Driver, for whatever reason, says close browserContextId 2
- Browser says, OK, but it doesn't check the id and just closes the only
  BrowserContext it knows about (which is 1)

By both re-using the same hard-coded ids, and not verifying that the ids sent
from the client correspond to the correct ids, any issues are going to be hard
to debug.

Currently LOADER_ID and FRAEM_ID are still hard-coded. Baby steps.
This commit is contained in:
Karl Seguin
2025-02-26 09:33:50 +08:00
committed by Pierre Tachoire
parent ccacac0597
commit a3e2b5246e
17 changed files with 1128 additions and 591 deletions

View File

@@ -29,7 +29,7 @@ pub fn processMessage(cmd: anytype) !void {
performSearch,
getSearchResults,
discardSearchResults,
}, cmd.action) orelse return error.UnknownMethod;
}, cmd.input.action) orelse return error.UnknownMethod;
switch (action) {
.enable => return cmd.sendResult(null, .{}),
@@ -133,14 +133,13 @@ fn getDocument(cmd: anytype) !void {
// pierce: ?bool = null,
// })) orelse return error.InvalidParams;
// retrieve the root node
const page = cmd.session.page orelse return error.NoPage;
const doc = page.doc orelse return error.NoDocument;
const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded;
const page = bc.session.currentPage() orelse return error.PageNotLoaded;
const doc = page.doc orelse return error.DocumentNotLoaded;
const state = cmd.cdp;
const node = parser.documentToNode(doc);
var n = try Node.init(node, &state.node_list);
_ = try n.initChildren(cmd.arena, node, &state.node_list);
var n = try Node.init(node, &bc.node_list);
_ = try n.initChildren(cmd.arena, node, &bc.node_list);
return cmd.sendResult(.{
.root = n,
@@ -184,21 +183,20 @@ fn performSearch(cmd: anytype) !void {
includeUserAgentShadowDOM: ?bool = null,
})) orelse return error.InvalidParams;
// retrieve the root node
const page = cmd.session.page orelse return error.NoPage;
const doc = page.doc orelse return error.NoDocument;
const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded;
const page = bc.session.currentPage() orelse return error.PageNotLoaded;
const doc = page.doc orelse return error.DocumentNotLoaded;
const list = try css.querySelectorAll(cmd.cdp.allocator, parser.documentToNode(doc), params.query);
const ln = list.nodes.items.len;
var ns = try NodeSearch.initCapacity(cmd.cdp.allocator, ln);
var state = cmd.cdp;
for (list.nodes.items) |n| {
const id = try state.node_list.set(n);
const id = try bc.node_list.set(n);
try ns.append(id);
}
try state.node_search_list.append(ns);
try bc.node_search_list.append(ns);
return cmd.sendResult(.{
.searchId = ns.name,
@@ -212,13 +210,14 @@ fn discardSearchResults(cmd: anytype) !void {
searchId: []const u8,
})) orelse return error.InvalidParams;
var state = cmd.cdp;
const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded;
// retrieve the search from context
for (state.node_search_list.items, 0..) |*s, i| {
for (bc.node_search_list.items, 0..) |*s, i| {
if (!std.mem.eql(u8, s.name, params.searchId)) continue;
s.deinit();
_ = state.node_search_list.swapRemove(i);
_ = bc.node_search_list.swapRemove(i);
break;
}
@@ -237,10 +236,11 @@ fn getSearchResults(cmd: anytype) !void {
return error.BadIndices;
}
const state = cmd.cdp;
const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded;
// retrieve the search from context
var ns: ?*const NodeSearch = undefined;
for (state.node_search_list.items) |s| {
for (bc.node_search_list.items) |s| {
if (!std.mem.eql(u8, s.name, params.searchId)) continue;
ns = &s;
break;