Centralize HTTP configuration in one struct

Gives us one nice place to access and document all HTTP-related configuration

Author: alexcrichton

src/cargo/core/package.rs

@@ -376,9 +376,7 @@ impl<'cfg> PackageSet<'cfg> {
         // that it's buggy, and we've empirically seen that it's buggy with HTTP
         // proxies.
         let mut multi = Multi::new();
-        let multiplexing = config
-            .get::<Option<bool>>("http.multiplexing")?
-            .unwrap_or(true);
+        let multiplexing = config.http_config()?.multiplexing.unwrap_or(true);
         multi
             .pipelining(false, multiplexing)
             .chain_err(|| "failed to enable multiplexing/pipelining in curl")?;

src/cargo/ops/registry.rs

@@ -408,34 +408,26 @@ pub fn http_handle_and_timeout(config: &Config) -> CargoResult<(Easy, HttpTimeou
 }
 
 pub fn needs_custom_http_transport(config: &Config) -> CargoResult<bool> {
-    let proxy_exists = http_proxy_exists(config)?;
-    let timeout = HttpTimeout::new(config)?.is_non_default();
-    let cainfo = config.get_path("http.cainfo")?;
-    let check_revoke = config.get_bool("http.check-revoke")?;
-    let user_agent = config.get_string("http.user-agent")?;
-    let ssl_version = config.get::<Option<SslVersionConfig>>("http.ssl-version")?;
-
-    Ok(proxy_exists
-        || timeout
-        || cainfo.is_some()
-        || check_revoke.is_some()
-        || user_agent.is_some()
-        || ssl_version.is_some())
+    Ok(http_proxy_exists(config)?
+        || *config.http_config()? != Default::default()
+        || env::var_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)? {
         handle.proxy(&proxy)?;
     }
-    if let Some(cainfo) = config.get_path("http.cainfo")? {
-        handle.cainfo(&cainfo.val)?;
+    if let Some(cainfo) = http.cainfo.clone() {
+        let cainfo = cainfo.resolve(config);
+        handle.cainfo(&cainfo)?;
     }
-    if let Some(check) = config.get_bool("http.check-revoke")? {
-        handle.ssl_options(SslOpt::new().no_revoke(!check.val))?;
+    if let Some(check) = http.check_revoke {
+        handle.ssl_options(SslOpt::new().no_revoke(!check))?;
     }
-    if let Some(user_agent) = config.get_string("http.user-agent")? {
-        handle.useragent(&user_agent.val)?;
+    if let Some(user_agent) = &http.user_agent {
+        handle.useragent(user_agent)?;
     } else {
         handle.useragent(&version().to_string())?;
     }
@@ -456,7 +448,7 @@ pub fn configure_http_handle(config: &Config, handle: &mut Easy) -> CargoResult<
         };
         Ok(version)
     }
-    if let Some(ssl_version) = config.get::<Option<SslVersionConfig>>("http.ssl-version")? {
+    if let Some(ssl_version) = &http_config.ssl_version {
         match ssl_version {
             SslVersionConfig::Single(s) => {
                 let version = to_ssl_version(s.as_str())?;
@@ -472,7 +464,7 @@ pub fn configure_http_handle(config: &Config, handle: &mut Easy) -> CargoResult<
         }
     }
 
-    if let Some(true) = config.get::<Option<bool>>("http.debug")? {
+    if let Some(true) = http.debug {
         handle.verbose(true)?;
         handle.debug_function(|kind, data| {
             let (prefix, level) = match kind {
@@ -513,11 +505,10 @@ pub struct HttpTimeout {
 
 impl HttpTimeout {
     pub fn new(config: &Config) -> CargoResult<HttpTimeout> {
-        let low_speed_limit = config
-            .get::<Option<u32>>("http.low-speed-limit")?
-            .unwrap_or(10);
+        let config = config.http_config()?;
+        let low_speed_limit = config.low_speed_limit.unwrap_or(10);
         let seconds = config
-            .get::<Option<u64>>("http.timeout")?
+            .timeout
             .or_else(|| env::var("HTTP_TIMEOUT").ok().and_then(|s| s.parse().ok()))
             .unwrap_or(30);
         Ok(HttpTimeout {
@@ -526,10 +517,6 @@ impl HttpTimeout {
         })
     }
 
-    fn is_non_default(&self) -> bool {
-        self.dur != Duration::new(30, 0) || self.low_speed_limit != 10
-    }
-
     pub fn configure(&self, handle: &mut Easy) -> CargoResult<()> {
         // The timeout option for libcurl by default times out the entire
         // transfer, but we probably don't want this. Instead we only set
@@ -548,8 +535,9 @@ impl HttpTimeout {
 /// 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>> {
-    if let Some(s) = config.get_string("http.proxy")? {
-        return Ok(Some(s.val));
+    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_str("http.proxy") {

src/cargo/util/config/mod.rs

@@ -94,6 +94,8 @@ pub struct Config {
     /// Lock, if held, of the global package cache along with the number of
     /// acquisitions so far.
     package_cache_lock: RefCell<Option<(Option<FileLock>, usize)>>,
+    /// HTTP configuration for Cargo
+    http_config: LazyCell<CargoHttpConfig>,
 }
 
 impl Config {
@@ -152,6 +154,7 @@ impl Config {
             profiles: LazyCell::new(),
             updated_sources: LazyCell::new(),
             package_cache_lock: RefCell::new(None),
+            http_config: LazyCell::new(),
         }
     }
 
@@ -916,6 +919,11 @@ impl Config {
         Ok(http)
     }
 
+    pub fn http_config(&self) -> CargoResult<&CargoHttpConfig> {
+        self.http_config
+            .try_borrow_with(|| Ok(self.get::<CargoHttpConfig>("http")?))
+    }
+
     pub fn crates_io_source_id<F>(&self, f: F) -> CargoResult<SourceId>
     where
         F: FnMut() -> CargoResult<SourceId>,
@@ -1402,6 +1410,29 @@ pub fn clippy_driver() -> PathBuf {
         .into()
 }
 
+#[derive(Clone, Debug, Deserialize)]
+pub struct SslVersionConfigRange {
+    pub min: Option<String>,
+    pub max: Option<String>,
+}
+
+#[derive(Debug, Default, Deserialize, PartialEq)]
+pub struct CargoHttpConfig {
+    pub proxy: Option<String>,
+    #[serde(rename = "low-speed-limit")]
+    pub low_speed_limit: Option<u32>,
+    pub timeout: Option<u64>,
+    pub cainfo: Option<ConfigRelativePath>,
+    #[serde(rename = "check-revoke")]
+    pub check_revoke: Option<bool>,
+    #[serde(rename = "user-agent")]
+    pub user_agent: Option<String>,
+    pub debug: Option<bool>,
+    pub multiplexing: Option<bool>,
+    #[serde(rename = "ssl-version")]
+    pub ssl_version: Option<SslVersionConfig>,
+}
+
 /// Configuration for `ssl-version` in `http` section
 /// There are two ways to configure:
 ///
@@ -1421,9 +1452,3 @@ pub enum SslVersionConfig {
     Single(String),
     Range(SslVersionConfigRange),
 }
-
-#[derive(Clone, Debug, Deserialize)]
-pub struct SslVersionConfigRange {
-    pub min: Option<String>,
-    pub max: Option<String>,
-}

src/cargo/util/config/path.rs

@@ -5,7 +5,7 @@ use std::path::PathBuf;
 /// Use with the `get` API to fetch a string that will be converted to a
 /// `PathBuf`. Relative paths are converted to absolute paths based on the
 /// location of the config file.
-#[derive(Deserialize)]
+#[derive(Debug, Deserialize, PartialEq, Clone)]
 #[serde(transparent)]
 pub struct ConfigRelativePath(Value<String>);
 

src/cargo/util/config/value.rs

@@ -2,8 +2,10 @@ use crate::util::config::Config;
 use serde::de;
 use std::fmt;
 use std::marker;
+use std::mem;
 use std::path::{Path, PathBuf};
 
+#[derive(Debug, PartialEq, Clone)]
 pub struct Value<T> {
     pub val: T,
     pub definition: Definition,
@@ -31,6 +33,16 @@ impl Definition {
     }
 }
 
+impl PartialEq for Definition {
+    fn eq(&self, other: &Definition) -> bool {
+        // configuration values are equivalent no matter where they're defined,
+        // but they need to be defined in the same location. For example if
+        // they're defined in the environment that's different than being
+        // defined in a file due to path interepretations.
+        mem::discriminant(self) == mem::discriminant(other)
+    }
+}
+
 impl fmt::Display for Definition {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {