Skip to content

Commit

Permalink
fix: .npmrc settings not being passed to install/add command (#26473)
Browse files Browse the repository at this point in the history 
We weren't passing the resolved npmrc settings to the install commands.
This lead us to always fall back to the default registry url instead of
using the one from npmrc.

Fixes #26139
Fixes #26033
Fixes #25924
Fixes #25822
Fixes #26152

---------

Co-authored-by: Bartek Iwańczuk <[email protected]>
  • Loading branch information
@marvinhagemeister @bartlomieju
marvinhagemeister and bartlomieju committed Oct 25, 2024
1 parent f62283f commit f4ef7b0
Show file tree
Hide file tree
Showing 39 changed files with 317 additions and 42 deletions.
1 change: 1 addition & 0 deletions cli/args/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,7 @@ fn discover_npmrc(
let resolved = npmrc
.as_resolved(npm_registry_url())
.context("Failed to resolve .npmrc options")?;
log::debug!(".npmrc found at: '{}'", path.display());
Ok(Arc::new(resolved))
}

Expand Down
1 change: 1 addition & 0 deletions cli/cache/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@ impl Loader for FetchCacher {
} else {
FetchPermissionsOptionRef::DynamicContainer(&permissions)
},
maybe_auth: None,
maybe_accept: None,
maybe_cache_setting: maybe_cache_setting.as_ref(),
},
Expand Down
30 changes: 29 additions & 1 deletion cli/file_fetcher.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use deno_graph::source::LoaderChecksum;
use deno_path_util::url_to_file_path;
use deno_runtime::deno_permissions::PermissionsContainer;
use deno_runtime::deno_web::BlobStore;
use http::header;
use log::debug;
use std::borrow::Cow;
use std::collections::HashMap;
Expand Down Expand Up @@ -181,6 +182,7 @@ pub enum FetchPermissionsOptionRef<'a> {
pub struct FetchOptions<'a> {
pub specifier: &'a ModuleSpecifier,
pub permissions: FetchPermissionsOptionRef<'a>,
pub maybe_auth: Option<(header::HeaderName, header::HeaderValue)>,
pub maybe_accept: Option<&'a str>,
pub maybe_cache_setting: Option<&'a CacheSetting>,
}
Expand Down Expand Up @@ -350,6 +352,7 @@ impl FileFetcher {
maybe_accept: Option<&str>,
cache_setting: &CacheSetting,
maybe_checksum: Option<&LoaderChecksum>,
maybe_auth: Option<(header::HeaderName, header::HeaderValue)>,
) -> Result<FileOrRedirect, AnyError> {
debug!(
"FileFetcher::fetch_remote_no_follow - specifier: {}",
Expand Down Expand Up @@ -442,6 +445,7 @@ impl FileFetcher {
.as_ref()
.map(|(_, etag)| etag.clone()),
maybe_auth_token: maybe_auth_token.clone(),
maybe_auth: maybe_auth.clone(),
maybe_progress_guard: maybe_progress_guard.as_ref(),
})
.await?
Expand Down Expand Up @@ -538,7 +542,18 @@ impl FileFetcher {
specifier: &ModuleSpecifier,
) -> Result<File, AnyError> {
self
.fetch_inner(specifier, FetchPermissionsOptionRef::AllowAll)
.fetch_inner(specifier, None, FetchPermissionsOptionRef::AllowAll)
.await
}

#[inline(always)]
pub async fn fetch_bypass_permissions_with_maybe_auth(
&self,
specifier: &ModuleSpecifier,
maybe_auth: Option<(header::HeaderName, header::HeaderValue)>,
) -> Result<File, AnyError> {
self
.fetch_inner(specifier, maybe_auth, FetchPermissionsOptionRef::AllowAll)
.await
}

Expand All @@ -552,6 +567,7 @@ impl FileFetcher {
self
.fetch_inner(
specifier,
None,
FetchPermissionsOptionRef::StaticContainer(permissions),
)
.await
Expand All @@ -560,12 +576,14 @@ impl FileFetcher {
async fn fetch_inner(
&self,
specifier: &ModuleSpecifier,
maybe_auth: Option<(header::HeaderName, header::HeaderValue)>,
permissions: FetchPermissionsOptionRef<'_>,
) -> Result<File, AnyError> {
self
.fetch_with_options(FetchOptions {
specifier,
permissions,
maybe_auth,
maybe_accept: None,
maybe_cache_setting: None,
})
Expand All @@ -585,12 +603,14 @@ impl FileFetcher {
max_redirect: usize,
) -> Result<File, AnyError> {
let mut specifier = Cow::Borrowed(options.specifier);
let mut maybe_auth = options.maybe_auth.clone();
for _ in 0..=max_redirect {
match self
.fetch_no_follow_with_options(FetchNoFollowOptions {
fetch_options: FetchOptions {
specifier: &specifier,
permissions: options.permissions,
maybe_auth: maybe_auth.clone(),
maybe_accept: options.maybe_accept,
maybe_cache_setting: options.maybe_cache_setting,
},
Expand All @@ -602,6 +622,10 @@ impl FileFetcher {
return Ok(file);
}
FileOrRedirect::Redirect(redirect_specifier) => {
// If we were redirected to another origin, don't send the auth header anymore.
if redirect_specifier.origin() != specifier.origin() {
maybe_auth = None;
}
specifier = Cow::Owned(redirect_specifier);
}
}
Expand Down Expand Up @@ -666,6 +690,7 @@ impl FileFetcher {
options.maybe_accept,
options.maybe_cache_setting.unwrap_or(&self.cache_setting),
maybe_checksum,
options.maybe_auth,
)
.await
}
Expand Down Expand Up @@ -756,6 +781,7 @@ mod tests {
FetchOptions {
specifier,
permissions: FetchPermissionsOptionRef::AllowAll,
maybe_auth: None,
maybe_accept: None,
maybe_cache_setting: Some(&file_fetcher.cache_setting),
},
Expand Down Expand Up @@ -1255,6 +1281,7 @@ mod tests {
FetchOptions {
specifier: &specifier,
permissions: FetchPermissionsOptionRef::AllowAll,
maybe_auth: None,
maybe_accept: None,
maybe_cache_setting: Some(&file_fetcher.cache_setting),
},
Expand All @@ -1268,6 +1295,7 @@ mod tests {
FetchOptions {
specifier: &specifier,
permissions: FetchPermissionsOptionRef::AllowAll,
maybe_auth: None,
maybe_accept: None,
maybe_cache_setting: Some(&file_fetcher.cache_setting),
},
Expand Down
21 changes: 21 additions & 0 deletions cli/http_util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use deno_runtime::deno_fetch;
use deno_runtime::deno_fetch::create_http_client;
use deno_runtime::deno_fetch::CreateHttpClientOptions;
use deno_runtime::deno_tls::RootCertStoreProvider;
use http::header;
use http::header::HeaderName;
use http::header::HeaderValue;
use http::header::ACCEPT;
Expand Down Expand Up @@ -204,6 +205,7 @@ pub struct FetchOnceArgs<'a> {
pub maybe_accept: Option<String>,
pub maybe_etag: Option<String>,
pub maybe_auth_token: Option<AuthToken>,
pub maybe_auth: Option<(header::HeaderName, header::HeaderValue)>,
pub maybe_progress_guard: Option<&'a UpdateGuard>,
}

Expand Down Expand Up @@ -382,6 +384,8 @@ impl HttpClient {
request
.headers_mut()
.insert(AUTHORIZATION, authorization_val);
} else if let Some((header, value)) = args.maybe_auth {
request.headers_mut().insert(header, value);
}
if let Some(accept) = args.maybe_accept {
let accepts_val = HeaderValue::from_str(&accept)?;
Expand Down Expand Up @@ -792,6 +796,7 @@ mod test {
maybe_etag: None,
maybe_auth_token: None,
maybe_progress_guard: None,
maybe_auth: None,
})
.await;
if let Ok(FetchOnceResult::Code(body, headers)) = result {
Expand All @@ -818,6 +823,7 @@ mod test {
maybe_etag: None,
maybe_auth_token: None,
maybe_progress_guard: None,
maybe_auth: None,
})
.await;
if let Ok(FetchOnceResult::Code(body, headers)) = result {
Expand Down Expand Up @@ -845,6 +851,7 @@ mod test {
maybe_etag: None,
maybe_auth_token: None,
maybe_progress_guard: None,
maybe_auth: None,
})
.await;
if let Ok(FetchOnceResult::Code(body, headers)) = result {
Expand All @@ -866,6 +873,7 @@ mod test {
maybe_etag: Some("33a64df551425fcc55e".to_string()),
maybe_auth_token: None,
maybe_progress_guard: None,
maybe_auth: None,
})
.await;
assert_eq!(res.unwrap(), FetchOnceResult::NotModified);
Expand All @@ -885,6 +893,7 @@ mod test {
maybe_etag: None,
maybe_auth_token: None,
maybe_progress_guard: None,
maybe_auth: None,
})
.await;
if let Ok(FetchOnceResult::Code(body, headers)) = result {
Expand Down Expand Up @@ -914,6 +923,7 @@ mod test {
maybe_etag: None,
maybe_auth_token: None,
maybe_progress_guard: None,
maybe_auth: None,
})
.await;
if let Ok(FetchOnceResult::Code(body, _)) = result {
Expand All @@ -939,6 +949,7 @@ mod test {
maybe_etag: None,
maybe_auth_token: None,
maybe_progress_guard: None,
maybe_auth: None,
})
.await;
if let Ok(FetchOnceResult::Redirect(url, _)) = result {
Expand Down Expand Up @@ -974,6 +985,7 @@ mod test {
maybe_etag: None,
maybe_auth_token: None,
maybe_progress_guard: None,
maybe_auth: None,
})
.await;
if let Ok(FetchOnceResult::Code(body, headers)) = result {
Expand Down Expand Up @@ -1021,6 +1033,7 @@ mod test {
maybe_etag: None,
maybe_auth_token: None,
maybe_progress_guard: None,
maybe_auth: None,
})
.await;

Expand Down Expand Up @@ -1083,6 +1096,7 @@ mod test {
maybe_etag: None,
maybe_auth_token: None,
maybe_progress_guard: None,
maybe_auth: None,
})
.await;

Expand Down Expand Up @@ -1136,6 +1150,7 @@ mod test {
maybe_etag: None,
maybe_auth_token: None,
maybe_progress_guard: None,
maybe_auth: None,
})
.await;
if let Ok(FetchOnceResult::Code(body, headers)) = result {
Expand Down Expand Up @@ -1177,6 +1192,7 @@ mod test {
maybe_etag: None,
maybe_auth_token: None,
maybe_progress_guard: None,
maybe_auth: None,
})
.await;
if let Ok(FetchOnceResult::Code(body, headers)) = result {
Expand All @@ -1199,6 +1215,7 @@ mod test {
maybe_etag: Some("33a64df551425fcc55e".to_string()),
maybe_auth_token: None,
maybe_progress_guard: None,
maybe_auth: None,
})
.await;
assert_eq!(res.unwrap(), FetchOnceResult::NotModified);
Expand Down Expand Up @@ -1233,6 +1250,7 @@ mod test {
maybe_etag: None,
maybe_auth_token: None,
maybe_progress_guard: None,
maybe_auth: None,
})
.await;
if let Ok(FetchOnceResult::Code(body, headers)) = result {
Expand Down Expand Up @@ -1262,6 +1280,7 @@ mod test {
maybe_etag: None,
maybe_auth_token: None,
maybe_progress_guard: None,
maybe_auth: None,
})
.await;
assert!(result.is_err());
Expand All @@ -1283,6 +1302,7 @@ mod test {
maybe_etag: None,
maybe_auth_token: None,
maybe_progress_guard: None,
maybe_auth: None,
})
.await;

Expand All @@ -1306,6 +1326,7 @@ mod test {
maybe_etag: None,
maybe_auth_token: None,
maybe_progress_guard: None,
maybe_auth: None,
})
.await;

Expand Down
6 changes: 5 additions & 1 deletion cli/lsp/npm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use dashmap::DashMap;
use deno_core::anyhow::anyhow;
use deno_core::error::AnyError;
use deno_core::serde_json;
use deno_npm::npm_rc::NpmRc;
use deno_semver::package::PackageNv;
use deno_semver::Version;
use serde::Deserialize;
Expand All @@ -25,7 +26,10 @@ pub struct CliNpmSearchApi {

impl CliNpmSearchApi {
pub fn new(file_fetcher: Arc<FileFetcher>) -> Self {
let resolver = NpmFetchResolver::new(file_fetcher.clone());
let resolver = NpmFetchResolver::new(
file_fetcher.clone(),
Arc::new(NpmRc::default().as_resolved(npm_registry_url()).unwrap()),
);
Self {
file_fetcher,
resolver,
Expand Down
1 change: 1 addition & 0 deletions cli/lsp/registries.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,7 @@ impl ModuleRegistry {
.fetch_with_options(FetchOptions {
specifier: &specifier,
permissions: FetchPermissionsOptionRef::AllowAll,
maybe_auth: None,
maybe_accept: Some("application/vnd.deno.reg.v2+json, application/vnd.deno.reg.v1+json;q=0.9, application/json;q=0.8"),
maybe_cache_setting: None,
})
Expand Down
2 changes: 1 addition & 1 deletion cli/npm/managed/cache/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use crate::cache::CACHE_PERM;
use crate::util::fs::atomic_write_file_with_retries;
use crate::util::fs::hard_link_dir_recursive;

mod registry_info;
pub mod registry_info;
mod tarball;
mod tarball_extract;

Expand Down
Loading

0 comments on commit f4ef7b0

Please sign in to comment.