From 8972ebc9cc33ef1df21c899c35006ea712f7f91f Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 12 Dec 2022 21:30:44 -0500 Subject: [PATCH] fix: always derive http client from cli flags (#17029) I'm not sure how to test this. It doesn't seem to have an existing test. Closes #15921 --- cli/args/flags.rs | 5 ---- cli/main.rs | 2 +- cli/proc_state.rs | 4 ++- cli/tools/run.rs | 5 +++- cli/tools/standalone.rs | 13 +++++---- cli/tools/upgrade.rs | 62 ++++++++++++++++++----------------------- 6 files changed, 42 insertions(+), 49 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index ac2b7a0624..7c3eae7787 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -203,7 +203,6 @@ pub struct UpgradeFlags { pub canary: bool, pub version: Option, pub output: Option, - pub ca_file: Option, } #[derive(Clone, Debug, Eq, PartialEq, Deserialize, Serialize)] @@ -2882,14 +2881,12 @@ fn upgrade_parse(flags: &mut Flags, matches: &clap::ArgMatches) { } else { None }; - let ca_file = matches.value_of("cert").map(|s| s.to_string()); flags.subcommand = DenoSubcommand::Upgrade(UpgradeFlags { dry_run, force, canary, version, output, - ca_file, }); } @@ -3302,7 +3299,6 @@ mod tests { canary: false, version: None, output: None, - ca_file: None, }), ..Flags::default() } @@ -5754,7 +5750,6 @@ mod tests { canary: false, version: None, output: None, - ca_file: Some("example.crt".to_owned()), }), ca_file: Some("example.crt".to_owned()), ..Flags::default() diff --git a/cli/main.rs b/cli/main.rs index a47c9a01a1..ecff408d7c 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -195,7 +195,7 @@ async fn run_subcommand(flags: Flags) -> Result { Ok(0) } DenoSubcommand::Upgrade(upgrade_flags) => { - tools::upgrade::upgrade(upgrade_flags).await?; + tools::upgrade::upgrade(flags, upgrade_flags).await?; Ok(0) } DenoSubcommand::Vendor(vendor_flags) => { diff --git a/cli/proc_state.rs b/cli/proc_state.rs index a238bd5d2a..13c8d24142 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -78,6 +78,7 @@ pub struct ProcState(Arc); pub struct Inner { pub dir: DenoDir, pub file_fetcher: FileFetcher, + pub http_client: HttpClient, pub options: Arc, pub emit_cache: EmitCache, pub emit_options: deno_ast::EmitOptions, @@ -226,7 +227,7 @@ impl ProcState { let api = RealNpmRegistryApi::new( registry_url, npm_cache.clone(), - http_client, + http_client.clone(), progress_bar.clone(), ); let maybe_lockfile = lockfile.as_ref().cloned(); @@ -256,6 +257,7 @@ impl ProcState { .finish(), emit_options, file_fetcher, + http_client, graph_data: Default::default(), lockfile, maybe_import_map, diff --git a/cli/tools/run.rs b/cli/tools/run.rs index d714c55d32..617ec28f6d 100644 --- a/cli/tools/run.rs +++ b/cli/tools/run.rs @@ -45,7 +45,10 @@ To grant permissions, set them before the script argument. For example: // Run a background task that checks for available upgrades. If an earlier // run of this background task found a new version of Deno. - super::upgrade::check_for_upgrades(ps.dir.upgrade_check_file_path()); + super::upgrade::check_for_upgrades( + ps.http_client.clone(), + ps.dir.upgrade_check_file_path(), + ); let main_module = if NpmPackageReference::from_str(&run_flags.script).is_ok() { diff --git a/cli/tools/standalone.rs b/cli/tools/standalone.rs index c276119ead..558adc460e 100644 --- a/cli/tools/standalone.rs +++ b/cli/tools/standalone.rs @@ -5,6 +5,7 @@ use crate::args::Flags; use crate::cache::DenoDir; use crate::graph_util::create_graph_and_maybe_check; use crate::graph_util::error_for_any_npm_specifier; +use crate::http_util::HttpClient; use crate::standalone::Metadata; use crate::standalone::MAGIC_TRAILER; use crate::util::path::path_has_trailing_slash; @@ -18,7 +19,6 @@ use deno_core::serde_json; use deno_core::url::Url; use deno_graph::ModuleSpecifier; use deno_runtime::colors; -use deno_runtime::deno_fetch::reqwest::Client; use deno_runtime::permissions::Permissions; use std::env; use std::fs; @@ -64,7 +64,8 @@ pub async fn compile( // Select base binary based on target let original_binary = - get_base_binary(deno_dir, compile_flags.target.clone()).await?; + get_base_binary(&ps.http_client, deno_dir, compile_flags.target.clone()) + .await?; let final_bin = create_standalone_binary( original_binary, @@ -82,6 +83,7 @@ pub async fn compile( } async fn get_base_binary( + client: &HttpClient, deno_dir: &DenoDir, target: Option, ) -> Result, AnyError> { @@ -103,7 +105,8 @@ async fn get_base_binary( let binary_path = download_directory.join(&binary_path_suffix); if !binary_path.exists() { - download_base_binary(&download_directory, &binary_path_suffix).await?; + download_base_binary(client, &download_directory, &binary_path_suffix) + .await?; } let archive_data = tokio::fs::read(binary_path).await?; @@ -114,14 +117,12 @@ async fn get_base_binary( } async fn download_base_binary( + client: &HttpClient, output_directory: &Path, binary_path_suffix: &str, ) -> Result<(), AnyError> { let download_url = format!("https://dl.deno.land/{}", binary_path_suffix); - let client_builder = Client::builder(); - let client = client_builder.build()?; - log::info!("Checking {}", &download_url); let res = client.get(&download_url).send().await?; diff --git a/cli/tools/upgrade.rs b/cli/tools/upgrade.rs index 82949410b8..1e0be728ee 100644 --- a/cli/tools/upgrade.rs +++ b/cli/tools/upgrade.rs @@ -2,8 +2,11 @@ //! This module provides feature to upgrade deno executable +use crate::args::Flags; use crate::args::UpgradeFlags; use crate::colors; +use crate::http_util::HttpClient; +use crate::proc_state::ProcState; use crate::util::display::human_download_size; use crate::util::progress_bar::ProgressBar; use crate::util::progress_bar::ProgressBarStyle; @@ -15,8 +18,6 @@ use deno_core::error::AnyError; use deno_core::futures::future::BoxFuture; use deno_core::futures::FutureExt; use deno_core::futures::StreamExt; -use deno_runtime::deno_fetch::reqwest; -use deno_runtime::deno_fetch::reqwest::Client; use once_cell::sync::Lazy; use std::borrow::Cow; use std::env; @@ -49,13 +50,15 @@ trait UpdateCheckerEnvironment: Clone + Send + Sync { #[derive(Clone)] struct RealUpdateCheckerEnvironment { + http_client: HttpClient, cache_file_path: PathBuf, current_time: chrono::DateTime, } impl RealUpdateCheckerEnvironment { - pub fn new(cache_file_path: PathBuf) -> Self { + pub fn new(http_client: HttpClient, cache_file_path: PathBuf) -> Self { Self { + http_client, cache_file_path, // cache the current time current_time: chrono::Utc::now(), @@ -65,12 +68,12 @@ impl RealUpdateCheckerEnvironment { impl UpdateCheckerEnvironment for RealUpdateCheckerEnvironment { fn latest_version(&self) -> BoxFuture<'static, Result> { - async { - let client = build_http_client(None)?; + let http_client = self.http_client.clone(); + async move { if version::is_canary() { - get_latest_canary_version(&client).await + get_latest_canary_version(&http_client).await } else { - get_latest_release_version(&client).await + get_latest_release_version(&http_client).await } } .boxed() @@ -161,12 +164,12 @@ impl UpdateChecker { } } -pub fn check_for_upgrades(cache_file_path: PathBuf) { +pub fn check_for_upgrades(http_client: HttpClient, cache_file_path: PathBuf) { if env::var("DENO_NO_UPDATE_CHECK").is_ok() { return; } - let env = RealUpdateCheckerEnvironment::new(cache_file_path); + let env = RealUpdateCheckerEnvironment::new(http_client, cache_file_path); let update_checker = UpdateChecker::new(env); if update_checker.should_check_for_new_version() { @@ -236,7 +239,11 @@ async fn fetch_and_store_latest_version< ); } -pub async fn upgrade(upgrade_flags: UpgradeFlags) -> Result<(), AnyError> { +pub async fn upgrade( + flags: Flags, + upgrade_flags: UpgradeFlags, +) -> Result<(), AnyError> { + let ps = ProcState::build(flags).await?; let current_exe_path = std::env::current_exe()?; let metadata = fs::metadata(¤t_exe_path)?; let permissions = metadata.permissions(); @@ -258,7 +265,7 @@ pub async fn upgrade(upgrade_flags: UpgradeFlags) -> Result<(), AnyError> { ), current_exe_path.display()); } - let client = build_http_client(upgrade_flags.ca_file)?; + let client = ps.http_client.clone(); let install_version = match upgrade_flags.version { Some(passed_version) => { @@ -316,7 +323,11 @@ pub async fn upgrade(upgrade_flags: UpgradeFlags) -> Result<(), AnyError> { { log::info!( "Local deno version {} is the most recent release", - crate::version::deno() + if upgrade_flags.canary { + crate::version::GIT_COMMIT_HASH.to_string() + } else { + crate::version::deno() + } ); return Ok(()); } else { @@ -342,7 +353,7 @@ pub async fn upgrade(upgrade_flags: UpgradeFlags) -> Result<(), AnyError> { ) }; - let archive_data = download_package(client, &download_url).await?; + let archive_data = download_package(&client, &download_url).await?; log::info!("Deno is upgrading to version {}", &install_version); @@ -388,27 +399,8 @@ pub async fn upgrade(upgrade_flags: UpgradeFlags) -> Result<(), AnyError> { Ok(()) } -fn build_http_client( - ca_file: Option, -) -> Result { - let mut client_builder = - Client::builder().user_agent(version::get_user_agent()); - - // If we have been provided a CA Certificate, add it into the HTTP client - let ca_file = ca_file.or_else(|| env::var("DENO_CERT").ok()); - if let Some(ca_file) = ca_file { - let buf = std::fs::read(ca_file)?; - let cert = reqwest::Certificate::from_pem(&buf)?; - client_builder = client_builder.add_root_certificate(cert); - } - - let client = client_builder.build()?; - - Ok(client) -} - async fn get_latest_release_version( - client: &Client, + client: &HttpClient, ) -> Result { let res = client .get("https://dl.deno.land/release-latest.txt") @@ -419,7 +411,7 @@ async fn get_latest_release_version( } async fn get_latest_canary_version( - client: &Client, + client: &HttpClient, ) -> Result { let res = client .get("https://dl.deno.land/canary-latest.txt") @@ -430,7 +422,7 @@ async fn get_latest_canary_version( } async fn download_package( - client: Client, + client: &HttpClient, download_url: &str, ) -> Result, AnyError> { log::info!("Downloading {}", &download_url);