Skip to content

Commit

Permalink
fix: disable multiplexing on macOS for bad curl
Browse files Browse the repository at this point in the history
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@821f6e2

However, a custom built Cargo can still link against old system libcurl.
Multiplexing needs to be disabled when those versions are detected.

We only take care of macOS now, since it is one of the major platforms.
For other OS we encourage not to stick to older versions of licurl.
  • Loading branch information
weihanglo committed Jun 6, 2023
1 parent a2e6862 commit 0a3c39b
Showing 1 changed file with 83 additions and 12 deletions.
95 changes: 83 additions & 12 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ use crate::core::{features, CliUnstable, Shell, SourceId, Workspace, WorkspaceRo
use crate::ops::{self, RegistryCredentialConfig};
use crate::util::auth::Secret;
use crate::util::errors::CargoResult;
use crate::util::network;
use crate::util::CanonicalUrl;
use crate::util::{internal, toml as cargo_toml};
use crate::util::{try_canonicalize, validate_package_name};
Expand Down Expand Up @@ -1717,18 +1718,15 @@ impl Config {
}

pub fn http_config(&self) -> CargoResult<&CargoHttpConfig> {
self.http_config
.try_borrow_with(|| {
let http_config = self.get::<CargoHttpConfig>("http")?;
if cfg!(target_os = "macos") {
let versions = ["7.87", "7.88"];
let curl_v = curl::Version::get().version();
if versions.iter().any(|v| curl_v.starts_with(v)) {
http_config.multiplexing = Some(false);
}
}
Ok(http_config)
})
self.http_config.try_borrow_with(|| {
let mut http = self.get::<CargoHttpConfig>("http")?;
#[cfg(target_os = "macos")]
{
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 @@ -2760,3 +2758,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, a custom built Cargo can still link against old system libcurl.
/// Multiplexing needs to be disabled when those versions are detected.
///
/// We only take care of macOS now, since it is one of the major platforms.
/// For other OS we encourage not to stick to older versions of licurl.
#[cfg(target_os = "macos")]
fn disables_multiplexing_for_bad_curl(
curl_version: &str,
http: &mut CargoHttpConfig,
config: &Config,
) {
if network::proxy::http_proxy_exists(http, config) && http.multiplexing.is_none() {
let bad_curl_versions = ["7.87", "7.88"];
if bad_curl_versions
.iter()
.any(|v| curl_version.starts_with(v))
{
http.multiplexing = Some(false);
}
}
}

#[cfg(test)]
mod tests {
use super::disables_multiplexing_for_bad_curl;
use super::CargoHttpConfig;
use super::Config;
use super::Shell;

#[cfg(target_os = "macos")]
#[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);
}
}
}

0 comments on commit 0a3c39b

Please sign in to comment.