From 1971cb642ab190cbe929e78e608f6f37e89e0d12 Mon Sep 17 00:00:00 2001 From: bors Date: Wed, 7 Jun 2023 21:16:46 +0000 Subject: [PATCH 1/2] Auto merge of #12234 - weihanglo:multiplexing, r=ehuss fix: disable multiplexing for some versions of curl --- src/bin/cargo/main.rs | 4 -- src/cargo/ops/registry.rs | 52 +++++---------------- src/cargo/util/config/mod.rs | 81 ++++++++++++++++++++++++++++++++- src/cargo/util/network/mod.rs | 1 + src/cargo/util/network/proxy.rs | 42 +++++++++++++++++ 5 files changed, 133 insertions(+), 47 deletions(-) create mode 100644 src/cargo/util/network/proxy.rs diff --git a/src/bin/cargo/main.rs b/src/bin/cargo/main.rs index 55da2997fdb..9fb6635ea77 100644 --- a/src/bin/cargo/main.rs +++ b/src/bin/cargo/main.rs @@ -293,10 +293,6 @@ fn init_git(config: &Config) { /// configured to use libcurl instead of the built-in networking support so /// that those configuration settings can be used. fn init_git_transports(config: &Config) { - // Only use a custom transport if any HTTP options are specified, - // such as proxies or custom certificate authorities. The custom - // transport, however, is not as well battle-tested. - match cargo::ops::needs_custom_http_transport(config) { Ok(true) => {} _ => return, diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 89d8b274738..a8efb549274 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -35,6 +35,7 @@ use crate::util::auth::{ use crate::util::config::{Config, SslVersionConfig, SslVersionConfigRange}; use crate::util::errors::CargoResult; use crate::util::important_paths::find_root_manifest_for_wd; +use crate::util::network; use crate::util::{truncate_with_ellipsis, IntoUrl}; use crate::util::{Progress, ProgressStyle}; use crate::{drop_print, drop_println, version}; @@ -611,16 +612,22 @@ pub fn http_handle_and_timeout(config: &Config) -> CargoResult<(Easy, HttpTimeou Ok((handle, timeout)) } +// Only use a custom transport if any HTTP options are specified, +// such as proxies or custom certificate authorities. +// +// The custom transport, however, is not as well battle-tested. pub fn needs_custom_http_transport(config: &Config) -> CargoResult { - Ok(http_proxy_exists(config)? - || *config.http_config()? != Default::default() - || config.get_env_os("HTTP_TIMEOUT").is_some()) + Ok( + network::proxy::http_proxy_exists(config.http_config()?, config) + || *config.http_config()? != Default::default() + || config.get_env_os("HTTP_TIMEOUT").is_some(), + ) } /// Configure a libcurl http handle with the defaults options for Cargo pub fn configure_http_handle(config: &Config, handle: &mut Easy) -> CargoResult { let http = config.http_config()?; - if let Some(proxy) = http_proxy(config)? { + if let Some(proxy) = network::proxy::http_proxy(http) { handle.proxy(&proxy)?; } if let Some(cainfo) = &http.cainfo { @@ -773,43 +780,6 @@ impl HttpTimeout { } } -/// Finds an explicit HTTP proxy if one is available. -/// -/// Favor cargo's `http.proxy`, then git's `http.proxy`. Proxies specified -/// via environment variables are picked up by libcurl. -fn http_proxy(config: &Config) -> CargoResult> { - let http = config.http_config()?; - if let Some(s) = &http.proxy { - return Ok(Some(s.clone())); - } - if let Ok(cfg) = git2::Config::open_default() { - if let Ok(s) = cfg.get_string("http.proxy") { - return Ok(Some(s)); - } - } - Ok(None) -} - -/// Determine if an http proxy exists. -/// -/// Checks the following for existence, in order: -/// -/// * cargo's `http.proxy` -/// * git's `http.proxy` -/// * `http_proxy` env var -/// * `HTTP_PROXY` env var -/// * `https_proxy` env var -/// * `HTTPS_PROXY` env var -fn http_proxy_exists(config: &Config) -> CargoResult { - if http_proxy(config)?.is_some() { - Ok(true) - } else { - Ok(["http_proxy", "HTTP_PROXY", "https_proxy", "HTTPS_PROXY"] - .iter() - .any(|v| config.get_env(v).is_ok())) - } -} - pub fn registry_login( config: &Config, token: Option>, diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index f080f9a99e4..076b7829949 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -1717,8 +1717,12 @@ impl Config { } pub fn http_config(&self) -> CargoResult<&CargoHttpConfig> { - self.http_config - .try_borrow_with(|| self.get::("http")) + self.http_config.try_borrow_with(|| { + let mut http = self.get::("http")?; + let curl_v = curl::Version::get(); + disables_multiplexing_for_bad_curl(curl_v.version(), &mut http, self); + Ok(http) + }) } pub fn future_incompat_config(&self) -> CargoResult<&CargoFutureIncompatConfig> { @@ -2731,3 +2735,76 @@ impl Tool { } } } + +/// Disable HTTP/2 multiplexing for some broken versions of libcurl. +/// +/// In certain versions of libcurl when proxy is in use with HTTP/2 +/// multiplexing, connections will continue stacking up. This was +/// fixed in libcurl 8.0.0 in curl/curl@821f6e2a89de8aec1c7da3c0f381b92b2b801efc +/// +/// However, Cargo can still link against old system libcurl if it is from a +/// custom built one or on macOS. For those cases, multiplexing needs to be +/// disabled when those versions are detected. +fn disables_multiplexing_for_bad_curl( + curl_version: &str, + http: &mut CargoHttpConfig, + config: &Config, +) { + use crate::util::network; + + if network::proxy::http_proxy_exists(http, config) && http.multiplexing.is_none() { + let bad_curl_versions = ["7.87.0", "7.88.0", "7.88.1"]; + if bad_curl_versions + .iter() + .any(|v| curl_version.starts_with(v)) + { + log::info!("disabling multiplexing with proxy, curl version is {curl_version}"); + http.multiplexing = Some(false); + } + } +} + +#[cfg(test)] +mod tests { + use super::disables_multiplexing_for_bad_curl; + use super::CargoHttpConfig; + use super::Config; + use super::Shell; + + #[test] + fn disables_multiplexing() { + let mut config = Config::new(Shell::new(), "".into(), "".into()); + config.set_search_stop_path(std::path::PathBuf::new()); + config.set_env(Default::default()); + + let mut http = CargoHttpConfig::default(); + http.proxy = Some("127.0.0.1:3128".into()); + disables_multiplexing_for_bad_curl("7.88.1", &mut http, &config); + assert_eq!(http.multiplexing, Some(false)); + + let cases = [ + (None, None, "7.87.0", None), + (None, None, "7.88.0", None), + (None, None, "7.88.1", None), + (None, None, "8.0.0", None), + (Some("".into()), None, "7.87.0", Some(false)), + (Some("".into()), None, "7.88.0", Some(false)), + (Some("".into()), None, "7.88.1", Some(false)), + (Some("".into()), None, "8.0.0", None), + (Some("".into()), Some(false), "7.87.0", Some(false)), + (Some("".into()), Some(false), "7.88.0", Some(false)), + (Some("".into()), Some(false), "7.88.1", Some(false)), + (Some("".into()), Some(false), "8.0.0", Some(false)), + ]; + + for (proxy, multiplexing, curl_v, result) in cases { + let mut http = CargoHttpConfig { + multiplexing, + proxy, + ..Default::default() + }; + disables_multiplexing_for_bad_curl(curl_v, &mut http, &config); + assert_eq!(http.multiplexing, result); + } + } +} diff --git a/src/cargo/util/network/mod.rs b/src/cargo/util/network/mod.rs index 60a380343b7..2006bb65fb6 100644 --- a/src/cargo/util/network/mod.rs +++ b/src/cargo/util/network/mod.rs @@ -2,6 +2,7 @@ use std::task::Poll; +pub mod proxy; pub mod retry; pub mod sleep; diff --git a/src/cargo/util/network/proxy.rs b/src/cargo/util/network/proxy.rs new file mode 100644 index 00000000000..e305cf1ff8e --- /dev/null +++ b/src/cargo/util/network/proxy.rs @@ -0,0 +1,42 @@ +//! Utilities for network proxies. + +use crate::util::config::CargoHttpConfig; +use crate::util::config::Config; + +/// Proxy environment variables that are picked up by libcurl. +const LIBCURL_HTTP_PROXY_ENVS: [&str; 4] = + ["http_proxy", "HTTP_PROXY", "https_proxy", "HTTPS_PROXY"]; + +/// Finds an explicit HTTP proxy if one is available. +/// +/// Favor [Cargo's `http.proxy`], then [Git's `http.proxy`]. +/// Proxies specified via environment variables are picked up by libcurl. +/// See [`LIBCURL_HTTP_PROXY_ENVS`]. +/// +/// [Cargo's `http.proxy`]: https://doc.rust-lang.org/nightly/cargo/reference/config.html#httpproxy +/// [Git's `http.proxy`]: https://git-scm.com/docs/git-config#Documentation/git-config.txt-httpproxy +pub fn http_proxy(http: &CargoHttpConfig) -> Option { + if let Some(s) = &http.proxy { + return Some(s.into()); + } + git2::Config::open_default() + .and_then(|cfg| cfg.get_string("http.proxy")) + .ok() +} + +/// Determine if an http proxy exists. +/// +/// Checks the following for existence, in order: +/// +/// * Cargo's `http.proxy` +/// * Git's `http.proxy` +/// * `http_proxy` env var +/// * `HTTP_PROXY` env var +/// * `https_proxy` env var +/// * `HTTPS_PROXY` env var +pub fn http_proxy_exists(http: &CargoHttpConfig, config: &Config) -> bool { + http_proxy(http).is_some() + || LIBCURL_HTTP_PROXY_ENVS + .iter() + .any(|v| config.get_env(v).is_ok()) +} From 3ca8c6ff2be99e1d33454adc40922f5acef463a7 Mon Sep 17 00:00:00 2001 From: bors Date: Wed, 7 Jun 2023 22:10:27 +0000 Subject: [PATCH 2/2] Auto merge of #12241 - weihanglo:fix-git-cli-output, r=epage test: loose overly matches for git cli output The output format should be stable I believe, but it turns out not. This is how `git fetch` man page says [^1]: ``` -> [] ``` In Git 2.41 they've changed the fetch output a bit [^2]. I think let's just loose it to prevent future breakages. [^1]: https://git-scm.com/docs/git-fetch#_output [^2]: https://github.blog/2023-06-01-highlights-from-git-2-41/ --- tests/testsuite/git.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index 09119203783..7c717e9671c 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -2711,7 +2711,7 @@ fn use_the_cli() { [UPDATING] git repository `[..]` [RUNNING] `git fetch [..]` From [..] - * [new ref] -> origin/HEAD + * [new ref] [..] -> origin/HEAD[..] [CHECKING] dep1 [..] [RUNNING] `rustc [..]` [CHECKING] foo [..]