From ab6fa0e39b43536cdd45911cf8c21d50f65365e5 Mon Sep 17 00:00:00 2001 From: SGavrl <277843151+SGavrl@users.noreply.github.com> Date: Sun, 26 Jul 2026 10:00:55 +0000 Subject: [PATCH] fix(js): scope fetch clients to browser contexts --- crates/obscura-js/src/ops.rs | 58 ++++++------ crates/obscura-net/src/client.rs | 9 ++ .../obscura/tests/sequential_runtime_fetch.rs | 89 +++++++++++++++++++ 3 files changed, 128 insertions(+), 28 deletions(-) create mode 100644 crates/obscura/tests/sequential_runtime_fetch.rs diff --git a/crates/obscura-js/src/ops.rs b/crates/obscura-js/src/ops.rs index 983aa09..64aad05 100644 --- a/crates/obscura-js/src/ops.rs +++ b/crates/obscura-js/src/ops.rs @@ -578,24 +578,9 @@ fn op_console_msg(state: &OpState, #[string] level: &str, #[string] msg: &str) { } } -// op_fetch_url backs JS-level `fetch()` and XHR. Pre-#139 it used a -// process-wide `OnceLock` initialised with no proxy, so -// every JS network call bypassed the configured upstream proxy. We now -// build a client per request, threading whatever `proxy_url` the page's -// ObscuraHttpClient was configured with. -// -// The per-request build cost is negligible (≪1ms) compared with the actual -// network round-trip; the simplification is worth not having to invalidate -// a cache when the proxy is reconfigured between fetches. -// -// Process-wide cache keyed by proxy URL. Previously we built a fresh -// reqwest::Client on every op_fetch_url call (every JS fetch(), XHR, -// dynamic script load). Each build re-initialised TLS roots and a -// fresh connection pool with zero reuse, costing ~5ms per fetch on top -// of any real network work. On an asset-heavy page with 30+ subresources -// that adds ~150ms of pure waste. With the cache, the first fetch on a -// given proxy pays the build cost once and every subsequent fetch reuses -// the same connection pool. +// Fallback cache for runtimes that have no owning ObscuraHttpClient, such as +// a standalone module loader. Browser pages use their context-scoped client +// below so sequential V8 runtimes never share an async network pool (#453). static FETCH_CLIENT_CACHE: std::sync::OnceLock< std::sync::RwLock>, > = std::sync::OnceLock::new(); @@ -634,13 +619,9 @@ fn build_request_client(proxy_url: Option<&str>) -> Result) -> Result std::time::Duration { + let timeout_ms = std::env::var("OBSCURA_FETCH_TIMEOUT_MS") + .ok() + .and_then(|s| s.parse().ok()) + .unwrap_or(30_000); + std::time::Duration::from_millis(timeout_ms) +} + /// Cap on the number of redirect hops op_fetch_url will follow. /// Matches reqwest's default policy of 10. const FETCH_REDIRECT_LIMIT: usize = 10; @@ -689,7 +678,7 @@ async fn op_fetch_url( } } - let (cookie_jar, in_flight, intercept_tx, proxy_url, callbacks) = { + let (cookie_jar, in_flight, intercept_tx, proxy_url, callbacks, http_client) = { let state_borrow = state.borrow(); let gs = state_borrow.borrow::().clone(); let mut gs = gs.borrow_mut(); @@ -721,7 +710,14 @@ async fn op_fetch_url( } else { None }; - (jar, in_flight, itx, proxy_url, gs.callbacks.clone()) + ( + jar, + in_flight, + itx, + proxy_url, + gs.callbacks.clone(), + gs.http_client.clone(), + ) }; // Slots the interception channel can override via Continue so a consumer @@ -804,8 +800,11 @@ async fn op_fetch_url( let method = override_method.unwrap_or(method); let body = override_body.unwrap_or(body); - let client = cached_request_client(proxy_url.as_deref()) - .map_err(deno_error::JsErrorBox::generic)?; + let client = match &http_client { + Some(client) => client.request_client().await, + None => cached_request_client(proxy_url.as_deref()) + .map_err(deno_error::JsErrorBox::generic)?, + }; let request_origin = url::Url::parse(&url) .ok() @@ -886,6 +885,7 @@ async fn op_fetch_url( if needs_preflight { let preflight = client .request(reqwest::Method::OPTIONS, &url) + .timeout(fetch_timeout()) .header("Origin", &page_origin) .header("Access-Control-Request-Method", method.as_str()) .header( @@ -919,7 +919,9 @@ async fn op_fetch_url( let mut current_body = body; let mut redirects_followed: usize = 0; let response = loop { - let mut req = client.request(current_method.clone(), ¤t_url); + let mut req = client + .request(current_method.clone(), ¤t_url) + .timeout(fetch_timeout()); if is_cross_origin { req = req.header("Origin", &page_origin); diff --git a/crates/obscura-net/src/client.rs b/crates/obscura-net/src/client.rs index f259872..5aa450b 100644 --- a/crates/obscura-net/src/client.rs +++ b/crates/obscura-net/src/client.rs @@ -466,6 +466,15 @@ impl ObscuraHttpClient { }).await } + /// Clone the request client owned by this browser context. + /// + /// Scripted fetch/XHR uses the same pool as navigation instead of a + /// process-global client. This keeps its async network state inside the + /// same ownership boundary as the V8 runtime (issue #453). + pub async fn request_client(&self) -> Client { + self.get_client().await.clone() + } + /// Read-only accessor for the proxy URL the client was configured with /// (if any). Exposed so callers outside the `obscura-net` crate — notably /// `op_fetch_url` in `obscura-js` (#139) — can route their own reqwest diff --git a/crates/obscura/tests/sequential_runtime_fetch.rs b/crates/obscura/tests/sequential_runtime_fetch.rs new file mode 100644 index 0000000..bb42bbd --- /dev/null +++ b/crates/obscura/tests/sequential_runtime_fetch.rs @@ -0,0 +1,89 @@ +use std::io::{Read, Write}; + +use obscura::Browser; + +fn spawn_server() -> String { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + std::thread::spawn(move || { + for incoming in listener.incoming() { + let Ok(mut stream) = incoming else { + continue; + }; + std::thread::spawn(move || { + let mut request = [0u8; 2048]; + let _ = stream.read(&mut request); + let request = std::str::from_utf8(&request).unwrap_or(""); + let path = request.split_whitespace().nth(1).unwrap_or("/"); + let (content_type, body) = if path.starts_with("/api") { + ("application/json", r#"{"ok":true}"#) + } else { + ( + "text/html", + "fixture", + ) + }; + let response = format!( + "HTTP/1.1 200 OK\r\nContent-Type: {}\r\nContent-Length: {}\r\nAccess-Control-Allow-Origin: *\r\nConnection: close\r\n\r\n{}", + content_type, + body.len(), + body, + ); + let _ = stream.write_all(response.as_bytes()); + let _ = stream.shutdown(std::net::Shutdown::Both); + }); + } + }); + format!("http://{}", addr) +} + +async fn run_browser(base: &str) { + let browser = Browser::new().unwrap(); + let mut page = browser.new_page().await.unwrap(); + page.goto(base).await.unwrap(); + + let script = format!( + r#"(function() {{ + var done = 0; + function mark() {{ + done += 1; + document.body.setAttribute('data-done', String(done)); + }} + for (var i = 0; i < 8; i++) {{ + fetch('{base}/api?fetch=' + i) + .then(function(r) {{ return r.json(); }}) + .then(function() {{ return fetch('{base}/api?nested=' + i); }}) + .then(mark) + .catch(function() {{}}); + }} + for (var j = 0; j < 4; j++) {{ + var xhr = new XMLHttpRequest(); + xhr.open('GET', '{base}/api?xhr=' + j); + xhr.addEventListener('load', mark); + xhr.send(); + }} + }})()"#, + ); + page.evaluate(&script); + + for _ in 0..20 { + page.settle(250).await; + if page.evaluate("document.body.getAttribute('data-done')") == serde_json::json!("12") { + break; + } + } + assert_eq!( + page.evaluate("document.body.getAttribute('data-done')"), + serde_json::json!("12") + ); +} + +#[tokio::test(flavor = "current_thread")] +async fn sequential_browsers_complete_concurrent_fetch_and_xhr() { + std::env::set_var("OBSCURA_ALLOW_PRIVATE_NETWORK", "1"); + let base = spawn_server(); + + for _ in 0..8 { + run_browser(&base).await; + } +}