diff --git a/plans/261021-1028-tauri-v2-port/phase-03-data-layer-ureq.md b/plans/261021-1028-tauri-v2-port/phase-03-data-layer-ureq.md index 7ecdab9..5f9a12f 100644 --- a/plans/261021-1028-tauri-v2-port/phase-03-data-layer-ureq.md +++ b/plans/261021-1028-tauri-v2-port/phase-03-data-layer-ureq.md @@ -30,7 +30,7 @@ only two Win32 couplings: `src/net/winhttp.rs`, and the `CommandExt` import in - User-Agent `claude-code-usage-bubble/`. - A per-request User-Agent replaces the agent's, as the Codex provider requires. - Proxy: - - Env proxies come from ureq. + - Env proxies: `net::env_proxy()` walks the variables ureq reads (`ALL_PROXY`, `HTTPS_PROXY`, `HTTP_PROXY`, either case) and keeps the first HTTP/HTTPS one, skipping SOCKS entries with a warning because the `socks-proxy` feature is off. `NO_PROXY` applies to the chosen proxy. - `cfg(windows)`, only when no env proxy is set: `platform::windows::system_proxy()` reads `HKCU\Software\Microsoft\Windows\CurrentVersion\Internet Settings` (`ProxyEnable`, `ProxyServer`, `ProxyOverride`). Nothing when `ProxyEnable` is not 1. - Parsing, a pure function in `core` with tests: a `ProxyServer` of the form `http=h:p;https=h:p;…` uses the `https=` entry, else the `http=` entry; a bare `h:p` applies to all schemes. `ProxyOverride` is split on `;`; each entry is a case-insensitive glob (`*` wildcard) against the host, and `` matches dotless hosts. - `net::Client` holds a proxied agent and a direct agent and picks per request: direct when the request host matches a `ProxyOverride` entry. The parsed proxy URL is also handed to the updater builder in Phase 09. @@ -82,8 +82,12 @@ only two Win32 couplings: `src/net/winhttp.rs`, and the `CommandExt` import in proxied reply. - `net::Error::Status` is kept so the providers compile unchanged; the client never builds it, because every status comes back as a `Response`. -- `net::system_proxy()` returns the system proxy only when no env proxy is - set, and `Client::system_proxy()` exposes the one in use for the updater +- `net::Client::new` consults `net::system_proxy()` only when + `net::env_proxy()` finds no usable proxy. A SOCKS env entry is skipped + rather than passed to ureq, which without `socks-proxy` fails every request + for `socks5h` and silently connects directly for `socks5`; skipping lets an + HTTP env proxy or the system proxy take over. `Client::system_proxy()` + exposes the system proxy in use for the updater `Client::system_proxy()` exposes the one in use for the updater builder in Phase 09. Proxy URLs are never logged (they may hold credentials). - An empty `CODEX_HOME` now counts as unset, as in the Codex CLI. Before, it diff --git a/src/net/mod.rs b/src/net/mod.rs index 349b6f5..6fe82ae 100644 --- a/src/net/mod.rs +++ b/src/net/mod.rs @@ -48,15 +48,74 @@ impl From for Error { } } -/// The static system proxy that applies when no environment proxy is set. +/// Proxy variables in the order `ureq` reads them; the first usable one wins. +const ENV_PROXY_VARS: &[&str] = &[ + "ALL_PROXY", + "all_proxy", + "HTTPS_PROXY", + "https_proxy", + "HTTP_PROXY", + "http_proxy", +]; + +/// The environment proxy, if one is usable. /// -/// Environment proxies (`HTTPS_PROXY`, `ALL_PROXY`, `NO_PROXY`, …) are read by -/// `ureq` itself and win over the system proxy. On Windows the fallback is the -/// `Internet Settings` proxy; other platforms have none. -pub fn system_proxy() -> Option { - if Proxy::try_from_env().is_some() { - return None; +/// Walks the variables `ureq` reads, but skips SOCKS entries: this build has no +/// `socks-proxy` support, so `ureq` would either fail every request (`socks5h`) +/// or silently connect directly (`socks5`). Proxy tools often set +/// `ALL_PROXY=socks5h://…` next to `HTTPS_PROXY=http://…`, so skipping lets the +/// HTTP entry, or failing that the system proxy, take over. `NO_PROXY` applies +/// to the chosen proxy. +pub fn env_proxy() -> Option { + env_proxy_from(|name| std::env::var(name).ok()) +} + +fn env_proxy_from(var: impl Fn(&str) -> Option) -> Option { + let no_proxy = var("NO_PROXY").or_else(|| var("no_proxy")); + for name in ENV_PROXY_VARS { + let Some(value) = var(name) else { continue }; + // Invalid values are skipped, as `ureq` does. Values are not logged: + // they may carry credentials. + let Ok(parsed) = Proxy::new(&value) else { + continue; + }; + if !is_connect_proxy(&parsed) { + log::warn!("{name} ignored, only http and https proxies are supported"); + continue; + } + // Rebuild so the proxy carries the `NO_PROXY` bypass list, which only + // the builder accepts. + let mut builder = Proxy::builder(parsed.protocol()) + .host(parsed.host()) + .port(parsed.port()); + if let Some(user) = parsed.username() { + builder = builder.username(user); + } + if let Some(password) = parsed.password() { + builder = builder.password(password); + } + for expr in no_proxy.iter().flat_map(|v| v.split(',')) { + let expr = expr.trim(); + if !expr.is_empty() { + builder = builder.no_proxy(expr); + } + } + match builder.build() { + Ok(proxy) => return Some(proxy), + Err(err) => log::warn!("{name} ignored, it is not a valid proxy URI: {err}"), + } } + None +} + +/// Whether `proxy` is an HTTP CONNECT proxy, the only kind this build supports. +fn is_connect_proxy(proxy: &Proxy) -> bool { + matches!(proxy.protocol(), ProxyProtocol::Http | ProxyProtocol::Https) +} + +/// The static system proxy: the `Internet Settings` proxy on Windows, none +/// elsewhere. It applies only when [`env_proxy`] finds nothing. +pub fn system_proxy() -> Option { #[cfg(windows)] { crate::platform::windows::system_proxy() @@ -82,7 +141,9 @@ pub struct Client { impl Client { /// Create a client whose default User-Agent is `user_agent`. pub fn new(user_agent: &str) -> Result { - Self::build(user_agent, Proxy::try_from_env(), system_proxy()) + let env = env_proxy(); + let system = if env.is_some() { None } else { system_proxy() }; + Self::build(user_agent, env, system) } /// A client that ignores every proxy, for loopback tests. @@ -102,7 +163,7 @@ impl Client { // Without ureq's `socks-proxy` feature a manually configured // SOCKS proxy panics at connect time, so only CONNECT proxies // are accepted here. - Ok(p) if !matches!(p.protocol(), ProxyProtocol::Http | ProxyProtocol::Https) => { + Ok(p) if !is_connect_proxy(&p) => { log::warn!("system proxy ignored, only http and https proxies are supported"); None } @@ -433,6 +494,60 @@ mod tests { assert_eq!(client.system_proxy(), None); } + fn env(vars: &[(&str, &str)]) -> impl Fn(&str) -> Option { + let vars: Vec<(String, String)> = vars + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(); + move |name| vars.iter().find(|(k, _)| k == name).map(|(_, v)| v.clone()) + } + + #[test] + fn socks_env_proxy_is_skipped_for_the_http_one() { + let proxy = serve_once("200 OK", &[], Vec::new()); + let origin = serve_once("200 OK", &[], Vec::new()); + let chosen = env_proxy_from(env(&[ + ("all_proxy", "socks5h://127.0.0.1:9"), + ("https_proxy", &proxy.base), + ])) + .expect("http env proxy"); + assert_eq!(chosen.protocol(), ProxyProtocol::Http); + + let client = Client::build(USER_AGENT, Some(chosen), None).expect("client"); + // The fake proxy does not tunnel; the CONNECT reaching it is the proof. + let _ = client.get(&format!("{}/x", origin.base)).send(); + let request = received(&proxy); + let authority = origin.base.trim_start_matches("http://"); + assert!( + request.starts_with(&format!("CONNECT {authority} HTTP/1.1")), + "{request}" + ); + } + + #[test] + fn socks_only_env_proxy_yields_none() { + assert!(env_proxy_from(env(&[("ALL_PROXY", "socks5://127.0.0.1:1080")])).is_none()); + assert!(env_proxy_from(env(&[("HTTPS_PROXY", "bad host:x")])).is_none()); + assert!(env_proxy_from(env(&[])).is_none()); + } + + #[test] + fn env_proxy_keeps_credentials_and_no_proxy() { + let chosen = env_proxy_from(env(&[ + ("HTTPS_PROXY", "http://user:pa%40ss@proxy.corp:3128"), + ("NO_PROXY", "localhost, .internal"), + ])) + .expect("env proxy"); + assert_eq!(chosen.host(), "proxy.corp"); + assert_eq!(chosen.port(), 3128); + assert_eq!(chosen.username(), Some("user")); + assert_eq!(chosen.password(), Some("pa%40ss")); + let uri = |s: &str| s.parse::().expect("uri"); + assert!(chosen.is_no_proxy(&uri("http://localhost/"))); + assert!(chosen.is_no_proxy(&uri("https://api.internal/"))); + assert!(!chosen.is_no_proxy(&uri("https://api.anthropic.com/"))); + } + #[test] fn socks_system_proxy_falls_back_to_direct() { let client = Client::build(