Auto merge of rust-lang#12234 - weihanglo:multiplexing, r=ehuss

fix: disable multiplexing for some versions of curl

Author: bors

Diff for: 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 for: 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<bool> {
-    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<HttpTimeout> {
     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<Option<String>> {
-    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<bool> {
-    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<Secret<&str>>,

Diff for: 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::<CargoHttpConfig>("http"))
+        self.http_config.try_borrow_with(|| {
+            let mut http = self.get::<CargoHttpConfig>("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 for: src/cargo/util/network/mod.rs

@@ -2,6 +2,7 @@
 
 use std::task::Poll;
 
+pub mod proxy;
 pub mod retry;
 pub mod sleep;
 

Diff for: 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<String> {
+    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())
+}