Repository: seanmonstar/reqwest (language: rust)

Issue:
Added support for the NO_PROXY environment variable
Hello there,

Using the use_sys_proxy() feature does not support the NO_PROXY environment variable.
This causes Domains or IP's which should not go through the proxy to not work.

Most languages/tools support a simple comma separated list of items to skip proxy for.
This list can contain the following.
- **FQDN**: github.com, localhost etc.. - Only matches github.com, not gist.github.com
- **FQDN Started with a dot**: .github.com - matches gist.github.com, api.github.com, but not github.com
- **IP Address**: 127.0.0.1,::1
- **CIDR**: 10.0.0.0/8

Example:
```bash
NO_PROXY=localhost,127.0.0.1,::1,github.com,.github.com,10.0.0.0/8,192.168.0.5
```



Discussion / hints:
@seanmonstar 
I'm not that high level skilled in rust, but i can find my way.
I was looking at the code, and am a bit puzzled as where the best place is to put this.

I figured the best thing would be to add it to the `ProxyScheme` or a bit more global in the `Intercept` enum. But i'm a bit at a loss where to start and where/how to add it.

Because i think i need to catch it in the `Proxy::intercept()` call, and return `None` in case the `Dst.host()` matches the NO_PROXY value. So i probably need to have something available within that function, which can do that pattern checking.

If possible, could you point me in a bit in the right direction?
Thx in advance :).
The behavior would likely be contained in the [`Intercept::System`](https://github.com/seanmonstar/reqwest/blob/3b235932623ea99303fbe04718baaf2cbc12d88e/src/proxy.rs#L444) variant. The matching happens [here](https://github.com/seanmonstar/reqwest/blob/3b235932623ea99303fbe04718baaf2cbc12d88e/src/proxy.rs#L258).

I'm not too familiar with all the rules that `NO_PROXY` can use. It kind of looks like the rules differ in curl vs Golang.
There are some basics at least. But there are a lot of differences indeed.

Thx for the pointers, i will take a look there.
I would be interested in seeing this problem fixed.

Thank you!
Curl works by splitting the "domains" in the `no_proxy` environment by comma (`,`), checking whether that matches exactly the domain, or whether the domain being considered is a "subdomain" of one of the `no_proxy` domains.  I use quotes because you can specify a number like `1` and curl will consider `192.168.1.1` to be a "subdomain" and skip the proxy.  (This is very handy for telling curl to only use the proxy for FQDN queries and bypass the proxy for IP-only queries.)

Fix patch:
diff --git a/Cargo.toml b/Cargo.toml
index 63df5981d..4c50ffaa2 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -82,6 +82,7 @@ mime_guess = "2.0"
 percent-encoding = "2.1"
 tokio = { version = "0.2.5", default-features = false, features = ["tcp", "time"] }
 pin-project-lite = "0.1.1"
+ipnet = "2.3"
 
 # TODO: candidates for optional features
 
diff --git a/src/proxy.rs b/src/proxy.rs
index cc39511b0..59f3ce6ef 100644
--- a/src/proxy.rs
+++ b/src/proxy.rs
@@ -5,11 +5,13 @@ use std::sync::Arc;
 
 use crate::{IntoUrl, Url};
 use http::{header::HeaderValue, Uri};
+use ipnet::IpNet;
 use percent_encoding::percent_decode;
 use std::collections::HashMap;
 use std::env;
 #[cfg(target_os = "windows")]
 use std::error::Error;
+use std::net::IpAddr;
 #[cfg(target_os = "windows")]
 use winreg::enums::HKEY_CURRENT_USER;
 #[cfg(target_os = "windows")]
@@ -50,6 +52,31 @@ use winreg::RegKey;
 #[derive(Clone)]
 pub struct Proxy {
     intercept: Intercept,
+    no_proxy: Option<NoProxy>,
+}
+
+/// Represents a possible matching entry for an IP address
+#[derive(Clone, Debug)]
+enum Ip {
+    Address(IpAddr),
+    Network(IpNet),
+}
+
+/// A wrapper around a list of IP cidr blocks or addresses with a [IpMatcher::contains] method for
+/// checking if an IP address is contained within the matcher
+#[derive(Clone, Debug, Default)]
+struct IpMatcher(Vec<Ip>);
+
+/// A wrapper around a list of domains with a [DomainMatcher::contains] method for checking if a
+/// domain is contained within the matcher
+#[derive(Clone, Debug, Default)]
+struct DomainMatcher(Vec<String>);
+
+/// A configuration for filtering out requests that shouldn't be proxied
+#[derive(Clone, Debug, Default)]
+struct NoProxy {
+    ips: IpMatcher,
+    domains: DomainMatcher,
 }
 
 /// A particular scheme used for proxying requests.
@@ -184,15 +211,20 @@ impl Proxy {
     }
 
     pub(crate) fn system() -> Proxy {
-        if cfg!(feature = "__internal_proxy_sys_no_cache") {
+        let mut 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()))
-        }
+        };
+        proxy.no_proxy = NoProxy::new();
+        proxy
     }
 
     fn new(intercept: Intercept) -> Proxy {
-        Proxy { intercept }
+        Proxy {
+            intercept,
+            no_proxy: None,
+        }
     }
 
     /// Set the `Proxy-Authorization` header using Basic auth.
