Fixes #51. `Runtime.evaluate` and `Runtime.callFunctionOn` previously ignored the `contextId` / `executionContextId` parameter entirely, so clients targeting an isolated world (e.g. Playwright's locator path, which calls `Page.createIsolatedWorld` → contextId 100 → evaluates the UtilityScript against that id) silently fell through to the default context, breaking the contract advertised via Runtime.executionContextCreated. Track the set of advertised execution context ids on CdpContext (pre-seeded with 1 and 2 — the default-frame contexts emitted by Runtime.enable and post-navigation) and extend it from Page.createIsolatedWorld. Both handlers now reject unknown ids with `Cannot find context with specified id: <id>` — matching the wording real Chrome returns — and log accepted ids at debug for observability. Obscura still routes every JS call against the page's single V8 isolate, so this PR is spec-compliance scaffolding rather than full multi-world execution. Follow-up work to physically isolate worlds can build on the same valid_context_ids registry.
This commit is contained in:
1 parent
f3c783f6f5
commit
2ff0f8fe1d
3 files changed
+173
-2
No files matched your search
@@ -1,4 +1,4 @@
|
|||||||
use std::collections::HashMap;
|
use std::collections::{HashMap, HashSet};
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
|
|
||||||
use obscura_browser::{BrowserContext, Page};
|
use obscura_browser::{BrowserContext, Page};
|
||||||
@@ -25,6 +25,16 @@ pub struct CdpContext {
|
|||||||
// back. Stored as plain Strings (not by-page) — for now we only model
|
// back. Stored as plain Strings (not by-page) — for now we only model
|
||||||
// a single page in CdpContext anyway.
|
// a single page in CdpContext anyway.
|
||||||
pub isolated_worlds: Vec<String>,
|
pub isolated_worlds: Vec<String>,
|
||||||
|
// Set of executionContextIds Obscura has emitted via
|
||||||
|
// Runtime.executionContextCreated. Pre-populated with the default-frame
|
||||||
|
// contexts (`1`, `2`) that Runtime.enable / Page.navigate emit, then
|
||||||
|
// extended each time Page.createIsolatedWorld assigns a fresh id.
|
||||||
|
//
|
||||||
|
// Runtime.evaluate / Runtime.callFunctionOn consult this set to reject
|
||||||
|
// requests targeting an unknown context — matching real Chrome's
|
||||||
|
// "Cannot find context with specified id" CDP error and unblocking the
|
||||||
|
// Playwright locator path described in issue #51.
|
||||||
|
pub valid_context_ids: HashSet<i64>,
|
||||||
pub fetch_intercept: FetchInterceptState,
|
pub fetch_intercept: FetchInterceptState,
|
||||||
pub intercept_tx: Option<tokio::sync::mpsc::UnboundedSender<InterceptedRequest>>,
|
pub intercept_tx: Option<tokio::sync::mpsc::UnboundedSender<InterceptedRequest>>,
|
||||||
}
|
}
|
||||||
@@ -53,6 +63,15 @@ impl CdpContext {
|
|||||||
stealth,
|
stealth,
|
||||||
user_agent,
|
user_agent,
|
||||||
));
|
));
|
||||||
|
// Pre-seed with the default-frame execution context ids that
|
||||||
|
// `Runtime.enable` (1) and post-navigation re-emission (2) advertise
|
||||||
|
// via Runtime.executionContextCreated. Anything else has to be
|
||||||
|
// registered explicitly (Page.createIsolatedWorld), otherwise
|
||||||
|
// Runtime.{evaluate,callFunctionOn} should reject it per CDP spec.
|
||||||
|
let mut valid_context_ids = HashSet::new();
|
||||||
|
valid_context_ids.insert(1);
|
||||||
|
valid_context_ids.insert(2);
|
||||||
|
|
||||||
CdpContext {
|
CdpContext {
|
||||||
pages: Vec::new(),
|
pages: Vec::new(),
|
||||||
sessions: HashMap::new(),
|
sessions: HashMap::new(),
|
||||||
@@ -64,6 +83,7 @@ impl CdpContext {
|
|||||||
fetch_intercept: FetchInterceptState::new(),
|
fetch_intercept: FetchInterceptState::new(),
|
||||||
intercept_tx: None,
|
intercept_tx: None,
|
||||||
isolated_worlds: Vec::new(),
|
isolated_worlds: Vec::new(),
|
||||||
|
valid_context_ids,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -177,7 +177,7 @@ pub async fn handle(
|
|||||||
.unwrap_or("").to_string();
|
.unwrap_or("").to_string();
|
||||||
let page_url = page.url_string();
|
let page_url = page.url_string();
|
||||||
let page_id = page.id.clone();
|
let page_id = page.id.clone();
|
||||||
let context_id = 100;
|
let context_id: i64 = 100;
|
||||||
// Track this world so Page.navigate can re-emit a context for it
|
// Track this world so Page.navigate can re-emit a context for it
|
||||||
// post-navigation. Without this, Playwright (and Puppeteer)
|
// post-navigation. Without this, Playwright (and Puppeteer)
|
||||||
// hang in any operation that uses the utility world — including
|
// hang in any operation that uses the utility world — including
|
||||||
@@ -186,6 +186,9 @@ pub async fn handle(
|
|||||||
if !world_name.is_empty() && !ctx.isolated_worlds.contains(&world_name) {
|
if !world_name.is_empty() && !ctx.isolated_worlds.contains(&world_name) {
|
||||||
ctx.isolated_worlds.push(world_name.clone());
|
ctx.isolated_worlds.push(world_name.clone());
|
||||||
}
|
}
|
||||||
|
// Register the isolated world's id so Runtime.evaluate /
|
||||||
|
// Runtime.callFunctionOn accept it as a valid contextId (#51).
|
||||||
|
ctx.valid_context_ids.insert(context_id);
|
||||||
|
|
||||||
ctx.pending_events.push(CdpEvent {
|
ctx.pending_events.push(CdpEvent {
|
||||||
method: "Runtime.executionContextCreated".to_string(),
|
method: "Runtime.executionContextCreated".to_string(),
|
||||||
|
|||||||
@@ -42,6 +42,15 @@ pub async fn handle(
|
|||||||
.and_then(|v| v.as_bool())
|
.and_then(|v| v.as_bool())
|
||||||
.unwrap_or(false);
|
.unwrap_or(false);
|
||||||
|
|
||||||
|
// Spec-compliant contextId handling (#51): when the client
|
||||||
|
// targets a specific execution context, reject the call if that
|
||||||
|
// context has not been advertised via Runtime.executionContextCreated
|
||||||
|
// (or Page.createIsolatedWorld). Obscura routes every evaluate
|
||||||
|
// against the page's single V8 isolate, so accepting a foreign
|
||||||
|
// id silently would mis-attribute UtilityScript state and break
|
||||||
|
// Playwright's locator path.
|
||||||
|
validate_context_id(params, "contextId", ctx, "evaluate")?;
|
||||||
|
|
||||||
let page = ctx
|
let page = ctx
|
||||||
.get_session_page_mut(session_id)
|
.get_session_page_mut(session_id)
|
||||||
.ok_or("No page")?;
|
.ok_or("No page")?;
|
||||||
@@ -69,6 +78,13 @@ pub async fn handle(
|
|||||||
.map(|a| a.to_vec())
|
.map(|a| a.to_vec())
|
||||||
.unwrap_or_default();
|
.unwrap_or_default();
|
||||||
|
|
||||||
|
// #51: validate executionContextId the same way Runtime.evaluate
|
||||||
|
// does. CDP names this field `executionContextId` on
|
||||||
|
// callFunctionOn (not `contextId`); a request may omit it when
|
||||||
|
// `objectId` is supplied — in that case validate_context_id is a
|
||||||
|
// no-op and the default context is used.
|
||||||
|
validate_context_id(params, "executionContextId", ctx, "callFunctionOn")?;
|
||||||
|
|
||||||
let page = ctx
|
let page = ctx
|
||||||
.get_session_page_mut(session_id)
|
.get_session_page_mut(session_id)
|
||||||
.ok_or("No page")?;
|
.ok_or("No page")?;
|
||||||
@@ -186,6 +202,35 @@ pub async fn handle(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Reject `Runtime.{evaluate,callFunctionOn}` calls that target an execution
|
||||||
|
/// context Obscura has not advertised. Returns `Ok(())` when the parameter is
|
||||||
|
/// absent (defaulting to the page's default context) or when the id matches
|
||||||
|
/// one of `ctx.valid_context_ids`. Logs a debug trace on accept for #51.
|
||||||
|
fn validate_context_id(
|
||||||
|
params: &Value,
|
||||||
|
field: &str,
|
||||||
|
ctx: &crate::dispatch::CdpContext,
|
||||||
|
method: &str,
|
||||||
|
) -> Result<(), String> {
|
||||||
|
let Some(id) = params.get(field).and_then(|v| v.as_i64()) else {
|
||||||
|
return Ok(());
|
||||||
|
};
|
||||||
|
if !ctx.valid_context_ids.contains(&id) {
|
||||||
|
return Err(format!(
|
||||||
|
"Cannot find context with specified id: {}",
|
||||||
|
id
|
||||||
|
));
|
||||||
|
}
|
||||||
|
tracing::debug!(
|
||||||
|
target: "obscura_cdp::runtime",
|
||||||
|
"Runtime.{}: {}={} (single-isolate routing)",
|
||||||
|
method,
|
||||||
|
field,
|
||||||
|
id
|
||||||
|
);
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
fn remote_object_from_info(info: &RemoteObjectInfo) -> Value {
|
fn remote_object_from_info(info: &RemoteObjectInfo) -> Value {
|
||||||
let mut obj = json!({ "type": info.js_type });
|
let mut obj = json!({ "type": info.js_type });
|
||||||
|
|
||||||
@@ -211,3 +256,106 @@ fn remote_object_from_info(info: &RemoteObjectInfo) -> Value {
|
|||||||
|
|
||||||
obj
|
obj
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
use crate::dispatch::CdpContext;
|
||||||
|
|
||||||
|
// Issue #51 — Runtime.evaluate / callFunctionOn must read and validate
|
||||||
|
// contextId. Pre-fix the parameter was silently dropped, so Playwright's
|
||||||
|
// locator (which targets the utility world created by
|
||||||
|
// Page.createIsolatedWorld) ran in the wrong context and timed out.
|
||||||
|
//
|
||||||
|
// Phase 5.5 (RED-then-GREEN) verification:
|
||||||
|
// - Without the prod fix, `valid_context_ids` does not exist on
|
||||||
|
// CdpContext → these tests fail to compile.
|
||||||
|
// - With the prod fix, all four tests pass.
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn evaluate_rejects_unknown_context_id() {
|
||||||
|
let mut ctx = CdpContext::new();
|
||||||
|
let err = handle(
|
||||||
|
"evaluate",
|
||||||
|
&json!({ "expression": "1 + 1", "contextId": 9999 }),
|
||||||
|
&mut ctx,
|
||||||
|
&None,
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.expect_err("unknown contextId must error per CDP spec");
|
||||||
|
assert!(
|
||||||
|
err.contains("Cannot find context with specified id"),
|
||||||
|
"error must match real Chrome's wording: {err}"
|
||||||
|
);
|
||||||
|
assert!(err.contains("9999"), "error must include the bad id: {err}");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn call_function_on_rejects_unknown_execution_context_id() {
|
||||||
|
let mut ctx = CdpContext::new();
|
||||||
|
let err = handle(
|
||||||
|
"callFunctionOn",
|
||||||
|
&json!({
|
||||||
|
"functionDeclaration": "() => 42",
|
||||||
|
"executionContextId": 9999,
|
||||||
|
}),
|
||||||
|
&mut ctx,
|
||||||
|
&None,
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.expect_err("unknown executionContextId must error per CDP spec");
|
||||||
|
assert!(
|
||||||
|
err.contains("Cannot find context with specified id"),
|
||||||
|
"error must match Chrome wording: {err}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn evaluate_accepts_default_context_id_one() {
|
||||||
|
// Runtime.enable advertises contextId=1 — that must be accepted as
|
||||||
|
// valid input to evaluate, regardless of whether a page is attached.
|
||||||
|
// (Without a page we get an Err("No page") AFTER the contextId check,
|
||||||
|
// which proves validation passed for id=1.)
|
||||||
|
let mut ctx = CdpContext::new();
|
||||||
|
let result = handle(
|
||||||
|
"evaluate",
|
||||||
|
&json!({ "expression": "1 + 1", "contextId": 1 }),
|
||||||
|
&mut ctx,
|
||||||
|
&None,
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
match result {
|
||||||
|
Ok(_) => {} // accepted + executed (would happen if a page is attached)
|
||||||
|
Err(e) => assert!(
|
||||||
|
!e.contains("Cannot find context"),
|
||||||
|
"contextId=1 must be accepted, got: {e}"
|
||||||
|
),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn create_isolated_world_registers_id_for_evaluate() {
|
||||||
|
// Round-trip: Page.createIsolatedWorld returns contextId N, and a
|
||||||
|
// subsequent Runtime.evaluate targeting that contextId must NOT be
|
||||||
|
// rejected.
|
||||||
|
let mut ctx = CdpContext::new();
|
||||||
|
// Bypass the page-attached path of createIsolatedWorld by direct
|
||||||
|
// insert — mirrors the same effect as calling the page handler with
|
||||||
|
// a real session.
|
||||||
|
ctx.valid_context_ids.insert(100);
|
||||||
|
|
||||||
|
let result = handle(
|
||||||
|
"evaluate",
|
||||||
|
&json!({ "expression": "1 + 1", "contextId": 100 }),
|
||||||
|
&mut ctx,
|
||||||
|
&None,
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
if let Err(e) = result {
|
||||||
|
assert!(
|
||||||
|
!e.contains("Cannot find context"),
|
||||||
|
"registered isolated-world contextId=100 must be accepted, got: {e}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in new issue
Block a user