From b5fdfb9d25a636725d3aeae055bd1e740f6ec1d6 Mon Sep 17 00:00:00 2001 From: Kitson Kelly Date: Tue, 21 Dec 2021 13:40:22 +1100 Subject: [PATCH] feat(lsp): supply accept header when fetching registry config (#13159) Closes #13153 --- cli/file_fetcher.rs | 34 +++++++++++++++++++++++++++----- cli/http_util.rs | 46 ++++++++++++++++++++++++++++++++++++++++--- cli/lsp/registries.rs | 6 +++++- test_util/src/lib.rs | 7 +++++++ 4 files changed, 84 insertions(+), 9 deletions(-) diff --git a/cli/file_fetcher.rs b/cli/file_fetcher.rs index c10c72e531..3ac8f9bd4f 100644 --- a/cli/file_fetcher.rs +++ b/cli/file_fetcher.rs @@ -474,6 +474,7 @@ impl FileFetcher { maybe_headers: Some(headers), }) } + /// Asynchronously fetch remote source file specified by the URL following /// redirects. /// @@ -484,6 +485,7 @@ impl FileFetcher { specifier: &ModuleSpecifier, permissions: &mut Permissions, redirect_limit: i64, + maybe_accept: Option, ) -> Pin> + Send>> { debug!("FileFetcher::fetch_remote() - specifier: {}", specifier); if redirect_limit < 0 { @@ -539,6 +541,7 @@ impl FileFetcher { match fetch_once(FetchOnceArgs { client, url: specifier.clone(), + maybe_accept: maybe_accept.clone(), maybe_etag, maybe_auth_token, }) @@ -551,7 +554,12 @@ impl FileFetcher { FetchOnceResult::Redirect(redirect_url, headers) => { file_fetcher.http_cache.set(&specifier, headers, &[])?; file_fetcher - .fetch_remote(&redirect_url, &mut permissions, redirect_limit - 1) + .fetch_remote( + &redirect_url, + &mut permissions, + redirect_limit - 1, + maybe_accept, + ) .await } FetchOnceResult::Code(bytes, headers) => { @@ -574,6 +582,15 @@ impl FileFetcher { permissions: &mut Permissions, ) -> Result { debug!("FileFetcher::fetch() - specifier: {}", specifier); + self.fetch_with_accept(specifier, permissions, None).await + } + + pub async fn fetch_with_accept( + &self, + specifier: &ModuleSpecifier, + permissions: &mut Permissions, + maybe_accept: Option<&str>, + ) -> Result { let scheme = get_validated_scheme(specifier)?; permissions.check_specifier(specifier)?; if let Some(file) = self.cache.get(specifier) { @@ -600,7 +617,14 @@ impl FileFetcher { format!("A remote specifier was requested: \"{}\", but --no-remote is specified.", specifier), )) } else { - let result = self.fetch_remote(specifier, permissions, 10).await; + let result = self + .fetch_remote( + specifier, + permissions, + 10, + maybe_accept.map(String::from), + ) + .await; if let Ok(file) = &result { self.cache.insert(specifier.clone(), file.clone()); } @@ -710,7 +734,7 @@ mod tests { let _http_server_guard = test_util::http_server(); let (file_fetcher, _) = setup(CacheSetting::ReloadAll, None); let result: Result = file_fetcher - .fetch_remote(specifier, &mut Permissions::allow_all(), 1) + .fetch_remote(specifier, &mut Permissions::allow_all(), 1, None) .await; assert!(result.is_ok()); let (_, headers, _) = file_fetcher.http_cache.get(specifier).unwrap(); @@ -1375,12 +1399,12 @@ mod tests { .unwrap(); let result = file_fetcher - .fetch_remote(&specifier, &mut Permissions::allow_all(), 2) + .fetch_remote(&specifier, &mut Permissions::allow_all(), 2, None) .await; assert!(result.is_ok()); let result = file_fetcher - .fetch_remote(&specifier, &mut Permissions::allow_all(), 1) + .fetch_remote(&specifier, &mut Permissions::allow_all(), 1, None) .await; assert!(result.is_err()); diff --git a/cli/http_util.rs b/cli/http_util.rs index 562cd06f27..78a22d3e5b 100644 --- a/cli/http_util.rs +++ b/cli/http_util.rs @@ -9,6 +9,7 @@ use deno_core::error::generic_error; use deno_core::error::AnyError; use deno_core::url::Url; use deno_runtime::deno_fetch::reqwest::header::HeaderValue; +use deno_runtime::deno_fetch::reqwest::header::ACCEPT; use deno_runtime::deno_fetch::reqwest::header::AUTHORIZATION; use deno_runtime::deno_fetch::reqwest::header::IF_NONE_MATCH; use deno_runtime::deno_fetch::reqwest::header::LOCATION; @@ -209,6 +210,7 @@ pub enum FetchOnceResult { pub struct FetchOnceArgs { pub client: Client, pub url: Url, + pub maybe_accept: Option, pub maybe_etag: Option, pub maybe_auth_token: Option, } @@ -224,14 +226,17 @@ pub async fn fetch_once( let mut request = args.client.get(args.url.clone()); if let Some(etag) = args.maybe_etag { - let if_none_match_val = HeaderValue::from_str(&etag).unwrap(); + let if_none_match_val = HeaderValue::from_str(&etag)?; request = request.header(IF_NONE_MATCH, if_none_match_val); } if let Some(auth_token) = args.maybe_auth_token { - let authorization_val = - HeaderValue::from_str(&auth_token.to_string()).unwrap(); + let authorization_val = HeaderValue::from_str(&auth_token.to_string())?; request = request.header(AUTHORIZATION, authorization_val); } + if let Some(accept) = args.maybe_accept { + let accepts_val = HeaderValue::from_str(&accept)?; + request = request.header(ACCEPT, accepts_val); + } let response = request.send().await?; if response.status() == StatusCode::NOT_MODIFIED { @@ -324,6 +329,7 @@ mod tests { let result = fetch_once(FetchOnceArgs { client, url, + maybe_accept: None, maybe_etag: None, maybe_auth_token: None, }) @@ -348,6 +354,7 @@ mod tests { let result = fetch_once(FetchOnceArgs { client, url, + maybe_accept: None, maybe_etag: None, maybe_auth_token: None, }) @@ -373,6 +380,7 @@ mod tests { let result = fetch_once(FetchOnceArgs { client: client.clone(), url: url.clone(), + maybe_accept: None, maybe_etag: None, maybe_auth_token: None, }) @@ -392,6 +400,7 @@ mod tests { let res = fetch_once(FetchOnceArgs { client, url, + maybe_accept: None, maybe_etag: Some("33a64df551425fcc55e".to_string()), maybe_auth_token: None, }) @@ -409,6 +418,7 @@ mod tests { let result = fetch_once(FetchOnceArgs { client, url, + maybe_accept: None, maybe_etag: None, maybe_auth_token: None, }) @@ -427,6 +437,27 @@ mod tests { } } + #[tokio::test] + async fn test_fetch_accept() { + let _http_server_guard = test_util::http_server(); + // Relies on external http server. See target/debug/test_server + let url = Url::parse("http://127.0.0.1:4545/echo_accept").unwrap(); + let client = create_test_client(); + let result = fetch_once(FetchOnceArgs { + client, + url, + maybe_accept: Some("application/json".to_string()), + maybe_etag: None, + maybe_auth_token: None, + }) + .await; + if let Ok(FetchOnceResult::Code(body, _)) = result { + assert_eq!(body, r#"{"accept":"application/json"}"#.as_bytes()); + } else { + panic!(); + } + } + #[tokio::test] async fn test_fetch_once_with_redirect() { let _http_server_guard = test_util::http_server(); @@ -438,6 +469,7 @@ mod tests { let result = fetch_once(FetchOnceArgs { client, url, + maybe_accept: None, maybe_etag: None, maybe_auth_token: None, }) @@ -511,6 +543,7 @@ mod tests { let result = fetch_once(FetchOnceArgs { client, url, + maybe_accept: None, maybe_etag: None, maybe_auth_token: None, }) @@ -543,6 +576,7 @@ mod tests { let result = fetch_once(FetchOnceArgs { client, url, + maybe_accept: None, maybe_etag: None, maybe_auth_token: None, }) @@ -578,6 +612,7 @@ mod tests { let result = fetch_once(FetchOnceArgs { client, url, + maybe_accept: None, maybe_etag: None, maybe_auth_token: None, }) @@ -614,6 +649,7 @@ mod tests { let result = fetch_once(FetchOnceArgs { client, url, + maybe_accept: None, maybe_etag: None, maybe_auth_token: None, }) @@ -653,6 +689,7 @@ mod tests { let result = fetch_once(FetchOnceArgs { client: client.clone(), url: url.clone(), + maybe_accept: None, maybe_etag: None, maybe_auth_token: None, }) @@ -673,6 +710,7 @@ mod tests { let res = fetch_once(FetchOnceArgs { client, url, + maybe_accept: None, maybe_etag: Some("33a64df551425fcc55e".to_string()), maybe_auth_token: None, }) @@ -705,6 +743,7 @@ mod tests { let result = fetch_once(FetchOnceArgs { client, url, + maybe_accept: None, maybe_etag: None, maybe_auth_token: None, }) @@ -732,6 +771,7 @@ mod tests { let result = fetch_once(FetchOnceArgs { client, url, + maybe_accept: None, maybe_etag: None, maybe_auth_token: None, }) diff --git a/cli/lsp/registries.rs b/cli/lsp/registries.rs index fc7e13fb7a..912a6a9ded 100644 --- a/cli/lsp/registries.rs +++ b/cli/lsp/registries.rs @@ -482,7 +482,11 @@ impl ModuleRegistry { ) -> Result, AnyError> { let fetch_result = self .file_fetcher - .fetch(specifier, &mut Permissions::allow_all()) + .fetch_with_accept( + specifier, + &mut Permissions::allow_all(), + Some("application/vnd.deno.reg.v2+json, application/vnd.deno.reg.v1+json;q=0.9, application/json;q=0.8"), + ) .await; // if there is an error fetching, we will cache an empty file, so that // subsequent requests they are just an empty doc which will error without diff --git a/test_util/src/lib.rs b/test_util/src/lib.rs index 4ad6f1c3c2..79afdde127 100644 --- a/test_util/src/lib.rs +++ b/test_util/src/lib.rs @@ -912,6 +912,13 @@ async fn main_server( ); Ok(res) } + (_, "/echo_accept") => { + let accept = req.headers().get("accept").map(|v| v.to_str().unwrap()); + let res = Response::new(Body::from( + serde_json::json!({ "accept": accept }).to_string(), + )); + Ok(res) + } _ => { let mut file_path = testdata_path(); file_path.push(&req.uri().path()[1..]);