Remove HTTP client creation

Cargo.toml

@@ -22,14 +22,10 @@ exclude = [
 [features]
 # By default we use rustls for TLS
 default = ["rustls-tls"]
-rustls-tls = ["ureq/tls", "rustls-pemfile", "rustls"]
+rustls-tls = ["ureq/tls", "rustls"]
 # If this feature is enabled we instead use the native TLS implementation for the
 # target platform
-native-tls = [
-    "ureq/native-tls",
-    "native-tls-crate/vendored",
-    "rustls-pemfile",
-]
+native-tls = ["ureq/native-tls", "native-tls-crate/vendored"]
 
 [dependencies]
 # Easy errors

src/ctx.rs

@@ -1,5 +1,3 @@
-use std::time::Duration;
-
 use crate::{
     splat::SdkHeaders,
     util::{ProgressTarget, Sha256},
@@ -25,93 +23,8 @@ pub struct Ctx {
 }
 
 impl Ctx {
-    fn http_client(read_timeout: Option<Duration>) -> Result<ureq::Agent, Error> {
-        let mut builder = ureq::builder();
-
-        #[cfg(feature = "native-tls")]
-        'custom: {
-            // "common"? env vars that people who use custom certs use? I guess
-            // this is easy to expand if it's not the case. /shrug
-            const CERT_ENVS: &[&str] = &["REQUESTS_CA_BUNDLE", "CURL_CA_BUNDLE", "SSL_CERT_FILE"];
-
-            let Some((env, cert_path)) = CERT_ENVS.iter().find_map(|env| {
-                std::env::var_os(env).map(|var| (env, std::path::PathBuf::from(var)))
-            }) else {
-                break 'custom;
-            };
-
-            fn build(
-                cert_path: &std::path::Path,
-            ) -> anyhow::Result<native_tls_crate::TlsConnector> {
-                let mut tls_builder = native_tls_crate::TlsConnector::builder();
-                let mut reader = std::io::BufReader::new(std::fs::File::open(cert_path)?);
-                for cert in rustls_pemfile::certs(&mut reader)? {
-                    tls_builder
-                        .add_root_certificate(native_tls_crate::Certificate::from_pem(&cert)?);
-                }
-                Ok(tls_builder.build()?)
-            }
-
-            let tls_connector = build(&cert_path).with_context(|| {
-                format!(
-                    "failed to add custom cert from path '{}' configured by env var '{env}'",
-                    cert_path.display()
-                )
-            })?;
-
-            builder = builder.tls_connector(std::sync::Arc::new(tls_connector));
-        }
-
-        #[cfg(feature = "rustls-tls")]
-        'custom: {
-            // "common"? env vars that people who use custom certs use? I guess
-            // this is easy to expand if it's not the case. /shrug
-            const CERT_ENVS: &[&str] = &["REQUESTS_CA_BUNDLE", "CURL_CA_BUNDLE", "SSL_CERT_FILE"];
-
-            let Some((env, cert_path)) = CERT_ENVS.iter().find_map(|env| {
-                std::env::var_os(env).map(|var| (env, std::path::PathBuf::from(var)))
-            }) else {
-                break 'custom;
-            };
-
-            fn build(cert_path: &std::path::Path) -> anyhow::Result<rustls::ClientConfig> {
-                let mut reader = std::io::BufReader::new(std::fs::File::open(cert_path)?);
-                let certs = rustls_pemfile::certs(&mut reader)?;
-                let mut root_certs = rustls::RootCertStore::empty();
-                root_certs.add_parsable_certificates(&certs);
-                let client_config = rustls::ClientConfig::builder()
-                    .with_safe_defaults()
-                    .with_root_certificates(root_certs)
-                    .with_no_client_auth();
-                Ok(client_config)
-            }
-
-            let client_config = build(&cert_path).with_context(|| {
-                format!(
-                    "failed to add custom cert from path '{}' configured by env var '{env}'",
-                    cert_path.display()
-                )
-            })?;
-
-            builder = builder.tls_config(std::sync::Arc::new(client_config));
-        }
-
-        // Allow user to specify timeout values in the case of bad/slow proxies
-        // or MS itself being terrible, but default to a minute, which is _far_
-        // more than it should take in normal situations, as by default ureq
-        // sets no timeout on the response
-        builder = builder.timeout_read(read_timeout.unwrap_or(Duration::from_secs(60)));
-
-        if let Ok(proxy) = std::env::var("https_proxy") {
-            let proxy = ureq::Proxy::new(proxy)?;
-            builder = builder.proxy(proxy);
-        }
-        Ok(builder.build())
-    }
-
-    pub fn with_temp(dt: ProgressTarget, read_timeout: Option<Duration>) -> Result<Self, Error> {
+    pub fn with_temp(dt: ProgressTarget, client: ureq::Agent) -> Result<Self, Error> {
         let td = tempfile::TempDir::new()?;
-        let client = Self::http_client(read_timeout)?;
 
         Ok(Self {
             work_dir: PathBuf::from_path_buf(td.path().to_owned()).map_err(|pb| {
@@ -126,10 +39,8 @@ impl Ctx {
     pub fn with_dir(
         mut work_dir: PathBuf,
         dt: ProgressTarget,
-        read_timeout: Option<Duration>,
+        client: ureq::Agent,
     ) -> Result<Self, Error> {
-        let client = Self::http_client(read_timeout)?;
-
         work_dir.push("dl");
         std::fs::create_dir_all(&work_dir)?;
         work_dir.pop();

src/lib.rs

@@ -18,6 +18,7 @@ pub mod util;
 pub use ctx::Ctx;
 pub use minimize::MinimizeConfig;
 pub use splat::SplatConfig;
+pub use ureq;
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
 pub enum Arch {

src/main.rs

@@ -212,9 +212,12 @@ pub struct Args {
     /// Whether to include the Active Template Library (ATL) in the installation
     #[arg(long)]
     include_atl: bool,
-    /// Specifies a timeout for how long a single download is allowed to take. The default is 60s.
-    #[arg(short, long, value_parser = parse_duration)]
-    timeout: Option<Duration>,
+    /// Specifies a timeout for how long a single download is allowed to take.
+    #[arg(short, long, value_parser = parse_duration, default_value = "60s")]
+    timeout: Duration,
+    /// An HTTPS proxy to use
+    #[arg(long, env = "HTTPS_PROXY")]
+    https_proxy: Option<String>,
     /// The architectures to include
     #[arg(
         long,
@@ -259,19 +262,36 @@ fn main() -> Result<(), Error> {
 
     let draw_target = xwin::util::ProgressTarget::Stdout;
 
+    let client = {
+        let mut builder = ureq::AgentBuilder::new().timeout_read(args.timeout);
+
+        if let Some(proxy) = args.https_proxy {
+            let proxy = ureq::Proxy::new(proxy).context("failed to parse https proxy address")?;
+            builder = builder.proxy(proxy);
+        }
+
+        builder.build()
+    };
+
     let ctx = if args.temp {
-        xwin::Ctx::with_temp(draw_target, args.timeout)?
+        xwin::Ctx::with_temp(draw_target, client)?
     } else {
         let cache_dir = match &args.cache_dir {
             Some(cd) => cd.clone(),
             None => cwd.join(".xwin-cache"),
         };
-        xwin::Ctx::with_dir(cache_dir, draw_target, args.timeout)?
+        xwin::Ctx::with_dir(cache_dir, draw_target, client)?
     };
 
     let ctx = std::sync::Arc::new(ctx);
 
-    let pkg_manifest = load_manifest(&ctx, &args, draw_target)?;
+    let pkg_manifest = load_manifest(
+        &ctx,
+        args.manifest.as_ref(),
+        &args.manifest_version,
+        &args.channel,
+        draw_target,
+    )?;
 
     let arches = args.arch.into_iter().fold(0, |acc, arch| acc | arch as u32);
     let variants = args
@@ -453,7 +473,9 @@ fn print_packages(payloads: &[xwin::Payload]) {
 
 fn load_manifest(
     ctx: &xwin::Ctx,
-    args: &Args,
+    manifest: Option<&PathBuf>,
+    manifest_version: &str,
+    channel: &str,
     dt: xwin::util::ProgressTarget,
 ) -> anyhow::Result<xwin::manifest::PackageManifest> {
     let manifest_pb = ia::ProgressBar::with_draw_target(Some(0), dt.into())
@@ -467,19 +489,14 @@ fn load_manifest(
     manifest_pb.set_prefix("Manifest");
     manifest_pb.set_message("📥 downloading");
 
-    let manifest = match &args.manifest {
+    let manifest = match manifest {
         Some(manifest_path) => {
             let manifest_content = std::fs::read_to_string(manifest_path)
                 .with_context(|| format!("failed to read path '{}'", manifest_path))?;
             serde_json::from_str(&manifest_content)
                 .with_context(|| format!("failed to deserialize manifest in '{}'", manifest_path))?
         }
-        None => xwin::manifest::get_manifest(
-            ctx,
-            &args.manifest_version,
-            &args.channel,
-            manifest_pb.clone(),
-        )?,
+        None => xwin::manifest::get_manifest(ctx, manifest_version, channel, manifest_pb.clone())?,
     };
 
     let pkg_manifest = xwin::manifest::get_package_manifest(ctx, &manifest, manifest_pb.clone())?;

tests/compiles.rs

@@ -3,7 +3,7 @@ fn verify_compiles() {
     let ctx = xwin::Ctx::with_dir(
         xwin::PathBuf::from(".xwin-cache/compile-test"),
         xwin::util::ProgressTarget::Hidden,
-        None,
+        ureq::agent(),
     )
     .unwrap();
 
@@ -141,7 +141,7 @@ fn verify_compiles_minimized() {
     let ctx = xwin::Ctx::with_dir(
         xwin::PathBuf::from(".xwin-cache/compile-test-minimized"),
         xwin::util::ProgressTarget::Hidden,
-        None,
+        ureq::agent(),
     )
     .unwrap();
 

tests/deterministic.rs

@@ -6,7 +6,7 @@ fn verify_deterministic() {
     let ctx = xwin::Ctx::with_dir(
         PathBuf::from(".xwin-cache/deterministic"),
         xwin::util::ProgressTarget::Hidden,
-        None,
+        ureq::agent(),
     )
     .unwrap();
 

tests/snapshots/xwin.snap

@@ -75,8 +75,14 @@ Options:
           installation
 
   -t, --timeout <TIMEOUT>
-          Specifies a timeout for how long a single download is allowed to take.
-          The default is 60s
+          Specifies a timeout for how long a single download is allowed to take
+
+          [default: 60s]
+
+      --https-proxy <HTTPS_PROXY>
+          An HTTPS proxy to use
+
+          [env: HTTPS_PROXY]
 
       --arch <ARCH>
           The architectures to include