Fixes the remaining two paths from #139: JS-level fetch()/XHR (op_fetch_url) and dynamic ES module imports (ObscuraModuleLoader) both built their reqwest clients with `Client::builder().build()` — no proxy — so every JS-initiated network call silently bypassed BrowserContext.proxy_url. (The third path, StealthHttpClient, was already fixed in main: page.rs:61 uses with_proxy.) Changes: * obscura-net: expose ObscuraHttpClient::proxy_url() so callers outside the crate can route their own reqwest requests through the same upstream proxy. The field was already there — just lacked an accessor. * obscura-js/module_loader: add `proxy_url: Option<String>` and `with_proxy(base_url, proxy_url)`. ObscuraModuleLoader::new keeps the pre-#139 direct-connection behaviour. load() builds a reqwest client per import, threading the proxy and bubbling up "Invalid module proxy" on bad URLs. * obscura-js/ops.rs: replace the process-wide `static SHARED_HTTP_CLIENT: OnceLock<reqwest::Client>` (which could never be reconfigured after first call) with a per-request `build_request_client(proxy_url)` that pulls the configured proxy out of SharedState.http_client.proxy_url() each call. Per-request build cost is ~1ms vs the network round-trip, and we no longer have a global singleton that lies about its config. * obscura-js/runtime: add ObscuraJsRuntime::with_base_url_and_proxy. with_base_url delegates to it with None so existing tests / callers are unaffected. * obscura-browser/page.rs: thread context.proxy_url through to with_base_url_and_proxy when constructing the JS runtime.
This commit is contained in:
1 parent
a750797d15
commit
f3c783f6f5
5 files changed
+139
-16
No files matched your search
@@ -136,7 +136,14 @@ impl Page {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
let mut rt = ObscuraJsRuntime::with_base_url(&self.url_string());
|
// Thread the BrowserContext's proxy through to the ES-module loader
|
||||||
|
// and op_fetch_url so dynamic imports and JS fetch() honour the
|
||||||
|
// configured upstream proxy (#139). When proxy_url is None this is
|
||||||
|
// equivalent to with_base_url() (direct connection).
|
||||||
|
let mut rt = ObscuraJsRuntime::with_base_url_and_proxy(
|
||||||
|
&self.url_string(),
|
||||||
|
self.context.proxy_url.clone(),
|
||||||
|
);
|
||||||
rt.set_url(&self.url_string());
|
rt.set_url(&self.url_string());
|
||||||
rt.set_title(&self.title);
|
rt.set_title(&self.title);
|
||||||
|
|
||||||
|
|||||||
@@ -10,12 +10,21 @@ use deno_core::RequestedModuleType;
|
|||||||
|
|
||||||
pub struct ObscuraModuleLoader {
|
pub struct ObscuraModuleLoader {
|
||||||
pub base_url: String,
|
pub base_url: String,
|
||||||
|
/// Proxy URL threaded through to every dynamic ES-module fetch (#139).
|
||||||
|
/// `None` keeps the pre-#139 direct-connection behaviour for callers
|
||||||
|
/// that haven't been updated.
|
||||||
|
pub proxy_url: Option<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl ObscuraModuleLoader {
|
impl ObscuraModuleLoader {
|
||||||
pub fn new(base_url: &str) -> Self {
|
pub fn new(base_url: &str) -> Self {
|
||||||
|
Self::with_proxy(base_url, None)
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn with_proxy(base_url: &str, proxy_url: Option<String>) -> Self {
|
||||||
ObscuraModuleLoader {
|
ObscuraModuleLoader {
|
||||||
base_url: base_url.to_string(),
|
base_url: base_url.to_string(),
|
||||||
|
proxy_url,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -52,13 +61,32 @@ impl ModuleLoader for ObscuraModuleLoader {
|
|||||||
_requested_module_type: RequestedModuleType,
|
_requested_module_type: RequestedModuleType,
|
||||||
) -> ModuleLoadResponse {
|
) -> ModuleLoadResponse {
|
||||||
let url = module_specifier.to_string();
|
let url = module_specifier.to_string();
|
||||||
|
// Capture the loader's proxy here so the async closure below owns a
|
||||||
|
// plain Option<String> rather than borrowing &self across an `await`.
|
||||||
|
let proxy_url = self.proxy_url.clone();
|
||||||
|
|
||||||
ModuleLoadResponse::Async(Pin::from(Box::new(async move {
|
ModuleLoadResponse::Async(Pin::from(Box::new(async move {
|
||||||
let client = reqwest::Client::builder()
|
let mut builder = reqwest::Client::builder();
|
||||||
|
if let Some(ref proxy) = proxy_url {
|
||||||
|
match reqwest::Proxy::all(proxy) {
|
||||||
|
Ok(p) => builder = builder.proxy(p),
|
||||||
|
Err(e) => {
|
||||||
|
return Err(io_err(format!(
|
||||||
|
"Invalid module proxy '{}': {}",
|
||||||
|
proxy, e
|
||||||
|
)));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
let client = builder
|
||||||
.build()
|
.build()
|
||||||
.map_err(|e| io_err(format!("HTTP client error: {}", e)))?;
|
.map_err(|e| io_err(format!("HTTP client error: {}", e)))?;
|
||||||
|
|
||||||
tracing::debug!("Loading ES module: {}", url);
|
tracing::debug!(
|
||||||
|
"Loading ES module: {} (proxy: {})",
|
||||||
|
url,
|
||||||
|
proxy_url.as_deref().unwrap_or("direct")
|
||||||
|
);
|
||||||
|
|
||||||
let resp = client
|
let resp = client
|
||||||
.get(&url)
|
.get(&url)
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
use std::cell::RefCell;
|
use std::cell::RefCell;
|
||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
use std::rc::Rc;
|
use std::rc::Rc;
|
||||||
use std::sync::{Arc, OnceLock};
|
use std::sync::Arc;
|
||||||
|
|
||||||
use deno_core::op2;
|
use deno_core::op2;
|
||||||
use deno_core::OpState;
|
use deno_core::OpState;
|
||||||
@@ -281,14 +281,25 @@ fn op_console_msg(state: &OpState, #[string] level: &str, #[string] msg: &str) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
static SHARED_HTTP_CLIENT: OnceLock<reqwest::Client> = OnceLock::new();
|
// op_fetch_url backs JS-level `fetch()` and XHR. Pre-#139 it used a
|
||||||
|
// process-wide `OnceLock<reqwest::Client>` initialised with no proxy, so
|
||||||
fn get_shared_client() -> &'static reqwest::Client {
|
// every JS network call bypassed the configured upstream proxy. We now
|
||||||
SHARED_HTTP_CLIENT.get_or_init(|| {
|
// build a client per request, threading whatever `proxy_url` the page's
|
||||||
reqwest::Client::builder()
|
// ObscuraHttpClient was configured with.
|
||||||
.build()
|
//
|
||||||
.expect("failed to build shared reqwest::Client")
|
// 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.
|
||||||
|
fn build_request_client(proxy_url: Option<&str>) -> Result<reqwest::Client, String> {
|
||||||
|
let mut builder = reqwest::Client::builder();
|
||||||
|
if let Some(proxy) = proxy_url {
|
||||||
|
let p = reqwest::Proxy::all(proxy)
|
||||||
|
.map_err(|e| format!("Invalid op_fetch_url proxy '{}': {}", proxy, e))?;
|
||||||
|
builder = builder.proxy(p);
|
||||||
|
}
|
||||||
|
builder
|
||||||
|
.build()
|
||||||
|
.map_err(|e| format!("failed to build reqwest::Client: {}", e))
|
||||||
}
|
}
|
||||||
|
|
||||||
#[op2(async)]
|
#[op2(async)]
|
||||||
@@ -317,7 +328,7 @@ async fn op_fetch_url(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
let (cookie_jar, in_flight, intercept_tx) = {
|
let (cookie_jar, in_flight, intercept_tx, proxy_url) = {
|
||||||
let state_borrow = state.borrow();
|
let state_borrow = state.borrow();
|
||||||
let gs = state_borrow.borrow::<SharedState>().clone();
|
let gs = state_borrow.borrow::<SharedState>().clone();
|
||||||
let mut gs = gs.borrow_mut();
|
let mut gs = gs.borrow_mut();
|
||||||
@@ -334,6 +345,10 @@ async fn op_fetch_url(
|
|||||||
}
|
}
|
||||||
let jar = gs.cookie_jar.clone();
|
let jar = gs.cookie_jar.clone();
|
||||||
let in_flight = gs.http_client.as_ref().map(|c| c.in_flight.clone());
|
let in_flight = gs.http_client.as_ref().map(|c| c.in_flight.clone());
|
||||||
|
// #139: thread the configured proxy through to the per-request
|
||||||
|
// reqwest::Client. Without this, op_fetch_url silently bypasses
|
||||||
|
// BrowserContext.proxy_url for every JS fetch() / XHR call.
|
||||||
|
let proxy_url = gs.http_client.as_ref().and_then(|c| c.proxy_url().map(|s| s.to_string()));
|
||||||
tracing::debug!("op_fetch_url: intercept_enabled={}, has_tx={}", gs.intercept_enabled, gs.intercept_tx.is_some());
|
tracing::debug!("op_fetch_url: intercept_enabled={}, has_tx={}", gs.intercept_enabled, gs.intercept_tx.is_some());
|
||||||
let itx = if gs.intercept_enabled {
|
let itx = if gs.intercept_enabled {
|
||||||
gs.intercept_counter += 1;
|
gs.intercept_counter += 1;
|
||||||
@@ -341,7 +356,7 @@ async fn op_fetch_url(
|
|||||||
} else {
|
} else {
|
||||||
None
|
None
|
||||||
};
|
};
|
||||||
(jar, in_flight, itx)
|
(jar, in_flight, itx, proxy_url)
|
||||||
};
|
};
|
||||||
|
|
||||||
if let Some((tx, request_id)) = intercept_tx {
|
if let Some((tx, request_id)) = intercept_tx {
|
||||||
@@ -385,7 +400,8 @@ async fn op_fetch_url(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
let client = get_shared_client();
|
let client = build_request_client(proxy_url.as_deref())
|
||||||
|
.map_err(deno_error::JsErrorBox::generic)?;
|
||||||
|
|
||||||
let request_origin = url::Url::parse(&url)
|
let request_origin = url::Url::parse(&url)
|
||||||
.ok()
|
.ok()
|
||||||
|
|||||||
@@ -33,10 +33,17 @@ impl ObscuraJsRuntime {
|
|||||||
}
|
}
|
||||||
|
|
||||||
pub fn with_base_url(base_url: &str) -> Self {
|
pub fn with_base_url(base_url: &str) -> Self {
|
||||||
|
Self::with_base_url_and_proxy(base_url, None)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Construct a runtime whose ES-module loader routes dynamic imports
|
||||||
|
/// through `proxy_url` (#139). `None` is equivalent to `with_base_url`
|
||||||
|
/// (direct connection).
|
||||||
|
pub fn with_base_url_and_proxy(base_url: &str, proxy_url: Option<String>) -> Self {
|
||||||
let state = Rc::new(RefCell::new(ObscuraState::new()));
|
let state = Rc::new(RefCell::new(ObscuraState::new()));
|
||||||
let state_clone = state.clone();
|
let state_clone = state.clone();
|
||||||
|
|
||||||
let module_loader = Rc::new(ObscuraModuleLoader::new(base_url));
|
let module_loader = Rc::new(ObscuraModuleLoader::with_proxy(base_url, proxy_url));
|
||||||
|
|
||||||
let mut runtime = JsRuntime::new(RuntimeOptions {
|
let mut runtime = JsRuntime::new(RuntimeOptions {
|
||||||
extensions: vec![build_extension()],
|
extensions: vec![build_extension()],
|
||||||
@@ -1589,4 +1596,61 @@ mod tests {
|
|||||||
assert_eq!(inf, serde_json::Value::Null);
|
assert_eq!(inf, serde_json::Value::Null);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Issue #139 — proxy_url must thread through to both the ES-module
|
||||||
|
// loader (module_loader.rs) and op_fetch_url's reqwest client
|
||||||
|
// (ops.rs::build_request_client). Pre-fix both built clients with
|
||||||
|
// `Client::builder().build()` — no proxy — so JS fetch/XHR and
|
||||||
|
// dynamic imports silently bypassed BrowserContext.proxy_url.
|
||||||
|
//
|
||||||
|
// Phase 5.5 RED check: each test references a symbol that does NOT
|
||||||
|
// exist on main (proxy_url() accessor, with_proxy ctor,
|
||||||
|
// with_base_url_and_proxy ctor), so the tests fail to compile without
|
||||||
|
// the prod fix.
|
||||||
|
#[test]
|
||||||
|
fn http_client_round_trips_proxy_url() {
|
||||||
|
use obscura_net::{CookieJar, ObscuraHttpClient};
|
||||||
|
let jar = std::sync::Arc::new(CookieJar::new());
|
||||||
|
let configured =
|
||||||
|
ObscuraHttpClient::with_options(jar.clone(), Some("http://proxy.test:8080"));
|
||||||
|
assert_eq!(
|
||||||
|
configured.proxy_url(),
|
||||||
|
Some("http://proxy.test:8080"),
|
||||||
|
"proxy_url() must expose the value passed to with_options"
|
||||||
|
);
|
||||||
|
|
||||||
|
let direct = ObscuraHttpClient::with_options(jar, None);
|
||||||
|
assert_eq!(
|
||||||
|
direct.proxy_url(),
|
||||||
|
None,
|
||||||
|
"proxy_url() must return None when no proxy was configured"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn module_loader_stores_proxy_for_dynamic_imports() {
|
||||||
|
use crate::module_loader::ObscuraModuleLoader;
|
||||||
|
let loader = ObscuraModuleLoader::with_proxy(
|
||||||
|
"https://example.com/",
|
||||||
|
Some("http://proxy.test:8080".to_string()),
|
||||||
|
);
|
||||||
|
assert_eq!(loader.proxy_url.as_deref(), Some("http://proxy.test:8080"));
|
||||||
|
assert_eq!(loader.base_url, "https://example.com/");
|
||||||
|
|
||||||
|
// Default constructor must keep the historical "no proxy" behaviour.
|
||||||
|
let direct = ObscuraModuleLoader::new("https://example.com/");
|
||||||
|
assert_eq!(direct.proxy_url, None);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn runtime_with_base_url_and_proxy_constructs_successfully() {
|
||||||
|
// Sanity-check the public ctor that page.rs uses to thread proxy
|
||||||
|
// through to the module loader. Direct (None) and proxied paths
|
||||||
|
// must both initialise the JS environment.
|
||||||
|
let _direct = ObscuraJsRuntime::with_base_url_and_proxy("https://example.com/", None);
|
||||||
|
let _proxied = ObscuraJsRuntime::with_base_url_and_proxy(
|
||||||
|
"https://example.com/",
|
||||||
|
Some("http://proxy.test:8080".to_string()),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
@@ -212,6 +212,14 @@ impl ObscuraHttpClient {
|
|||||||
}).await
|
}).await
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// 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
|
||||||
|
/// requests through the same upstream proxy.
|
||||||
|
pub fn proxy_url(&self) -> Option<&str> {
|
||||||
|
self.proxy_url.as_deref()
|
||||||
|
}
|
||||||
|
|
||||||
pub async fn fetch(&self, url: &Url) -> Result<Response, ObscuraNetError> {
|
pub async fn fetch(&self, url: &Url) -> Result<Response, ObscuraNetError> {
|
||||||
self.fetch_with_method(Method::GET, url, None).await
|
self.fetch_with_method(Method::GET, url, None).await
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user