Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: disable multiplexing for some versions of curl #12234

Merged
merged 2 commits into from
Jun 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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, JobsConfig, 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 @@ -2750,3 +2754,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);
weihanglo marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

#[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())
}