From cd1a2b079fb7403422d7813afbf0cf1c5d178f8a Mon Sep 17 00:00:00 2001 From: Codex GPT-6 Date: Wed, 23 Sep 2026 21:58:45 +0200 Subject: [PATCH 1/2] fix(gix-transport): honor proxy configuration with `reqwest` (#3015) The blocking `reqwest` backend received `http::Options` but constructed its client before seeing them. A configured `http.proxy` was silently ignored, allowing direct requests even when only the proxy could reach the destination. The regression fails with a direct DNS error against `git.invalid` before this change. Configure the client from the effective proxy and bypass list, preserving connection reuse until that configuration or its credentials change. Explicit configuration overrides the environment, an empty proxy disables proxying, and `no_proxy=*` also bypasses numeric addresses. Preserve the configuration when restarting a failed worker so invalid proxy settings cannot turn into a direct connection on retry. Select environment proxies for each actual request URL, after request hooks and accepted redirects. Explicit proxy configuration still applies to both schemes. Use Git/libcurl's documented port 1080 for HTTP proxies without a port, preserving explicit ports and the usual HTTPS default. For `reqwest`, leave imported environment proxy values out of explicit transport overrides so redirects can select the destination scheme. Keep `curl`'s existing setup. Prepare credential helpers for the imported proxy candidates so username-only environment URLs still obtain the matching password, including after a redirect. Preserve explicit empty settings while treating empty environment values as absent during fallback. Validate environment proxies only when their destination scheme is actually requested, including redirects and URL-changing request hooks. An unused scheme's proxy must not break a valid request, and a configured global bypass must ignore disabled proxies entirely. Reuse `reqwest` for proxy routing, Basic credentials and SOCKS, and use the existing credential helper callback to obtain, approve or reject proxy credentials. Reject unsupported schemes, Unix socket paths, SOCKS4 user IDs (which reqwest otherwise silently drops), and explicit authentication methods that `reqwest` cannot honor. Document its preemptive Basic handling of `anyauth` and its current requirement for a TLS feature when using SOCKS. `reqwest` strips proxy authentication on authority-changing redirects without reapplying it. Execute each accepted hop separately, retaining the existing redirect safety policy and reapplying proxy authentication for that destination. Preserve POST-to-GET conversion, replay buffered 307/308 bodies, and stop when streamed bodies cannot be replayed. Do not carry origin credentials across authorities or proxy headers to bypassed origins. Reuse helper credentials while the selected proxy stays the same. Preserve explicit `Authorization` headers over credentials embedded in the origin URL, matching the previous request-builder precedence. Use the bypass matcher from `reqwest`'s existing `hyper-util` dependency before validating or authenticating a proxy. A bypassed destination must not need proxy credentials or store unvalidated ones. This introduces a direct dependency but no new packages. Close streamed upload bodies before reporting proxy setup errors, so the writer and header reader cannot block waiting for each other. Approve proxy credentials even when the origin returns an error, reject them only for a proxy authentication response (407), and leave them alone when a connection fails without a response. Regression tests cover the redirect, default-port and credential rejection findings from review. Include HTTPS CONNECT rejections: `reqwest` hides their status and hyper-util does not export its error type, so match the specific source message until a typed API becomes available. Test 407, other tunnel failures and dropped connections to protect that distinction. Git reference: `http.c` and `t/t5564-http-proxy.sh` at `d38352cd43ab9745686d697872408bc3249a153f`. The recording-proxy regression also runs `git ls-remote` with Git 2.54.0 (Apple Git-157) and verifies the same absolute request target. Routing, bypass, environment precedence, inline authentication and SOCKS tests are shared with the curl backend; the credential-helper regression is specific to reqwest because `curl` currently loses the helper password when its proxy URL has a username. Validation: - Reproduced the issue with the failing recording-proxy test first. - Regressions cover unused proxy settings, global bypass of invalid settings, SOCKS4 user IDs, request-hook URL changes, environment helper selection, authenticated redirects, buffered/streamed POST redirects, explicit authorization precedence, and upload setup failures. - Transport tests with plain reqwest, Rust TLS and native TLS. - The complete curl HTTP transport integration suite, including HTTPS proxy selection after redirects. - Repository transport-option tests with both reqwest and curl, including the imported-environment regression from review. - `cargo fmt --all -- --check` and transport documentation generation. - Library Clippy with `--no-deps -- -D warnings`; test Clippy completes with existing warnings in the older transport tests. --- Cargo.lock | 1 + gix-transport/Cargo.toml | 6 +- .../src/client/blocking_io/http/mod.rs | 10 +- .../client/blocking_io/http/reqwest/remote.rs | 421 ++++++-- .../tests/client/blocking_io/http/mod.rs | 1 + .../tests/client/blocking_io/http/proxy.rs | 993 ++++++++++++++++++ gix/src/config/tree/sections/http.rs | 6 +- gix/src/repository/config/transport.rs | 97 +- .../repository/config/transport_options.rs | 94 ++ 9 files changed, 1546 insertions(+), 83 deletions(-) create mode 100644 gix-transport/tests/client/blocking_io/http/proxy.rs diff --git a/Cargo.lock b/Cargo.lock index 7c6d8fd6280..6e60c0be216 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2650,6 +2650,7 @@ dependencies = [ "gix-testtools", "gix-transport", "gix-url", + "hyper-util", "insta", "parking_lot", "pin-project-lite", diff --git a/gix-transport/Cargo.toml b/gix-transport/Cargo.toml index a837ec5e209..1bba5a65474 100644 --- a/gix-transport/Cargo.toml +++ b/gix-transport/Cargo.toml @@ -40,7 +40,8 @@ http-client-curl-openssl = ["http-client-curl", "curl/ssl"] ## Implies `http-client` and adds support for http transports using the blocking version of `reqwest`. ## NOTE: `https://` is NOT supported by default. You must enable one of the `http-client-reqwest-rust-tls` ## or `http-client-reqwest-native-tls` features to enable HTTPS support. -http-client-reqwest = ["reqwest", "http-client"] +## With the current reqwest version, SOCKS proxies also require one of the TLS features. +http-client-reqwest = ["reqwest", "http-client", "dep:hyper-util"] ## Stacks with `http-client-reqwest` and enables `https://` via the `rustls` crate. http-client-reqwest-rust-tls = ["http-client-reqwest", "reqwest/rustls"] ## Stacks with `http-client-reqwest` and enables `https://` via the `rustls` crate. @@ -125,7 +126,8 @@ curl = { version = "0.4", optional = true, default-features = false } # for http-client-reqwest # all but the 'default-tls' feature -reqwest = { version = "0.13.4", optional = true, default-features = false, features = ["blocking", "charset", "http2"] } +reqwest = { version = "0.13.4", optional = true, default-features = false, features = ["blocking", "charset", "http2", "socks"] } +hyper-util = { version = "0.1.20", optional = true, default-features = false, features = ["client-proxy"] } ## If used in conjunction with `async-client`, the `connect()` method will become available along with supporting the git protocol over TCP, ## where the TCP stream is created using this crate. diff --git a/gix-transport/src/client/blocking_io/http/mod.rs b/gix-transport/src/client/blocking_io/http/mod.rs index 89aa778099c..950b3d68d67 100644 --- a/gix-transport/src/client/blocking_io/http/mod.rs +++ b/gix-transport/src/client/blocking_io/http/mod.rs @@ -32,8 +32,11 @@ pub mod curl; /// The experimental `reqwest` backend. /// -/// It doesn't support any of the shared http options yet, but can be seen as example on how to integrate blocking `http` backends. -/// There is also nothing that would prevent it from becoming a fully-featured HTTP backend except for demand and time. +/// Supports extra headers, redirects, HTTP/HTTPS and SOCKS proxies, proxy bypass lists, and proxy credential helpers. +/// HTTP proxy authentication uses preemptive Basic authentication for both `Basic` and `AnyAuth`; other methods and +/// Unix socket proxy paths and SOCKS4 user IDs are rejected. With the current reqwest version, SOCKS proxies also require +/// a TLS feature. +/// Other shared HTTP options are not supported yet. #[cfg(feature = "http-client-reqwest")] pub mod reqwest; @@ -145,6 +148,7 @@ pub struct Options { /// A curl-style proxy declaration of the form `[protocol://][user[:password]@]proxyhost[:port]`. /// /// Note that an empty string means the proxy is disabled entirely. + /// If unset, the backend selects proxy environment variables for each requested URL. /// Refers to `http.proxy`. pub proxy: Option, /// The comma-separated list of hosts to not send through the `proxy`, or `*` to entirely disable all proxying. @@ -155,6 +159,8 @@ pub struct Options { pub proxy_auth_method: options::ProxyAuthMethod, /// If authentication is needed for the proxy as its URL contains a username, this method must be set to provide a password /// for it before making the request, and to store it if the connection succeeds. + /// When `proxy` is unset, reqwest creates each `Get` action from the selected environment proxy URL, which can change + /// on redirects. The callback must select credentials for that URL. pub proxy_authenticate: Option<(gix_credentials::helper::Action, Arc>)>, /// The `HTTP` `USER_AGENT` string presented to an `HTTP` server, notably not the user agent present to the `git` server. /// diff --git a/gix-transport/src/client/blocking_io/http/reqwest/remote.rs b/gix-transport/src/client/blocking_io/http/reqwest/remote.rs index 3a800f96a97..b6198318fb7 100644 --- a/gix-transport/src/client/blocking_io/http/reqwest/remote.rs +++ b/gix-transport/src/client/blocking_io/http/reqwest/remote.rs @@ -1,5 +1,6 @@ use std::{ any::Any, + error::Error as _, io::{Read, Write}, str::FromStr, sync::Arc, @@ -8,10 +9,11 @@ use std::{ use gix_error::{Error, ErrorExt, Result, ResultExt, message}; use gix_features::io::pipe; use parking_lot::Mutex; +use reqwest::{Method, StatusCode, header}; use crate::client::blocking_io::http::{ self, - options::FollowRedirects, + options::{FollowRedirects, ProxyAuthMethod}, redirect::{self, Action as RedirectAction}, reqwest::Remote, traits::PostBodyDataKind, @@ -34,6 +36,14 @@ fn authority_changed(curr_url: &reqwest::Url, prev_url: &reqwest::Url) -> bool { || curr_url.port_or_known_default() != prev_url.port_or_known_default() } +#[derive(Default)] +struct RedirectState { + action: RedirectAction, + tail: String, + next_url: Option, + count: usize, +} + impl Default for Remote { fn default() -> Self { let (req_send, req_recv) = std::sync::mpsc::sync_channel(0); @@ -42,61 +52,70 @@ impl Default for Remote { let redirected_base_url_shared_for_field = redirected_base_url_shared.clone(); let handle = std::thread::spawn(move || -> Result { let mut follow = None; - let redirect_action = Arc::new(Mutex::new(RedirectAction::Stop)); - let redirect_tail = Arc::new(Mutex::new(String::new())); - - // We may error while configuring, which is expected as part of the internal protocol. The error will be - // received and the sender of the request might restart us. - let client = reqwest::blocking::ClientBuilder::new() - .connect_timeout(std::time::Duration::from_secs(20)) - .http1_title_case_headers() - .redirect(reqwest::redirect::Policy::custom({ - let redirect_action = redirect_action.clone(); - let redirect_tail = redirect_tail.clone(); - move |attempt| { - match *redirect_action.lock() { - RedirectAction::Follow => { - let curr_url = attempt.url(); - let prev_urls = attempt.previous(); - // emulate default git behaviour which relies on curl default behaviour apparently. - const CURL_DEFAULT_REDIRS: usize = 50; - if prev_urls.len() >= CURL_DEFAULT_REDIRS { - return attempt.error("too many redirects"); - } + let redirects = Arc::new(Mutex::new(RedirectState::default())); - match prev_urls.last() { - Some(prev_url) if !redirect::scheme_is_safe(curr_url.as_str(), prev_url.as_str()) => { + // Reuse connections until the effective proxy configuration (including credentials) changes. + let mut client: Option<(Option, reqwest::blocking::Client)> = None; + let create_client = |proxy: Option<&gix_url::Url>| -> Result<_> { + let mut builder = reqwest::blocking::ClientBuilder::new() + .no_proxy() + .connect_timeout(std::time::Duration::from_secs(20)) + .http1_title_case_headers() + .redirect(reqwest::redirect::Policy::custom({ + let redirects = redirects.clone(); + move |attempt| { + let mut redirects = redirects.lock(); + match redirects.action { + RedirectAction::Follow => { + let curr_url = attempt.url(); + let prev_urls = attempt.previous(); + // emulate default git behaviour which relies on curl default behaviour apparently. + const CURL_DEFAULT_REDIRS: usize = 50; + redirects.count += 1; + if redirects.count >= CURL_DEFAULT_REDIRS { + return attempt.error("too many redirects"); + } + if let Some(prev_url) = prev_urls.last() + && !redirect::scheme_is_safe(curr_url.as_str(), prev_url.as_str()) + { // Don't follow insecure protocol redirects, particularly https-to-http downgrades. - attempt.stop() + return attempt.stop(); } - Some(prev_url) if authority_changed(curr_url, prev_url) => { - // Allowed only if the tail doesn't change. - let redirect_tail = redirect_tail.lock(); - if curr_url.as_str().ends_with(redirect_tail.as_str()) { - attempt.follow() - } else { - let curr_url = curr_url.as_str().to_owned(); - let redirect_tail = redirect_tail.to_string(); - attempt.error(format!( - "redirect url {curr_url:?} does not end with expected request suffix {redirect_tail:?}", - )) - } + if prev_urls.last().is_some_and(|prev_url| authority_changed(curr_url, prev_url)) + && !curr_url.as_str().ends_with(&redirects.tail) + { + let curr_url = curr_url.as_str().to_owned(); + let redirect_tail = &redirects.tail; + return attempt.error(format!( + "redirect url {curr_url:?} does not end with expected request suffix {redirect_tail:?}", + )); } - _ => attempt.follow(), + // Execute each hop ourselves: reqwest otherwise strips proxy credentials on + // authority changes without adding them again for the selected proxy. + redirects.next_url = Some(curr_url.clone()); + attempt.stop() } + RedirectAction::RejectConfiguredHeaders => { + attempt.error("refusing to follow redirect after request headers were configured") + } + RedirectAction::Stop => attempt.stop(), } - RedirectAction::RejectConfiguredHeaders => { - attempt.error("refusing to follow redirect after request headers were configured") - } - RedirectAction::Stop => attempt.stop(), } - } - })) - .build() - .map_err(classify_reqwest) - .or_raise(|| message("Could not initialize HTTP client"))?; + })); + if let Some(proxy) = proxy { + builder = builder.proxy( + reqwest::Proxy::all(proxy.to_bstring().to_string()) + .map_err(classify_reqwest) + .or_raise(|| message("Could not configure HTTP proxy"))?, + ); + } + builder + .build() + .map_err(classify_reqwest) + .or_raise(|| message("Could not initialize HTTP client")) + }; - for Request { + 'requests: for Request { url, base_url, headers, @@ -106,13 +125,35 @@ impl Default for Remote { { let redirected_base_url = redirected_base_url_shared.lock().clone(); let effective_url = redirect::swap_tails(redirected_base_url.as_deref(), &base_url, url.clone()); + let no_proxy = config.no_proxy.clone().or_else(|| proxy_env("no_proxy", "NO_PROXY")); let has_configured_extra_headers = !config.extra_headers.is_empty(); - let mut req_builder = if upload_body_kind.is_some() { - client.post(&effective_url) - } else { - client.get(&effective_url) + let mut req = reqwest::blocking::Request::new( + if upload_body_kind.is_some() { + Method::POST + } else { + Method::GET + }, + reqwest::Url::parse(&effective_url).or_raise(|| message("Request configuration failed"))?, + ); + *req.headers_mut() = headers; + if !req.url().username().is_empty() || req.url().password().is_some() { + use base64::Engine; + let origin = + gix_url::parse(req.url().as_str()).or_raise(|| message("Request configuration failed"))?; + let value = format!( + "Basic {}", + base64::engine::general_purpose::STANDARD.encode(format!( + "{}:{}", + origin.user().unwrap_or_default(), + origin.password().unwrap_or_default() + )) + ); + req.headers_mut() + .entry(header::AUTHORIZATION) + .or_insert(value.parse().expect("base64 is a valid header")); + req.url_mut().set_username("").ok(); + req.url_mut().set_password(None).ok(); } - .headers(headers); let (post_body_tx, mut post_body_rx) = pipe::unidirectional(0); let (mut response_body_tx, response_body_rx) = pipe::unidirectional(0); let (mut headers_tx, headers_rx) = pipe::unidirectional(0); @@ -128,21 +169,17 @@ impl Default for Remote { // Shut down as something is off. break; } - req_builder = match upload_body_kind { + *req.body_mut() = match upload_body_kind { Some(PostBodyDataKind::BoundedAndFitsIntoMemory) => { let mut buf = Vec::::with_capacity(512); post_body_rx .read_to_end(&mut buf) .or_raise(|| message("Could not finish reading all data to post to the remote"))?; - req_builder.body(buf) + Some(buf.into()) } - Some(PostBodyDataKind::Unbounded) => req_builder.body(reqwest::blocking::Body::new(post_body_rx)), - None => req_builder, + Some(PostBodyDataKind::Unbounded) => Some(reqwest::blocking::Body::new(post_body_rx)), + None => None, }; - let mut req = req_builder - .build() - .map_err(classify_reqwest) - .or_raise(|| message("Could not build HTTP request"))?; let mut has_configure_request = false; if let Some(ref mut request_options) = config.backend.as_ref().and_then(|backend| backend.lock().ok()) && let Some(options) = request_options.downcast_mut::() @@ -151,22 +188,163 @@ impl Default for Remote { has_configure_request = true; configure_request(&mut req).or_raise(|| message("Request configuration failed"))?; } - let follow = follow.get_or_insert(config.follow_redirects); let may_follow_redirects = matches!(*follow, FollowRedirects::Initial | FollowRedirects::All); let has_configured_request_headers = has_configure_request || has_configured_extra_headers; - *redirect_action.lock() = - RedirectAction::from_request(may_follow_redirects, has_configured_request_headers); - url.strip_prefix(&base_url) - .expect("BUG: caller assures `base_url` is subset of `url`") - .clone_into(&mut redirect_tail.lock()); + *redirects.lock() = RedirectState { + action: RedirectAction::from_request(may_follow_redirects, has_configured_request_headers), + tail: url + .strip_prefix(&base_url) + .expect("BUG: caller assures `base_url` is subset of `url`") + .into(), + ..Default::default() + }; if *follow == FollowRedirects::Initial { *follow = FollowRedirects::None; } + let mut proxy_credentials: Option<(gix_url::Url, gix_credentials::protocol::Outcome)> = None; + let response = loop { + let mut proxy = if bypasses_proxy(no_proxy.as_deref(), req.url()) { + None + } else { + match proxy_url(&config, req.url().scheme()) { + Ok(proxy) => proxy, + Err(err) => { + drop(req); // Release a streamed upload before sending its error to the header reader. + headers_tx.channel.send(Err(std::io::Error::other(err))).ok(); + continue 'requests; + } + } + }; + let mut proxy_auth_action = None; + if let Some(proxy) = proxy.as_mut() + && let Some((action, authenticate)) = &config.proxy_authenticate + && (config.proxy.is_some() || proxy.user.is_some()) + { + if proxy_credentials.as_ref().is_none_or(|(previous, _)| previous != proxy) { + let action = if config.proxy.is_some() { + action.clone() + } else { + gix_credentials::helper::Action::get_for_url(proxy.to_bstring()) + }; + let credentials = match authenticate.lock().expect("no panics in other threads")(action) + .or_raise(|| message("Could not obtain proxy credentials")) + .and_then(|credentials| { + credentials.ok_or_else(|| { + message("The proxy credential helper returned no credentials").raise() + }) + }) { + Ok(credentials) => credentials, + Err(err) => { + drop(req); + headers_tx.channel.send(Err(std::io::Error::other(err))).ok(); + continue 'requests; + } + }; + proxy_credentials = Some((proxy.clone(), credentials)); + } + let (_, credentials) = proxy_credentials.as_ref().expect("credentials were initialized above"); + proxy.user = Some(credentials.identity.username.clone()); + proxy.password = Some(credentials.identity.password.clone()); + proxy_auth_action = Some((credentials.next.clone(), authenticate)); + } + if client + .as_ref() + .is_none_or(|(previous_proxy, _)| previous_proxy != &proxy) + { + let new_client = match create_client(proxy.as_ref()) { + Ok(client) => client, + Err(err) => { + drop(req); + headers_tx.channel.send(Err(std::io::Error::other(err))).ok(); + continue 'requests; + } + }; + client = Some((proxy, new_client)); + } + let (_, client) = client.as_ref().expect("client was initialized above"); + let mut next_req = req.try_clone().unwrap_or_else(|| { + // A streamed POST can still redirect to a GET. Reqwest stops 307/308 redirects + // before invoking our policy when it cannot replay the body. + let body = req.body_mut().take(); + let copy = req.try_clone().expect("a request without a body can be cloned"); + *req.body_mut() = body; + copy + }); + let response = client.execute(req); + let proxy_auth_failed = match &response { + Ok(res) => res.status() == StatusCode::PROXY_AUTHENTICATION_REQUIRED, + Err(err) => { + // ponytail: reqwest hides CONNECT's status and hyper-util's error type is private. + // Use a typed check when either dependency exposes one. + err.is_connect() + && std::iter::successors(err.source(), |&err| err.source()) + .any(|err| err.to_string() == "tunnel error: proxy authorization required") + } + }; + if (response.is_ok() || proxy_auth_failed) + && let Some((action, authenticate)) = proxy_auth_action + { + let action = if proxy_auth_failed { + action.erase() + } else { + action.store() + }; + if let Err(err) = authenticate.lock().expect("no panics in other threads")(action) { + headers_tx.channel.send(Err(std::io::Error::other(err))).ok(); + continue 'requests; + } + } + let Some(next_url) = redirects.lock().next_url.take() else { + break response; + }; + let res = match response { + Ok(res) => res, + Err(err) => break Err(err), + }; + if res.status() == StatusCode::SEE_OTHER + || (matches!(res.status(), StatusCode::MOVED_PERMANENTLY | StatusCode::FOUND) + && next_req.method() == Method::POST) + { + if next_req.method() != Method::HEAD { + *next_req.method_mut() = Method::GET; + } + *next_req.body_mut() = None; + for name in [ + header::CONTENT_LENGTH, + header::CONTENT_TYPE, + header::TRANSFER_ENCODING, + header::CONTENT_ENCODING, + ] { + next_req.headers_mut().remove(name); + } + } + if authority_changed(&next_url, next_req.url()) { + for name in [ + header::AUTHORIZATION, + header::COOKIE, + header::WWW_AUTHENTICATE, + header::PROXY_AUTHORIZATION, + ] { + next_req.headers_mut().remove(name); + } + next_req.headers_mut().remove("cookie2"); + } + let mut referer = next_req.url().clone(); + referer.set_username("").ok(); + referer.set_password(None).ok(); + referer.set_fragment(None); + if let Ok(value) = header::HeaderValue::from_str(referer.as_str()) { + next_req.headers_mut().insert(header::REFERER, value); + } + *next_req.url_mut() = next_url; + req = next_req; + }; + let mut www_authenticate = Vec::new(); - let mut res = match client.execute(req).and_then(|res| { + let mut res = match response.and_then(|res| { if res.status() == reqwest::StatusCode::UNAUTHORIZED { www_authenticate = res .headers() @@ -263,8 +441,11 @@ impl Remote { .expect("thread handle present") .join() .expect("handler thread should never panic") - .expect_err("something should have gone wrong with curl (we join on error only)"); - *self = Remote::default(); + .expect_err("something should have gone wrong with HTTP (we join on error only)"); + *self = Remote { + config: std::mem::take(&mut self.config), + ..Remote::default() + }; err_that_brought_thread_down.and_raise(message("Could not initialize the http client")) } @@ -315,6 +496,82 @@ impl Remote { } } +fn proxy_env(lower: &str, upper: &str) -> Option { + [lower, upper] + .into_iter() + .find_map(|name| std::env::var(name).ok().filter(|value| !value.is_empty())) +} + +fn bypasses_proxy(no_proxy: Option<&str>, url: &reqwest::Url) -> bool { + let Some(no_proxy) = no_proxy.filter(|value| !value.is_empty()) else { + return false; + }; + // The wildcard in reqwest's matcher currently only covers domain names, not IP addresses. + no_proxy == "*" + || url + .host_str() + .and_then(|host| format!("http://{host}").parse().ok()) + .is_some_and(|uri| { + // Use reqwest's own bypass rules before proxy validation and credential lookup. + // This dummy intercept target is only needed to construct the matcher, and is never contacted. + hyper_util::client::proxy::matcher::Matcher::builder() + .all("http://proxy.invalid") + .no(no_proxy) + .build() + .intercept(&uri) + .is_none() + }) +} + +fn proxy_url(config: &http::Options, scheme: &str) -> Result> { + let proxy = config.proxy.clone().or_else(|| { + // Like Git and curl, ignore uppercase HTTP_PROXY, which can originate in a CGI request. + if scheme == "https" { + proxy_env("https_proxy", "HTTPS_PROXY") + } else { + std::env::var("http_proxy").ok().filter(|value| !value.is_empty()) + } + .or_else(|| proxy_env("all_proxy", "ALL_PROXY")) + }); + let Some(mut proxy) = proxy.filter(|proxy| !proxy.is_empty()) else { + return Ok(None); + }; + if !proxy.contains("://") { + proxy.insert_str(0, "http://"); + } + let mut proxy = gix_url::parse(proxy.as_str()).or_raise(|| message("Invalid proxy URL"))?; + if !matches!( + proxy.scheme.as_str(), + "http" | "https" | "socks4" | "socks4a" | "socks5" | "socks5h" + ) { + return Err(message!("Unsupported proxy scheme '{}'", proxy.scheme.as_str()).raise()); + } + if !proxy.path.is_empty() && proxy.path.as_slice() != b"/" { + return Err(message("The reqwest backend does not support Unix socket proxy paths").raise()); + } + if matches!(proxy.scheme.as_str(), "socks4" | "socks4a") + && (proxy.user.is_some() || (config.proxy.is_some() && config.proxy_authenticate.is_some())) + { + return Err(message("The reqwest backend does not support SOCKS4 proxy user IDs").raise()); + } + if proxy.scheme == gix_url::Scheme::Http && proxy.port.is_none() { + // Git/libcurl's default for an HTTP proxy is 1080, unlike reqwest's 80. + proxy.port = Some(1080); + } + if matches!(proxy.scheme, gix_url::Scheme::Http | gix_url::Scheme::Https) + && (proxy.user.is_some() || (config.proxy.is_some() && config.proxy_authenticate.is_some())) + && !matches!( + config.proxy_auth_method, + ProxyAuthMethod::AnyAuth | ProxyAuthMethod::Basic + ) + { + return Err(message("The reqwest backend only supports Basic HTTP proxy authentication").raise()); + } + // Validate before constructing the client, without checking an unused destination scheme. + reqwest::Proxy::all(proxy.to_bstring().to_string()).or_raise(|| message("Invalid proxy URL"))?; + Ok(Some(proxy)) +} + /// Add one `name: value` header line to `header_map`, ignoring malformed or unsupported input in `header_line`. /// /// Git configuration may provide arbitrary extra header lines, so invalid names or values are skipped instead of @@ -388,3 +645,27 @@ pub(crate) struct Response { pub body: pipe::Reader, pub upload_body: pipe::Writer, } + +#[cfg(test)] +mod tests { + #[test] + fn http_proxy_default_port_matches_curl() -> gix_testtools::Result { + for (input, port) in [ + ("proxy.example", Some(1080)), + ("http://proxy.example", Some(1080)), + ("http://proxy.example:80", Some(80)), + ("https://proxy.example", None), + ] { + let proxy = super::proxy_url( + &super::http::Options { + proxy: Some(input.into()), + ..Default::default() + }, + "http", + )? + .expect("the proxy is configured"); + assert_eq!(proxy.port, port, "curl-style proxy port for {input}"); + } + Ok(()) + } +} diff --git a/gix-transport/tests/client/blocking_io/http/mod.rs b/gix-transport/tests/client/blocking_io/http/mod.rs index ed5c3b22887..62bea39b9eb 100644 --- a/gix-transport/tests/client/blocking_io/http/mod.rs +++ b/gix-transport/tests/client/blocking_io/http/mod.rs @@ -24,6 +24,7 @@ use crate::{ }; mod mock; +mod proxy; #[cfg(feature = "http-client-curl")] type Remote = http::curl::Curl; diff --git a/gix-transport/tests/client/blocking_io/http/proxy.rs b/gix-transport/tests/client/blocking_io/http/proxy.rs new file mode 100644 index 00000000000..4bf0b680d2e --- /dev/null +++ b/gix-transport/tests/client/blocking_io/http/proxy.rs @@ -0,0 +1,993 @@ +use std::{ + io::{self, Read, Write}, + net::TcpListener, + time::{Duration, Instant}, +}; + +#[cfg(feature = "http-client-reqwest")] +use gix_error::{ErrorExt, ResultExt}; +use gix_transport::client::blocking_io::http::{self, Http}; + +use super::Remote; +use crate::http_helpers::read_request_lines; + +fn proxy_environment() -> gix_testtools::Env<'static> { + [ + "http_proxy", + "HTTP_PROXY", + "https_proxy", + "HTTPS_PROXY", + "all_proxy", + "ALL_PROXY", + "no_proxy", + "NO_PROXY", + ] + .into_iter() + .fold(gix_testtools::Env::new(), gix_testtools::Env::unset) +} + +fn accept(listener: TcpListener) -> io::Result { + listener.set_nonblocking(true)?; + let deadline = Instant::now() + Duration::from_secs(5); + let stream = loop { + match listener.accept() { + Ok((stream, _)) => break stream, + Err(err) if err.kind() == io::ErrorKind::WouldBlock && Instant::now() < deadline => { + std::thread::sleep(Duration::from_millis(10)); + } + Err(err) => return Err(err), + } + }; + stream.set_nonblocking(false)?; + stream.set_read_timeout(Some(Duration::from_secs(5)))?; + stream.set_write_timeout(Some(Duration::from_secs(5)))?; + Ok(stream) +} + +fn serve(listener: TcpListener, status: Option) -> std::thread::JoinHandle>> { + std::thread::spawn(move || { + let stream = accept(listener)?; + let mut reader = io::BufReader::new(stream); + let lines = read_request_lines(&mut reader); + let body_len = lines + .iter() + .find_map(|line| { + line.to_ascii_lowercase() + .strip_prefix("content-length:")? + .trim() + .parse() + .ok() + }) + .unwrap_or(0); + reader.read_exact(&mut vec![0; body_len])?; + if let Some(status) = status { + write!( + reader.get_mut(), + "HTTP/1.1 {status} status\r\nContent-Length: 2\r\nConnection: close\r\n\r\nok" + )?; + } + Ok(lines) + }) +} + +fn request( + remote: &mut H, + url: &str, + listener: TcpListener, + post: bool, +) -> gix_testtools::Result> { + let server = serve(listener, Some(200)); + let response = if post { + let mut response = remote.post( + url, + url, + ["Accept: */*"], + http::PostBodyDataKind::BoundedAndFitsIntoMemory, + )?; + response.post_body.write_all(b"pack")?; + drop(response.post_body); + http::GetResponse { + headers: response.headers, + body: response.body, + } + } else { + remote.get(url, url, ["Accept: */*"])? + }; + let mut headers = response.headers; + let mut body = response.body; + io::copy(&mut headers, &mut io::sink())?; + let mut received = String::new(); + body.read_to_string(&mut received)?; + assert_eq!(received, "ok", "the request must reach the selected listener"); + Ok(server.join().expect("recording server must not panic")?) +} + +#[test] +fn configured_proxy_is_used_for_get_and_post() -> gix_testtools::Result { + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + let _env = proxy_environment(); + let listener = TcpListener::bind("127.0.0.1:0")?; + let mut remote = Remote::default(); + remote.configure(&http::Options { + // Also exercise curl's scheme-less proxy syntax. + proxy: Some(listener.local_addr()?.to_string()), + no_proxy: Some(String::new()), + ..Default::default() + })?; + let url = "http://git.invalid/repo/info/refs?service=git-upload-pack"; + for (post, method) in [(false, "GET"), (true, "POST")] { + let lines = request(&mut remote, url, listener.try_clone()?, post)?; + assert_eq!( + lines[0], + format!("{method} {url} HTTP/1.1"), + "the proxy receives an absolute URL" + ); + } + let proxy = listener.local_addr()?.to_string(); + let server = serve(listener, Some(200)); + let directory = gix_testtools::tempfile::tempdir()?; + gix_testtools::git_command(directory.path()) + .args([ + "-c", + &format!("http.proxy={proxy}"), + "ls-remote", + "http://git.invalid/repo", + ]) + .output()?; + let lines = server.join().expect("recording server must not panic")?; + assert_eq!( + lines[0], + format!("GET {url} HTTP/1.1"), + "Git uses the same proxy request target" + ); + Ok(()) +} + +#[test] +fn configured_proxy_and_empty_bypass_override_environment() -> gix_testtools::Result { + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + let listener = TcpListener::bind("127.0.0.1:0")?; + let unused = TcpListener::bind("127.0.0.1:0")?; + let _env = proxy_environment() + .set("http_proxy", format!("http://{}", unused.local_addr()?)) + .set("no_proxy", "*"); + drop(unused); + let mut remote = Remote::default(); + remote.configure(&http::Options { + proxy: Some(format!("http://{}", listener.local_addr()?)), + no_proxy: Some(String::new()), + ..Default::default() + })?; + request(&mut remote, "http://git.invalid/repo", listener, false)?; + Ok(()) +} + +#[test] +fn configured_bypass_applies_to_environment_proxy() -> gix_testtools::Result { + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + let listener = TcpListener::bind("127.0.0.1:0")?; + let _env = proxy_environment() + .set("http_proxy", format!("http://{}", listener.local_addr()?)) + .set("no_proxy", "*"); + let mut remote = Remote::default(); + remote.configure(&http::Options { + no_proxy: Some(String::new()), + ..Default::default() + })?; + request(&mut remote, "http://git.invalid/repo", listener, false)?; + Ok(()) +} + +#[test] +fn bypass_and_empty_proxy_use_direct_connection() -> gix_testtools::Result { + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + let unused = TcpListener::bind("127.0.0.1:0")?; + let proxy = format!("http://{}", unused.local_addr()?); + drop(unused); + let _env = proxy_environment().set("http_proxy", proxy.clone()); + for (proxy, no_proxy) in [ + (proxy.clone(), "*"), + ("htpp://proxy.invalid".into(), "*"), + (proxy.clone(), "example.com,127.0.0.1"), + (proxy, "127.0.0.0/8"), + (String::new(), ""), + ] { + let listener = TcpListener::bind("127.0.0.1:0")?; + let url = format!("http://{}/repo", listener.local_addr()?); + let mut remote = Remote::default(); + remote.configure(&http::Options { + proxy: Some(proxy), + no_proxy: Some(no_proxy.into()), + ..Default::default() + })?; + let lines = request(&mut remote, &url, listener, false)?; + assert_eq!( + lines[0], "GET /repo HTTP/1.1", + "bypassed requests go directly to the origin" + ); + } + Ok(()) +} + +#[test] +fn reconfiguration_changes_proxy() -> gix_testtools::Result { + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + let _env = proxy_environment(); + let mut remote = Remote::default(); + for _ in 0..2 { + let listener = TcpListener::bind("127.0.0.1:0")?; + remote.configure(&http::Options { + proxy: Some(format!("http://{}", listener.local_addr()?)), + no_proxy: Some(String::new()), + ..Default::default() + })?; + request(&mut remote, "http://git.invalid/repo", listener, false)?; + } + Ok(()) +} + +fn check_proxy_credentials( + use_helper: bool, + bypass: bool, + status: Option, + https: bool, + environment_proxy: bool, +) -> gix_testtools::Result { + use std::sync::{Arc, Mutex}; + + use gix_credentials::{helper::Action, protocol::Context}; + + let listener = TcpListener::bind("127.0.0.1:0")?; + let address = listener.local_addr()?; + let mut options = http::Options { + proxy: Some(format!( + "http://user{}@{address}", + if use_helper { "" } else { ":p%3Aass" } + )), + proxy_auth_method: http::options::ProxyAuthMethod::Basic, + no_proxy: Some(if bypass { "127.0.0.1" } else { "" }.into()), + ..Default::default() + }; + let actions = Arc::new(Mutex::new(Vec::new())); + if use_helper { + let actions = actions.clone(); + options.proxy_authenticate = Some(( + Action::get_for_url(options.proxy.as_deref().expect("proxy is set")), + Arc::new(Mutex::new(move |action: Action| { + actions.lock().expect("no panics").push(action.as_arg(true).to_owned()); + if bypass { + return Ok(None); + } + Ok(if let Action::Get(_) = action { + Some(gix_credentials::protocol::Outcome { + identity: gix_sec::identity::Account { + username: "user".into(), + password: "p:ass".into(), + oauth_refresh_token: None, + }, + next: Context { + username: Some("user".into()), + password: Some("p:ass".into()), + ..Default::default() + } + .into(), + }) + } else { + None + }) + })), + )); + } + let _environment = if environment_proxy { + Some(gix_testtools::Env::new().set( + if https { "https_proxy" } else { "http_proxy" }, + options.proxy.take().expect("proxy is configured"), + )) + } else { + None + }; + let mut remote = H::default(); + remote.configure(&options)?; + let url = if bypass { + format!("http://{address}/repo") + } else if https { + "https://git.invalid/repo".into() + } else { + "http://git.invalid/repo".into() + }; + let server = serve(listener, status); + let mut response = remote.get(&url, &url, ["Accept: */*"])?; + let result = io::copy(&mut response.headers, &mut io::sink()); + assert_eq!( + result.is_ok(), + !https && status.is_some_and(|status| status < 400), + "HTTP errors and dropped connections are reported: {status:?}" + ); + io::copy(&mut response.body, &mut io::sink())?; + let lines = server.join().expect("recording server must not panic")?; + if https { + assert_eq!( + lines[0], "CONNECT git.invalid:443 HTTP/1.1", + "HTTPS authenticates the proxy tunnel" + ); + } + let proxy_auth = lines + .iter() + .find(|line| line.to_ascii_lowercase().starts_with("proxy-authorization:")); + assert_eq!( + proxy_auth.and_then(|line| line.split_once(':').map(|(_, value)| value.trim())), + (!bypass).then_some("Basic dXNlcjpwOmFzcw=="), + "decoded credentials authenticate only requests that use the proxy: helper={use_helper}, bypass={bypass}" + ); + assert!( + !lines + .iter() + .any(|line| line.to_ascii_lowercase().starts_with("authorization:")), + "proxy credentials must never become origin credentials" + ); + if use_helper { + let expected = match status { + _ if bypass => Vec::new(), + Some(407) => vec!["get", "erase"], + Some(_) if !https => vec!["get", "store"], + _ => vec!["get"], + }; + assert_eq!( + *actions.lock().expect("no panics"), + expected, + "only proxy authentication rejection invalidates credentials: status={status:?}, https={https}, response={result:?}" + ); + } + Ok(()) +} + +#[test] +fn basic_proxy_credentials_do_not_authenticate_the_origin() -> gix_testtools::Result { + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + let _env = proxy_environment(); + for bypass in [false, true] { + check_proxy_credentials::(false, bypass, Some(200), false, false)?; + } + Ok(()) +} + +#[test] +fn proxy_authentication_survives_http_redirects_without_leaking_to_bypassed_origins() -> gix_testtools::Result { + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + let _env = proxy_environment(); + for bypass in [false, true] { + let listener = TcpListener::bind("127.0.0.1:0")?; + let proxy_address = listener.local_addr()?; + let target = if bypass { + TcpListener::bind("127.0.0.1:0")? + } else { + listener.try_clone()? + }; + let target_url = if bypass { + format!("http://{}/repo", target.local_addr()?) + } else { + "http://redirected.invalid/repo".into() + }; + let server = std::thread::spawn(move || -> io::Result<_> { + let mut reader = io::BufReader::new(accept(listener)?); + let first = read_request_lines(&mut reader); + write!( + reader.get_mut(), + "HTTP/1.1 302 Found\r\nLocation: {target_url}\r\nContent-Length: 0\r\nConnection: close\r\n\r\n" + )?; + drop(reader); + let mut reader = io::BufReader::new(accept(target)?); + let second = read_request_lines(&mut reader); + reader + .get_mut() + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\nok")?; + Ok([first, second]) + }); + let mut remote = Remote::default(); + remote.configure(&http::Options { + proxy: Some(format!("http://user:pass@{proxy_address}")), + proxy_auth_method: http::options::ProxyAuthMethod::Basic, + no_proxy: Some(if bypass { "127.0.0.1" } else { "" }.into()), + ..Default::default() + })?; + let mut response = remote.get( + "http://git.invalid/repo", + "http://git.invalid/repo", + ["Authorization: Basic b3JpZ2luOnNlY3JldA=="], + )?; + io::copy(&mut response.headers, &mut io::sink())?; + io::copy(&mut response.body, &mut io::sink())?; + let [first, second] = server.join().expect("redirect proxy must not panic")?; + for (lines, uses_proxy) in [(&first, true), (&second, !bypass)] { + let auth = lines.iter().find_map(|line| { + let (name, value) = line.split_once(':')?; + name.eq_ignore_ascii_case("proxy-authorization").then(|| value.trim()) + }); + assert_eq!( + auth, + uses_proxy.then_some("Basic dXNlcjpwYXNz"), + "each proxied request is authenticated" + ); + } + assert!( + !second + .iter() + .any(|line| line.to_ascii_lowercase().starts_with("authorization:")), + "origin credentials do not cross authorities" + ); + } + Ok(()) +} + +#[cfg(feature = "http-client-reqwest")] +#[test] +fn reqwest_preserves_explicit_origin_authorization_with_url_credentials() -> gix_testtools::Result { + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + let _env = proxy_environment(); + for explicit in [false, true] { + let listener = TcpListener::bind("127.0.0.1:0")?; + let url = format!("http://user:pass@{}/repo", listener.local_addr()?); + let mut remote = http::reqwest::Remote::default(); + remote.configure(&http::Options { + proxy: Some(String::new()), + extra_headers: if explicit { + vec!["Authorization: Bearer explicit-token".into()] + } else { + Vec::new() + }, + ..Default::default() + })?; + let lines = request(&mut remote, &url, listener, false)?; + let authorization = lines.iter().find_map(|line| { + let (name, value) = line.split_once(':')?; + name.eq_ignore_ascii_case("authorization").then(|| value.trim()) + }); + assert_eq!( + authorization, + Some(if explicit { + "Bearer explicit-token" + } else { + "Basic dXNlcjpwYXNz" + }), + "explicit authentication takes precedence over URL credentials" + ); + } + Ok(()) +} + +#[cfg(feature = "http-client-reqwest")] +#[test] +fn reqwest_proxy_setup_errors_release_streamed_uploads() -> gix_testtools::Result { + use std::sync::{Arc, Mutex, mpsc}; + + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + let _env = proxy_environment().set("http_proxy", "http://invalid:port"); + for helper_error in [false, true] { + let (send, receive) = mpsc::channel(); + let worker = std::thread::spawn(move || { + let mut remote = http::reqwest::Remote::default(); + if helper_error { + remote + .configure(&http::Options { + proxy: Some("http://user@127.0.0.1:9".into()), + proxy_authenticate: Some(( + gix_credentials::helper::Action::get_for_url("http://user@127.0.0.1:9"), + Arc::new(Mutex::new(|_| { + Err( + gix_error::message("The handler asked to stop trying to obtain credentials") + .raise(), + ) + })), + )), + ..Default::default() + }) + .expect("options can be configured"); + } + let mut response = remote + .post( + "http://git.invalid/repo", + "http://git.invalid/repo", + ["Accept: */*"], + http::PostBodyDataKind::Unbounded, + ) + .expect("streamed requests return their pipes before sending the body"); + let write_error = response + .post_body + .write_all(b"pack") + .expect_err("failed setup releases the upload writer"); + drop(response.post_body); + let header_error = + io::copy(&mut response.headers, &mut io::sink()).expect_err("the setup error is reported"); + send.send((write_error.kind(), format!("{header_error:?}"))) + .expect("test receives the result"); + }); + let (kind, error) = receive.recv_timeout(Duration::from_secs(5))?; + worker.join().expect("upload worker must not panic"); + assert_eq!( + kind, + io::ErrorKind::BrokenPipe, + "the failed request closes its upload pipe" + ); + assert!( + error.contains(if helper_error { + "Could not obtain proxy credentials" + } else { + "Invalid proxy URL" + }), + "the original proxy setup error remains available: {error}" + ); + } + Ok(()) +} + +#[cfg(feature = "http-client-reqwest")] +#[test] +fn reqwest_proxy_redirects_preserve_post_body_rules() -> gix_testtools::Result { + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + let _env = proxy_environment(); + for streaming in [false, true] { + for status in [301, 302, 303, 307, 308] { + let listener = TcpListener::bind("127.0.0.1:0")?; + let address = listener.local_addr()?; + let keeps_body = status >= 307; + let follows = !streaming || !keeps_body; + let server = std::thread::spawn(move || -> io::Result<_> { + let mut reader = io::BufReader::new(accept(listener.try_clone()?)?); + read_request_lines(&mut reader); + let mut body = [0; 4]; + reader.read_exact(&mut body)?; + assert_eq!(&body, b"pack", "the first POST contains the upload"); + write!( + reader.get_mut(), + "HTTP/1.1 {status} Redirect\r\nLocation: http://redirected.invalid/repo\r\nContent-Length: 0\r\nConnection: close\r\n\r\n" + )?; + drop(reader); + if !follows { + return Ok(None); + } + let mut reader = io::BufReader::new(accept(listener)?); + let lines = read_request_lines(&mut reader); + let mut body = if keeps_body { vec![0; 4] } else { Vec::new() }; + reader.read_exact(&mut body)?; + reader + .get_mut() + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: close\r\n\r\n")?; + Ok(Some((lines, body))) + }); + let mut remote = http::reqwest::Remote::default(); + remote.configure(&http::Options { + proxy: Some(format!("http://{address}")), + no_proxy: Some(String::new()), + ..Default::default() + })?; + let mut response = remote.post( + "http://git.invalid/repo", + "http://git.invalid/repo", + [ + "Content-Length: 4", + "Content-Type: application/x-git-upload-pack-request", + ], + if streaming { + http::PostBodyDataKind::Unbounded + } else { + http::PostBodyDataKind::BoundedAndFitsIntoMemory + }, + )?; + response.post_body.write_all(b"pack")?; + drop(response.post_body); + io::copy(&mut response.headers, &mut io::sink())?; + io::copy(&mut response.body, &mut io::sink())?; + let redirected = server.join().expect("redirect server must not panic")?; + assert_eq!( + redirected.is_some(), + follows, + "streamed uploads cannot be replayed for {status}" + ); + if let Some((lines, body)) = redirected { + let method = if keeps_body { "POST" } else { "GET" }; + assert_eq!( + lines[0], + format!("{method} http://redirected.invalid/repo HTTP/1.1"), + "redirect method for {status}" + ); + assert_eq!( + body.as_slice(), + if keeps_body { b"pack".as_slice() } else { b"" }, + "redirect body for {status}" + ); + assert_eq!( + lines + .iter() + .any(|line| line.to_ascii_lowercase().starts_with("content-type:")), + keeps_body, + "redirects to GET drop the upload headers" + ); + } + } + } + Ok(()) +} + +#[cfg(feature = "http-client-reqwest")] +#[test] +fn reqwest_proxy_credentials_come_from_helpers() -> gix_testtools::Result { + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + let _env = proxy_environment(); + for status in [Some(200), Some(201), Some(401), Some(404), Some(500), Some(407), None] { + check_proxy_credentials::(true, false, status, false, false)?; + } + check_proxy_credentials::(true, true, Some(200), false, false)?; + for status in [Some(200), Some(407)] { + check_proxy_credentials::(true, false, status, false, true)?; + } + #[cfg(any( + feature = "http-client-reqwest-rust-tls", + feature = "http-client-reqwest-rust-tls-trust-dns", + feature = "http-client-reqwest-native-tls" + ))] + for status in [Some(407), Some(403), None] { + check_proxy_credentials::(true, false, status, true, false)?; + check_proxy_credentials::(true, false, status, true, true)?; + } + Ok(()) +} + +#[cfg(feature = "http-client-reqwest")] +#[test] +fn reqwest_rejects_unsupported_proxy_configuration_on_every_attempt() -> gix_testtools::Result { + use http::options::ProxyAuthMethod; + + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + let _env = proxy_environment(); + for (proxy, method, message) in [ + ( + "htpp://proxy.invalid", + ProxyAuthMethod::AnyAuth, + "Unsupported proxy scheme", + ), + ( + "http://proxy.invalid:invalid", + ProxyAuthMethod::AnyAuth, + "Invalid proxy URL", + ), + ( + "socks5://localhost/proxy.sock", + ProxyAuthMethod::AnyAuth, + "Unix socket proxy paths", + ), + ( + "socks4://alice@proxy.invalid", + ProxyAuthMethod::AnyAuth, + "SOCKS4 proxy user IDs", + ), + ( + "socks4a://alice@proxy.invalid", + ProxyAuthMethod::AnyAuth, + "SOCKS4 proxy user IDs", + ), + ( + "http://user:secret@proxy.invalid", + ProxyAuthMethod::Digest, + "only supports Basic", + ), + ( + "http://user:secret@proxy.invalid", + ProxyAuthMethod::Negotiate, + "only supports Basic", + ), + ( + "http://user:secret@proxy.invalid", + ProxyAuthMethod::Ntlm, + "only supports Basic", + ), + ] { + let mut remote = http::reqwest::Remote::default(); + remote.configure(&http::Options { + proxy: Some(proxy.into()), + proxy_auth_method: method, + ..Default::default() + })?; + for _ in 0..2 { + let error = match remote.get("http://127.0.0.1:9/repo", "http://127.0.0.1:9/repo", ["Accept: */*"]) { + Ok(mut response) => format!( + "{:?}", + io::copy(&mut response.headers, &mut io::sink()) + .expect_err("invalid proxy settings must fail on every request") + ), + Err(error) => format!("{error:?}"), + }; + assert!( + format!("{error:?}").contains(message), + "expected {message:?}, got {error:?}" + ); + } + } + Ok(()) +} + +#[test] +fn environment_proxy_fallback_and_precedence() -> gix_testtools::Result { + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + for variable in ["http_proxy", "all_proxy", "ALL_PROXY"] { + let listener = TcpListener::bind("127.0.0.1:0")?; + let environment = proxy_environment().set(variable, format!("http://{}", listener.local_addr()?)); + let environment = if variable != "http_proxy" { + environment.set("http_proxy", "") + } else { + environment + }; + #[cfg(not(windows))] + let environment = if variable == "ALL_PROXY" { + environment.set("all_proxy", "") + } else { + environment + }; + // Windows environment variable names are case-insensitive. + #[cfg(not(windows))] + let environment = environment + .set("HTTP_PROXY", "http://127.0.0.1:9") + .set("no_proxy", "example.invalid") + .set("NO_PROXY", "*"); + let _env = environment; + request(&mut Remote::default(), "http://git.invalid/repo", listener, false)?; + } + Ok(()) +} + +// Reqwest 0.13's connector rejects SOCKS URLs when built without TLS. +#[cfg(any( + feature = "http-client-curl", + feature = "http-client-reqwest-rust-tls", + feature = "http-client-reqwest-rust-tls-trust-dns", + feature = "http-client-reqwest-native-tls" +))] +#[test] +fn socks5h_resolves_the_destination_at_the_proxy() -> gix_testtools::Result { + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + let _env = proxy_environment(); + let listener = TcpListener::bind("127.0.0.1:0")?; + let mut remote = Remote::default(); + remote.configure(&http::Options { + proxy: Some(format!("socks5h://{}", listener.local_addr()?)), + no_proxy: Some(String::new()), + ..Default::default() + })?; + let server = std::thread::spawn(move || -> gix_testtools::Result { + let mut stream = accept(listener)?; + let mut greeting = [0; 2]; + stream.read_exact(&mut greeting)?; + assert_eq!(greeting[0], 5, "SOCKS5 negotiation"); + let mut methods = vec![0; usize::from(greeting[1])]; + stream.read_exact(&mut methods)?; + assert!(methods.contains(&0), "the proxy can select unauthenticated access"); + stream.write_all(&[5, 0])?; + let mut connect = [0; 5]; + stream.read_exact(&mut connect)?; + assert_eq!(&connect[..4], [5, 1, 0, 3], "SOCKS5h sends a domain, without local DNS"); + let mut host = vec![0; usize::from(connect[4])]; + stream.read_exact(&mut host)?; + assert_eq!(host, b"git.invalid", "the proxy receives the original hostname"); + let mut port = [0; 2]; + stream.read_exact(&mut port)?; + assert_eq!( + u16::from_be_bytes(port), + 80, + "the proxy connects to the destination port" + ); + // Reject after recording the request; no external server is needed. + stream.write_all(&[5, 4, 0, 1, 0, 0, 0, 0, 0, 0])?; + Ok(()) + }); + let mut response = remote.get("http://git.invalid/repo", "http://git.invalid/repo", ["Accept: */*"])?; + let error = io::copy(&mut response.headers, &mut io::sink()).expect_err("the proxy rejection is reported"); + server + .join() + .expect("SOCKS server must not panic") + .map_err(|server_error| format!("SOCKS server failed: {server_error}; client error: {error:?}"))?; + Ok(()) +} + +#[cfg(any( + feature = "http-client-curl-rust-tls", + feature = "http-client-curl-openssl", + feature = "http-client-reqwest-rust-tls", + feature = "http-client-reqwest-rust-tls-trust-dns", + feature = "http-client-reqwest-native-tls" +))] +#[test] +fn redirects_select_environment_proxy_for_new_scheme() -> gix_testtools::Result { + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + for use_http_proxy in [false, true] { + let http_listener = TcpListener::bind("127.0.0.1:0")?; + let https_proxy = TcpListener::bind("127.0.0.1:0")?; + let http_address = http_listener.local_addr()?; + let environment = proxy_environment().set("HTTPS_PROXY", format!("http://{}", https_proxy.local_addr()?)); + let _env = if use_http_proxy { + environment.set("http_proxy", format!("http://{http_address}")) + } else { + environment + }; + let redirect = std::thread::spawn(move || -> io::Result> { + let mut reader = io::BufReader::new(accept(http_listener)?); + let lines = read_request_lines(&mut reader); + reader.get_mut().write_all( + b"HTTP/1.1 302 Found\r\nLocation: https://git.invalid/repo\r\nContent-Length: 0\r\nConnection: close\r\n\r\n", + )?; + Ok(lines) + }); + let tunnel = serve(https_proxy, Some(403)); + let url = if use_http_proxy { + "http://git.invalid/repo".into() + } else { + format!("http://{http_address}/repo") + }; + let mut remote = Remote::default(); + let mut response = remote.get(&url, &url, ["Accept: */*"])?; + let error = io::copy(&mut response.headers, &mut io::sink()).expect_err("the proxy rejected the tunnel"); + let lines = redirect.join().expect("redirect server must not panic")?; + assert_eq!( + lines[0], + if use_http_proxy { + "GET http://git.invalid/repo HTTP/1.1" + } else { + "GET /repo HTTP/1.1" + }, + "the initial request uses the HTTP proxy only when configured" + ); + let lines = tunnel + .join() + .expect("CONNECT server must not panic") + .map_err(|server_error| format!("HTTPS proxy was not used: {server_error}; client error: {error:?}"))?; + assert_eq!( + lines[0], "CONNECT git.invalid:443 HTTP/1.1", + "the redirect selects HTTPS_PROXY, independently of the initial HTTP proxy" + ); + } + Ok(()) +} + +#[cfg(any( + feature = "http-client-curl-rust-tls", + feature = "http-client-curl-openssl", + feature = "http-client-reqwest-rust-tls", + feature = "http-client-reqwest-rust-tls-trust-dns", + feature = "http-client-reqwest-native-tls" +))] +#[test] +fn https_uses_connect_and_https_proxy_environment() -> gix_testtools::Result { + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + let listener = TcpListener::bind("127.0.0.1:0")?; + let _env = proxy_environment() + .set("http_proxy", "http://proxy.invalid:invalid") + .set("HTTPS_PROXY", format!("http://user:pass@{}", listener.local_addr()?)); + let server = std::thread::spawn(move || -> io::Result> { + let mut reader = io::BufReader::new(accept(listener)?); + let lines = read_request_lines(&mut reader); + reader + .get_mut() + .write_all(b"HTTP/1.1 403 Forbidden\r\nContent-Length: 0\r\nConnection: close\r\n\r\n")?; + Ok(lines) + }); + let mut remote = Remote::default(); + remote.configure(&http::Options { + proxy_auth_method: http::options::ProxyAuthMethod::Basic, + no_proxy: Some(String::new()), + ..Default::default() + })?; + let mut response = remote.get("https://git.invalid/repo", "https://git.invalid/repo", ["Accept: */*"])?; + assert!( + io::copy(&mut response.headers, &mut io::sink()).is_err(), + "the proxy rejected the tunnel" + ); + let lines = server.join().expect("CONNECT server must not panic")?; + assert_eq!( + lines[0], "CONNECT git.invalid:443 HTTP/1.1", + "HTTPS uses a proxy tunnel" + ); + assert!( + lines.iter().any(|line| { + line.split_once(':').is_some_and(|(name, value)| { + name.eq_ignore_ascii_case("proxy-authorization") && value.trim() == "Basic dXNlcjpwYXNz" + }) + }), + "proxy credentials are attached to CONNECT: {lines:?}" + ); + Ok(()) +} + +#[cfg(any( + feature = "http-client-reqwest-rust-tls", + feature = "http-client-reqwest-rust-tls-trust-dns", + feature = "http-client-reqwest-native-tls" +))] +#[test] +fn reqwest_defers_invalid_https_proxy_until_redirect() -> gix_testtools::Result { + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + let _env = proxy_environment().set("HTTPS_PROXY", "socks5://localhost/proxy.sock"); + let listener = TcpListener::bind("127.0.0.1:0")?; + let url = format!("http://{}/repo", listener.local_addr()?); + let mut remote = http::reqwest::Remote::default(); + remote.configure(&http::Options { + follow_redirects: http::options::FollowRedirects::All, + ..Default::default() + })?; + request(&mut remote, &url, listener.try_clone()?, false)?; + + let redirect = std::thread::spawn(move || -> io::Result<()> { + let mut reader = io::BufReader::new(accept(listener)?); + read_request_lines(&mut reader); + reader.get_mut().write_all( + b"HTTP/1.1 302 Found\r\nLocation: https://git.invalid/repo\r\nContent-Length: 0\r\nConnection: close\r\n\r\n", + ) + }); + let mut response = remote.get(&url, &url, ["Accept: */*"])?; + let error = io::copy(&mut response.headers, &mut io::sink()).expect_err("the HTTPS proxy is invalid"); + assert!( + format!("{error:?}").contains("Unix socket proxy paths"), + "the cached client rejects the redirect before sending an unproxied request: {error:?}" + ); + redirect.join().expect("redirect server must not panic")?; + Ok(()) +} + +#[cfg(feature = "http-client-reqwest")] +#[test] +fn reqwest_checks_proxy_errors_after_request_url_changes() -> gix_testtools::Result { + use std::sync::{Arc, Mutex}; + + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + let _env = proxy_environment().set("HTTPS_PROXY", "socks5://localhost/proxy.sock"); + let mut remote = http::reqwest::Remote::default(); + remote.configure(&http::Options { + backend: Some(Arc::new(Mutex::new(http::reqwest::Options { + configure_request: Some(Box::new(|request| { + *request.url_mut() = "https://git.invalid/repo".parse::().or_error()?; + Ok(()) + })), + }))), + ..Default::default() + })?; + let mut response = remote.get("http://git.invalid/repo", "http://git.invalid/repo", ["Accept: */*"])?; + let error = io::copy(&mut response.headers, &mut io::sink()).expect_err("the HTTPS proxy is invalid"); + assert!( + format!("{error:?}").contains("Unix socket proxy paths"), + "request URL changes must not bypass proxy validation: {error:?}" + ); + Ok(()) +} diff --git a/gix/src/config/tree/sections/http.rs b/gix/src/config/tree/sections/http.rs index 737592e8aee..78821673149 100644 --- a/gix/src/config/tree/sections/http.rs +++ b/gix/src/config/tree/sections/http.rs @@ -14,12 +14,12 @@ impl Http { pub const SSL_VERIFY: keys::Boolean = keys::Boolean::new_boolean("sslVerify", &config::Tree::HTTP) .with_note("also see the `gitoxide.http.sslNoVerify` key"); /// The `http.proxy` key. - pub const PROXY: keys::String = - keys::String::new_string("proxy", &config::Tree::HTTP).with_deviation("fails on strings with illformed UTF-8"); + pub const PROXY: keys::String = keys::String::new_string("proxy", &config::Tree::HTTP) + .with_deviation("fails on strings with illformed UTF-8; the reqwest backend rejects Unix socket proxy paths and SOCKS4 user IDs"); /// The `http.proxyAuthMethod` key. pub const PROXY_AUTH_METHOD: ProxyAuthMethod = ProxyAuthMethod::new_proxy_auth_method("proxyAuthMethod", &config::Tree::HTTP) - .with_deviation("implemented like git, but never actually tried"); + .with_deviation("the reqwest backend uses preemptive Basic authentication for anyauth/basic and rejects other HTTP proxy authentication methods"); /// The `http.version` key. pub const VERSION: Version = Version::new_with_validate("version", &config::Tree::HTTP, validate::Version) .with_deviation("fails on illformed UTF-8"); diff --git a/gix/src/repository/config/transport.rs b/gix/src/repository/config/transport.rs index fe2534ef9f0..bdb19f18ed0 100644 --- a/gix/src/repository/config/transport.rs +++ b/gix/src/repository/config/transport.rs @@ -168,13 +168,20 @@ impl crate::Repository { .try_into_u32(config.integer_filter("http.lowSpeedLimit", &mut trusted_only)) .with_leniency(lenient)? .unwrap_or_default(); + // Reqwest selects environment proxies per destination, including after redirects. + // Keep curl's existing configuration and credential-helper setup. + let mut explicit_proxy = |meta: &gix_config::file::Metadata| { + (cfg!(feature = "blocking-http-transport-curl") + || meta.source != gix_config::Source::EnvOverride) + && trusted_only(meta) + }; opts.proxy = proxy( remote_name .and_then(|name| { config .string_filter( &format!("remote.{}.{}", name, Remote::PROXY.name), - &mut trusted_only, + &mut explicit_proxy, ) .map(|v| (v, Some(name), &Remote::PROXY)) }) @@ -182,13 +189,13 @@ impl crate::Repository { let key = "http.proxy"; debug_assert_eq!(key, config::tree::Http::PROXY.logical_name()); let http_proxy = config - .string_filter(key, &mut trusted_only) + .string_filter(key, &mut explicit_proxy) .map(|v| (v, None, &config::tree::Http::PROXY)) .or_else(|| { let key = "gitoxide.http.proxy"; debug_assert_eq!(key, gitoxide::Http::PROXY.logical_name()); config - .string_filter(key, &mut trusted_only) + .string_filter(key, &mut explicit_proxy) .map(|v| (v, None, &gitoxide::Http::PROXY)) }); if url.scheme == Https { @@ -196,7 +203,7 @@ impl crate::Repository { let key = "gitoxide.https.proxy"; debug_assert_eq!(key, gitoxide::Https::PROXY.logical_name()); config - .string_filter(key, &mut trusted_only) + .string_filter(key, &mut explicit_proxy) .map(|v| (v, None, &gitoxide::Https::PROXY)) }) } else { @@ -207,7 +214,7 @@ impl crate::Repository { let key = "gitoxide.http.allProxy"; debug_assert_eq!(key, gitoxide::Http::ALL_PROXY.logical_name()); config - .string_filter(key, &mut trusted_only) + .string_filter(key, &mut explicit_proxy) .map(|v| (v, None, &gitoxide::Http::ALL_PROXY)) }), lenient, @@ -216,7 +223,7 @@ impl crate::Repository { let key = "gitoxide.http.noProxy"; debug_assert_eq!(key, gitoxide::Http::NO_PROXY.logical_name()); opts.no_proxy = config - .string_filter(key, &mut trusted_only) + .string_filter(key, &mut explicit_proxy) .and_then(|v| try_to_string(v, lenient, None, &gitoxide::Http::NO_PROXY).transpose()) .transpose()?; } @@ -267,6 +274,84 @@ impl crate::Repository { )) }) .transpose()?; + #[cfg(all( + feature = "blocking-http-transport-reqwest", + not(feature = "blocking-http-transport-curl") + ))] + if opts.proxy.is_none() { + use crate::bstr::ByteSlice; + use gix_error::ErrorExt; + + // Prepare only the imported proxy candidates: the callback must be Send + Sync, + // whereas the repository/configuration can use Rc in non-parallel builds. + let mut candidates = Vec::new(); + for key in ["gitoxide.http.proxy", "gitoxide.https.proxy", "gitoxide.http.allProxy"] { + for value in config + .strings_filter(key, &mut |meta: &gix_config::file::Metadata| { + meta.source == gix_config::Source::EnvOverride && trusted_only(meta) + }) + .into_iter() + .flatten() + { + let Some(value) = value.to_str().ok().filter(|value| !value.is_empty()) else { + continue; + }; + let value = if value.contains("://") { + value.to_owned() + } else { + format!("http://{value}") + }; + let Ok(mut url) = gix_url::parse(value.as_str()) else { + continue; // The backend reports invalid settings only when selected. + }; + if url.user().is_none() { + continue; + } + let helpers = + self.config_snapshot().credential_helpers(url.clone()).map_err(Arc::new); + // Match the backend's routing URL, but preserve the original declaration for + // URL-specific credential configuration and credential-store lookups. + if url.scheme == Http && url.port.is_none() { + url.port = Some(1080); + } + candidates.push((url.to_bstring(), helpers)); + } + } + if let Some((first_url, _)) = candidates.first() { + let action = gix_credentials::helper::Action::get_for_url(first_url.clone()); + let mut selected = None; + opts.proxy_authenticate = Some(( + action, + Arc::new(Mutex::new(move |action: gix_credentials::helper::Action| { + let get = action.expects_output(); + if get { + let url = action + .context() + .and_then(|context| context.url.clone().or_else(|| context.to_url())) + .ok_or_else(|| { + gix_error::message("The URL for proxy authentication is missing") + .raise() + })?; + let url = gix_url::parse(&url)?.to_bstring(); + selected = candidates.iter().position(|(candidate, _)| candidate == &url); + } + let Some(index) = selected else { return Ok(None) }; + let (_, helpers) = &mut candidates[index]; + let (cascade, normalized_action, prompt_opts) = + helpers.as_mut().map_err(|source| { + source.clone().and_raise(gix_error::message( + "Could not configure credential helpers for the proxy URL", + )) + })?; + cascade.invoke( + if get { normalized_action.clone() } else { action }, + prompt_opts.clone(), + ) + })) + as Arc>, + )); + } + } opts.connect_timeout = { let key = "gitoxide.http.connectTimeout"; debug_assert_eq!(key, gitoxide::Http::CONNECT_TIMEOUT.logical_name()); diff --git a/gix/tests/gix/repository/config/transport_options.rs b/gix/tests/gix/repository/config/transport_options.rs index faf9ed82d0c..1f48caf9822 100644 --- a/gix/tests/gix/repository/config/transport_options.rs +++ b/gix/tests/gix/repository/config/transport_options.rs @@ -23,6 +23,100 @@ mod http { .to_owned() } + #[cfg(all( + feature = "blocking-http-transport-reqwest", + not(feature = "blocking-http-transport-curl") + ))] + #[test] + fn environment_proxy_selection_is_left_to_the_backend() -> gix_testtools::Result { + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + let _environment = gix_testtools::Env::new() + .set("http_proxy", "http://http.invalid") + .set("HTTPS_PROXY", "http://https.invalid") + .set("ALL_PROXY", "http://all.invalid") + .set("no_proxy", "env.invalid"); + for (fixture, proxy, no_proxy) in [ + ("http-verbose", None, None), + ("http-proxy-empty", Some(""), None), + ("gitoxide-http-proxy-only", Some("http://http-fallback"), None), + ("http-no-proxy", None, Some("no validation done here")), + ] { + let repo = repo_opts(fixture, |mut opts| { + opts.permissions.env.http_transport = gix::sec::Permission::Allow; + opts + }); + assert!( + repo.config_snapshot().string("gitoxide.http.proxy").is_some(), + "the configuration snapshot imports proxy environment variables" + ); + for url in ["http://host.local/repo", "https://host.local/repo"] { + let opts = http_options(&repo, None, url); + assert_eq!( + opts.proxy.as_deref(), + proxy, + "only explicit proxy settings may override per-scheme environment routing: {fixture}, {url}" + ); + assert_eq!( + opts.no_proxy.as_deref(), + no_proxy, + "explicit bypass settings override the environment: {fixture}" + ); + } + } + Ok(()) + } + + #[test] + fn environment_proxies_keep_credential_helpers() -> gix_testtools::Result { + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + let _environment = gix_testtools::Env::new() + .set("http_proxy", "http://one@http.invalid:3128") + .set("https_proxy", "http://two@https.invalid:3129") + .set("HTTPS_PROXY", "http://two@https.invalid:3129") + .set("all_proxy", "http://three@all.invalid") + .set("ALL_PROXY", "http://three@all.invalid"); + let repo = repo_opts("http-verbose", |mut opts| { + opts.permissions.env.http_transport = gix::sec::Permission::Allow; + opts.config_overrides([ + "credential.helper=", + "credential.http://http.invalid:3128.helper=!f() { echo password=http-secret; }; f", + "credential.http://https.invalid:3129.helper=!f() { echo password=https-secret; }; f", + "credential.http://all.invalid.helper=!f() { echo password=all-secret; }; f", + "gitoxide.credentials.terminalPrompt=false", + ]) + }); + let opts = http_options(&repo, None, "http://host.local/repo"); + let (_, authenticate) = opts + .proxy_authenticate + .as_ref() + .expect("environment proxy usernames still enable credential helpers"); + let cases = [ + ("http://one@http.invalid:3128", "http-secret"), + ("http://two@https.invalid:3129", "https-secret"), + ("http://three@all.invalid:1080", "all-secret"), + ]; + let count = if cfg!(feature = "blocking-http-transport-curl") { + 1 + } else { + cases.len() + }; + for (url, password) in cases.into_iter().take(count) { + let credentials = + authenticate.lock().expect("no panics")(gix::credentials::helper::Action::get_for_url(url))? + .expect("the selected proxy has a credential helper"); + assert_eq!( + credentials.identity.password, password, + "helper selection follows the proxy URL" + ); + authenticate.lock().expect("no panics")(credentials.next.store())?; + } + Ok(()) + } + #[test] fn remote_overrides() { let repo = repo("http-remote-override"); From ee771d89c513d48962844467e38307a2332b9bce Mon Sep 17 00:00:00 2001 From: Codex GPT-6 Date: Thu, 24 Sep 2026 00:06:07 +0200 Subject: [PATCH 2/2] fix(gix-transport): honor proxy helper credentials with `curl` Libcurl prioritizes user information in `http.proxy` over `CURLOPT_PROXYUSERNAME` and `CURLOPT_PROXYPASSWORD`. A URL such as `http://user@proxy` therefore discarded the helper's password and sent `user:` instead. Like Git, remove URL credentials before passing the proxy to libcurl when a helper is configured, and keep using the dedicated credential options. Preserve explicit empty proxies and use `gix-url` to retain the proxy scheme, host, port and path. Run the existing recording-proxy credential check against both backends. It reproduced the empty password with curl before the fix; reqwest's helper credentials already worked for HTTP, HTTPS CONNECT, and environment-selected proxies. Validation: 39 curl and 46 reqwest transport tests passed with Rust TLS, along with formatting and strict library Clippy. Clippy reports the existing removed-lint warning. --- .../src/client/blocking_io/http/curl/remote.rs | 13 ++++++++++++- .../tests/client/blocking_io/http/proxy.rs | 1 + 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/gix-transport/src/client/blocking_io/http/curl/remote.rs b/gix-transport/src/client/blocking_io/http/curl/remote.rs index b434606f1fb..2ed245e0913 100644 --- a/gix-transport/src/client/blocking_io/http/curl/remote.rs +++ b/gix-transport/src/client/blocking_io/http/curl/remote.rs @@ -493,7 +493,18 @@ pub fn new() -> Worker { } let mut proxy_auth_action = None; - if let Some(proxy) = proxy { + if let Some(mut proxy) = proxy { + if proxy_authenticate.is_some() && !proxy.is_empty() { + if !proxy.contains("://") { + proxy.insert_str(0, "http://"); + } + let mut proxy_url = + gix_url::parse(proxy.as_str()).or_raise(|| message("Could not parse proxy URL"))?; + // Libcurl gives URL credentials precedence over the helper's username/password options. + proxy_url.user = None; + proxy_url.password = None; + proxy = proxy_url.to_bstring().to_string(); + } curl!(handle.proxy(&proxy)); let proxy_type = if proxy.starts_with("socks5h") { curl::easy::ProxyType::Socks5Hostname diff --git a/gix-transport/tests/client/blocking_io/http/proxy.rs b/gix-transport/tests/client/blocking_io/http/proxy.rs index 4bf0b680d2e..603648de011 100644 --- a/gix-transport/tests/client/blocking_io/http/proxy.rs +++ b/gix-transport/tests/client/blocking_io/http/proxy.rs @@ -360,6 +360,7 @@ fn basic_proxy_credentials_do_not_authenticate_the_origin() -> gix_testtools::Re for bypass in [false, true] { check_proxy_credentials::(false, bypass, Some(200), false, false)?; } + check_proxy_credentials::(true, false, Some(200), false, false)?; Ok(()) }