From 50b436100e2a1e1a16f64727f82b85da54085d2d Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 1 Jul 2022 00:03:43 +1000 Subject: [PATCH 1/3] Reuse `Client` everywhere instead of recreate one which pools the connection to the same site (github.com). This commit also sets `USER_AGENT` so that quickinstall can reuse it. Signed-off-by: Jiahao XU --- src/fetchers/quickinstall.rs | 9 ++------- src/helpers.rs | 11 +++++++---- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/fetchers/quickinstall.rs b/src/fetchers/quickinstall.rs index 120dd6cb..50076394 100644 --- a/src/fetchers/quickinstall.rs +++ b/src/fetchers/quickinstall.rs @@ -7,13 +7,10 @@ use tokio::task::JoinHandle; use url::Url; use super::Data; -use crate::{ - download_and_extract, new_reqwest_client_builder, remote_exists, BinstallError, PkgFmt, -}; +use crate::{download_and_extract, new_reqwest_client, remote_exists, BinstallError, PkgFmt}; const BASE_URL: &str = "https://github.com/alsuren/cargo-quickinstall/releases/download"; const STATS_URL: &str = "https://warehouse-clerk-tmp.vercel.app/api/crate"; -const USER_AGENT: &str = concat!(env!("CARGO_PKG_NAME"), "/", env!("CARGO_PKG_VERSION")); pub struct QuickInstall { package: String, @@ -91,9 +88,7 @@ impl QuickInstall { let url = Url::parse(&stats_url)?; debug!("Sending installation report to quickinstall ({url})"); - new_reqwest_client_builder() - .user_agent(USER_AGENT) - .build()? + new_reqwest_client()? .request(Method::HEAD, url.clone()) .send() .await diff --git a/src/helpers.rs b/src/helpers.rs index d48e97b6..04a8473b 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -59,8 +59,10 @@ pub fn load_manifest_path>( }) } -pub fn new_reqwest_client_builder() -> ClientBuilder { - let mut builder = ClientBuilder::new(); +fn new_reqwest_client_builder() -> ClientBuilder { + const USER_AGENT: &str = concat!(env!("CARGO_PKG_NAME"), "/", env!("CARGO_PKG_VERSION")); + + let mut builder = ClientBuilder::new().user_agent(USER_AGENT); if let Some(ReqwestConfig { secure, min_tls }) = REQWESTGLOBALCONFIG.get() { if *secure { @@ -77,8 +79,9 @@ pub fn new_reqwest_client_builder() -> ClientBuilder { builder } -pub fn new_reqwest_client() -> reqwest::Result { - new_reqwest_client_builder().build() +pub fn new_reqwest_client() -> reqwest::Result<&'static Client> { + static CLIENT: OnceCell = OnceCell::new(); + CLIENT.get_or_try_init(|| new_reqwest_client_builder().build()) } pub async fn remote_exists(url: Url, method: Method) -> Result { From 6582eefd25865aa15fb2b65e98764c2c9ed3df62 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 1 Jul 2022 00:14:50 +1000 Subject: [PATCH 2/3] Refactor: Replace `REQWESTCONFIG` with `initialize_reqwest_client` so that we don't need two `OnceCell`s. Signed-off-by: Jiahao XU --- src/fetchers/quickinstall.rs | 2 +- src/helpers.rs | 52 ++++++++++++++++++------------------ src/main.rs | 9 ++----- 3 files changed, 29 insertions(+), 34 deletions(-) diff --git a/src/fetchers/quickinstall.rs b/src/fetchers/quickinstall.rs index 50076394..3cb07fd8 100644 --- a/src/fetchers/quickinstall.rs +++ b/src/fetchers/quickinstall.rs @@ -88,7 +88,7 @@ impl QuickInstall { let url = Url::parse(&stats_url)?; debug!("Sending installation report to quickinstall ({url})"); - new_reqwest_client()? + new_reqwest_client() .request(Method::HEAD, url.clone()) .send() .await diff --git a/src/helpers.rs b/src/helpers.rs index 04a8473b..57091807 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -35,15 +35,6 @@ pub use path_ext::*; mod tls_version; pub use tls_version::TLSVersion; -#[derive(Debug)] -pub struct ReqwestConfig { - pub secure: bool, - pub min_tls: Option, -} - -/// (secure mode, min TLS version) -pub static REQWESTGLOBALCONFIG: OnceCell = OnceCell::new(); - /// Load binstall metadata from the crate `Cargo.toml` at the provided path pub fn load_manifest_path>( manifest_path: P, @@ -59,33 +50,42 @@ pub fn load_manifest_path>( }) } -fn new_reqwest_client_builder() -> ClientBuilder { +static CLIENT: OnceCell = OnceCell::new(); + +/// Should only be called once in main::entry. +pub fn initialize_reqwest_client( + secure: bool, + min_tls: Option, +) -> Result<(), BinstallError> { const USER_AGENT: &str = concat!(env!("CARGO_PKG_NAME"), "/", env!("CARGO_PKG_VERSION")); let mut builder = ClientBuilder::new().user_agent(USER_AGENT); - if let Some(ReqwestConfig { secure, min_tls }) = REQWESTGLOBALCONFIG.get() { - if *secure { - builder = builder - .https_only(true) - .min_tls_version(tls::Version::TLS_1_2) - } - - if let Some(ver) = *min_tls { - builder = builder.min_tls_version(ver); - } + if secure { + builder = builder + .https_only(true) + .min_tls_version(tls::Version::TLS_1_2); } - builder + if let Some(ver) = min_tls { + builder = builder.min_tls_version(ver); + } + + let client = builder.build()?; + + CLIENT + .set(client) + .expect("Reqwest client already initialized"); + + Ok(()) } -pub fn new_reqwest_client() -> reqwest::Result<&'static Client> { - static CLIENT: OnceCell = OnceCell::new(); - CLIENT.get_or_try_init(|| new_reqwest_client_builder().build()) +pub fn new_reqwest_client() -> &'static Client { + CLIENT.get().expect("Reqwest client is not initialized") } pub async fn remote_exists(url: Url, method: Method) -> Result { - let req = new_reqwest_client()? + let req = new_reqwest_client() .request(method.clone(), url.clone()) .send() .await @@ -98,7 +98,7 @@ async fn create_request( ) -> Result>, BinstallError> { debug!("Downloading from: '{url}'"); - new_reqwest_client()? + new_reqwest_client() .get(url.clone()) .send() .await diff --git a/src/main.rs b/src/main.rs index 5a155bad..506cc478 100644 --- a/src/main.rs +++ b/src/main.rs @@ -200,13 +200,8 @@ async fn entry() -> Result<()> { bin_dir: opts.bin_dir.take(), }; - // Initialize REQWESTGLOBALCONFIG - REQWESTGLOBALCONFIG - .set(ReqwestConfig { - secure: opts.secure, - min_tls: opts.min_tls_version.map(|v| v.into()), - }) - .unwrap(); + // Initialize reqwest client + initialize_reqwest_client(opts.secure, opts.min_tls_version.map(|v| v.into()))?; // Setup logging let mut log_config = ConfigBuilder::new(); From 5ad572fa422d52396b030163096dda89df7ed261 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 1 Jul 2022 00:15:54 +1000 Subject: [PATCH 3/3] Rename `new_reqwest_client` > `get_reqwest_client` Signed-off-by: Jiahao XU --- src/fetchers/quickinstall.rs | 4 ++-- src/helpers.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/fetchers/quickinstall.rs b/src/fetchers/quickinstall.rs index 3cb07fd8..591bb6dc 100644 --- a/src/fetchers/quickinstall.rs +++ b/src/fetchers/quickinstall.rs @@ -7,7 +7,7 @@ use tokio::task::JoinHandle; use url::Url; use super::Data; -use crate::{download_and_extract, new_reqwest_client, remote_exists, BinstallError, PkgFmt}; +use crate::{download_and_extract, get_reqwest_client, remote_exists, BinstallError, PkgFmt}; const BASE_URL: &str = "https://github.com/alsuren/cargo-quickinstall/releases/download"; const STATS_URL: &str = "https://warehouse-clerk-tmp.vercel.app/api/crate"; @@ -88,7 +88,7 @@ impl QuickInstall { let url = Url::parse(&stats_url)?; debug!("Sending installation report to quickinstall ({url})"); - new_reqwest_client() + get_reqwest_client() .request(Method::HEAD, url.clone()) .send() .await diff --git a/src/helpers.rs b/src/helpers.rs index 57091807..73a553bd 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -80,12 +80,12 @@ pub fn initialize_reqwest_client( Ok(()) } -pub fn new_reqwest_client() -> &'static Client { +pub fn get_reqwest_client() -> &'static Client { CLIENT.get().expect("Reqwest client is not initialized") } pub async fn remote_exists(url: Url, method: Method) -> Result { - let req = new_reqwest_client() + let req = get_reqwest_client() .request(method.clone(), url.clone()) .send() .await @@ -98,7 +98,7 @@ async fn create_request( ) -> Result>, BinstallError> { debug!("Downloading from: '{url}'"); - new_reqwest_client() + get_reqwest_client() .get(url.clone()) .send() .await