From a6bb7411b29da70a4df69fe0b9a5c66c8bf4257e Mon Sep 17 00:00:00 2001 From: lisa0314 Date: Fri, 24 Jul 2026 15:03:53 +0800 Subject: [PATCH] fix(js): let dynamic scripts finish settling Keep pumping the event loop past the general 500ms settle limit only while the dynamic-script queue remains pending, with a 3s hard cap. Add a deterministic CDP regression for direct and nested external scripts whose fetch crosses the original settle deadline. --- crates/obscura-browser/src/page.rs | 21 +++- .../tests/dynamic_script_onload_fires.rs | 119 ++++++++++++++++++ crates/obscura-js/src/runtime.rs | 12 ++ 3 files changed, 148 insertions(+), 4 deletions(-) create mode 100644 crates/obscura-cdp/tests/dynamic_script_onload_fires.rs diff --git a/crates/obscura-browser/src/page.rs b/crates/obscura-browser/src/page.rs index 64fc64c..2e169fd 100644 --- a/crates/obscura-browser/src/page.rs +++ b/crates/obscura-browser/src/page.rs @@ -744,6 +744,11 @@ impl Page { } if let Some(js) = &mut self.js { + let dynamic_settle_ms = std::env::var("OBSCURA_DYNAMIC_SCRIPT_SETTLE_MS") + .ok() + .and_then(|s| s.parse::().ok()) + .unwrap_or(3_000) + .max(500); // Bound the post-script settle loop by wall clock, not just by the // 10ms-tick branch. The old code only consulted `deadline` inside // the `Err(_)` arm (when the inner tick timed out), so a steady @@ -753,14 +758,22 @@ impl Page { // On busy sites this could keep the V8 lock held for tens of // seconds, wedging the entire CDP dispatcher (see triage for // issue series around the 40-site compat sweep). + // A dynamic external script may still be in flight at 500ms. Keep + // pumping only while that queue is pending, up to a separate bounded + // budget, so normal pages and unrelated fetches retain the fast path. // A single run_event_loop poll that pins the thread inside V8 makes // the per-poll tokio timeouts below useless, so guard the whole loop - // with a watchdog that fires ~250ms past its 500ms deadline. - let settle_wd = js.arm_watchdog(std::time::Duration::from_millis(750)); - let deadline = tokio::time::Instant::now() + tokio::time::Duration::from_millis(500); + // with a watchdog that fires 250ms past the longest deadline. + let settle_wd = js.arm_watchdog(std::time::Duration::from_millis(dynamic_settle_ms + 250)); + let started = tokio::time::Instant::now(); + let deadline = started + tokio::time::Duration::from_millis(500); + let dynamic_deadline = started + tokio::time::Duration::from_millis(dynamic_settle_ms); let mut idle_count = 0u32; loop { - if tokio::time::Instant::now() >= deadline { + let now = tokio::time::Instant::now(); + if now >= deadline + && (now >= dynamic_deadline || !js.has_pending_dynamic_scripts()) + { break; } let result = tokio::time::timeout( diff --git a/crates/obscura-cdp/tests/dynamic_script_onload_fires.rs b/crates/obscura-cdp/tests/dynamic_script_onload_fires.rs new file mode 100644 index 0000000..d80a92e --- /dev/null +++ b/crates/obscura-cdp/tests/dynamic_script_onload_fires.rs @@ -0,0 +1,119 @@ +// Regression for issue #474: external scripts inserted after a timer must be +// fetched and execute before the post-navigation event-loop settle completes. + +use obscura_cdp::dispatch::{dispatch, CdpContext}; +use obscura_cdp::types::CdpRequest; +use serde_json::{json, Value}; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use tokio::net::TcpListener; + +async fn serve() -> String { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + loop { + let (mut socket, _) = listener.accept().await.unwrap(); + tokio::spawn(async move { + let mut buf = [0u8; 2048]; + let read = socket.read(&mut buf).await.unwrap(); + let request = String::from_utf8_lossy(&buf[..read]); + let (content_type, body) = if request.starts_with("GET /direct.js") { + tokio::time::sleep(std::time::Duration::from_millis(600)).await; + ("application/javascript", "window.__directExecuted = true;") + } else if request.starts_with("GET /nested.js") { + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + ("application/javascript", "window.__nestedExecuted = true;") + } else { + ( + "text/html", + r#" +
stage1
+ +"#, + ) + }; + let response = format!( + "HTTP/1.1 200 OK\r\nContent-Type: {content_type}\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ); + let _ = socket.write_all(response.as_bytes()).await; + }); + } + }); + format!("http://{addr}/") +} + +async fn cdp( + ctx: &mut CdpContext, + id: u64, + method: &str, + params: Value, + session_id: &str, +) -> Value { + let response = dispatch( + &CdpRequest { + id, + method: method.to_string(), + params, + session_id: Some(session_id.to_string()), + }, + ctx, + ) + .await; + assert!( + response.error.is_none(), + "CDP {method} failed: {:?}", + response.error + ); + response.result.unwrap_or_else(|| json!({})) +} + +#[tokio::test(flavor = "current_thread")] +async fn dynamic_external_scripts_execute_and_fire_load() { + std::env::set_var("OBSCURA_ALLOW_PRIVATE_NETWORK", "1"); + let url = serve().await; + let mut ctx = CdpContext::new(); + let page_id = ctx.create_page(); + let session_id = "session-1"; + ctx.sessions.insert(session_id.to_string(), page_id); + + cdp( + &mut ctx, + 1, + "Page.navigate", + json!({"url": url, "waitUntil": "load"}), + session_id, + ) + .await; + + let result = cdp( + &mut ctx, + 2, + "Runtime.evaluate", + json!({ + "expression": "JSON.stringify({directExecuted: !!window.__directExecuted, directLoaded: !!window.__directLoaded, nestedExecuted: !!window.__nestedExecuted, nestedLoaded: !!window.__nestedLoaded})", + "returnByValue": true, + }), + session_id, + ) + .await; + assert_eq!( + result["result"]["value"], + r#"{"directExecuted":true,"directLoaded":true,"nestedExecuted":true,"nestedLoaded":true}"#, + "dynamic scripts must execute and fire load before navigation settles" + ); +} diff --git a/crates/obscura-js/src/runtime.rs b/crates/obscura-js/src/runtime.rs index ca7ade2..26905a1 100644 --- a/crates/obscura-js/src/runtime.rs +++ b/crates/obscura-js/src/runtime.rs @@ -859,6 +859,18 @@ impl ObscuraJsRuntime { .map_err(|e| format!("Event loop error: {}", e)) } + /// Whether the serialized dynamic-script queue is still fetching or + /// evaluating a script. The queue variables are global lexicals rather + /// than window properties, so page code cannot overwrite this state. + pub fn has_pending_dynamic_scripts(&mut self) -> bool { + self.evaluate( + "typeof __dynScriptBusy !== 'undefined' && (__dynScriptBusy || __dynScriptQueue.length > 0)", + ) + .ok() + .and_then(|value| value.as_bool()) + .unwrap_or(false) + } + /// Arm a hard wall-clock backstop on synchronous V8 work. A page stuck in a /// synchronous loop or a microtask storm pins the OS thread inside V8, so /// `tokio::time::timeout` (which can only cancel at await points) never