fix(js): scope fetch clients to browser contexts
This commit is contained in:
@@ -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<reqwest::Client>` 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::collections::HashMap<String, reqwest::Client>>,
|
||||
> = std::sync::OnceLock::new();
|
||||
@@ -634,13 +619,9 @@ fn build_request_client(proxy_url: Option<&str>) -> Result<reqwest::Client, Stri
|
||||
// send().await errors, which op_fetch_url propagates and the fetch shim turns
|
||||
// into an XHR `error`/`loadend`. 30s matches the other clients in the
|
||||
// workspace; OBSCURA_FETCH_TIMEOUT_MS overrides it for tighter cloud limits.
|
||||
let timeout_ms: u64 = std::env::var("OBSCURA_FETCH_TIMEOUT_MS")
|
||||
.ok()
|
||||
.and_then(|s| s.parse().ok())
|
||||
.unwrap_or(30_000);
|
||||
let mut builder = reqwest::Client::builder()
|
||||
.redirect(reqwest::redirect::Policy::none())
|
||||
.timeout(std::time::Duration::from_millis(timeout_ms))
|
||||
.timeout(fetch_timeout())
|
||||
// SSRF guard: also reject hostnames that resolve to a private/loopback IP.
|
||||
.dns_resolver(std::sync::Arc::new(obscura_net::SsrfGuardResolver::new(false)))
|
||||
// Be explicit about pool size: default is unbounded which is fine,
|
||||
@@ -659,6 +640,14 @@ fn build_request_client(proxy_url: Option<&str>) -> Result<reqwest::Client, Stri
|
||||
.map_err(|e| format!("failed to build reqwest::Client: {}", e))
|
||||
}
|
||||
|
||||
fn fetch_timeout() -> 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::<SharedState>().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);
|
||||
|
||||
@@ -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
|
||||
|
||||
89
crates/obscura/tests/sequential_runtime_fetch.rs
Normal file
89
crates/obscura/tests/sequential_runtime_fetch.rs
Normal file
@@ -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",
|
||||
"<!doctype html><html><head><title>fixture</title></head><body></body></html>",
|
||||
)
|
||||
};
|
||||
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;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user