fix: proxy could use wrong credentials if many matched (#3098)

Closes #3097

Author: seanmonstar

src/async_impl/client.rs

@@ -2714,8 +2714,11 @@ impl Client {
         }
 
         for proxy in self.inner.proxies.iter() {
-            if let Some(header) = proxy.http_non_tunnel_basic_auth(dst) {
-                headers.insert(PROXY_AUTHORIZATION, header);
+            if let Some(proxy) = proxy.intercept(dst) {
+                if let Some(header) = proxy.http_non_tunnel_basic_auth() {
+                    headers.insert(PROXY_AUTHORIZATION, header);
+                }
+                // Use only the first matching proxy, as the connector does.
                 break;
             }
         }
@@ -2731,10 +2734,13 @@ impl Client {
         }
 
         for proxy in self.inner.proxies.iter() {
-            if let Some(iter) = proxy.http_non_tunnel_custom_headers(dst) {
-                iter.iter().for_each(|(key, value)| {
-                    headers.insert(key, value.clone());
-                });
+            if let Some(proxy) = proxy.intercept(dst) {
+                if let Some(iter) = proxy.http_non_tunnel_custom_headers() {
+                    iter.iter().for_each(|(key, value)| {
+                        headers.insert(key, value.clone());
+                    });
+                }
+                // Use only the first matching proxy, as the connector does.
                 break;
             }
         }

src/proxy.rs

@@ -550,31 +550,9 @@ impl Matcher {
         self.maybe_has_http_auth
     }
 
-    pub(crate) fn http_non_tunnel_basic_auth(&self, dst: &Uri) -> Option<HeaderValue> {
-        if let Some(proxy) = self.intercept(dst) {
-            let scheme = proxy.uri().scheme();
-            if scheme == Some(&Scheme::HTTP) || scheme == Some(&Scheme::HTTPS) {
-                return proxy.basic_auth().cloned();
-            }
-        }
-
-        None
-    }
-
     pub(crate) fn maybe_has_http_custom_headers(&self) -> bool {
         self.maybe_has_http_custom_headers
     }
-
-    pub(crate) fn http_non_tunnel_custom_headers(&self, dst: &Uri) -> Option<HeaderMap> {
-        if let Some(proxy) = self.intercept(dst) {
-            let scheme = proxy.uri().scheme();
-            if scheme == Some(&Scheme::HTTP) || scheme == Some(&Scheme::HTTPS) {
-                return proxy.custom_headers().cloned();
-            }
-        }
-
-        None
-    }
 }
 
 impl fmt::Debug for Matcher {
@@ -587,6 +565,24 @@ impl fmt::Debug for Matcher {
 }
 
 impl Intercepted {
+    pub(crate) fn http_non_tunnel_basic_auth(&self) -> Option<HeaderValue> {
+        let scheme = self.uri().scheme();
+        if scheme == Some(&Scheme::HTTP) || scheme == Some(&Scheme::HTTPS) {
+            return self.basic_auth().cloned();
+        }
+
+        None
+    }
+
+    pub(crate) fn http_non_tunnel_custom_headers(&self) -> Option<HeaderMap> {
+        let scheme = self.uri().scheme();
+        if scheme == Some(&Scheme::HTTP) || scheme == Some(&Scheme::HTTPS) {
+            return self.custom_headers().cloned();
+        }
+
+        None
+    }
+
     pub(crate) fn uri(&self) -> &http::Uri {
         self.inner.uri()
     }

tests/proxy.rs

@@ -71,6 +71,48 @@ async fn http_proxy_basic_auth() {
     assert_eq!(res.status(), reqwest::StatusCode::OK);
 }
 
+#[tokio::test]
+async fn http_proxy_does_not_use_later_proxy_basic_auth() {
+    http_proxy_does_not_use_later_proxy_headers(false).await;
+}
+
+#[tokio::test]
+async fn http_proxy_does_not_use_later_proxy_custom_headers() {
+    http_proxy_does_not_use_later_proxy_headers(true).await;
+}
+
+async fn http_proxy_does_not_use_later_proxy_headers(custom: bool) {
+    let url = "http://hyper.rs.local/prox";
+    let server = server::http(move |req| {
+        assert!(!req.headers().contains_key("proxy-authorization"));
+        assert!(!req.headers().contains_key("x-proxy-secret"));
+
+        async { http::Response::default() }
+    });
+
+    let later_proxy = reqwest::Proxy::http("http://unused.proxy.local").unwrap();
+    let later_proxy = if custom {
+        let mut headers = reqwest::header::HeaderMap::new();
+        headers.insert("proxy-authorization", "secret".parse().unwrap());
+        headers.insert("x-proxy-secret", "secret".parse().unwrap());
+        later_proxy.headers(headers)
+    } else {
+        later_proxy.basic_auth("Aladdin", "open sesame")
+    };
+
+    let res = reqwest::Client::builder()
+        .proxy(reqwest::Proxy::http(format!("http://{}", server.addr())).unwrap())
+        .proxy(later_proxy)
+        .build()
+        .unwrap()
+        .get(url)
+        .send()
+        .await
+        .unwrap();
+
+    assert_eq!(res.status(), reqwest::StatusCode::OK);
+}
+
 #[tokio::test]
 async fn http_proxy_basic_auth_parsed() {
     let url = "http://hyper.rs.local/prox";