From fc34e100ededda3beb73af31782359b622b2cad2 Mon Sep 17 00:00:00 2001 From: Stefan Zwanenburg Date: Tue, 4 Mar 2025 08:45:22 +0100 Subject: [PATCH 1/5] Add --noproxy argument to ignore proxy for hosts or IP addresses. Related issue: #296 --- src/cli.rs | 26 ++++++++++++++++++++++++++ src/main.rs | 8 ++------ src/to_curl.rs | 4 ++++ tests/cli.rs | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 6 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 00b5b8e0..19ada23f 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -295,6 +295,14 @@ Example: --print=Hb" #[clap(long, value_name = "PROTOCOL:URL", number_of_values = 1)] pub proxy: Vec, + /// Comma-separated list of hosts for which not to use a proxy, if one is specified. + /// + /// - A '*' matches all hosts, and effectively disables proxies altogether. + /// - IP addresses are allowed, as are subnets (in CIDR notation, i.e.: '127.0.0.0/8'). + /// - Any other entry in the list is assumed to be a hostname. + #[clap(long, value_name = "no-proxy-list", value_delimiter = ',')] + pub noproxy: Vec, + /// If "no", skip SSL verification. If a file path, use it as a CA bundle. /// /// Specifying a CA bundle will disable the system's built-in root certificates. @@ -1087,6 +1095,24 @@ impl FromStr for Proxy { } } +impl Proxy { + pub fn into_reqwest_proxy(self, noproxy: &[String]) -> anyhow::Result { + let proxy = match self { + Proxy::Http(url) => reqwest::Proxy::http(url), + Proxy::Https(url) => reqwest::Proxy::https(url), + Proxy::All(url) => reqwest::Proxy::all(url), + }?; + + let mut noproxy_comma_delimited = noproxy.join(","); + if noproxy.contains(&"*".to_string()) { + // reqwest's NoProxy wildcard doesn't apply to IP addresses, while curl's does + noproxy_comma_delimited.push_str(",0.0.0.0/0,::/0"); + } + + Ok(proxy.no_proxy(reqwest::NoProxy::from_string(&noproxy_comma_delimited))) + } +} + #[derive(Debug, Clone)] pub struct Resolve { pub domain: String, diff --git a/src/main.rs b/src/main.rs index 6a1eac0f..b1c4afa4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -41,7 +41,7 @@ use utils::reason_phrase; use crate::auth::{Auth, DigestAuthMiddleware}; use crate::buffer::Buffer; -use crate::cli::{Cli, FormatOptions, HttpVersion, Print, Proxy, Verify}; +use crate::cli::{Cli, FormatOptions, HttpVersion, Print, Verify}; use crate::download::{download_file, get_file_size}; use crate::middleware::ClientWithMiddleware; use crate::printer::Printer; @@ -274,11 +274,7 @@ fn run(args: Cli) -> Result { } for proxy in args.proxy.into_iter().rev() { - client = client.proxy(match proxy { - Proxy::Http(url) => reqwest::Proxy::http(url), - Proxy::Https(url) => reqwest::Proxy::https(url), - Proxy::All(url) => reqwest::Proxy::all(url), - }?); + client = client.proxy(proxy.into_reqwest_proxy(&args.noproxy)?); } client = match args.http_version { diff --git a/src/to_curl.rs b/src/to_curl.rs index adeebf1f..6cf4d159 100644 --- a/src/to_curl.rs +++ b/src/to_curl.rs @@ -233,6 +233,10 @@ pub fn translate(args: Cli) -> Result { } } } + if !args.noproxy.is_empty() { + cmd.arg("--noproxy"); + cmd.arg(args.noproxy.join(",")); + } if let Some(timeout) = args.timeout.and_then(|t| t.as_duration()) { cmd.arg("--max-time"); cmd.arg(timeout.as_secs_f64().to_string()); diff --git a/tests/cli.rs b/tests/cli.rs index b3adf374..27b86c7a 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -1136,6 +1136,53 @@ fn proxy_multiple_valid_proxies() { cmd.assert().success(); } +fn noproxy_test(noproxy_arg: &str) { + let mut proxy_server = server::http(|_| async move { + hyper::Response::builder() + .status(200) + .body("Proxy shouldn't have been used.".into()) + .unwrap() + }); + let actual_server = server::http(|_| async move { + hyper::Response::builder() + .status(200) + .body("".into()) + .unwrap() + }); + + get_command() + .arg(format!("--proxy=http:{}", proxy_server.base_url())) + .arg(format!("--noproxy={}", noproxy_arg)) + .arg("GET") + .arg(actual_server.base_url().as_str()) + .assert() + .success(); + + proxy_server.disable_hit_checks(); + proxy_server.assert_hits(0); + actual_server.assert_hits(1); +} + +#[test] +fn noproxy_wildcard() { + noproxy_test("*"); +} + +#[test] +fn noproxy_ip() { + noproxy_test("127.0.0.1"); +} + +#[test] +fn noproxy_ip_cidr() { + noproxy_test("127.0.0.0/8"); +} + +#[test] +fn noproxy_multiple() { + noproxy_test("127.0.0.2,127.0.0.1"); +} + // temporarily disabled for builds not using rustls #[cfg(all(feature = "online-tests", feature = "rustls"))] #[ignore = "endpoint is randomly timing out"] From 51ca27d704e4113b7829ba0e1c9fda3831e06e51 Mon Sep 17 00:00:00 2001 From: Stefan Zwanenburg Date: Wed, 5 Mar 2025 13:51:51 +0100 Subject: [PATCH 2/5] --noproxy: Trim whitespace in arg values & add extra tests --- src/cli.rs | 27 +++++++++++++++++++++--- tests/cli.rs | 58 ++++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 73 insertions(+), 12 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 19ada23f..1b1a4eaa 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,3 +1,4 @@ +use std::borrow::Borrow; use std::convert::TryFrom; use std::env; use std::ffi::OsString; @@ -301,7 +302,7 @@ Example: --print=Hb" /// - IP addresses are allowed, as are subnets (in CIDR notation, i.e.: '127.0.0.0/8'). /// - Any other entry in the list is assumed to be a hostname. #[clap(long, value_name = "no-proxy-list", value_delimiter = ',')] - pub noproxy: Vec, + pub noproxy: Vec, /// If "no", skip SSL verification. If a file path, use it as a CA bundle. /// @@ -1096,7 +1097,7 @@ impl FromStr for Proxy { } impl Proxy { - pub fn into_reqwest_proxy(self, noproxy: &[String]) -> anyhow::Result { + pub fn into_reqwest_proxy(self, noproxy: &[NoProxy]) -> anyhow::Result { let proxy = match self { Proxy::Http(url) => reqwest::Proxy::http(url), Proxy::Https(url) => reqwest::Proxy::https(url), @@ -1104,7 +1105,7 @@ impl Proxy { }?; let mut noproxy_comma_delimited = noproxy.join(","); - if noproxy.contains(&"*".to_string()) { + if noproxy.contains(&"*".into()) { // reqwest's NoProxy wildcard doesn't apply to IP addresses, while curl's does noproxy_comma_delimited.push_str(",0.0.0.0/0,::/0"); } @@ -1113,6 +1114,21 @@ impl Proxy { } } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct NoProxy(String); + +impl From<&str> for NoProxy { + fn from(s: &str) -> Self { + Self(s.trim().to_string()) + } +} + +impl Borrow for NoProxy { + fn borrow(&self) -> &str { + &self.0 + } +} + #[derive(Debug, Clone)] pub struct Resolve { pub domain: String, @@ -1502,6 +1518,11 @@ mod tests { ); } + #[test] + fn noproxy_trims_whitespace() { + assert_eq!(NoProxy::from("*"), NoProxy::from(" * ")); + } + #[test] fn executable_name() { let args = Cli::try_parse_from(["xhs", "example.org"]).unwrap(); diff --git a/tests/cli.rs b/tests/cli.rs index 27b86c7a..6382e163 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -1136,14 +1136,18 @@ fn proxy_multiple_valid_proxies() { cmd.assert().success(); } -fn noproxy_test(noproxy_arg: &str) { +enum NoProxyTestType { + ProxyUsed, + ProxyNotUsed, +} +fn noproxy_test(noproxy_arg: &str, test_type: NoProxyTestType) { let mut proxy_server = server::http(|_| async move { hyper::Response::builder() .status(200) .body("Proxy shouldn't have been used.".into()) .unwrap() }); - let actual_server = server::http(|_| async move { + let mut actual_server = server::http(|_| async move { hyper::Response::builder() .status(200) .body("".into()) @@ -1158,29 +1162,65 @@ fn noproxy_test(noproxy_arg: &str) { .assert() .success(); - proxy_server.disable_hit_checks(); - proxy_server.assert_hits(0); - actual_server.assert_hits(1); + if let NoProxyTestType::ProxyNotUsed = test_type { + proxy_server.disable_hit_checks(); + proxy_server.assert_hits(0); + actual_server.assert_hits(1); + } else { + proxy_server.assert_hits(1); + actual_server.disable_hit_checks(); + actual_server.assert_hits(0); + } } #[test] fn noproxy_wildcard() { - noproxy_test("*"); + noproxy_test("*", NoProxyTestType::ProxyNotUsed); } #[test] fn noproxy_ip() { - noproxy_test("127.0.0.1"); + noproxy_test("127.0.0.1", NoProxyTestType::ProxyNotUsed); } #[test] fn noproxy_ip_cidr() { - noproxy_test("127.0.0.0/8"); + noproxy_test("127.0.0.0/8", NoProxyTestType::ProxyNotUsed); } #[test] fn noproxy_multiple() { - noproxy_test("127.0.0.2,127.0.0.1"); + noproxy_test("127.0.0.2,127.0.0.1", NoProxyTestType::ProxyNotUsed); +} + +#[test] +fn noproxy_whitespace() { + noproxy_test("example.test, 127.0.0.1", NoProxyTestType::ProxyNotUsed); +} + +#[test] +fn noproxy_whitespace_wildcard() { + noproxy_test("example.test, *", NoProxyTestType::ProxyNotUsed); +} + +#[test] +fn noproxy_whitespace_ip() { + noproxy_test("127.0.0.2, 127.0.0.1", NoProxyTestType::ProxyNotUsed); +} + +#[test] +fn noproxy_other_host() { + noproxy_test("example.test", NoProxyTestType::ProxyUsed); +} + +#[test] +fn noproxy_other_ip() { + noproxy_test("127.0.0.2", NoProxyTestType::ProxyUsed); +} + +#[test] +fn noproxy_other_ip_cidr() { + noproxy_test("127.0.1.0/24", NoProxyTestType::ProxyUsed); } // temporarily disabled for builds not using rustls From 824ed3ddf920bb794a557fe7519074c4bd053e0d Mon Sep 17 00:00:00 2001 From: Stefan Zwanenburg Date: Wed, 5 Mar 2025 13:58:22 +0100 Subject: [PATCH 3/5] Rename --noproxy to --disable-proxy-for --- src/cli.rs | 21 +++++++++++--------- src/main.rs | 2 +- src/to_curl.rs | 4 ++-- tests/cli.rs | 54 ++++++++++++++++++++++++++++---------------------- 4 files changed, 45 insertions(+), 36 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 1b1a4eaa..292b082e 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -302,7 +302,7 @@ Example: --print=Hb" /// - IP addresses are allowed, as are subnets (in CIDR notation, i.e.: '127.0.0.0/8'). /// - Any other entry in the list is assumed to be a hostname. #[clap(long, value_name = "no-proxy-list", value_delimiter = ',')] - pub noproxy: Vec, + pub disable_proxy_for: Vec, /// If "no", skip SSL verification. If a file path, use it as a CA bundle. /// @@ -1097,15 +1097,18 @@ impl FromStr for Proxy { } impl Proxy { - pub fn into_reqwest_proxy(self, noproxy: &[NoProxy]) -> anyhow::Result { + pub fn into_reqwest_proxy( + self, + disable_proxy_for: &[DisableProxyFor], + ) -> anyhow::Result { let proxy = match self { Proxy::Http(url) => reqwest::Proxy::http(url), Proxy::Https(url) => reqwest::Proxy::https(url), Proxy::All(url) => reqwest::Proxy::all(url), }?; - let mut noproxy_comma_delimited = noproxy.join(","); - if noproxy.contains(&"*".into()) { + let mut noproxy_comma_delimited = disable_proxy_for.join(","); + if disable_proxy_for.contains(&"*".into()) { // reqwest's NoProxy wildcard doesn't apply to IP addresses, while curl's does noproxy_comma_delimited.push_str(",0.0.0.0/0,::/0"); } @@ -1115,15 +1118,15 @@ impl Proxy { } #[derive(Debug, Clone, PartialEq, Eq)] -pub struct NoProxy(String); +pub struct DisableProxyFor(String); -impl From<&str> for NoProxy { +impl From<&str> for DisableProxyFor { fn from(s: &str) -> Self { Self(s.trim().to_string()) } } -impl Borrow for NoProxy { +impl Borrow for DisableProxyFor { fn borrow(&self) -> &str { &self.0 } @@ -1519,8 +1522,8 @@ mod tests { } #[test] - fn noproxy_trims_whitespace() { - assert_eq!(NoProxy::from("*"), NoProxy::from(" * ")); + fn disable_proxy_for_trims_whitespace() { + assert_eq!(DisableProxyFor::from("*"), DisableProxyFor::from(" * ")); } #[test] diff --git a/src/main.rs b/src/main.rs index b1c4afa4..0781649a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -274,7 +274,7 @@ fn run(args: Cli) -> Result { } for proxy in args.proxy.into_iter().rev() { - client = client.proxy(proxy.into_reqwest_proxy(&args.noproxy)?); + client = client.proxy(proxy.into_reqwest_proxy(&args.disable_proxy_for)?); } client = match args.http_version { diff --git a/src/to_curl.rs b/src/to_curl.rs index 6cf4d159..776ac87d 100644 --- a/src/to_curl.rs +++ b/src/to_curl.rs @@ -233,9 +233,9 @@ pub fn translate(args: Cli) -> Result { } } } - if !args.noproxy.is_empty() { + if !args.disable_proxy_for.is_empty() { cmd.arg("--noproxy"); - cmd.arg(args.noproxy.join(",")); + cmd.arg(args.disable_proxy_for.join(",")); } if let Some(timeout) = args.timeout.and_then(|t| t.as_duration()) { cmd.arg("--max-time"); diff --git a/tests/cli.rs b/tests/cli.rs index 6382e163..2b6d07c4 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -1136,11 +1136,11 @@ fn proxy_multiple_valid_proxies() { cmd.assert().success(); } -enum NoProxyTestType { +enum DisableProxyForTestType { ProxyUsed, ProxyNotUsed, } -fn noproxy_test(noproxy_arg: &str, test_type: NoProxyTestType) { +fn disable_proxy_for_test(disable_proxy_for_arg: &str, test_type: DisableProxyForTestType) { let mut proxy_server = server::http(|_| async move { hyper::Response::builder() .status(200) @@ -1156,13 +1156,13 @@ fn noproxy_test(noproxy_arg: &str, test_type: NoProxyTestType) { get_command() .arg(format!("--proxy=http:{}", proxy_server.base_url())) - .arg(format!("--noproxy={}", noproxy_arg)) + .arg(format!("--disable-proxy-for={}", disable_proxy_for_arg)) .arg("GET") .arg(actual_server.base_url().as_str()) .assert() .success(); - if let NoProxyTestType::ProxyNotUsed = test_type { + if let DisableProxyForTestType::ProxyNotUsed = test_type { proxy_server.disable_hit_checks(); proxy_server.assert_hits(0); actual_server.assert_hits(1); @@ -1174,53 +1174,59 @@ fn noproxy_test(noproxy_arg: &str, test_type: NoProxyTestType) { } #[test] -fn noproxy_wildcard() { - noproxy_test("*", NoProxyTestType::ProxyNotUsed); +fn disable_proxy_for_wildcard() { + disable_proxy_for_test("*", DisableProxyForTestType::ProxyNotUsed); } #[test] -fn noproxy_ip() { - noproxy_test("127.0.0.1", NoProxyTestType::ProxyNotUsed); +fn disable_proxy_for_ip() { + disable_proxy_for_test("127.0.0.1", DisableProxyForTestType::ProxyNotUsed); } #[test] -fn noproxy_ip_cidr() { - noproxy_test("127.0.0.0/8", NoProxyTestType::ProxyNotUsed); +fn disable_proxy_for_ip_cidr() { + disable_proxy_for_test("127.0.0.0/8", DisableProxyForTestType::ProxyNotUsed); } #[test] -fn noproxy_multiple() { - noproxy_test("127.0.0.2,127.0.0.1", NoProxyTestType::ProxyNotUsed); +fn disable_proxy_for_multiple() { + disable_proxy_for_test("127.0.0.2,127.0.0.1", DisableProxyForTestType::ProxyNotUsed); } #[test] -fn noproxy_whitespace() { - noproxy_test("example.test, 127.0.0.1", NoProxyTestType::ProxyNotUsed); +fn disable_proxy_for_whitespace() { + disable_proxy_for_test( + "example.test, 127.0.0.1", + DisableProxyForTestType::ProxyNotUsed, + ); } #[test] -fn noproxy_whitespace_wildcard() { - noproxy_test("example.test, *", NoProxyTestType::ProxyNotUsed); +fn disable_proxy_for_whitespace_wildcard() { + disable_proxy_for_test("example.test, *", DisableProxyForTestType::ProxyNotUsed); } #[test] -fn noproxy_whitespace_ip() { - noproxy_test("127.0.0.2, 127.0.0.1", NoProxyTestType::ProxyNotUsed); +fn disable_proxy_for_whitespace_ip() { + disable_proxy_for_test( + "127.0.0.2, 127.0.0.1", + DisableProxyForTestType::ProxyNotUsed, + ); } #[test] -fn noproxy_other_host() { - noproxy_test("example.test", NoProxyTestType::ProxyUsed); +fn disable_proxy_for_other_host() { + disable_proxy_for_test("example.test", DisableProxyForTestType::ProxyUsed); } #[test] -fn noproxy_other_ip() { - noproxy_test("127.0.0.2", NoProxyTestType::ProxyUsed); +fn disable_proxy_for_other_ip() { + disable_proxy_for_test("127.0.0.2", DisableProxyForTestType::ProxyUsed); } #[test] -fn noproxy_other_ip_cidr() { - noproxy_test("127.0.1.0/24", NoProxyTestType::ProxyUsed); +fn disable_proxy_for_other_ip_cidr() { + disable_proxy_for_test("127.0.1.0/24", DisableProxyForTestType::ProxyUsed); } // temporarily disabled for builds not using rustls From 4e88c91b9e09c6d589f9ff786f6c8a4aa6896551 Mon Sep 17 00:00:00 2001 From: Stefan Zwanenburg Date: Wed, 5 Mar 2025 14:00:31 +0100 Subject: [PATCH 4/5] Added link to reqwest issue for its different handling of NoProxy --- src/cli.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cli.rs b/src/cli.rs index 292b082e..a8a45b5c 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1110,6 +1110,7 @@ impl Proxy { let mut noproxy_comma_delimited = disable_proxy_for.join(","); if disable_proxy_for.contains(&"*".into()) { // reqwest's NoProxy wildcard doesn't apply to IP addresses, while curl's does + // See: https://github.com/seanmonstar/reqwest/issues/2579 noproxy_comma_delimited.push_str(",0.0.0.0/0,::/0"); } From e6941936835d1a36c8f531283bb6ce86abdfb40f Mon Sep 17 00:00:00 2001 From: Stefan Zwanenburg Date: Wed, 5 Mar 2025 14:20:57 +0100 Subject: [PATCH 5/5] Handle `NO_PROXY`/`no_proxy` env variables & fix long help for --disable-proxy-for --- src/cli.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index a8a45b5c..47a69e7f 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -298,9 +298,14 @@ Example: --print=Hb" /// Comma-separated list of hosts for which not to use a proxy, if one is specified. /// - /// - A '*' matches all hosts, and effectively disables proxies altogether. - /// - IP addresses are allowed, as are subnets (in CIDR notation, i.e.: '127.0.0.0/8'). + /// - A "*" matches all hosts, and effectively disables proxies altogether. + /// + /// - IP addresses are allowed, as are subnets (in CIDR notation, i.e.: "127.0.0.0/8"). + /// /// - Any other entry in the list is assumed to be a hostname. + /// + /// The environment variable "NO_PROXY"/"no_proxy" can also be used, but its completely ignored + /// if --disable-proxy-for is passed. #[clap(long, value_name = "no-proxy-list", value_delimiter = ',')] pub disable_proxy_for: Vec, @@ -1114,7 +1119,10 @@ impl Proxy { noproxy_comma_delimited.push_str(",0.0.0.0/0,::/0"); } - Ok(proxy.no_proxy(reqwest::NoProxy::from_string(&noproxy_comma_delimited))) + Ok(proxy.no_proxy( + reqwest::NoProxy::from_string(&noproxy_comma_delimited) + .or_else(reqwest::NoProxy::from_env), + )) } }