Remove username and password when parsing proxies (#686)

src/connect.rs

@@ -1,5 +1,5 @@
 use futures_util::FutureExt;
-use http::uri::Scheme;
+use http::uri::{Scheme, Authority};
 use hyper::client::connect::{Connect, Connected, Destination};
 use tokio::io::{AsyncRead, AsyncWrite};
 
@@ -255,22 +255,13 @@ impl Connector {
     ) -> Result<(Conn, Connected), io::Error> {
         log::trace!("proxy({:?}) intercepts {:?}", proxy_scheme, dst);
 
-        let (puri, _auth) = match proxy_scheme {
+        let (proxy_dst, _auth) = match proxy_scheme {
-            ProxyScheme::Http { uri, auth, .. } => (uri, auth),
+            ProxyScheme::Http { host, auth } => (into_dst(Scheme::HTTP, host), auth),
+            ProxyScheme::Https { host, auth } => (into_dst(Scheme::HTTPS, host), auth),
             #[cfg(feature = "socks")]
             ProxyScheme::Socks5 { .. } => return this.connect_socks(dst, proxy_scheme),
         };
 
-        let mut ndst = dst.clone();
-
-        let new_scheme = puri.scheme_part().map(Scheme::as_str).unwrap_or("http");
-        ndst.set_scheme(new_scheme)
-            .expect("proxy target scheme should be valid");
-
-        ndst.set_host(puri.host().expect("proxy target should have host"))
-            .expect("proxy target host should be valid");
-
-        ndst.set_port(puri.port_part().map(|port| port.as_u16()));
 
         #[cfg(feature = "tls")]
         let auth = _auth;
@@ -285,7 +276,7 @@ impl Connector {
                     http.set_nodelay(self.nodelay);
                     let tls_connector = tokio_tls::TlsConnector::from(tls.clone());
                     let http = hyper_tls::HttpsConnector::from((http, tls_connector));
-                    let (conn, connected) = http.connect(ndst).await?;
+                    let (conn, connected) = http.connect(proxy_dst).await?;
                     log::trace!("tunneling HTTPS over proxy");
                     let tunneled = tunnel(conn, host.clone(), port, self.user_agent.clone(), auth).await?;
                     let tls_connector = tokio_tls::TlsConnector::from(tls.clone());
@@ -313,7 +304,7 @@ impl Connector {
                     http.set_nodelay(no_delay);
                     let http = hyper_rustls::HttpsConnector::from((http, tls_proxy.clone()));
                     let tls = tls.clone();
-                    let (conn, connected) = http.connect(ndst).await?;
+                    let (conn, connected) = http.connect(proxy_dst).await?;
                     log::trace!("tunneling HTTPS over proxy");
                     let maybe_dnsname = DNSNameRef::try_from_ascii_str(&host)
                         .map(|dnsname| dnsname.to_owned())
@@ -336,10 +327,24 @@ impl Connector {
             Inner::Http(_) => (),
         }
 
-        self.connect_with_maybe_proxy(ndst, true).await
+        self.connect_with_maybe_proxy(proxy_dst, true).await
     }
 }
 
+fn into_dst(scheme: Scheme, host: Authority) -> Destination {
+    use std::convert::TryInto;
+
+    // TODO: Should the `http` crate get `From<(Scheme, Authority)> for Uri`?
+    http::Uri::builder()
+        .scheme(scheme)
+        .authority(host)
+        .path_and_query(http::uri::PathAndQuery::from_static("/"))
+        .build()
+        .expect("scheme and authority is valid Uri")
+        .try_into()
+        .expect("scheme and authority is valid Destination")
+}
+
 //#[cfg(feature = "trust-dns")]
 //fn http_connector() -> crate::Result<HttpConnector> {
 //    TrustDnsResolver::new()

src/proxy.rs

@@ -60,7 +60,11 @@ pub struct Proxy {
 pub enum ProxyScheme {
     Http {
         auth: Option<HeaderValue>,
-        uri: hyper::Uri,
+        host: http::uri::Authority,
+    },
+    Https {
+        auth: Option<HeaderValue>,
+        host: http::uri::Authority,
     },
     #[cfg(feature = "socks")]
     Socks5 {
@@ -226,6 +230,7 @@ impl Proxy {
             | Intercept::Http(ProxyScheme::Http { ref auth, .. }) => auth.clone(),
             Intercept::Custom(ref custom) => custom.call(uri).and_then(|scheme| match scheme {
                 ProxyScheme::Http { auth, .. } => auth,
+                ProxyScheme::Https { auth, .. } => auth,
                 #[cfg(feature = "socks")]
                 _ => None,
             }),
@@ -278,10 +283,18 @@ impl ProxyScheme {
     // To start conservative, keep builders private for now.
 
     /// Proxy traffic via the specified URL over HTTP
-    fn http<T: IntoUrl>(url: T) -> crate::Result<Self> {
+    fn http(host: &str) -> crate::Result<Self> {
         Ok(ProxyScheme::Http {
             auth: None,
-            uri: crate::into_url::expect_uri(&url.into_url()?),
+            host: host.parse().map_err(crate::error::builder)?,
+        })
+    }
+
+    /// Proxy traffic via the specified URL over HTTPS
+    fn https(host: &str) -> crate::Result<Self> {
+        Ok(ProxyScheme::Https {
+            auth: None,
+            host: host.parse().map_err(crate::error::builder)?,
         })
     }
 
@@ -330,7 +343,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()));
@@ -338,11 +355,32 @@ impl ProxyScheme {
         }
     }
 
+    fn if_no_auth(mut self, update: &Option<HeaderValue>) -> Self {
+        match self {
+            ProxyScheme::Http { ref mut auth, .. } => {
+                if auth.is_none() {
+                    *auth = update.clone();
+                }
+            },
+            ProxyScheme::Https { ref mut auth, .. } => {
+                if auth.is_none() {
+                    *auth = update.clone();
+                }
+            },
+            #[cfg(feature = "socks")]
+            ProxyScheme::Socks5 { .. } => {}
+        }
+
+        self
+    }
+
     /// Convert a URL into a proxy scheme
     ///
     /// Supported schemes: HTTP, HTTPS, (SOCKS5, SOCKS5H if `socks` feature is enabled).
     // Private for now...
     fn parse(url: Url) -> crate::Result<Self> {
+        use url::Position;
+
         // Resolve URL to a host and port
         #[cfg(feature = "socks")]
         let to_addr = || {
@@ -357,7 +395,8 @@ impl ProxyScheme {
         };
 
         let mut scheme = match url.scheme() {
-            "http" | "https" => Self::http(url.clone())?,
+            "http" => Self::http(&url[Position::BeforeHost..Position::AfterPort])?,
+            "https" => Self::https(&url[Position::BeforeHost..Position::AfterPort])?,
             #[cfg(feature = "socks")]
             "socks5" => Self::socks5(to_addr()?)?,
             #[cfg(feature = "socks")]
@@ -375,13 +414,22 @@ impl ProxyScheme {
     }
 
     #[cfg(test)]
-    fn http_uri(&self) -> &http::Uri {
+    fn scheme(&self) -> &str {
         match self {
-            ProxyScheme::Http { ref uri, .. } => {
+            ProxyScheme::Http { .. } => "http",
-                uri
+            ProxyScheme::Https { .. } => "https",
-            },
             #[cfg(feature = "socks")]
-            _ => panic!("socks not expected"),
+            ProxyScheme::Socks5 => "socks5",
+        }
+    }
+
+    #[cfg(test)]
+    fn host(&self) -> &str {
+        match self {
+            ProxyScheme::Http { host, .. } => host.as_str(),
+            ProxyScheme::Https { host, .. } => host.as_str(),
+            #[cfg(feature = "socks")]
+            ProxyScheme::Socks5 => panic!("socks5"),
         }
     }
 }
@@ -433,20 +481,7 @@ impl Custom {
 
         (self.func)(&url)
             .and_then(|result| result.ok())
-            .map(|scheme| match scheme {
+            .map(|scheme| scheme.if_no_auth(&self.auth))
-                ProxyScheme::Http { auth, uri } => {
-                    if auth.is_some() {
-                        ProxyScheme::Http { auth, uri }
-                    } else {
-                        ProxyScheme::Http {
-                            auth: self.auth.clone(),
-                            uri,
-                        }
-                    }
-                }
-                #[cfg(feature = "socks")]
-                socks => socks,
-            })
     }
 }
 
@@ -659,11 +694,18 @@ mod tests {
     }
 
     fn intercepted_uri(p: &Proxy, s: &str) -> Uri {
-        match p.intercept(&url(s)).unwrap() {
+        let (scheme, host) = match p.intercept(&url(s)).unwrap() {
-            ProxyScheme::Http { uri, .. } => uri,
+            ProxyScheme::Http { host, .. } => ("http", host),
+            ProxyScheme::Https { host, .. } => ("https", host),
             #[cfg(feature = "socks")]
             _ => panic!("intercepted as socks"),
-        }
+        };
+        http::Uri::builder()
+            .scheme(scheme)
+            .authority(host)
+            .path_and_query("/")
+            .build()
+            .expect("intercepted_uri")
     }
 
     #[test]
@@ -727,6 +769,19 @@ mod tests {
         assert!(p.intercept(&url(other)).is_none());
     }
 
+    #[test]
+    fn test_proxy_scheme_parse() {
+        let ps = "http://foo:bar@localhost:1239".into_proxy_scheme().unwrap();
+
+        match ps {
+            ProxyScheme::Http { auth, host } => {
+                assert_eq!(auth.unwrap(), encode_basic_auth("foo", "bar"));
+                assert_eq!(host, "localhost:1239");
+            },
+            other => panic!("unexpected: {:?}", other),
+        }
+    }
+
     #[test]
     fn test_get_sys_proxies_parsing() {
         // save system setting first.
@@ -744,7 +799,9 @@ mod tests {
         env::set_var("http_proxy", "http://127.0.0.1/");
         let proxies = get_sys_proxies();
 
-        assert_eq!(proxies["http"].http_uri(), "http://127.0.0.1/");
+        let p = &proxies["http"];
+        assert_eq!(p.scheme(), "http");
+        assert_eq!(p.host(), "127.0.0.1");
 
         // reset user setting when guards drop
     }
@@ -759,7 +816,7 @@ mod tests {
         env::set_var("HTTP_PROXY", "http://evil/");
 
         // not in CGI yet
-        assert_eq!(get_sys_proxies()["http"].http_uri(), "http://evil/");
+        assert_eq!(get_sys_proxies()["http"].host(), "evil");
 
         // set like we're in CGI
         env::set_var("REQUEST_METHOD", "GET");