diff options
Diffstat (limited to 'cli')
| -rw-r--r-- | cli/Cargo.toml | 2 | ||||
| -rw-r--r-- | cli/args/mod.rs | 1 | ||||
| -rw-r--r-- | cli/http_util.rs | 56 | ||||
| -rw-r--r-- | cli/npm/managed/cache/tarball.rs | 41 |
4 files changed, 74 insertions, 26 deletions
diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 38cb8402b..5f029c714 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -72,7 +72,7 @@ deno_emit = "=0.42.0" deno_graph = { version = "=0.78.0", features = ["tokio_executor"] } deno_lint = { version = "=0.60.0", features = ["docs"] } deno_lockfile.workspace = true -deno_npm = "=0.21.1" +deno_npm = "=0.21.2" deno_runtime = { workspace = true, features = ["include_js_files_for_snapshotting"] } deno_semver = "=0.5.4" deno_task_shell = "=0.16.1" diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 6b2d36129..d5e37acc8 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -619,6 +619,7 @@ pub fn create_default_npmrc() -> Arc<ResolvedNpmRc> { config: Default::default(), }, scopes: Default::default(), + registry_configs: Default::default(), }) } diff --git a/cli/http_util.rs b/cli/http_util.rs index 5042f5078..7fcce616b 100644 --- a/cli/http_util.rs +++ b/cli/http_util.rs @@ -7,7 +7,6 @@ use crate::version::get_user_agent; use cache_control::Cachability; use cache_control::CacheControl; use chrono::DateTime; -use deno_core::anyhow::bail; use deno_core::error::custom_error; use deno_core::error::generic_error; use deno_core::error::AnyError; @@ -30,6 +29,7 @@ use std::sync::Arc; use std::thread::ThreadId; use std::time::Duration; use std::time::SystemTime; +use thiserror::Error; // TODO(ry) HTTP headers are not unique key, value pairs. There may be more than // one header line with the same key. This should be changed to something like @@ -260,6 +260,27 @@ impl HttpClientProvider { } } +#[derive(Debug, Error)] +#[error("Bad response: {:?}{}", .status_code, .response_text.as_ref().map(|s| format!("\n\n{}", s)).unwrap_or_else(String::new))] +pub struct BadResponseError { + pub status_code: StatusCode, + pub response_text: Option<String>, +} + +#[derive(Debug, Error)] +pub enum DownloadError { + #[error(transparent)] + Reqwest(#[from] reqwest::Error), + #[error(transparent)] + ToStr(#[from] reqwest::header::ToStrError), + #[error("Redirection from '{}' did not provide location header", .request_url)] + NoRedirectHeader { request_url: Url }, + #[error("Too many redirects.")] + TooManyRedirects, + #[error(transparent)] + BadResponse(#[from] BadResponseError), +} + #[derive(Debug)] pub struct HttpClient { #[allow(clippy::disallowed_types)] // reqwest::Client allowed here @@ -409,7 +430,7 @@ impl HttpClient { url: impl reqwest::IntoUrl, maybe_header: Option<(HeaderName, HeaderValue)>, progress_guard: &UpdateGuard, - ) -> Result<Option<Vec<u8>>, AnyError> { + ) -> Result<Option<Vec<u8>>, DownloadError> { self .download_inner(url, maybe_header, Some(progress_guard)) .await @@ -429,7 +450,7 @@ impl HttpClient { url: impl reqwest::IntoUrl, maybe_header: Option<(HeaderName, HeaderValue)>, progress_guard: Option<&UpdateGuard>, - ) -> Result<Option<Vec<u8>>, AnyError> { + ) -> Result<Option<Vec<u8>>, DownloadError> { let response = self.get_redirected_response(url, maybe_header).await?; if response.status() == 404 { @@ -437,26 +458,25 @@ impl HttpClient { } else if !response.status().is_success() { let status = response.status(); let maybe_response_text = response.text().await.ok(); - bail!( - "Bad response: {:?}{}", - status, - match maybe_response_text { - Some(text) => format!("\n\n{text}"), - None => String::new(), - } - ); + return Err(DownloadError::BadResponse(BadResponseError { + status_code: status, + response_text: maybe_response_text + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()), + })); } get_response_body_with_progress(response, progress_guard) .await .map(Some) + .map_err(Into::into) } async fn get_redirected_response( &self, url: impl reqwest::IntoUrl, mut maybe_header: Option<(HeaderName, HeaderValue)>, - ) -> Result<reqwest::Response, AnyError> { + ) -> Result<reqwest::Response, DownloadError> { let mut url = url.into_url()?; let mut builder = self.get(url.clone()); if let Some((header_name, header_value)) = maybe_header.as_ref() { @@ -486,7 +506,7 @@ impl HttpClient { return Ok(new_response); } } - Err(custom_error("Http", "Too many redirects.")) + Err(DownloadError::TooManyRedirects) } else { Ok(response) } @@ -496,7 +516,7 @@ impl HttpClient { async fn get_response_body_with_progress( response: reqwest::Response, progress_guard: Option<&UpdateGuard>, -) -> Result<Vec<u8>, AnyError> { +) -> Result<Vec<u8>, reqwest::Error> { if let Some(progress_guard) = progress_guard { if let Some(total_size) = response.content_length() { progress_guard.set_total_size(total_size); @@ -546,7 +566,7 @@ fn resolve_url_from_location(base_url: &Url, location: &str) -> Url { fn resolve_redirect_from_response( request_url: &Url, response: &reqwest::Response, -) -> Result<Url, AnyError> { +) -> Result<Url, DownloadError> { debug_assert!(response.status().is_redirection()); if let Some(location) = response.headers().get(LOCATION) { let location_string = location.to_str()?; @@ -554,9 +574,9 @@ fn resolve_redirect_from_response( let new_url = resolve_url_from_location(request_url, location_string); Ok(new_url) } else { - Err(generic_error(format!( - "Redirection from '{request_url}' did not provide location header" - ))) + Err(DownloadError::NoRedirectHeader { + request_url: request_url.clone(), + }) } } diff --git a/cli/npm/managed/cache/tarball.rs b/cli/npm/managed/cache/tarball.rs index 042c3cbb2..46186b87c 100644 --- a/cli/npm/managed/cache/tarball.rs +++ b/cli/npm/managed/cache/tarball.rs @@ -15,8 +15,11 @@ use deno_npm::npm_rc::ResolvedNpmRc; use deno_npm::registry::NpmPackageVersionDistInfo; use deno_runtime::deno_fs::FileSystem; use deno_semver::package::PackageNv; +use reqwest::StatusCode; +use reqwest::Url; use crate::args::CacheSetting; +use crate::http_util::DownloadError; use crate::http_util::HttpClientProvider; use crate::npm::common::maybe_auth_header_for_npm_registry; use crate::util::progress_bar::ProgressBar; @@ -138,8 +141,6 @@ impl TarballCache { let tarball_cache = self.clone(); async move { let registry_url = tarball_cache.npmrc.get_registry_url(&package_nv.name); - let registry_config = - tarball_cache.npmrc.get_registry_config(&package_nv.name).clone(); let package_folder = tarball_cache.cache.package_folder_for_nv_and_url(&package_nv, registry_url); let should_use_cache = tarball_cache.cache.should_use_cache_for_package(&package_nv); @@ -161,14 +162,40 @@ impl TarballCache { bail!("Tarball URL was empty."); } - let maybe_auth_header = - maybe_auth_header_for_npm_registry(®istry_config); + // IMPORTANT: npm registries may specify tarball URLs at different URLS than the + // registry, so we MUST get the auth for the tarball URL and not the registry URL. + let tarball_uri = Url::parse(&dist.tarball)?; + let maybe_registry_config = + tarball_cache.npmrc.tarball_config(&tarball_uri); + let maybe_auth_header = maybe_registry_config.and_then(|c| maybe_auth_header_for_npm_registry(c)); let guard = tarball_cache.progress_bar.update(&dist.tarball); - let maybe_bytes = tarball_cache.http_client_provider + let result = tarball_cache.http_client_provider .get_or_create()? - .download_with_progress(&dist.tarball, maybe_auth_header, &guard) - .await?; + .download_with_progress(tarball_uri, maybe_auth_header, &guard) + .await; + let maybe_bytes = match result { + Ok(maybe_bytes) => maybe_bytes, + Err(DownloadError::BadResponse(err)) => { + if err.status_code == StatusCode::UNAUTHORIZED + && maybe_registry_config.is_none() + && tarball_cache.npmrc.get_registry_config(&package_nv.name).auth_token.is_some() + { + bail!( + concat!( + "No auth for tarball URI, but present for scoped registry.\n\n", + "Tarball URI: {}\n", + "Scope URI: {}\n\n", + "More info here: https://github.com/npm/cli/wiki/%22No-auth-for-URI,-but-auth-present-for-scoped-registry%22" + ), + dist.tarball, + registry_url, + ) + } + return Err(err.into()) + }, + Err(err) => return Err(err.into()), + }; match maybe_bytes { Some(bytes) => { let extraction_mode = if should_use_cache || !package_folder_exists { |