mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-21 00:21:49 +00:00
refactor(rust): read proxy env vars through the settings lookup
reqwest and hyper each read HTTP(S)_PROXY, ALL_PROXY and NO_PROXY from the process on their own, so tests could not inject them and the pooled client key ignored proxy changes. EnvironmentProxies now reads them through Lookup with the same precedence hyper used, the resolved config carries them (empty when the transport does not trust the env), and both the provider clients and the media fetcher build from that one value. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
caf37c8b6f
commit
a41885e48e
5 changed files with 191 additions and 27 deletions
|
|
@ -6,6 +6,7 @@ use std::{
|
|||
|
||||
use crate::{
|
||||
error::Error,
|
||||
proxy::EnvironmentProxies,
|
||||
settings::{HttpSettings, SslVerify, TcpKeepalive},
|
||||
tls::{CipherSelection, KeyExchangeGroup, Tls12CipherSuite, Unsupported},
|
||||
};
|
||||
|
|
@ -26,7 +27,7 @@ pub struct HttpClientConfig {
|
|||
pub force_ipv4: bool,
|
||||
pub http2: bool,
|
||||
pub user_agent: Option<String>,
|
||||
pub trust_proxy_env: bool,
|
||||
pub proxies: EnvironmentProxies,
|
||||
pub connect_timeout: Duration,
|
||||
pub tcp_keepalive: Option<TcpKeepalive>,
|
||||
pub pool_idle_timeout: Duration,
|
||||
|
|
@ -72,7 +73,11 @@ impl From<&HttpSettings> for Resolution {
|
|||
force_ipv4: settings.force_ipv4,
|
||||
http2: settings.http2,
|
||||
user_agent: settings.user_agent.clone(),
|
||||
trust_proxy_env: settings.trust_proxy_env,
|
||||
proxies: if settings.trust_proxy_env {
|
||||
settings.proxies.clone()
|
||||
} else {
|
||||
EnvironmentProxies::default()
|
||||
},
|
||||
connect_timeout: settings.connect_timeout,
|
||||
tcp_keepalive: settings.tcp_keepalive,
|
||||
pool_idle_timeout: settings.pool_idle_timeout,
|
||||
|
|
@ -111,11 +116,11 @@ impl TryFrom<&HttpClientConfig> for reqwest::ClientBuilder {
|
|||
Some(agent) => with_protocol.user_agent(agent),
|
||||
None => with_protocol,
|
||||
};
|
||||
Ok(if config.trust_proxy_env {
|
||||
with_agent
|
||||
} else {
|
||||
with_agent.no_proxy()
|
||||
})
|
||||
Ok(config
|
||||
.proxies
|
||||
.reqwest_proxies()
|
||||
.into_iter()
|
||||
.fold(with_agent.no_proxy(), reqwest::ClientBuilder::proxy))
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -227,6 +232,25 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
fn proxies() -> EnvironmentProxies {
|
||||
EnvironmentProxies::from_environment(&|name: &str| {
|
||||
(name == "HTTPS_PROXY").then(|| "http://proxy.corp:3128".to_string())
|
||||
})
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn proxies_are_dropped_when_the_transport_does_not_trust_the_environment() {
|
||||
let settings = HttpSettings {
|
||||
trust_proxy_env: false,
|
||||
proxies: proxies(),
|
||||
..HttpSettings::default()
|
||||
};
|
||||
assert_eq!(
|
||||
Resolution::from(&settings).config.proxies,
|
||||
EnvironmentProxies::default()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn connection_settings_carry_over_unchanged() {
|
||||
let keepalive = TcpKeepalive {
|
||||
|
|
@ -240,6 +264,7 @@ mod tests {
|
|||
http2: true,
|
||||
user_agent: Some("litellm/1.0".into()),
|
||||
trust_proxy_env: true,
|
||||
proxies: proxies(),
|
||||
connect_timeout: Duration::from_secs(7),
|
||||
tcp_keepalive: Some(keepalive),
|
||||
pool_idle_timeout: Duration::from_secs(45),
|
||||
|
|
@ -256,7 +281,7 @@ mod tests {
|
|||
force_ipv4: true,
|
||||
http2: true,
|
||||
user_agent: Some("litellm/1.0".into()),
|
||||
trust_proxy_env: true,
|
||||
proxies: proxies(),
|
||||
connect_timeout: Duration::from_secs(7),
|
||||
tcp_keepalive: Some(keepalive),
|
||||
pool_idle_timeout: Duration::from_secs(45),
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ use std::{
|
|||
|
||||
use reqwest::dns::Resolve;
|
||||
|
||||
use crate::{config::HttpClientConfig, error::Error};
|
||||
use crate::{config::HttpClientConfig, error::Error, proxy::EnvironmentProxies};
|
||||
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
|
||||
pub enum ClientVariant {
|
||||
|
|
@ -52,7 +52,7 @@ impl HttpClientPool {
|
|||
let effective = match variant {
|
||||
ClientVariant::Media => HttpClientConfig {
|
||||
client_certificate: None,
|
||||
trust_proxy_env: false,
|
||||
proxies: EnvironmentProxies::default(),
|
||||
..config.clone()
|
||||
},
|
||||
ClientVariant::UnpinnedMedia => HttpClientConfig {
|
||||
|
|
@ -138,6 +138,13 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
fn proxied_through(proxy: &str) -> EnvironmentProxies {
|
||||
let proxy = proxy.to_owned();
|
||||
EnvironmentProxies::from_environment(&move |name: &str| {
|
||||
(name == "HTTP_PROXY").then(|| proxy.clone())
|
||||
})
|
||||
}
|
||||
|
||||
async fn serve(
|
||||
status_line: &'static str,
|
||||
) -> (SocketAddr, Arc<AtomicUsize>, Arc<Mutex<Vec<String>>>) {
|
||||
|
|
@ -202,6 +209,50 @@ mod tests {
|
|||
assert_eq!(connections.load(Ordering::SeqCst), 3);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn provider_clients_route_through_the_resolved_proxy_not_the_process_environment() {
|
||||
let (proxy, connections, requests) = serve("HTTP/1.1 204 No Content").await;
|
||||
let config = HttpClientConfig {
|
||||
proxies: proxied_through(&format!("http://user:secret@{proxy}")),
|
||||
..config("a")
|
||||
};
|
||||
let response = get(
|
||||
&pool(),
|
||||
&config,
|
||||
ClientVariant::Provider,
|
||||
"http://upstream.invalid/v1/ocr",
|
||||
)
|
||||
.await;
|
||||
assert_eq!(response.status(), 204);
|
||||
assert_eq!(connections.load(Ordering::SeqCst), 1);
|
||||
let request = requests.lock().unwrap().concat();
|
||||
assert!(request.starts_with("GET http://upstream.invalid/v1/ocr HTTP/1.1"));
|
||||
assert!(request.contains("proxy-authorization: Basic dXNlcjpzZWNyZXQ="));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn no_proxy_hosts_bypass_the_resolved_proxy() {
|
||||
let (upstream, _, _) = serve("HTTP/1.1 204 No Content").await;
|
||||
let (proxy, proxy_connections, _) = serve("HTTP/1.1 502 Bad Gateway").await;
|
||||
let config = HttpClientConfig {
|
||||
proxies: EnvironmentProxies::from_environment(&move |name: &str| match name {
|
||||
"HTTP_PROXY" => Some(format!("http://{proxy}")),
|
||||
"NO_PROXY" => Some("127.0.0.1".into()),
|
||||
_ => None,
|
||||
}),
|
||||
..config("a")
|
||||
};
|
||||
let response = get(
|
||||
&pool(),
|
||||
&config,
|
||||
ClientVariant::Provider,
|
||||
&format!("http://{upstream}/v1/ocr"),
|
||||
)
|
||||
.await;
|
||||
assert_eq!(response.status(), 204);
|
||||
assert_eq!(proxy_connections.load(Ordering::SeqCst), 0);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn expired_clients_are_rebuilt() {
|
||||
let (address, connections, _) = serve("HTTP/1.1 204 No Content").await;
|
||||
|
|
@ -220,9 +271,12 @@ mod tests {
|
|||
let (address, connections, _) = serve("HTTP/1.1 204 No Content").await;
|
||||
let pool = HttpClientPool::new(Arc::new(FixedResolver(address)));
|
||||
let url = format!("http://media.invalid:{}/doc", address.port());
|
||||
for trust_proxy_env in [true, false] {
|
||||
for proxies in [
|
||||
proxied_through("http://proxy.invalid:3128"),
|
||||
EnvironmentProxies::default(),
|
||||
] {
|
||||
let config = HttpClientConfig {
|
||||
trust_proxy_env,
|
||||
proxies,
|
||||
..config("a")
|
||||
};
|
||||
get(&pool, &config, ClientVariant::Media, &url).await;
|
||||
|
|
|
|||
|
|
@ -1,15 +1,98 @@
|
|||
use hyper_util::client::proxy::matcher::Matcher;
|
||||
use litellm_core_utils::settings::Lookup;
|
||||
|
||||
pub struct EnvironmentProxies(Matcher);
|
||||
#[derive(Clone, Debug, Default, PartialEq, Eq, Hash)]
|
||||
pub struct EnvironmentProxies {
|
||||
all: String,
|
||||
http: String,
|
||||
https: String,
|
||||
no: String,
|
||||
}
|
||||
|
||||
impl EnvironmentProxies {
|
||||
pub fn from_environment() -> Self {
|
||||
Self(Matcher::from_system())
|
||||
pub fn from_environment(env: &impl Lookup) -> Self {
|
||||
if env.get("REQUEST_METHOD").is_some() {
|
||||
return Self::default();
|
||||
}
|
||||
let first = |upper: &str, lower: &str| {
|
||||
env.get(upper)
|
||||
.or_else(|| env.get(lower))
|
||||
.unwrap_or_default()
|
||||
};
|
||||
Self {
|
||||
all: first("ALL_PROXY", "all_proxy"),
|
||||
http: first("HTTP_PROXY", "http_proxy"),
|
||||
https: first("HTTPS_PROXY", "https_proxy"),
|
||||
no: first("NO_PROXY", "no_proxy"),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn apply_to(&self, url: &reqwest::Url) -> bool {
|
||||
let matcher = Matcher::builder()
|
||||
.all(self.all.clone())
|
||||
.http(self.http.clone())
|
||||
.https(self.https.clone())
|
||||
.no(self.no.clone())
|
||||
.build();
|
||||
url.as_str()
|
||||
.parse::<http::Uri>()
|
||||
.is_ok_and(|uri| self.0.intercept(&uri).is_some())
|
||||
.is_ok_and(|uri| matcher.intercept(&uri).is_some())
|
||||
}
|
||||
|
||||
pub(crate) fn reqwest_proxies(&self) -> Vec<reqwest::Proxy> {
|
||||
let no_proxy = reqwest::NoProxy::from_string(&self.no);
|
||||
[
|
||||
reqwest::Proxy::http(self.http.as_str()),
|
||||
reqwest::Proxy::https(self.https.as_str()),
|
||||
reqwest::Proxy::all(self.all.as_str()),
|
||||
]
|
||||
.into_iter()
|
||||
.filter_map(Result::ok)
|
||||
.map(|proxy| proxy.no_proxy(no_proxy.clone()))
|
||||
.collect()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use rstest::rstest;
|
||||
|
||||
use super::*;
|
||||
|
||||
fn env_of(values: &'static [(&'static str, &'static str)]) -> impl Fn(&str) -> Option<String> {
|
||||
move |name| {
|
||||
values
|
||||
.iter()
|
||||
.find(|(key, _)| *key == name)
|
||||
.map(|(_, value)| value.to_string())
|
||||
}
|
||||
}
|
||||
|
||||
fn url(value: &str) -> reqwest::Url {
|
||||
reqwest::Url::parse(value).unwrap()
|
||||
}
|
||||
|
||||
#[rstest]
|
||||
#[case::http_only(&[("HTTP_PROXY", "http://proxy:3128")], "http://api.test/", true)]
|
||||
#[case::http_proxy_skips_https(&[("HTTP_PROXY", "http://proxy:3128")], "https://api.test/", false)]
|
||||
#[case::all_covers_https(&[("ALL_PROXY", "http://proxy:3128")], "https://api.test/", true)]
|
||||
#[case::lowercase(&[("https_proxy", "http://proxy:3128")], "https://api.test/", true)]
|
||||
#[case::no_proxy_bypass(&[("HTTPS_PROXY", "http://proxy:3128"), ("NO_PROXY", "api.test")], "https://api.test/", false)]
|
||||
#[case::cgi_ignores_everything(&[("HTTPS_PROXY", "http://proxy:3128"), ("REQUEST_METHOD", "GET")], "https://api.test/", false)]
|
||||
#[case::uppercase_wins_even_when_empty(&[("HTTPS_PROXY", ""), ("https_proxy", "http://proxy:3128")], "https://api.test/", false)]
|
||||
fn proxies_follow_the_injected_environment(
|
||||
#[case] env: &'static [(&'static str, &'static str)],
|
||||
#[case] target: &str,
|
||||
#[case] expected: bool,
|
||||
) {
|
||||
let proxies = EnvironmentProxies::from_environment(&env_of(env));
|
||||
assert_eq!(proxies.apply_to(&url(target)), expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_empty_environment_proxies_nothing() {
|
||||
let proxies = EnvironmentProxies::from_environment(&env_of(&[]));
|
||||
assert_eq!(proxies, EnvironmentProxies::default());
|
||||
assert!(proxies.reqwest_proxies().is_empty());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,6 +5,8 @@ use std::{
|
|||
|
||||
use litellm_core_utils::settings::{Layer, Lookup, merge};
|
||||
|
||||
use crate::proxy::EnvironmentProxies;
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
|
||||
pub enum SslVerify {
|
||||
Enabled,
|
||||
|
|
@ -44,6 +46,7 @@ pub struct HttpSettingsLayer {
|
|||
pub user_agent: Option<String>,
|
||||
pub tcp_keepalive: Option<TcpKeepalive>,
|
||||
pub pool_idle_timeout: Option<Duration>,
|
||||
pub proxies: Option<EnvironmentProxies>,
|
||||
}
|
||||
|
||||
impl HttpSettingsLayer {
|
||||
|
|
@ -71,6 +74,8 @@ impl HttpSettingsLayer {
|
|||
pool_idle_timeout: env
|
||||
.parsed::<u32>("AIOHTTP_KEEPALIVE_TIMEOUT")
|
||||
.map(|timeout| Duration::from_secs(u64::from(timeout))),
|
||||
proxies: Some(EnvironmentProxies::from_environment(env))
|
||||
.filter(|proxies| *proxies != EnvironmentProxies::default()),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -95,6 +100,7 @@ impl Layer for HttpSettingsLayer {
|
|||
user_agent: self.user_agent.or(lower.user_agent),
|
||||
tcp_keepalive: self.tcp_keepalive.or(lower.tcp_keepalive),
|
||||
pool_idle_timeout: self.pool_idle_timeout.or(lower.pool_idle_timeout),
|
||||
proxies: self.proxies.or(lower.proxies),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -110,6 +116,7 @@ pub struct HttpSettings {
|
|||
pub http2: bool,
|
||||
pub user_agent: Option<String>,
|
||||
pub trust_proxy_env: bool,
|
||||
pub proxies: EnvironmentProxies,
|
||||
pub connect_timeout: Duration,
|
||||
pub tcp_keepalive: Option<TcpKeepalive>,
|
||||
pub pool_idle_timeout: Duration,
|
||||
|
|
@ -127,6 +134,7 @@ impl Default for HttpSettings {
|
|||
http2: false,
|
||||
user_agent: None,
|
||||
trust_proxy_env: true,
|
||||
proxies: EnvironmentProxies::default(),
|
||||
connect_timeout: Duration::from_secs(10),
|
||||
tcp_keepalive: None,
|
||||
pool_idle_timeout: Duration::from_secs(120),
|
||||
|
|
@ -160,6 +168,7 @@ impl HttpSettings {
|
|||
pool_idle_timeout: merged
|
||||
.pool_idle_timeout
|
||||
.unwrap_or(defaults.pool_idle_timeout),
|
||||
proxies: merged.proxies.unwrap_or_default(),
|
||||
..defaults
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@ use std::{
|
|||
time::Duration,
|
||||
};
|
||||
|
||||
use litellm_http::{ClientVariant, EnvironmentProxies, HttpClientConfig, HttpClientPool};
|
||||
use litellm_http::{ClientVariant, HttpClientConfig, HttpClientPool};
|
||||
use reqwest::{
|
||||
Url,
|
||||
dns::{Addrs, Name, Resolve, Resolving},
|
||||
|
|
@ -102,12 +102,8 @@ impl MediaFetcher {
|
|||
config: &HttpClientConfig,
|
||||
url_policy: UrlPolicy,
|
||||
) -> Result<Self, litellm_http::Error> {
|
||||
let uses_proxy: ProxyMatch = if config.trust_proxy_env {
|
||||
let proxies = EnvironmentProxies::from_environment();
|
||||
Arc::new(move |url| proxies.apply_to(url))
|
||||
} else {
|
||||
Arc::new(|_| false)
|
||||
};
|
||||
let proxies = config.proxies.clone();
|
||||
let uses_proxy: ProxyMatch = Arc::new(move |url| proxies.apply_to(url));
|
||||
Self::with_resolution(
|
||||
pool,
|
||||
config,
|
||||
|
|
@ -443,10 +439,7 @@ mod tests {
|
|||
url_policy: UrlPolicy,
|
||||
uses_proxy: bool,
|
||||
) -> MediaFetcher {
|
||||
let direct = HttpClientConfig {
|
||||
trust_proxy_env: false,
|
||||
..Resolution::from(&HttpSettings::default()).config
|
||||
};
|
||||
let direct = Resolution::from(&HttpSettings::default()).config;
|
||||
MediaFetcher::with_resolution(
|
||||
&HttpClientPool::new(Arc::new(LoopbackDnsResolver(pinned_address))),
|
||||
&direct,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue