support http proxy addresses with no scheme

Other implementations (curl and Go) accept HTTPS_PROXY values with no
protocol scheme. When the scheme is not present, http:// is assumed.
For example 192.168.1.1 is interpreted as http://192.168.1.1

This commit adds support for http proxy addresses without a scheme by
retrying the URL parsing mechanisms with http:// prepended.

src/into_url.rs

@@ -13,6 +13,8 @@ pub trait PolyfillTryInto {
     // Besides parsing as a valid `Url`, the `Url` must be a valid
     // `http::Uri`, in that it makes sense to use in a network request.
     fn into_url(self) -> crate::Result<Url>;
+
+    fn _as_str(&self) -> &str;
 }
 
 impl PolyfillTryInto for Url {
@@ -23,18 +25,30 @@ impl PolyfillTryInto for Url {
             Err(crate::error::url_bad_scheme(self))
         }
     }
+
+    fn _as_str(&self) -> &str {
+        self.as_ref()
+    }
 }
 
 impl<'a> PolyfillTryInto for &'a str {
     fn into_url(self) -> crate::Result<Url> {
         Url::parse(self).map_err(crate::error::builder)?.into_url()
     }
+
+    fn _as_str(&self) -> &str {
+        self.as_ref()
+    }
 }
 
 impl<'a> PolyfillTryInto for &'a String {
     fn into_url(self) -> crate::Result<Url> {
         (&**self).into_url()
     }
+
+    fn _as_str(&self) -> &str {
+        self.as_ref()
+    }
 }
 
 if_hyper! {

src/proxy.rs

@@ -3,7 +3,8 @@ use std::fmt;
 use std::net::SocketAddr;
 use std::sync::Arc;
 
-use crate::{IntoUrl, Url};
+use crate::into_url::{IntoUrl, PolyfillTryInto};
+use crate::Url;
 use http::{header::HeaderValue, Uri};
 use ipnet::IpNet;
 use percent_encoding::percent_decode;
@@ -107,9 +108,33 @@ pub trait IntoProxyScheme {
     fn into_proxy_scheme(self) -> crate::Result<ProxyScheme>;
 }
 
-impl<T: IntoUrl> IntoProxyScheme for T {
+impl<S: IntoUrl> IntoProxyScheme for S {
     fn into_proxy_scheme(self) -> crate::Result<ProxyScheme> {
-        ProxyScheme::parse(self.into_url()?)
+        // validate the URL
+        let url = match self._as_str().into_url() {
+            Ok(ok) => ok,
+            Err(e) => {
+                // the issue could have been caused by a missing scheme, so we try adding http://
+                format!("http://{}", self._as_str())
+                    .into_url()
+                    .map_err(|_| {
+                        // return the original error
+                        crate::error::builder(e)
+                    })?
+            }
+        };
+        ProxyScheme::parse(url)
+    }
+}
+
+// These bounds are accidentally leaked by the blanket impl of IntoProxyScheme
+// for all types that implement IntoUrl. So, this function exists to detect
+// if we were to break those bounds for a user.
+fn _implied_bounds() {
+    fn prox<T: IntoProxyScheme>(_t: T) {}
+
+    fn url<T: IntoUrl>(t: T) {
+        prox(t);
     }
 }
 
@@ -842,8 +867,7 @@ fn extract_type_prefix(address: &str) -> Option<&str> {
     if let Some(indice) = address.find("://") {
         if indice == 0 {
             None
-        }
-        else {
+        } else {
             let prefix = &address[..indice];
             let contains_banned = prefix.contains(|c| c == ':' || c == '/');
 
@@ -853,8 +877,7 @@ fn extract_type_prefix(address: &str) -> Option<&str> {
                 None
             }
         }
-    }
-    else {
+    } else {
         None
     }
 }
@@ -977,6 +1000,33 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_proxy_scheme_ip_address_default_http() {
+        let ps = "192.168.1.1:8888".into_proxy_scheme().unwrap();
+
+        match ps {
+            ProxyScheme::Http { auth, host } => {
+                assert!(auth.is_none());
+                assert_eq!(host, "192.168.1.1:8888");
+            }
+            other => panic!("unexpected: {:?}", other),
+        }
+    }
+
+    #[test]
+    fn test_proxy_scheme_parse_default_http_with_auth() {
+        // this should fail because `foo` is interpreted as the scheme and no host can be found
+        let ps = "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),
+        }
+    }
+
     // Smallest possible content for a mutex
     struct MutexInner;
 
@@ -996,10 +1046,10 @@ mod tests {
         // to avoid assert! -> panic! -> Mutex Poisoned.
         let baseline_proxies = get_sys_proxies(None);
         // the system proxy setting url is invalid.
-        env::set_var("http_proxy", "123465");
+        env::set_var("http_proxy", "file://123465");
         let invalid_proxies = get_sys_proxies(None);
         // set valid proxy
-        env::set_var("http_proxy", "http://127.0.0.1/");
+        env::set_var("http_proxy", "127.0.0.1/");
         let valid_proxies = get_sys_proxies(None);
 
         // reset user setting when guards drop
@@ -1033,9 +1083,16 @@ mod tests {
         // set valid proxy
         let valid_proxies = get_sys_proxies(Some((1, String::from("http://127.0.0.1/"))));
         let valid_proxies_no_schema = get_sys_proxies(Some((1, String::from("127.0.0.1"))));
-        let valid_proxies_explicit_https = get_sys_proxies(Some((1, String::from("https://127.0.0.1/"))));
-        let multiple_proxies = get_sys_proxies(Some((1, String::from("http=127.0.0.1:8888;https=127.0.0.2:8888"))));
-        let multiple_proxies_explicit_schema = get_sys_proxies(Some((1, String::from("http=http://127.0.0.1:8888;https=https://127.0.0.2:8888"))));
+        let valid_proxies_explicit_https =
+            get_sys_proxies(Some((1, String::from("https://127.0.0.1/"))));
+        let multiple_proxies = get_sys_proxies(Some((
+            1,
+            String::from("http=127.0.0.1:8888;https=127.0.0.2:8888"),
+        )));
+        let multiple_proxies_explicit_schema = get_sys_proxies(Some((
+            1,
+            String::from("http=http://127.0.0.1:8888;https=https://127.0.0.2:8888"),
+        )));
 
         // reset user setting when guards drop
         drop(_g1);