From 5b745a82e1639cab7f348aa60d5f2ac3442b4fa7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E5=B0=8F=E7=99=BD?= <364772080@qq.com> Date: Sat, 19 Oct 2024 00:57:00 +0800 Subject: [PATCH] reqwest_client: Fix `socks` proxy settings (#19123) Closes #19362 This pull request includes several updates to the `reqwest_client` crate and its dependencies. The most important changes involve adding support for SOCKS proxies, improving error handling for proxy URIs, and adding tests for proxy functionality. ### Dependency Updates: * [`Cargo.toml`](diffhunk://#diff-2e9d962a08321605940b5a657135052fbcef87b5e360662bb527c96d9a615542L394-R401): Added support for SOCKS proxies in the `reqwest` dependency by including the `socks` feature. ### Code Improvements: * [`crates/reqwest_client/src/reqwest_client.rs`](diffhunk://#diff-8e036b034e987390be2f57373864b75d6983f0cf84e85c43793eb431d13538f3L47-R52): Improved error handling when parsing proxy URIs by logging errors instead of directly panicking. ### Testing Enhancements: * [`crates/reqwest_client/src/reqwest_client.rs`](diffhunk://#diff-8e036b034e987390be2f57373864b75d6983f0cf84e85c43793eb431d13538f3R274-R317): Added tests to verify the handling of various proxy URIs, including valid and invalid cases. Release Notes: - N/A --- Cargo.lock | 2 + Cargo.toml | 9 +++- crates/reqwest_client/src/reqwest_client.rs | 52 ++++++++++++++++++++- 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 88cf3d2c42..77e2a2760c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9303,6 +9303,7 @@ dependencies = [ "system-configuration 0.6.1", "tokio", "tokio-rustls 0.26.0", + "tokio-socks", "tokio-util", "tower-service", "url", @@ -12001,6 +12002,7 @@ dependencies = [ "futures-io", "futures-util", "thiserror", + "tokio", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 4a4ddb4424..b38a046d3d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -391,7 +391,14 @@ pulldown-cmark = { version = "0.12.0", default-features = false } rand = "0.8.5" regex = "1.5" repair_json = "0.1.0" -reqwest = { git = "https://github.com/zed-industries/reqwest.git", rev = "fd110f6998da16bbca97b6dddda9be7827c50e29", default-features = false, features = ["charset", "http2", "macos-system-configuration", "rustls-tls-native-roots", "stream"]} +reqwest = { git = "https://github.com/zed-industries/reqwest.git", rev = "fd110f6998da16bbca97b6dddda9be7827c50e29", default-features = false, features = [ + "charset", + "http2", + "macos-system-configuration", + "rustls-tls-native-roots", + "socks", + "stream", +] } rsa = "0.9.6" runtimelib = { version = "0.15", default-features = false, features = [ "async-dispatcher-runtime", diff --git a/crates/reqwest_client/src/reqwest_client.rs b/crates/reqwest_client/src/reqwest_client.rs index ade115bcd0..08054a0128 100644 --- a/crates/reqwest_client/src/reqwest_client.rs +++ b/crates/reqwest_client/src/reqwest_client.rs @@ -44,8 +44,12 @@ impl ReqwestClient { let mut client = reqwest::Client::builder() .use_rustls_tls() .default_headers(map); - if let Some(proxy) = proxy.clone() { - client = client.proxy(reqwest::Proxy::all(proxy.to_string())?); + if let Some(proxy) = proxy.clone().and_then(|proxy_uri| { + reqwest::Proxy::all(proxy_uri.to_string()) + .inspect_err(|e| log::error!("Failed to parse proxy URI {}: {}", proxy_uri, e)) + .ok() + }) { + client = client.proxy(proxy); } let client = client.build()?; let mut client: ReqwestClient = client.into(); @@ -232,3 +236,47 @@ impl http_client::HttpClient for ReqwestClient { .boxed() } } + +#[cfg(test)] +mod tests { + use http_client::{http, HttpClient}; + + use crate::ReqwestClient; + + #[test] + fn test_proxy_uri() { + let client = ReqwestClient::new(); + assert_eq!(client.proxy(), None); + + let proxy = http::Uri::from_static("http://localhost:10809"); + let client = ReqwestClient::proxy_and_user_agent(Some(proxy.clone()), "test").unwrap(); + assert_eq!(client.proxy(), Some(&proxy)); + + let proxy = http::Uri::from_static("https://localhost:10809"); + let client = ReqwestClient::proxy_and_user_agent(Some(proxy.clone()), "test").unwrap(); + assert_eq!(client.proxy(), Some(&proxy)); + + let proxy = http::Uri::from_static("socks4://localhost:10808"); + let client = ReqwestClient::proxy_and_user_agent(Some(proxy.clone()), "test").unwrap(); + assert_eq!(client.proxy(), Some(&proxy)); + + let proxy = http::Uri::from_static("socks4a://localhost:10808"); + let client = ReqwestClient::proxy_and_user_agent(Some(proxy.clone()), "test").unwrap(); + assert_eq!(client.proxy(), Some(&proxy)); + + let proxy = http::Uri::from_static("socks5://localhost:10808"); + let client = ReqwestClient::proxy_and_user_agent(Some(proxy.clone()), "test").unwrap(); + assert_eq!(client.proxy(), Some(&proxy)); + + let proxy = http::Uri::from_static("socks5h://localhost:10808"); + let client = ReqwestClient::proxy_and_user_agent(Some(proxy.clone()), "test").unwrap(); + assert_eq!(client.proxy(), Some(&proxy)); + } + + #[test] + #[should_panic] + fn test_invalid_proxy_uri() { + let proxy = http::Uri::from_static("file:///etc/hosts"); + ReqwestClient::proxy_and_user_agent(Some(proxy), "test").unwrap(); + } +}