From 7739e0312362980ea2791780a6d6a5f71dc70fe2 Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Thu, 17 Oct 2019 13:32:00 -0700 Subject: [PATCH] Enable "system" proxies by default (#683) If no proxies are configured for a client, the environment (system) will be inspected automatically to set up proxies. Configuring a `Proxy` on a client or calling `no_proxy` will disable the use of the automatic system proxy. Closes #403 --- .travis.yml | 2 +- Cargo.toml | 8 +++++- src/async_impl/client.rs | 32 +++++++++++++--------- src/blocking/client.rs | 13 ++++++--- src/lib.rs | 3 +++ src/proxy.rs | 58 +++++++++++++++++++++++++++------------- tests/proxy.rs | 18 ++++++------- 7 files changed, 87 insertions(+), 47 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1a61b79..ad997ba 100644 --- a/.travis.yml +++ b/.travis.yml @@ -102,4 +102,4 @@ env: - RUST_BACKTRACE=1 script: - cargo build $FEATURES - - cargo test -v $FEATURES -- --test-threads=1 + - cargo test -v $FEATURES --features __internal_proxy_sys_no_cache -- --test-threads=1 diff --git a/Cargo.toml b/Cargo.toml index e0361a4..b1aecc6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,12 @@ json = ["serde_json"] unstable-stream = [] +# Internal (PRIVATE!) features used to aid testing. +# Don't rely on these whatsoever. They may disappear at anytime. + +# When enabled, disable using the cached SYS_PROXIES. +__internal_proxy_sys_no_cache = [] + [dependencies] http = "0.1.15" url = "2.1" @@ -49,6 +55,7 @@ futures-core-preview = { version = "=0.3.0-alpha.19" } futures-util-preview = { version = "=0.3.0-alpha.19" } http-body = "=0.2.0-alpha.3" hyper = { version = "=0.13.0-alpha.4", default-features = false, features = ["tcp"] } +lazy_static = "1.4" log = "0.4" mime = "0.3.7" mime_guess = "2.0" @@ -150,4 +157,3 @@ required-features = ["cookies"] name = "gzip" path = "tests/gzip.rs" required-features = ["gzip"] - diff --git a/src/async_impl/client.rs b/src/async_impl/client.rs index 6b7881b..383b3b3 100644 --- a/src/async_impl/client.rs +++ b/src/async_impl/client.rs @@ -28,7 +28,6 @@ use crate::connect::Connector; #[cfg(feature = "cookies")] use crate::cookie; use crate::into_url::{expect_uri, try_uri}; -use crate::proxy::get_proxies; use crate::redirect::{self, remove_sensitive_headers, RedirectPolicy}; #[cfg(feature = "tls")] use crate::tls::TlsBackend; @@ -69,6 +68,7 @@ struct Config { #[cfg(feature = "tls")] identity: Option, proxies: Vec, + auto_sys_proxy: bool, redirect_policy: RedirectPolicy, referer: bool, timeout: Option, @@ -104,6 +104,7 @@ impl ClientBuilder { connect_timeout: None, max_idle_per_host: std::usize::MAX, proxies: Vec::new(), + auto_sys_proxy: true, redirect_policy: RedirectPolicy::default(), referer: true, timeout: None, @@ -131,7 +132,11 @@ impl ClientBuilder { /// cannot load the system configuration. pub fn build(self) -> crate::Result { let config = self.config; - let proxies = Arc::new(config.proxies); + let mut proxies = config.proxies; + if config.auto_sys_proxy { + proxies.push(Proxy::system()); + } + let proxies = Arc::new(proxies); let mut connector = { #[cfg(feature = "tls")] @@ -365,27 +370,28 @@ impl ClientBuilder { // Proxy options /// Add a `Proxy` to the list of proxies the `Client` will use. + /// + /// # Note + /// + /// Adding a proxy will disable the automatic usage of the "system" proxy. pub fn proxy(mut self, proxy: Proxy) -> ClientBuilder { self.config.proxies.push(proxy); + self.config.auto_sys_proxy = false; self } /// Clear all `Proxies`, so `Client` will use no proxy anymore. + /// + /// This also disables the automatic usage of the "system" proxy. pub fn no_proxy(mut self) -> ClientBuilder { self.config.proxies.clear(); + self.config.auto_sys_proxy = false; self } - /// Add system proxy setting to the list of proxies - pub fn use_sys_proxy(mut self) -> ClientBuilder { - let proxies = get_proxies(); - self.config.proxies.push(Proxy::custom(move |url| { - if proxies.contains_key(url.scheme()) { - Some((*proxies.get(url.scheme()).unwrap()).clone()) - } else { - None - } - })); + #[doc(hidden)] + #[deprecated(note = "the system proxy is used automatically")] + pub fn use_sys_proxy(self) -> ClientBuilder { self } @@ -776,7 +782,7 @@ impl Client { impl fmt::Debug for Client { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let mut builder = f.debug_struct("ClientBuilder"); + let mut builder = f.debug_struct("Client"); self.inner.fmt_fields(&mut builder); builder.finish() } diff --git a/src/blocking/client.rs b/src/blocking/client.rs index eb70f4d..7d1b14d 100644 --- a/src/blocking/client.rs +++ b/src/blocking/client.rs @@ -193,18 +193,25 @@ impl ClientBuilder { // Proxy options /// Add a `Proxy` to the list of proxies the `Client` will use. + /// + /// # Note + /// + /// Adding a proxy will disable the automatic usage of the "system" proxy. pub fn proxy(self, proxy: Proxy) -> ClientBuilder { self.with_inner(move |inner| inner.proxy(proxy)) } - /// Disable proxy setting. + /// Clear all `Proxies`, so `Client` will use no proxy anymore. + /// + /// This also disables the automatic usage of the "system" proxy. pub fn no_proxy(self) -> ClientBuilder { self.with_inner(move |inner| inner.no_proxy()) } - /// Enable system proxy setting. + #[doc(hidden)] + #[deprecated(note = "the system proxy is used automatically")] pub fn use_sys_proxy(self) -> ClientBuilder { - self.with_inner(move |inner| inner.use_sys_proxy()) + self } // Timeout options diff --git a/src/lib.rs b/src/lib.rs index 40843c9..bbbd182 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -263,6 +263,9 @@ if_hyper! { #[macro_use] extern crate doc_comment; + #[macro_use] + extern crate lazy_static; + #[cfg(test)] doctest!("../README.md"); diff --git a/src/proxy.rs b/src/proxy.rs index 0334a86..407677a 100644 --- a/src/proxy.rs +++ b/src/proxy.rs @@ -180,11 +180,13 @@ impl Proxy { })) } - /* - pub fn unix(path: P) -> Proxy { - + pub(crate) fn system() -> Proxy { + if cfg!(feature = "__internal_proxy_sys_no_cache") { + Proxy::new(Intercept::System(Arc::new(get_sys_proxies()))) + } else { + Proxy::new(Intercept::System(SYS_PROXIES.clone())) + } } - */ fn new(intercept: Intercept) -> Proxy { Proxy { intercept } @@ -248,6 +250,9 @@ impl Proxy { None } } + Intercept::System(ref map) => { + map.get(uri.scheme()).cloned() + } Intercept::Custom(ref custom) => custom.call(uri), } } @@ -257,6 +262,7 @@ impl Proxy { Intercept::All(_) => true, Intercept::Http(_) => uri.scheme() == "http", Intercept::Https(_) => uri.scheme() == "https", + Intercept::System(ref map) => map.contains_key(uri.scheme()), Intercept::Custom(ref custom) => custom.call(uri).is_some(), } } @@ -369,11 +375,14 @@ impl ProxyScheme { } } +type SystemProxyMap = HashMap; + #[derive(Clone, Debug)] enum Intercept { All(ProxyScheme), Http(ProxyScheme), Https(ProxyScheme), + System(Arc), Custom(Custom), } @@ -383,6 +392,7 @@ impl Intercept { Intercept::All(ref mut s) | Intercept::Http(ref mut s) | Intercept::Https(ref mut s) => s.set_basic_auth(username, password), + Intercept::System(_) => unimplemented!(), Intercept::Custom(ref mut custom) => { let header = encode_basic_auth(username, password); custom.auth = Some(header); @@ -484,6 +494,10 @@ impl Dst for Uri { } } +lazy_static! { + static ref SYS_PROXIES: Arc = Arc::new(get_sys_proxies()); +} + /// Get system proxies information. /// /// It can only support Linux, Unix like, and windows system. Note that it will always @@ -493,8 +507,8 @@ impl Dst for Uri { /// Returns: /// System proxies information as a hashmap like /// {"http": Url::parse("http://127.0.0.1:80"), "https": Url::parse("https://127.0.0.1:80")} -pub fn get_proxies() -> HashMap { - let proxies: HashMap = get_from_environment(); +fn get_sys_proxies() -> SystemProxyMap { + let proxies = get_from_environment(); // TODO: move the following #[cfg] to `if expression` when attributes on `if` expressions allowed #[cfg(target_os = "windows")] @@ -507,14 +521,14 @@ pub fn get_proxies() -> HashMap { proxies } -fn insert_proxy(proxies: &mut HashMap, schema: String, addr: String) { - if let Ok(valid_addr) = Url::parse(&addr) { +fn insert_proxy(proxies: &mut SystemProxyMap, schema: String, addr: String) { + if let Ok(valid_addr) = addr.into_proxy_scheme() { proxies.insert(schema, valid_addr); } } -fn get_from_environment() -> HashMap { - let mut proxies: HashMap = HashMap::new(); +fn get_from_environment() -> SystemProxyMap { + let mut proxies = HashMap::new(); const PROXY_KEY_ENDS: &str = "_proxy"; @@ -530,7 +544,7 @@ fn get_from_environment() -> HashMap { } #[cfg(target_os = "windows")] -fn get_from_registry_impl() -> Result, Box> { +fn get_from_registry_impl() -> Result> { let hkcu = RegKey::predef(HKEY_CURRENT_USER); let internet_setting: RegKey = hkcu.open_subkey("Software\\Microsoft\\Windows\\CurrentVersion\\Internet Settings")?; @@ -542,7 +556,7 @@ fn get_from_registry_impl() -> Result, Box> { return Ok(HashMap::new()); } - let mut proxies: HashMap = HashMap::new(); + let mut proxies = HashMap::new(); if proxy_server.contains("=") { // per-protocol settings. for p in proxy_server.split(";") { @@ -589,7 +603,7 @@ fn get_from_registry_impl() -> Result, Box> { } #[cfg(target_os = "windows")] -fn get_from_registry() -> HashMap { +fn get_from_registry() -> SystemProxyMap { get_from_registry_impl().unwrap_or(HashMap::new()) } @@ -685,24 +699,30 @@ mod tests { } #[test] - fn test_get_proxies() { + fn test_get_sys_proxies() { // save system setting first. let system_proxy = env::var("http_proxy"); // remove proxy. env::remove_var("http_proxy"); - assert_eq!(get_proxies().contains_key("http"), false); + assert_eq!(get_sys_proxies().contains_key("http"), false); // the system proxy setting url is invalid. env::set_var("http_proxy", "123465"); - assert_eq!(get_proxies().contains_key("http"), false); + assert_eq!(get_sys_proxies().contains_key("http"), false); // set valid proxy env::set_var("http_proxy", "http://127.0.0.1/"); - let proxies = get_proxies(); - let http_target = proxies.get("http").unwrap().as_str(); + let proxies = get_sys_proxies(); + + match &proxies["http"] { + ProxyScheme::Http { uri, .. } => { + assert_eq!(uri, "http://127.0.0.1/"); + }, + #[cfg(feature = "socks")] + _ => panic!("socks not expected"), + } - assert_eq!(http_target, "http://127.0.0.1/"); // reset user setting. match system_proxy { Err(_) => env::remove_var("http_proxy"), diff --git a/tests/proxy.rs b/tests/proxy.rs index d269200..11cab33 100644 --- a/tests/proxy.rs +++ b/tests/proxy.rs @@ -119,30 +119,28 @@ async fn test_no_proxy() { assert_eq!(res.status(), reqwest::StatusCode::OK); } +#[cfg_attr(not(feature = "__internal_proxy_sys_no_cache"), ignore)] #[tokio::test] async fn test_using_system_proxy() { - let url = "http://hyper.rs/prox"; + let url = "http://not.a.real.sub.hyper.rs/prox"; let server = server::http(move |req| { assert_eq!(req.method(), "GET"); assert_eq!(req.uri(), url); - assert_eq!(req.headers()["host"], "hyper.rs"); + assert_eq!(req.headers()["host"], "not.a.real.sub.hyper.rs"); async { http::Response::default() } }); + // Note: we're relying on the `__internal_proxy_sys_no_cache` feature to + // check the environment every time. + // save system setting first. let system_proxy = env::var("http_proxy"); // set-up http proxy. env::set_var("http_proxy", format!("http://{}", server.addr())); - let res = reqwest::Client::builder() - .use_sys_proxy() - .build() - .unwrap() - .get(url) - .send() - .await - .unwrap(); + // system proxy is used by default + let res = reqwest::get(url).await.unwrap(); assert_eq!(res.url().as_str(), url); assert_eq!(res.status(), reqwest::StatusCode::OK);