Skip to content

http-client: make proxy route fallback explicit #31916

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bolinfest wants to merge 1 commit into
base: main
Choose a base branch
from
+405 −75
Open

http-client: make proxy route fallback explicit #31916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions codex-rs/http-client/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,13 @@ rustls-native-certs = { workspace = true }
rustls-pki-types = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
sha2 = { workspace = true }
thiserror = { workspace = true }
tokio = { workspace = true, features = ["macros", "rt", "time", "sync"] }
tracing = { workspace = true }
tracing-opentelemetry = { workspace = true }
zstd = { workspace = true }

[target.'cfg(any(target_os = "windows", target_os = "macos"))'.dependencies]
sha2 = { workspace = true }

[target.'cfg(target_os = "macos")'.dependencies]
system-configuration = { workspace = true }

Expand Down
227 changes: 162 additions & 65 deletions codex-rs/http-client/src/outbound_proxy.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,21 @@ use std::sync::Mutex;
use std::sync::OnceLock;
use std::time::Duration;
use std::time::Instant;
#[cfg(any(target_os = "windows", target_os = "macos"))]
use tokio::sync::Semaphore;

use crate::custom_ca::BuildCustomCaTransportError;
use crate::custom_ca::build_reqwest_client_with_custom_ca;
use crate::default_client::HttpClient;
#[cfg(any(target_os = "windows", target_os = "macos"))]
use sha2::Digest;
#[cfg(any(target_os = "windows", target_os = "macos"))]
use sha2::Sha256;
use thiserror::Error;

const SYSTEM_PROXY_SUCCESS_CACHE_TTL: Duration = Duration::from_secs(60);
const SYSTEM_PROXY_UNAVAILABLE_CACHE_TTL: Duration = Duration::from_secs(5);
const SYSTEM_PROXY_CACHE_MAX_ENTRIES: usize = 256;
#[cfg(any(target_os = "windows", target_os = "macos"))]
static ASYNC_SYSTEM_PROXY_RESOLUTION_PERMIT: Semaphore = Semaphore::const_new(1);

#[cfg(target_os = "macos")]
mod macos;
Expand Down Expand Up @@ -100,24 +102,34 @@ pub enum OutboundProxyPolicy {

/// Resolved proxy route for a concrete outbound destination.
///
/// `TransportDefault` delegates environment-proxy handling to the underlying transport. Proxy
/// URLs are intentionally redacted from `Debug` output because they may contain credentials.
#[derive(Clone, PartialEq, Eq)]
/// `TransportDefault` preserves the underlying transport behavior only when system-proxy support
/// is disabled. When system resolution is enabled, environment and direct fallbacks are resolved
/// explicitly so the transport cannot repeat system discovery. Proxy URLs and no-proxy settings
/// are intentionally redacted from `Debug` output because they may contain credentials or private
/// hostnames.
#[derive(Clone, Hash, PartialEq, Eq)]
pub enum OutboundProxyRoute {
/// Preserve the underlying transport's existing proxy behavior.
TransportDefault,
/// Connect directly and bypass transport-level proxy discovery.
Direct,
/// Connect through the selected proxy URL.
Proxy { url: String },
Proxy {
url: String,
no_proxy: Option<String>,
},
}

impl fmt::Debug for OutboundProxyRoute {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::TransportDefault => f.write_str("TransportDefault"),
Self::Direct => f.write_str("Direct"),
Self::Proxy { .. } => f.debug_struct("Proxy").field("url", &"<redacted>").finish(),
Self::Proxy { .. } => f
.debug_struct("Proxy")
.field("url", &"<redacted>")
.field("no_proxy", &"<redacted>")
.finish(),
}
}
}
Expand Down Expand Up @@ -149,16 +161,64 @@ impl HttpClientFactory {
///
/// WebSocket schemes are resolved through their HTTP equivalents so platform PAC and system
/// proxy APIs apply the same policy to `ws`/`wss` and `http`/`https` destinations. When system
/// resolution is unavailable, the transport retains responsibility for environment-proxy
/// fallback.
/// resolution is unavailable, explicit environment settings are resolved before falling back
/// to a direct route.
pub fn resolve_proxy_route(&self, request_url: &str) -> OutboundProxyRoute {
resolve_proxy_route(
&ProcessEnv,
request_url,
self.outbound_proxy_policy,
resolve_system_proxy,
)
}

/// Resolves the proxy route for a concrete destination without blocking a Tokio worker.
pub async fn resolve_proxy_route_async(
&self,
request_url: String,
) -> io::Result<OutboundProxyRoute> {
if matches!(
self.outbound_proxy_policy,
OutboundProxyPolicy::ReqwestDefault
) {
return Ok(OutboundProxyRoute::TransportDefault);
}

if let Some(route) = self.cached_proxy_route(&request_url) {
return Ok(route);
}

#[cfg(not(any(target_os = "windows", target_os = "macos")))]
return Ok(self.resolve_proxy_route(&request_url));

#[cfg(any(target_os = "windows", target_os = "macos"))]
{
let permit = ASYNC_SYSTEM_PROXY_RESOLUTION_PERMIT
.acquire()
.await
.map_err(io::Error::other)?;
let factory = self.clone();
tokio::task::spawn_blocking(move || {
// Keep the permit with the blocking task: cancelling the caller must not allow a
// second PAC/WinHTTP lookup to start while this one is still running.
let _permit = permit;
factory.resolve_proxy_route(&request_url)
})
.await
.map_err(io::Error::other)
}
}

fn cached_proxy_route(&self, request_url: &str) -> Option<OutboundProxyRoute> {
let env_proxy_kind = EnvProxyKind::from_request_url(request_url);
let request_url = proxy_resolution_url(request_url);
if RequestOrigin::parse(&request_url).is_none() {
return Some(OutboundProxyRoute::Direct);
}
cached_system_proxy_decision(&request_url)
.map(|decision| route_from_system_decision(&ProcessEnv, env_proxy_kind, decision))
}

/// Builds an HTTP client for a concrete outbound route.
pub fn build_client(
&self,
Expand Down Expand Up @@ -196,6 +256,7 @@ impl HttpClientFactory {
}

fn resolve_proxy_route(
env: &dyn EnvSource,
request_url: &str,
outbound_proxy_policy: OutboundProxyPolicy,
resolve_system_proxy: impl FnOnce(&str, &RequestOrigin) -> SystemProxyDecision,
Expand All @@ -204,15 +265,79 @@ fn resolve_proxy_route(
return OutboundProxyRoute::TransportDefault;
}

let env_proxy_kind = EnvProxyKind::from_request_url(request_url);
let request_url = proxy_resolution_url(request_url);
let Some(origin) = RequestOrigin::parse(&request_url) else {
return OutboundProxyRoute::TransportDefault;
return OutboundProxyRoute::Direct;
};

match resolve_system_proxy(&request_url, &origin) {
route_from_system_decision(
env,
env_proxy_kind,
resolve_system_proxy(&request_url, &origin),
)
}

fn route_from_system_decision(
env: &dyn EnvSource,
env_proxy_kind: EnvProxyKind,
decision: SystemProxyDecision,
) -> OutboundProxyRoute {
match decision {
SystemProxyDecision::Direct => OutboundProxyRoute::Direct,
SystemProxyDecision::Proxy { url } => OutboundProxyRoute::Proxy { url },
SystemProxyDecision::Unavailable { .. } => OutboundProxyRoute::TransportDefault,
SystemProxyDecision::Proxy { url } => OutboundProxyRoute::Proxy {
url,
no_proxy: None,
},
SystemProxyDecision::Unavailable { .. } => resolve_env_proxy_route(env, env_proxy_kind),
}
}

fn resolve_env_proxy_route(
env: &dyn EnvSource,
env_proxy_kind: EnvProxyKind,
) -> OutboundProxyRoute {
let proxy_url = match env_proxy_kind {
EnvProxyKind::Https => {
proxy_env_value(env, "HTTPS_PROXY").or_else(|| proxy_env_value(env, "ALL_PROXY"))
}
EnvProxyKind::SecureWebSocket => proxy_env_value(env, "HTTPS_PROXY")
.or_else(|| proxy_env_value(env, "HTTP_PROXY"))
.or_else(|| proxy_env_value(env, "ALL_PROXY")),
EnvProxyKind::Http => {
proxy_env_value(env, "HTTP_PROXY").or_else(|| proxy_env_value(env, "ALL_PROXY"))
}
EnvProxyKind::Other => proxy_env_value(env, "ALL_PROXY"),
};
match proxy_url {
Some(url) => OutboundProxyRoute::Proxy {
url,
no_proxy: proxy_env_value(env, "NO_PROXY"),
},
None => OutboundProxyRoute::Direct,
}
}

#[derive(Clone, Copy)]
enum EnvProxyKind {
Http,
Https,
SecureWebSocket,
Other,
}

impl EnvProxyKind {
fn from_request_url(request_url: &str) -> Self {
let scheme = request_url
.parse::<http::Uri>()
.ok()
.and_then(|uri| uri.scheme_str().map(str::to_ascii_lowercase));
match scheme.as_deref() {
Some("http" | "ws") => Self::Http,
Some("https") => Self::Https,
Some("wss") => Self::SecureWebSocket,
Some(_) | None => Self::Other,
}
}
}

Expand Down Expand Up @@ -276,22 +401,26 @@ fn configure_proxy_for_route(
outbound_proxy_policy: OutboundProxyPolicy,
resolve_system_proxy: impl FnOnce(&str, &RequestOrigin) -> SystemProxyDecision,
) -> Result<reqwest::ClientBuilder, BuildRouteAwareHttpClientError> {
if matches!(outbound_proxy_policy, OutboundProxyPolicy::ReqwestDefault) {
return Ok(builder);
}
let origin = RequestOrigin::parse(request_url);

let Some(origin) = origin.as_ref() else {
return configure_env_proxy_handling(env, builder, /*origin*/ None, route_class);
};
let route = resolve_proxy_route(
env,
request_url,
outbound_proxy_policy,
resolve_system_proxy,
);
configure_builder_for_resolved_route(builder, route_class, &route)
}

match resolve_system_proxy(request_url, origin) {
SystemProxyDecision::Direct => Ok(builder.no_proxy()),
SystemProxyDecision::Proxy { url } => {
configure_concrete_proxy(builder, route_class, &url, /*no_proxy*/ None)
}
SystemProxyDecision::Unavailable { .. } => {
configure_env_proxy_handling(env, builder, Some(origin), route_class)
fn configure_builder_for_resolved_route(
builder: reqwest::ClientBuilder,
route_class: ClientRouteClass,
route: &OutboundProxyRoute,
) -> Result<reqwest::ClientBuilder, BuildRouteAwareHttpClientError> {
match route {
OutboundProxyRoute::TransportDefault => Ok(builder),
OutboundProxyRoute::Direct => Ok(builder.no_proxy()),
OutboundProxyRoute::Proxy { url, no_proxy } => {
let no_proxy = no_proxy.as_deref().and_then(reqwest::NoProxy::from_string);
configure_concrete_proxy(builder, route_class, url, no_proxy)
}
}
}
Expand All @@ -311,31 +440,6 @@ fn configure_concrete_proxy(
Ok(builder.proxy(proxy.no_proxy(no_proxy)))
}

fn configure_env_proxy_handling(
env: &dyn EnvSource,
builder: reqwest::ClientBuilder,
origin: Option<&RequestOrigin>,
route_class: ClientRouteClass,
) -> Result<reqwest::ClientBuilder, BuildRouteAwareHttpClientError> {
if let Some(origin) = origin {
let proxy_url = match origin.scheme.as_str() {
"https" => {
proxy_env_value(env, "HTTPS_PROXY").or_else(|| proxy_env_value(env, "ALL_PROXY"))
}
"http" => {
proxy_env_value(env, "HTTP_PROXY").or_else(|| proxy_env_value(env, "ALL_PROXY"))
}
_ => proxy_env_value(env, "ALL_PROXY"),
};
if let Some(proxy_url) = proxy_url {
let no_proxy = proxy_env_value(env, "NO_PROXY")
.and_then(|value| reqwest::NoProxy::from_string(&value));
return configure_concrete_proxy(builder, route_class, &proxy_url, no_proxy);
}
}
Ok(builder.no_proxy())
}

#[derive(Debug, Clone, PartialEq, Eq)]
#[allow(dead_code)]
struct RequestOrigin {
Expand Down Expand Up @@ -431,7 +535,6 @@ struct CachedSystemProxyDecision {
static SYSTEM_PROXY_CACHE: OnceLock<Mutex<HashMap<String, CachedSystemProxyDecision>>> =
OnceLock::new();

#[cfg(test)]
fn cached_system_proxy_decision(request_url: &str) -> Option<SystemProxyDecision> {
let cache = SYSTEM_PROXY_CACHE.get_or_init(|| Mutex::new(HashMap::new()));
let mut cache = cache.lock().ok()?;
Expand Down Expand Up @@ -494,17 +597,11 @@ fn insert_system_proxy_cache_entry(
}

fn system_proxy_cache_key(request_url: &str) -> String {
#[cfg(any(target_os = "windows", target_os = "macos"))]
{
// Keep URL-specific PAC decisions without retaining the raw routed URL.
let mut hasher = Sha256::new();
hasher.update(b"system-proxy-cache-v1\0");
hasher.update(request_url.as_bytes());
format!("{:x}", hasher.finalize())
}

#[cfg(not(any(target_os = "windows", target_os = "macos")))]
request_url.to_string()
// Keep URL-specific PAC decisions without retaining the raw routed URL.
let mut hasher = Sha256::new();
hasher.update(b"system-proxy-cache-v1\0");
hasher.update(request_url.as_bytes());
format!("{:x}", hasher.finalize())
}

#[cfg(any(test, target_os = "windows"))]
Expand Down
Loading
Loading