From f5c84920c225579af9c249bdac4a59a046ef8683 Mon Sep 17 00:00:00 2001 From: tokiedokie Date: Tue, 15 Sep 2020 14:18:48 +0900 Subject: [PATCH] fix(cli/http_utils): accept a single key-multiple values headers (#7375) --- cli/file_fetcher.rs | 146 ++++++++++++++++++++++++++++++++++++-------- cli/http_cache.rs | 25 +++++--- cli/http_util.rs | 84 ++++++++++++++++++++----- 3 files changed, 207 insertions(+), 48 deletions(-) diff --git a/cli/file_fetcher.rs b/cli/file_fetcher.rs index 2a18a9a6a6..c62e3732e6 100644 --- a/cli/file_fetcher.rs +++ b/cli/file_fetcher.rs @@ -404,7 +404,7 @@ impl SourceFileFetcher { }; let (mut source_file, headers) = result; - if let Some(redirect_to) = headers.get("location") { + if let Some(redirect_to) = headers.get("location").and_then(|e| e.first()) { let redirect_url = match Url::parse(redirect_to) { Ok(redirect_url) => redirect_url, Err(url::ParseError::RelativeUrlWithoutBase) => { @@ -427,9 +427,15 @@ impl SourceFileFetcher { let fake_filepath = PathBuf::from(module_url.path()); let (media_type, charset) = map_content_type( &fake_filepath, - headers.get("content-type").map(|e| e.as_str()), + headers + .get("content-type") + .and_then(|e| e.first()) + .map(|e| e.as_str()), ); - let types_header = headers.get("x-typescript-types").map(|e| e.to_string()); + let types_header = headers + .get("x-typescript-types") + .and_then(|e| e.first()) + .map(|e| e.to_string()); Ok(Some(SourceFile { url: module_url.clone(), filename: cache_filename, @@ -493,7 +499,10 @@ impl SourceFileFetcher { let dir = self.clone(); let module_url = module_url.clone(); let module_etag = match self.http_cache.get(&module_url) { - Ok((_, headers)) => headers.get("etag").map(String::from), + Ok((_, headers)) => headers + .get("etag") + .and_then(|e| e.first()) + .map(|e| e.to_string()), Err(_) => None, }; let permissions = permissions.clone(); @@ -532,11 +541,16 @@ impl SourceFileFetcher { let fake_filepath = PathBuf::from(module_url.path()); let (media_type, charset) = map_content_type( &fake_filepath, - headers.get("content-type").map(String::as_str), + headers + .get("content-type") + .and_then(|e| e.first()) + .map(|e| e.as_str()), ); - let types_header = - headers.get("x-typescript-types").map(String::to_string); + let types_header = headers + .get("x-typescript-types") + .and_then(|e| e.first()) + .map(|e| e.to_string()); let source_file = SourceFile { url: module_url.clone(), @@ -811,7 +825,9 @@ mod tests { metadata.headers = HashMap::new(); metadata .headers - .insert("content-type".to_string(), "text/javascript".to_string()); + .entry("content-type".to_string()) + .or_insert_with(Vec::new) + .push("text/javascript".to_string()); metadata.write(&cache_filename).unwrap(); let result2 = fetcher_1 @@ -834,13 +850,23 @@ mod tests { assert_eq!(&(r2.media_type), &msg::MediaType::JavaScript); let (_, headers) = fetcher_2.http_cache.get(&module_url_1).unwrap(); - assert_eq!(headers.get("content-type").unwrap(), "text/javascript"); + assert_eq!( + headers + .get("content-type") + .unwrap() + .first() + .unwrap() + .as_str(), + "text/javascript" + ); // Modify .headers.json again, but the other way around metadata.headers = HashMap::new(); metadata .headers - .insert("content-type".to_string(), "application/json".to_string()); + .entry("content-type".to_string()) + .or_insert_with(Vec::new) + .push("application/json".to_string()); metadata.write(&cache_filename).unwrap(); let result3 = fetcher_2 @@ -863,7 +889,13 @@ mod tests { assert_eq!(&(r3.media_type), &msg::MediaType::Json); let metadata = crate::http_cache::Metadata::read(&cache_filename).unwrap(); assert_eq!( - metadata.headers.get("content-type").unwrap(), + metadata + .headers + .get("content-type") + .unwrap() + .first() + .unwrap() + .as_str(), "application/json" ); @@ -913,7 +945,15 @@ mod tests { assert_eq!(r.source_code.bytes, expected); assert_eq!(&(r.media_type), &msg::MediaType::JavaScript); let (_, headers) = fetcher.http_cache.get(&module_url).unwrap(); - assert_eq!(headers.get("content-type").unwrap(), "text/javascript"); + assert_eq!( + headers + .get("content-type") + .unwrap() + .first() + .unwrap() + .as_str(), + "text/javascript" + ); // Modify .headers.json let mut metadata = @@ -921,7 +961,9 @@ mod tests { metadata.headers = HashMap::new(); metadata .headers - .insert("content-type".to_string(), "text/typescript".to_string()); + .entry("content-type".to_string()) + .or_insert_with(Vec::new) + .push("text/typescript".to_string()); metadata.write(&cache_filename).unwrap(); let result2 = fetcher @@ -943,7 +985,13 @@ mod tests { assert_eq!(&(r2.media_type), &msg::MediaType::TypeScript); let metadata = crate::http_cache::Metadata::read(&cache_filename).unwrap(); assert_eq!( - metadata.headers.get("content-type").unwrap(), + metadata + .headers + .get("content-type") + .unwrap() + .first() + .unwrap() + .as_str(), "text/typescript" ); @@ -967,7 +1015,15 @@ mod tests { // (due to http fetch) assert_eq!(&(r3.media_type), &msg::MediaType::JavaScript); let (_, headers) = fetcher.http_cache.get(&module_url).unwrap(); - assert_eq!(headers.get("content-type").unwrap(), "text/javascript"); + assert_eq!( + headers + .get("content-type") + .unwrap() + .first() + .unwrap() + .as_str(), + "text/javascript" + ); } #[tokio::test] @@ -1053,7 +1109,7 @@ mod tests { assert_eq!(fs::read_to_string(&redirect_source_filename).unwrap(), ""); let (_, headers) = fetcher.http_cache.get(&redirect_module_url).unwrap(); assert_eq!( - headers.get("location").unwrap(), + headers.get("location").unwrap().first().unwrap().as_str(), "http://localhost:4545/cli/tests/subdir/redirects/redirect1.js" ); // The target of redirection is downloaded instead. @@ -1106,10 +1162,16 @@ mod tests { assert_eq!(fs::read_to_string(&redirect_path).unwrap(), ""); let (_, headers) = fetcher.http_cache.get(&double_redirect_url).unwrap(); - assert_eq!(headers.get("location").unwrap(), &redirect_url.to_string()); + assert_eq!( + headers.get("location").unwrap().first().unwrap(), + &redirect_url.to_string() + ); let (_, headers) = fetcher.http_cache.get(&redirect_url).unwrap(); - assert_eq!(headers.get("location").unwrap(), &target_url.to_string()); + assert_eq!( + headers.get("location").unwrap().first().unwrap(), + &target_url.to_string() + ); // The target of redirection is downloaded instead. assert_eq!( @@ -1262,7 +1324,7 @@ mod tests { assert_eq!(fs::read_to_string(&redirect_source_filename).unwrap(), ""); let (_, headers) = fetcher.http_cache.get(&redirect_module_url).unwrap(); assert_eq!( - headers.get("location").unwrap(), + headers.get("location").unwrap().first().unwrap().as_str(), "/cli/tests/subdir/redirects/redirect1.js" ); // The target of redirection is downloaded instead. @@ -1376,7 +1438,9 @@ mod tests { metadata.headers = HashMap::new(); metadata .headers - .insert("content-type".to_string(), "text/javascript".to_string()); + .entry("content-type".to_string()) + .or_insert_with(Vec::new) + .push("text/javascript".to_string()); metadata.write(&cache_filename).unwrap(); let result2 = fetcher.fetch_cached_remote_source(&module_url, 1); @@ -1407,7 +1471,15 @@ mod tests { assert_eq!(r.source_code.bytes, b"export const loaded = true;\n"); assert_eq!(&(r.media_type), &msg::MediaType::TypeScript); let (_, headers) = fetcher.http_cache.get(module_url).unwrap(); - assert_eq!(headers.get("content-type").unwrap(), "text/typescript"); + assert_eq!( + headers + .get("content-type") + .unwrap() + .first() + .unwrap() + .as_str(), + "text/typescript" + ); } #[tokio::test] @@ -1431,7 +1503,15 @@ mod tests { assert_eq!(r2.source_code.bytes, b"export const loaded = true;\n"); assert_eq!(&(r2.media_type), &msg::MediaType::JavaScript); let (_, headers) = fetcher.http_cache.get(module_url).unwrap(); - assert_eq!(headers.get("content-type").unwrap(), "text/javascript"); + assert_eq!( + headers + .get("content-type") + .unwrap() + .first() + .unwrap() + .as_str(), + "text/javascript" + ); } #[tokio::test] @@ -1455,7 +1535,15 @@ mod tests { assert_eq!(r3.source_code.bytes, b"export const loaded = true;\n"); assert_eq!(&(r3.media_type), &msg::MediaType::TypeScript); let (_, headers) = fetcher.http_cache.get(module_url).unwrap(); - assert_eq!(headers.get("content-type").unwrap(), "text/typescript"); + assert_eq!( + headers + .get("content-type") + .unwrap() + .first() + .unwrap() + .as_str(), + "text/typescript" + ); } #[tokio::test] @@ -1811,7 +1899,10 @@ mod tests { assert_eq!(&(source.media_type), &msg::MediaType::TypeScript); let (_, headers) = fetcher.http_cache.get(&module_url).unwrap(); - assert_eq!(headers.get("etag").unwrap(), "33a64df551425fcc55e"); + assert_eq!( + headers.get("etag").unwrap().first().unwrap().as_str(), + "33a64df551425fcc55e" + ); let metadata_path = crate::http_cache::Metadata::filename( &fetcher.http_cache.get_cache_filename(&module_url), @@ -1936,7 +2027,12 @@ mod tests { let (_, headers) = fetcher.http_cache.get(&module_url).unwrap(); assert_eq!( - headers.get("content-type").unwrap(), + headers + .get("content-type") + .unwrap() + .first() + .unwrap() + .as_str(), &format!("application/typescript;charset={}", charset) ); } diff --git a/cli/http_cache.rs b/cli/http_cache.rs index 6b2465b383..d14f3d2add 100644 --- a/cli/http_cache.rs +++ b/cli/http_cache.rs @@ -215,11 +215,14 @@ mod tests { let cache = HttpCache::new(dir.path()); let url = Url::parse("https://deno.land/x/welcome.ts").unwrap(); let mut headers = HashMap::new(); - headers.insert( - "content-type".to_string(), - "application/javascript".to_string(), - ); - headers.insert("etag".to_string(), "as5625rqdsfb".to_string()); + headers + .entry("content-type".to_string()) + .or_insert_with(Vec::new) + .push("application/javascript".to_string()); + headers + .entry("etag".to_string()) + .or_insert_with(Vec::new) + .push("as5625rqdsfb".to_string()); let content = b"Hello world"; let r = cache.set(&url, headers, content); eprintln!("result {:?}", r); @@ -231,10 +234,18 @@ mod tests { file.read_to_string(&mut content).unwrap(); assert_eq!(content, "Hello world"); assert_eq!( - headers.get("content-type").unwrap(), + headers + .get("content-type") + .unwrap() + .first() + .unwrap() + .as_str(), "application/javascript" ); - assert_eq!(headers.get("etag").unwrap(), "as5625rqdsfb"); + assert_eq!( + headers.get("etag").unwrap().first().unwrap().as_str(), + "as5625rqdsfb" + ); assert_eq!(headers.get("foobar"), None); } diff --git a/cli/http_util.rs b/cli/http_util.rs index 015bfaa46e..ebd84972db 100644 --- a/cli/http_util.rs +++ b/cli/http_util.rs @@ -76,10 +76,7 @@ fn resolve_url_from_location(base_url: &Url, location: &str) -> Url { } } -// 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 -// Vec<(String, String)> -pub type HeadersMap = HashMap; +pub type HeadersMap = HashMap>; #[derive(Debug, PartialEq)] pub enum FetchOnceResult { @@ -112,7 +109,7 @@ pub async fn fetch_once( return Ok(FetchOnceResult::NotModified); } - let mut headers_: HashMap = HashMap::new(); + let mut headers_: HashMap> = HashMap::new(); let headers = response.headers(); if let Some(warning) = headers.get("X-Deno-Warning") { @@ -131,7 +128,10 @@ pub async fn fetch_once( .map(|e| e.to_str().unwrap().to_string()) .collect::>() .join(","); - headers_.insert(key_str, values_str); + headers_ + .entry(key_str) + .or_insert_with(Vec::new) + .push(values_str); } if response.status().is_redirection() { @@ -248,7 +248,15 @@ mod tests { let result = fetch_once(client, &url, None).await; if let Ok(FetchOnceResult::Code(body, headers)) = result { assert!(!body.is_empty()); - assert_eq!(headers.get("content-type").unwrap(), "application/json"); + assert_eq!( + headers + .get("content-type") + .unwrap() + .first() + .unwrap() + .as_str(), + "application/json" + ); assert_eq!(headers.get("etag"), None); assert_eq!(headers.get("x-typescript-types"), None); } else { @@ -269,7 +277,12 @@ mod tests { if let Ok(FetchOnceResult::Code(body, headers)) = result { assert_eq!(String::from_utf8(body).unwrap(), "console.log('gzip')"); assert_eq!( - headers.get("content-type").unwrap(), + headers + .get("content-type") + .unwrap() + .first() + .unwrap() + .as_str(), "application/javascript" ); assert_eq!(headers.get("etag"), None); @@ -289,10 +302,18 @@ mod tests { assert!(!body.is_empty()); assert_eq!(String::from_utf8(body).unwrap(), "console.log('etag')"); assert_eq!( - headers.get("content-type").unwrap(), + headers + .get("content-type") + .unwrap() + .first() + .unwrap() + .as_str(), "application/typescript" ); - assert_eq!(headers.get("etag").unwrap(), "33a64df551425fcc55e"); + assert_eq!( + headers.get("etag").unwrap().first().unwrap().as_str(), + "33a64df551425fcc55e" + ); } else { panic!(); } @@ -316,7 +337,12 @@ mod tests { assert!(!body.is_empty()); assert_eq!(String::from_utf8(body).unwrap(), "console.log('brotli');"); assert_eq!( - headers.get("content-type").unwrap(), + headers + .get("content-type") + .unwrap() + .first() + .unwrap() + .as_str(), "application/javascript" ); assert_eq!(headers.get("etag"), None); @@ -399,7 +425,15 @@ mod tests { let result = fetch_once(client, &url, None).await; if let Ok(FetchOnceResult::Code(body, headers)) = result { assert!(!body.is_empty()); - assert_eq!(headers.get("content-type").unwrap(), "application/json"); + assert_eq!( + headers + .get("content-type") + .unwrap() + .first() + .unwrap() + .as_str(), + "application/json" + ); assert_eq!(headers.get("etag"), None); assert_eq!(headers.get("x-typescript-types"), None); } else { @@ -426,7 +460,12 @@ mod tests { if let Ok(FetchOnceResult::Code(body, headers)) = result { assert_eq!(String::from_utf8(body).unwrap(), "console.log('gzip')"); assert_eq!( - headers.get("content-type").unwrap(), + headers + .get("content-type") + .unwrap() + .first() + .unwrap() + .as_str(), "application/javascript" ); assert_eq!(headers.get("etag"), None); @@ -452,10 +491,18 @@ mod tests { assert!(!body.is_empty()); assert_eq!(String::from_utf8(body).unwrap(), "console.log('etag')"); assert_eq!( - headers.get("content-type").unwrap(), + headers + .get("content-type") + .unwrap() + .first() + .unwrap() + .as_str(), "application/typescript" ); - assert_eq!(headers.get("etag").unwrap(), "33a64df551425fcc55e"); + assert_eq!( + headers.get("etag").unwrap().first().unwrap().as_str(), + "33a64df551425fcc55e" + ); assert_eq!(headers.get("x-typescript-types"), None); } else { panic!(); @@ -486,7 +533,12 @@ mod tests { assert!(!body.is_empty()); assert_eq!(String::from_utf8(body).unwrap(), "console.log('brotli');"); assert_eq!( - headers.get("content-type").unwrap(), + headers + .get("content-type") + .unwrap() + .first() + .unwrap() + .as_str(), "application/javascript" ); assert_eq!(headers.get("etag"), None);