fix(proxy): restore default port 1080 for SOCKS proxies without explicit port (#821)

Author: 0x676e67

src/core/client/proxy/matcher.rs

@@ -15,7 +15,7 @@
 use std::{fmt, net::IpAddr};
 
 use bytes::Bytes;
-use http::header::HeaderValue;
+use http::{header::HeaderValue, uri::Authority};
 use ipnet::IpNet;
 use percent_encoding::percent_decode_str;
 
@@ -328,14 +328,16 @@ fn parse_env_uri(val: &str) -> Option<Intercept> {
     let uri = val.parse::<http::Uri>().ok()?;
     let mut builder = http::Uri::builder();
     let mut is_httpish = false;
+    let mut is_socks = false;
     let mut auth = Auth::Empty;
 
     builder = builder.scheme(match uri.scheme() {
         Some(s) => {
             if s == &http::uri::Scheme::HTTP || s == &http::uri::Scheme::HTTPS {
                 is_httpish = true;
                 s.clone()
-            } else if s.as_str() == "socks5" || s.as_str() == "socks5h" {
+            } else if matches!(s.as_str(), "socks4" | "socks4a" | "socks5" | "socks5h") {
+                is_socks = true;
                 s.clone()
             } else {
                 // can't use this proxy scheme
@@ -349,7 +351,13 @@ fn parse_env_uri(val: &str) -> Option<Intercept> {
         }
     });
 
+    // If the scheme is a SOCKS protocol and no port is specified, set the default port to 1080.
     let authority = uri.authority()?;
+    let authority = if is_socks && authority.port().is_none() {
+        format!("{authority}:1080").parse::<Authority>().ok()?
+    } else {
+        authority.clone()
+    };
 
     if let Some((userinfo, host_port)) = authority.as_str().split_once('@') {
         let (user, pass) = userinfo.split_once(':')?;
@@ -365,15 +373,15 @@ fn parse_env_uri(val: &str) -> Option<Intercept> {
         }
         builder = builder.authority(host_port);
     } else {
-        builder = builder.authority(authority.clone());
+        builder = builder.authority(authority);
     }
 
     // removing any path, but we MUST specify one or the builder errors
     builder = builder.path_and_query("/");
 
-    let dst = builder.build().ok()?;
+    let uri = builder.build().ok()?;
 
-    Some(Intercept { uri: dst, auth })
+    Some(Intercept { uri, auth })
 }
 
 fn encode_basic_auth(user: &str, pass: Option<&str>) -> HeaderValue {
@@ -838,4 +846,25 @@ mod tests {
 
         assert!(m.intercept(&"http://rick.roll".parse().unwrap()).is_none());
     }
+
+    fn test_parse_socks(url: &str) {
+        let p = p! {
+            all = url,
+        };
+
+        let proxy = intercept(&p, "https://example.local");
+        assert_eq!(proxy.uri(), url);
+    }
+
+    #[test]
+    fn test_parse_socks4() {
+        test_parse_socks("socks4://localhost:8887");
+        test_parse_socks("socks4a://localhost:8887");
+    }
+
+    #[test]
+    fn test_parse_socks5() {
+        test_parse_socks("socks5://localhost:8887");
+        test_parse_socks("socks5h://localhost:8887");
+    }
 }

src/proxy.rs

@@ -126,7 +126,7 @@ pub trait IntoProxy {
 impl<S: IntoUrl> IntoProxy for S {
     fn into_proxy(self) -> crate::Result<Url> {
         match self.as_str().into_url() {
-            Ok(ok) => Ok(ok),
+            Ok(url) => Ok(url),
             Err(e) => {
                 let mut presumed_to_have_scheme = true;
                 let mut source = e.source();
@@ -602,4 +602,32 @@ mod tests {
             .into_matcher();
         assert!(m.maybe_has_http_auth(), "http forwards");
     }
+
+    fn test_socks_proxy_default_port(url: &str, url2: &str, port: u16) {
+        let m = Proxy::all(url).unwrap().into_matcher();
+
+        let http = "http://hyper.rs";
+        let https = "https://hyper.rs";
+
+        assert_eq!(intercepted_uri(&m, http).port_u16(), Some(1080));
+        assert_eq!(intercepted_uri(&m, https).port_u16(), Some(1080));
+
+        // custom port
+        let m = Proxy::all(url2).unwrap().into_matcher();
+
+        assert_eq!(intercepted_uri(&m, http).port_u16(), Some(port));
+        assert_eq!(intercepted_uri(&m, https).port_u16(), Some(port));
+    }
+
+    #[test]
+    fn test_socks4_proxy_default_port() {
+        test_socks_proxy_default_port("socks4://example.com", "socks4://example.com:1234", 1234);
+        test_socks_proxy_default_port("socks4a://example.com", "socks4a://example.com:1234", 1234);
+    }
+
+    #[test]
+    fn test_socks5_proxy_default_port() {
+        test_socks_proxy_default_port("socks5://example.com", "socks5://example.com:1234", 1234);
+        test_socks_proxy_default_port("socks5h://example.com", "socks5h://example.com:1234", 1234);
+    }
 }