@@ -255,7 +287,15 @@ impl Proxy {
                 }
             }
             Intercept::System(ref map) => {
-                map.get(uri.scheme()).cloned()
+                let in_no_proxy = self
+                    .no_proxy
+                    .as_ref()
+                    .map_or(false, |np| np.contains(uri.host()));
+                if in_no_proxy {
+                    None
+                } else {
+                    map.get(uri.scheme()).cloned()
+                }
             }
             Intercept::Custom(ref custom) => custom.call(uri),
         }
@@ -274,7 +314,91 @@ impl Proxy {
 
 impl fmt::Debug for Proxy {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        f.debug_tuple("Proxy").field(&self.intercept).finish()
+        f.debug_tuple("Proxy")
+            .field(&self.intercept)
+            .field(&self.no_proxy)
+            .finish()
+    }
+}
+
+impl NoProxy {
+    /// Returns a new no proxy configration if the NO_PROXY/no_proxy environment variable is set.
+    /// Returns None otherwise
+    fn new() -> Option<Self> {
+        let raw = env::var("NO_PROXY")
+            .or_else(|_| env::var("no_proxy"))
+            .unwrap_or_default();
+        if raw.is_empty() {
+            return None;
+        }
+        let mut ips = Vec::new();
+        let mut domains = Vec::new();
+        let parts = raw.split(',');
+        for part in parts {
+            match part.parse::<IpNet>() {
+                // If we can parse an IP net or address, then use it, otherwise, assume it is a domain
+                Ok(ip) => ips.push(Ip::Network(ip)),
+                Err(_) => match part.parse::<IpAddr>() {
+                    Ok(addr) => ips.push(Ip::Address(addr)),
+                    Err(_) => domains.push(part.to_owned()),
+                },
+            }
+        }
+        Some(NoProxy {
+            ips: IpMatcher(ips),
+            domains: DomainMatcher(domains),
+        })
+    }
+
+    fn contains(&self, host: &str) -> bool {
+        // According to RFC3986, raw IPv6 hosts will be wrapped in []. So we need to strip those off
+        // the end in order to parse correctly
+        let host = if host.starts_with('[') {
+            let x: &[_] = &['[', ']'];
+            host.trim_matches(x)
+        } else {
+            host
+        };
+        match host.parse::<IpAddr>() {
+            // If we can parse an IP addr, then use it, otherwise, assume it is a domain
+            Ok(ip) => self.ips.contains(ip),
+            Err(_) => self.domains.contains(host),
+        }
+    }
+}
+
+impl IpMatcher {
+    fn contains(&self, addr: IpAddr) -> bool {
+        for ip in self.0.iter() {
+            match ip {
+                Ip::Address(address) => {
+                    if &addr == address {
+                        return true;
+                    }
+                }
+                Ip::Network(net) => {
+                    if net.contains(&addr) {
+                        return true;
+                    }
+                }
+            }
+        }
+        false
+    }
+}
+
+impl DomainMatcher {
+    fn contains(&self, domain: &str) -> bool {
+        for d in self.0.iter() {
+            // First check for a "wildcard" domain match. A single "." will match anything.
+            // Otherwise, check that the domains are equal
+            if (d.starts_with('.') && domain.ends_with(d.get(1..).unwrap_or_default()))
+                || d == domain
+            {
+                return true;
+            }
+        }
+        false
     }
 }
 
