Skip to content

Commit

Permalink
Auto merge of rust-lang#12234 - weihanglo:multiplexing, r=ehuss
Browse files Browse the repository at this point in the history
fix: disable multiplexing for some versions of curl
  • Loading branch information
bors authored and weihanglo committed Jun 7, 2023
1 parent 64fb38c commit 1971cb6
Show file tree
Hide file tree
Showing 5 changed files with 133 additions and 47 deletions.
4 changes: 0 additions & 4 deletions src/bin/cargo/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
52 changes: 11 additions & 41 deletions src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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>>,
Expand Down
81 changes: 79 additions & 2 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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);
}
}
}
1 change: 1 addition & 0 deletions src/cargo/util/network/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use std::task::Poll;

pub mod proxy;
pub mod retry;
pub mod sleep;

Expand Down
42 changes: 42 additions & 0 deletions src/cargo/util/network/proxy.rs
Original file line number Diff line number Diff line change
@@ -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())
}

0 comments on commit 1971cb6

Please sign in to comment.