@@ -342,11 +466,11 @@ impl ProxyScheme {
             ProxyScheme::Http { ref mut auth, .. } => {
                 let header = encode_basic_auth(&username.into(), &password.into());
                 *auth = Some(header);
-            },
+            }
             ProxyScheme::Https { ref mut auth, .. } => {
                 let header = encode_basic_auth(&username.into(), &password.into());
                 *auth = Some(header);
-            },
+            }
             #[cfg(feature = "socks")]
             ProxyScheme::Socks5 { ref mut auth, .. } => {
                 *auth = Some((username.into(), password.into()));
@@ -360,12 +484,12 @@ impl ProxyScheme {
                 if auth.is_none() {
                     *auth = update.clone();
                 }
-            },
+            }
             ProxyScheme::Https { ref mut auth, .. } => {
                 if auth.is_none() {
                     *auth = update.clone();
                 }
-            },
+            }
             #[cfg(feature = "socks")]
             ProxyScheme::Socks5 { .. } => {}
         }
@@ -383,10 +507,11 @@ impl ProxyScheme {
         // Resolve URL to a host and port
         #[cfg(feature = "socks")]
         let to_addr = || {
-            let addrs = url.socket_addrs(|| match url.scheme() {
-                "socks5" | "socks5h" => Some(1080),
-                _ => None,
-            })
+            let addrs = url
+                .socket_addrs(|| match url.scheme() {
+                    "socks5" | "socks5h" => Some(1080),
+                    _ => None,
+                })
                 .map_err(crate::error::builder)?;
             addrs
                 .into_iter()
@@ -437,29 +562,15 @@ impl ProxyScheme {
 impl fmt::Debug for ProxyScheme {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
-            ProxyScheme::Http {
-                auth: _auth,
-                host,
-            } => {
-                write!(f, "http://{}", host)
-            },
-            ProxyScheme::Https {
-                auth: _auth,
-                host,
-            } => {
-                write!(f, "https://{}", host)
-            },
+            ProxyScheme::Http { auth: _auth, host } => write!(f, "http://{}", host),
+            ProxyScheme::Https { auth: _auth, host } => write!(f, "https://{}", host),
             #[cfg(feature = "socks")]
             ProxyScheme::Socks5 {
                 addr,
                 auth: _auth,
                 remote_dns,
             } => {
-                let h = if *remote_dns {
-                    "h"
-                } else {
-                    ""
-                };
+                let h = if *remote_dns { "h" } else { "" };
                 write!(f, "socks5{}://{}", h, addr)
             }
         }
@@ -543,9 +654,7 @@ pub(crate) trait Dst {
 #[doc(hidden)]
 impl Dst for Uri {
     fn scheme(&self) -> &str {
-        self.scheme()
-            .expect("Uri should have a scheme")
-            .as_str()
+        self.scheme().expect("Uri should have a scheme").as_str()
     }
 
     fn host(&self) -> &str {
@@ -649,11 +758,7 @@ fn get_from_registry_impl() -> Result<SystemProxyMap, Box<dyn Error>> {
             let protocol_parts: Vec<&str> = p.split("=").collect();
             match protocol_parts.as_slice() {
                 [protocol, address] => {
-                    insert_proxy(
-                        &mut proxies,
-                        *protocol,
-                        String::from(*address),
-                    );
+                    insert_proxy(&mut proxies, *protocol, String::from(*address));
                 }
                 _ => {
                     // Contains invalid protocol setting, just break the loop
@@ -668,16 +773,8 @@ fn get_from_registry_impl() -> Result<SystemProxyMap, Box<dyn Error>> {
         if proxy_server.starts_with("http:") {
             insert_proxy(&mut proxies, "http", proxy_server);
         } else {
-            insert_proxy(
-                &mut proxies,
-                "http",
-                format!("http://{}", proxy_server),
-            );
-            insert_proxy(
-                &mut proxies,
-                "https",
-                format!("https://{}", proxy_server),
-            );
+            insert_proxy(&mut proxies, "http", format!("http://{}", proxy_server));
+            insert_proxy(&mut proxies, "https", format!("https://{}", proxy_server));
         }
     }
     Ok(proxies)
@@ -691,8 +788,8 @@ fn get_from_registry() -> SystemProxyMap {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use std::sync::Mutex;
     use lazy_static::lazy_static;
+    use std::sync::Mutex;
 
     impl Dst for Url {
         fn scheme(&self) -> &str {
@@ -796,7 +893,7 @@ mod tests {
             ProxyScheme::Http { auth, host } => {
                 assert_eq!(auth.unwrap(), encode_basic_auth("foo", "bar"));
                 assert_eq!(host, "localhost:1239");
-            },
+            }
             other => panic!("unexpected: {:?}", other),
         }
     }
@@ -805,9 +902,7 @@ mod tests {
     struct MutexInner;
 
     lazy_static! {
-        static ref ENVLOCK: Mutex<MutexInner> = {
-            Mutex::new(MutexInner)
-        };
+        static ref ENVLOCK: Mutex<MutexInner> = Mutex::new(MutexInner);
     }
 
     #[test]
@@ -872,6 +967,96 @@ mod tests {
         assert!(!cgi_proxies.contains_key("http"));
     }
 
+    #[test]
+    fn test_sys_no_proxy() {
+        // Stop other threads from modifying process-global ENV while we are.
+        let _lock = ENVLOCK.lock();
+        // save system setting first.
+        let _g1 = env_guard("HTTP_PROXY");
+        let _g2 = env_guard("NO_PROXY");
+
+        let target = "http://example.domain/";
+        env::set_var("HTTP_PROXY", target);
+
+        env::set_var(
+            "NO_PROXY",
+            ".foo.bar,bar.baz,10.42.1.1/24,::1,10.124.7.8,2001::/17",
+        );
+
+        // Manually construct this so we aren't use the cache
+        let mut p = Proxy::new(Intercept::System(Arc::new(get_sys_proxies())));
+        p.no_proxy = NoProxy::new();
+
+        assert_eq!(intercepted_uri(&p, "http://hyper.rs"), target);
+        assert_eq!(intercepted_uri(&p, "http://foo.bar.baz"), target);
+        assert_eq!(intercepted_uri(&p, "http://10.43.1.1"), target);
+        assert_eq!(intercepted_uri(&p, "http://10.124.7.7"), target);
+        assert_eq!(intercepted_uri(&p, "http://[ffff:db8:a0b:12f0::1]"), target);
+        assert_eq!(intercepted_uri(&p, "http://[2005:db8:a0b:12f0::1]"), target);
+
+        assert!(p.intercept(&url("http://hello.foo.bar")).is_none());
+        assert!(p.intercept(&url("http://bar.baz")).is_none());
+        assert!(p.intercept(&url("http://10.42.1.100")).is_none());
+        assert!(p.intercept(&url("http://[::1]")).is_none());
+        assert!(p.intercept(&url("http://[2001:db8:a0b:12f0::1]")).is_none());
+        assert!(p.intercept(&url("http://10.124.7.8")).is_none());
+
+        // reset user setting when guards drop
+        drop(_g1);
+        drop(_g2);
+        // Let other threads run now
+        drop(_lock);
+    }
+
+    #[test]
+    fn test_no_proxy_load() {
+        // Stop other threads from modifying process-global ENV while we are.
+        let _lock = ENVLOCK.lock();
+
+        let _g1 = env_guard("no_proxy");
+        let domain = "lower.case";
+        env::set_var("no_proxy", domain);
+        // Manually construct this so we aren't use the cache
+        let mut p = Proxy::new(Intercept::System(Arc::new(get_sys_proxies())));
+        p.no_proxy = NoProxy::new();
+        assert_eq!(
+            p.no_proxy.expect("should have a no proxy set").domains.0[0],
+            domain
+        );
+
+        env::remove_var("no_proxy");
+        let _g2 = env_guard("NO_PROXY");
+        let domain = "upper.case";
+        env::set_var("NO_PROXY", domain);
+        // Manually construct this so we aren't use the cache
+        let mut p = Proxy::new(Intercept::System(Arc::new(get_sys_proxies())));
+        p.no_proxy = NoProxy::new();
+        assert_eq!(
+            p.no_proxy.expect("should have a no proxy set").domains.0[0],
+            domain
+        );
+
+        let _g3 = env_guard("HTTP_PROXY");
+        env::remove_var("NO_PROXY");
+        env::remove_var("no_proxy");
+        let target = "http://example.domain/";
+        env::set_var("HTTP_PROXY", target);
+
+        // Manually construct this so we aren't use the cache
+        let mut p = Proxy::new(Intercept::System(Arc::new(get_sys_proxies())));
+        p.no_proxy = NoProxy::new();
+        assert!(p.no_proxy.is_none(), "NoProxy shouldn't have been created");
+
+        assert_eq!(intercepted_uri(&p, "http://hyper.rs"), target);
+
+        // reset user setting when guards drop
+        drop(_g1);
+        drop(_g2);
+        drop(_g3);
+        // Let other threads run now
+        drop(_lock);
+    }
+
     /// Guard an environment variable, resetting it to the original value
     /// when dropped.
     fn env_guard(name: impl Into<String>) -> EnvGuard {


Test patch: