chore: fix recent regression with deno upgrade not handling redirects (#17045)

cli/file_fetcher.rs

@@ -4,6 +4,7 @@ use crate::args::CacheSetting;
 use crate::auth_tokens::AuthTokens;
 use crate::cache::HttpCache;
 use crate::colors;
+use crate::http_util::resolve_redirect_from_response;
 use crate::http_util::CacheSemantics;
 use crate::http_util::FetchOnceArgs;
 use crate::http_util::FetchOnceResult;
@@ -21,6 +22,11 @@ use deno_core::futures;
 use deno_core::futures::future::FutureExt;
 use deno_core::parking_lot::Mutex;
 use deno_core::ModuleSpecifier;
+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::StatusCode;
 use deno_runtime::deno_web::BlobStore;
 use deno_runtime::permissions::Permissions;
 use log::debug;
@@ -457,13 +463,15 @@ impl FileFetcher {
     let file_fetcher = self.clone();
     // A single pass of fetch either yields code or yields a redirect.
     async move {
-      match client
-        .fetch_once(FetchOnceArgs {
+      match fetch_once(
+        &client,
+        FetchOnceArgs {
           url: specifier.clone(),
           maybe_accept: maybe_accept.clone(),
           maybe_etag,
           maybe_auth_token,
-        })
+        },
+      )
       .await?
       {
         FetchOnceResult::NotModified => {
@@ -627,17 +635,99 @@ impl FileFetcher {
   }
 }
 
+/// Asynchronously fetches the given HTTP URL one pass only.
+/// If no redirect is present and no error occurs,
+/// yields Code(ResultPayload).
+/// If redirect occurs, does not follow and
+/// yields Redirect(url).
+async fn fetch_once(
+  http_client: &HttpClient,
+  args: FetchOnceArgs,
+) -> Result<FetchOnceResult, AnyError> {
+  let mut request = http_client.get_no_redirect(args.url.clone());
+
+  if let Some(etag) = args.maybe_etag {
+    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())?;
+    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 {
+    return Ok(FetchOnceResult::NotModified);
+  }
+
+  let mut result_headers = HashMap::new();
+  let response_headers = response.headers();
+
+  if let Some(warning) = response_headers.get("X-Deno-Warning") {
+    log::warn!(
+      "{} {}",
+      crate::colors::yellow("Warning"),
+      warning.to_str().unwrap()
+    );
+  }
+
+  for key in response_headers.keys() {
+    let key_str = key.to_string();
+    let values = response_headers.get_all(key);
+    let values_str = values
+      .iter()
+      .map(|e| e.to_str().unwrap().to_string())
+      .collect::<Vec<String>>()
+      .join(",");
+    result_headers.insert(key_str, values_str);
+  }
+
+  if response.status().is_redirection() {
+    let new_url = resolve_redirect_from_response(&args.url, &response)?;
+    return Ok(FetchOnceResult::Redirect(new_url, result_headers));
+  }
+
+  if response.status().is_client_error() || response.status().is_server_error()
+  {
+    let err = if response.status() == StatusCode::NOT_FOUND {
+      custom_error(
+        "NotFound",
+        format!("Import '{}' failed, not found.", args.url),
+      )
+    } else {
+      generic_error(format!(
+        "Import '{}' failed: {}",
+        args.url,
+        response.status()
+      ))
+    };
+    return Err(err);
+  }
+
+  let body = response.bytes().await?.to_vec();
+
+  Ok(FetchOnceResult::Code(body, result_headers))
+}
+
 #[cfg(test)]
 mod tests {
   use crate::cache::CachedUrlMetadata;
   use crate::http_util::HttpClient;
+  use crate::version;
 
   use super::*;
   use deno_core::error::get_custom_error_class;
   use deno_core::resolve_url;
   use deno_core::resolve_url_or_path;
+  use deno_core::url::Url;
+  use deno_runtime::deno_fetch::create_http_client;
   use deno_runtime::deno_web::Blob;
   use deno_runtime::deno_web::InMemoryBlobPart;
+  use std::fs::read;
   use test_util::TempDir;
 
   fn setup(
@@ -1640,4 +1730,480 @@ mod tests {
                    \u{5E2}\u{5D5}\u{5DC}\u{5DD}\");\u{A}";
     test_fetch_remote_encoded("windows-1255", "windows-1255", expected).await;
   }
+
+  fn create_test_client() -> HttpClient {
+    HttpClient::from_client(
+      create_http_client(
+        "test_client".to_string(),
+        None,
+        vec![],
+        None,
+        None,
+        None,
+      )
+      .unwrap(),
+    )
+  }
+
+  #[tokio::test]
+  async fn test_fetch_string() {
+    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/assets/fixture.json").unwrap();
+    let client = create_test_client();
+    let result = fetch_once(
+      &client,
+      FetchOnceArgs {
+        url,
+        maybe_accept: None,
+        maybe_etag: None,
+        maybe_auth_token: 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("etag"), None);
+      assert_eq!(headers.get("x-typescript-types"), None);
+    } else {
+      panic!();
+    }
+  }
+
+  #[tokio::test]
+  async fn test_fetch_gzip() {
+    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/run/import_compression/gziped")
+      .unwrap();
+    let client = create_test_client();
+    let result = fetch_once(
+      &client,
+      FetchOnceArgs {
+        url,
+        maybe_accept: None,
+        maybe_etag: None,
+        maybe_auth_token: None,
+      },
+    )
+    .await;
+    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(),
+        "application/javascript"
+      );
+      assert_eq!(headers.get("etag"), None);
+      assert_eq!(headers.get("x-typescript-types"), None);
+    } else {
+      panic!();
+    }
+  }
+
+  #[tokio::test]
+  async fn test_fetch_with_etag() {
+    let _http_server_guard = test_util::http_server();
+    let url = Url::parse("http://127.0.0.1:4545/etag_script.ts").unwrap();
+    let client = create_test_client();
+    let result = fetch_once(
+      &client,
+      FetchOnceArgs {
+        url: url.clone(),
+        maybe_accept: None,
+        maybe_etag: None,
+        maybe_auth_token: None,
+      },
+    )
+    .await;
+    if let Ok(FetchOnceResult::Code(body, headers)) = result {
+      assert!(!body.is_empty());
+      assert_eq!(String::from_utf8(body).unwrap(), "console.log('etag')");
+      assert_eq!(
+        headers.get("content-type").unwrap(),
+        "application/typescript"
+      );
+      assert_eq!(headers.get("etag").unwrap(), "33a64df551425fcc55e");
+    } else {
+      panic!();
+    }
+
+    let res = fetch_once(
+      &client,
+      FetchOnceArgs {
+        url,
+        maybe_accept: None,
+        maybe_etag: Some("33a64df551425fcc55e".to_string()),
+        maybe_auth_token: None,
+      },
+    )
+    .await;
+    assert_eq!(res.unwrap(), FetchOnceResult::NotModified);
+  }
+
+  #[tokio::test]
+  async fn test_fetch_brotli() {
+    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/run/import_compression/brotli")
+      .unwrap();
+    let client = create_test_client();
+    let result = fetch_once(
+      &client,
+      FetchOnceArgs {
+        url,
+        maybe_accept: None,
+        maybe_etag: None,
+        maybe_auth_token: None,
+      },
+    )
+    .await;
+    if let Ok(FetchOnceResult::Code(body, headers)) = result {
+      assert!(!body.is_empty());
+      assert_eq!(String::from_utf8(body).unwrap(), "console.log('brotli');");
+      assert_eq!(
+        headers.get("content-type").unwrap(),
+        "application/javascript"
+      );
+      assert_eq!(headers.get("etag"), None);
+      assert_eq!(headers.get("x-typescript-types"), None);
+    } else {
+      panic!();
+    }
+  }
+
+  #[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(
+      &client,
+      FetchOnceArgs {
+        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();
+    // Relies on external http server. See target/debug/test_server
+    let url = Url::parse("http://127.0.0.1:4546/assets/fixture.json").unwrap();
+    // Dns resolver substitutes `127.0.0.1` with `localhost`
+    let target_url =
+      Url::parse("http://localhost:4545/assets/fixture.json").unwrap();
+    let client = create_test_client();
+    let result = fetch_once(
+      &client,
+      FetchOnceArgs {
+        url,
+        maybe_accept: None,
+        maybe_etag: None,
+        maybe_auth_token: None,
+      },
+    )
+    .await;
+    if let Ok(FetchOnceResult::Redirect(url, _)) = result {
+      assert_eq!(url, target_url);
+    } else {
+      panic!();
+    }
+  }
+
+  #[tokio::test]
+  async fn test_fetch_with_cafile_string() {
+    let _http_server_guard = test_util::http_server();
+    // Relies on external http server. See target/debug/test_server
+    let url = Url::parse("https://localhost:5545/assets/fixture.json").unwrap();
+
+    let client = HttpClient::from_client(
+      create_http_client(
+        version::get_user_agent(),
+        None,
+        vec![read(
+          test_util::testdata_path()
+            .join("tls/RootCA.pem")
+            .to_str()
+            .unwrap(),
+        )
+        .unwrap()],
+        None,
+        None,
+        None,
+      )
+      .unwrap(),
+    );
+    let result = fetch_once(
+      &client,
+      FetchOnceArgs {
+        url,
+        maybe_accept: None,
+        maybe_etag: None,
+        maybe_auth_token: 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("etag"), None);
+      assert_eq!(headers.get("x-typescript-types"), None);
+    } else {
+      panic!();
+    }
+  }
+
+  #[tokio::test]
+  async fn test_fetch_with_default_certificate_store() {
+    let _http_server_guard = test_util::http_server();
+    // Relies on external http server with a valid mozilla root CA cert.
+    let url = Url::parse("https://deno.land").unwrap();
+    let client = HttpClient::from_client(
+      create_http_client(
+        version::get_user_agent(),
+        None, // This will load mozilla certs by default
+        vec![],
+        None,
+        None,
+        None,
+      )
+      .unwrap(),
+    );
+
+    let result = fetch_once(
+      &client,
+      FetchOnceArgs {
+        url,
+        maybe_accept: None,
+        maybe_etag: None,
+        maybe_auth_token: None,
+      },
+    )
+    .await;
+
+    println!("{:?}", result);
+    if let Ok(FetchOnceResult::Code(body, _headers)) = result {
+      assert!(!body.is_empty());
+    } else {
+      panic!();
+    }
+  }
+
+  // TODO(@justinmchase): Windows should verify certs too and fail to make this request without ca certs
+  #[cfg(not(windows))]
+  #[tokio::test]
+  #[ignore] // https://github.com/denoland/deno/issues/12561
+  async fn test_fetch_with_empty_certificate_store() {
+    use deno_runtime::deno_tls::rustls::RootCertStore;
+
+    let _http_server_guard = test_util::http_server();
+    // Relies on external http server with a valid mozilla root CA cert.
+    let url = Url::parse("https://deno.land").unwrap();
+    let client = HttpClient::new(
+      Some(RootCertStore::empty()), // no certs loaded at all
+      None,
+    )
+    .unwrap();
+
+    let result = fetch_once(
+      &client,
+      FetchOnceArgs {
+        url,
+        maybe_accept: None,
+        maybe_etag: None,
+        maybe_auth_token: None,
+      },
+    )
+    .await;
+
+    if let Ok(FetchOnceResult::Code(_body, _headers)) = result {
+      // This test is expected to fail since to CA certs have been loaded
+      panic!();
+    }
+  }
+
+  #[tokio::test]
+  async fn test_fetch_with_cafile_gzip() {
+    let _http_server_guard = test_util::http_server();
+    // Relies on external http server. See target/debug/test_server
+    let url =
+      Url::parse("https://localhost:5545/run/import_compression/gziped")
+        .unwrap();
+    let client = HttpClient::from_client(
+      create_http_client(
+        version::get_user_agent(),
+        None,
+        vec![read(
+          test_util::testdata_path()
+            .join("tls/RootCA.pem")
+            .to_str()
+            .unwrap(),
+        )
+        .unwrap()],
+        None,
+        None,
+        None,
+      )
+      .unwrap(),
+    );
+    let result = fetch_once(
+      &client,
+      FetchOnceArgs {
+        url,
+        maybe_accept: None,
+        maybe_etag: None,
+        maybe_auth_token: None,
+      },
+    )
+    .await;
+    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(),
+        "application/javascript"
+      );
+      assert_eq!(headers.get("etag"), None);
+      assert_eq!(headers.get("x-typescript-types"), None);
+    } else {
+      panic!();
+    }
+  }
+
+  #[tokio::test]
+  async fn test_fetch_with_cafile_with_etag() {
+    let _http_server_guard = test_util::http_server();
+    let url = Url::parse("https://localhost:5545/etag_script.ts").unwrap();
+    let client = HttpClient::from_client(
+      create_http_client(
+        version::get_user_agent(),
+        None,
+        vec![read(
+          test_util::testdata_path()
+            .join("tls/RootCA.pem")
+            .to_str()
+            .unwrap(),
+        )
+        .unwrap()],
+        None,
+        None,
+        None,
+      )
+      .unwrap(),
+    );
+    let result = fetch_once(
+      &client,
+      FetchOnceArgs {
+        url: url.clone(),
+        maybe_accept: None,
+        maybe_etag: None,
+        maybe_auth_token: None,
+      },
+    )
+    .await;
+    if let Ok(FetchOnceResult::Code(body, headers)) = result {
+      assert!(!body.is_empty());
+      assert_eq!(String::from_utf8(body).unwrap(), "console.log('etag')");
+      assert_eq!(
+        headers.get("content-type").unwrap(),
+        "application/typescript"
+      );
+      assert_eq!(headers.get("etag").unwrap(), "33a64df551425fcc55e");
+      assert_eq!(headers.get("x-typescript-types"), None);
+    } else {
+      panic!();
+    }
+
+    let res = fetch_once(
+      &client,
+      FetchOnceArgs {
+        url,
+        maybe_accept: None,
+        maybe_etag: Some("33a64df551425fcc55e".to_string()),
+        maybe_auth_token: None,
+      },
+    )
+    .await;
+    assert_eq!(res.unwrap(), FetchOnceResult::NotModified);
+  }
+
+  #[tokio::test]
+  async fn test_fetch_with_cafile_brotli() {
+    let _http_server_guard = test_util::http_server();
+    // Relies on external http server. See target/debug/test_server
+    let url =
+      Url::parse("https://localhost:5545/run/import_compression/brotli")
+        .unwrap();
+    let client = HttpClient::from_client(
+      create_http_client(
+        version::get_user_agent(),
+        None,
+        vec![read(
+          test_util::testdata_path()
+            .join("tls/RootCA.pem")
+            .to_str()
+            .unwrap(),
+        )
+        .unwrap()],
+        None,
+        None,
+        None,
+      )
+      .unwrap(),
+    );
+    let result = fetch_once(
+      &client,
+      FetchOnceArgs {
+        url,
+        maybe_accept: None,
+        maybe_etag: None,
+        maybe_auth_token: None,
+      },
+    )
+    .await;
+    if let Ok(FetchOnceResult::Code(body, headers)) = result {
+      assert!(!body.is_empty());
+      assert_eq!(String::from_utf8(body).unwrap(), "console.log('brotli');");
+      assert_eq!(
+        headers.get("content-type").unwrap(),
+        "application/javascript"
+      );
+      assert_eq!(headers.get("etag"), None);
+      assert_eq!(headers.get("x-typescript-types"), None);
+    } else {
+      panic!();
+    }
+  }
+
+  #[tokio::test]
+  async fn bad_redirect() {
+    let _g = test_util::http_server();
+    let url_str = "http://127.0.0.1:4545/bad_redirect";
+    let url = Url::parse(url_str).unwrap();
+    let client = create_test_client();
+    let result = fetch_once(
+      &client,
+      FetchOnceArgs {
+        url,
+        maybe_accept: None,
+        maybe_etag: None,
+        maybe_auth_token: None,
+      },
+    )
+    .await;
+    assert!(result.is_err());
+    let err = result.unwrap_err();
+    // Check that the error message contains the original URL
+    assert!(err.to_string().contains(url_str));
+  }
 }

cli/http_util.rs

@@ -14,14 +14,9 @@ use deno_core::futures::StreamExt;
 use deno_core::url::Url;
 use deno_runtime::deno_fetch::create_http_client;
 use deno_runtime::deno_fetch::reqwest;
-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;
-use deno_runtime::deno_fetch::reqwest::StatusCode;
+use deno_runtime::deno_fetch::reqwest::Response;
 use deno_runtime::deno_tls::rustls::RootCertStore;
-use log::debug;
 use std::collections::HashMap;
 use std::time::Duration;
 use std::time::SystemTime;
@@ -53,6 +48,24 @@ fn resolve_url_from_location(base_url: &Url, location: &str) -> Url {
   }
 }
 
+pub fn resolve_redirect_from_response(
+  request_url: &Url,
+  response: &Response,
+) -> Result<Url, AnyError> {
+  debug_assert!(response.status().is_redirection());
+  if let Some(location) = response.headers().get(LOCATION) {
+    let location_string = location.to_str().unwrap();
+    log::debug!("Redirecting to {:?}...", &location_string);
+    let new_url = resolve_url_from_location(request_url, location_string);
+    Ok(new_url)
+  } else {
+    Err(generic_error(format!(
+      "Redirection from '{}' did not provide location header",
+      request_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)>
@@ -242,19 +255,50 @@ impl HttpClient {
     Self(client)
   }
 
-  pub fn get<U: reqwest::IntoUrl>(&self, url: U) -> reqwest::RequestBuilder {
+  /// Do a GET request without following redirects.
+  pub fn get_no_redirect<U: reqwest::IntoUrl>(
+    &self,
+    url: U,
+  ) -> reqwest::RequestBuilder {
     self.0.get(url)
   }
 
+  pub async fn download_text<U: reqwest::IntoUrl>(
+    &self,
+    url: U,
+  ) -> Result<String, AnyError> {
+    let bytes = self.download(url).await?;
+    Ok(String::from_utf8(bytes)?)
+  }
+
+  pub async fn download<U: reqwest::IntoUrl>(
+    &self,
+    url: U,
+  ) -> Result<Vec<u8>, AnyError> {
+    let maybe_bytes = self.inner_download(url, None).await?;
+    match maybe_bytes {
+      Some(bytes) => Ok(bytes),
+      None => Err(custom_error("Http", "Not found.")),
+    }
+  }
+
   pub async fn download_with_progress<U: reqwest::IntoUrl>(
     &self,
     url: U,
     progress_guard: &UpdateGuard,
   ) -> Result<Option<Vec<u8>>, AnyError> {
-    let response = self.get(url).send().await?;
+    self.inner_download(url, Some(progress_guard)).await
+  }
+
+  async fn inner_download<U: reqwest::IntoUrl>(
+    &self,
+    url: U,
+    progress_guard: Option<&UpdateGuard>,
+  ) -> Result<Option<Vec<u8>>, AnyError> {
+    let response = self.get_redirected_response(url).await?;
 
     if response.status() == 404 {
-      Ok(None)
+      return Ok(None);
     } else if !response.status().is_success() {
       let status = response.status();
       let maybe_response_text = response.text().await.ok();
@@ -266,7 +310,10 @@ impl HttpClient {
           None => String::new(),
         }
       );
-    } else if let Some(total_size) = response.content_length() {
+    }
+
+    if let Some(progress_guard) = progress_guard {
+      if let Some(total_size) = response.content_length() {
         progress_guard.set_total_size(total_size);
         let mut current_size = 0;
         let mut data = Vec::with_capacity(total_size as usize);
@@ -277,283 +324,63 @@ impl HttpClient {
           progress_guard.set_position(current_size);
           data.extend(bytes.into_iter());
         }
-      Ok(Some(data))
-    } else {
+        return Ok(Some(data));
+      }
+    }
+
     let bytes = response.bytes().await?;
     Ok(Some(bytes.into()))
   }
-  }
 
-  /// Asynchronously fetches the given HTTP URL one pass only.
-  /// If no redirect is present and no error occurs,
-  /// yields Code(ResultPayload).
-  /// If redirect occurs, does not follow and
-  /// yields Redirect(url).
-  pub async fn fetch_once(
+  async fn get_redirected_response<U: reqwest::IntoUrl>(
     &self,
-    args: FetchOnceArgs,
-  ) -> Result<FetchOnceResult, AnyError> {
-    let mut request = self.get(args.url.clone());
-
-    if let Some(etag) = args.maybe_etag {
-      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())?;
-      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 {
-      return Ok(FetchOnceResult::NotModified);
-    }
-
-    let mut result_headers = HashMap::new();
-    let response_headers = response.headers();
-
-    if let Some(warning) = response_headers.get("X-Deno-Warning") {
-      log::warn!(
-        "{} {}",
-        crate::colors::yellow("Warning"),
-        warning.to_str().unwrap()
-      );
-    }
-
-    for key in response_headers.keys() {
-      let key_str = key.to_string();
-      let values = response_headers.get_all(key);
-      let values_str = values
-        .iter()
-        .map(|e| e.to_str().unwrap().to_string())
-        .collect::<Vec<String>>()
-        .join(",");
-      result_headers.insert(key_str, values_str);
-    }
-
-    if response.status().is_redirection() {
-      if let Some(location) = response.headers().get(LOCATION) {
-        let location_string = location.to_str().unwrap();
-        debug!("Redirecting to {:?}...", &location_string);
-        let new_url = resolve_url_from_location(&args.url, location_string);
-        return Ok(FetchOnceResult::Redirect(new_url, result_headers));
+    url: U,
+  ) -> Result<Response, AnyError> {
+    let mut url = url.into_url()?;
+    let mut response = self.get_no_redirect(url.clone()).send().await?;
+    let status = response.status();
+    if status.is_redirection() {
+      for _ in 0..5 {
+        let new_url = resolve_redirect_from_response(&url, &response)?;
+        let new_response = self.get_no_redirect(new_url.clone()).send().await?;
+        let status = new_response.status();
+        if status.is_redirection() {
+          response = new_response;
+          url = new_url;
         } else {
-        return Err(generic_error(format!(
-          "Redirection from '{}' did not provide location header",
-          args.url
-        )));
+          return Ok(new_response);
         }
       }
-
-    if response.status().is_client_error()
-      || response.status().is_server_error()
-    {
-      let err = if response.status() == StatusCode::NOT_FOUND {
-        custom_error(
-          "NotFound",
-          format!("Import '{}' failed, not found.", args.url),
-        )
+      Err(custom_error("Http", "Too many redirects."))
     } else {
-        generic_error(format!(
-          "Import '{}' failed: {}",
-          args.url,
-          response.status()
-        ))
-      };
-      return Err(err);
+      Ok(response)
     }
-
-    let body = response.bytes().await?.to_vec();
-
-    Ok(FetchOnceResult::Code(body, result_headers))
   }
 }
 
 #[cfg(test)]
-mod tests {
+mod test {
   use super::*;
-  use crate::version;
-  use deno_runtime::deno_fetch::create_http_client;
-  use std::fs::read;
-
-  fn create_test_client() -> HttpClient {
-    HttpClient::from_client(
-      create_http_client(
-        "test_client".to_string(),
-        None,
-        vec![],
-        None,
-        None,
-        None,
-      )
-      .unwrap(),
-    )
-  }
 
   #[tokio::test]
-  async fn test_fetch_string() {
+  async fn test_http_client_download_redirect() {
     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/assets/fixture.json").unwrap();
-    let client = create_test_client();
-    let result = client
-      .fetch_once(FetchOnceArgs {
-        url,
-        maybe_accept: None,
-        maybe_etag: None,
-        maybe_auth_token: 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("etag"), None);
-      assert_eq!(headers.get("x-typescript-types"), None);
-    } else {
-      panic!();
-    }
-  }
+    let client = HttpClient::new(None, None).unwrap();
 
-  #[tokio::test]
-  async fn test_fetch_gzip() {
-    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/run/import_compression/gziped")
+    // make a request to the redirect server
+    let text = client
+      .download_text("http://localhost:4546/subdir/redirects/redirect1.js")
+      .await
       .unwrap();
-    let client = create_test_client();
-    let result = client
-      .fetch_once(FetchOnceArgs {
-        url,
-        maybe_accept: None,
-        maybe_etag: None,
-        maybe_auth_token: None,
-      })
-      .await;
-    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(),
-        "application/javascript"
-      );
-      assert_eq!(headers.get("etag"), None);
-      assert_eq!(headers.get("x-typescript-types"), None);
-    } else {
-      panic!();
-    }
-  }
+    assert_eq!(text, "export const redirect = 1;\n");
 
-  #[tokio::test]
-  async fn test_fetch_with_etag() {
-    let _http_server_guard = test_util::http_server();
-    let url = Url::parse("http://127.0.0.1:4545/etag_script.ts").unwrap();
-    let client = create_test_client();
-    let result = client
-      .fetch_once(FetchOnceArgs {
-        url: url.clone(),
-        maybe_accept: None,
-        maybe_etag: None,
-        maybe_auth_token: None,
-      })
-      .await;
-    if let Ok(FetchOnceResult::Code(body, headers)) = result {
-      assert!(!body.is_empty());
-      assert_eq!(String::from_utf8(body).unwrap(), "console.log('etag')");
-      assert_eq!(
-        headers.get("content-type").unwrap(),
-        "application/typescript"
-      );
-      assert_eq!(headers.get("etag").unwrap(), "33a64df551425fcc55e");
-    } else {
-      panic!();
-    }
-
-    let res = client
-      .fetch_once(FetchOnceArgs {
-        url,
-        maybe_accept: None,
-        maybe_etag: Some("33a64df551425fcc55e".to_string()),
-        maybe_auth_token: None,
-      })
-      .await;
-    assert_eq!(res.unwrap(), FetchOnceResult::NotModified);
-  }
-
-  #[tokio::test]
-  async fn test_fetch_brotli() {
-    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/run/import_compression/brotli")
+    // now make one to the infinite redirects server
+    let err = client
+      .download_text("http://localhost:4549/subdir/redirects/redirect1.js")
+      .await
+      .err()
       .unwrap();
-    let client = create_test_client();
-    let result = client
-      .fetch_once(FetchOnceArgs {
-        url,
-        maybe_accept: None,
-        maybe_etag: None,
-        maybe_auth_token: None,
-      })
-      .await;
-    if let Ok(FetchOnceResult::Code(body, headers)) = result {
-      assert!(!body.is_empty());
-      assert_eq!(String::from_utf8(body).unwrap(), "console.log('brotli');");
-      assert_eq!(
-        headers.get("content-type").unwrap(),
-        "application/javascript"
-      );
-      assert_eq!(headers.get("etag"), None);
-      assert_eq!(headers.get("x-typescript-types"), None);
-    } else {
-      panic!();
-    }
-  }
-
-  #[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 = client
-      .fetch_once(FetchOnceArgs {
-        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();
-    // Relies on external http server. See target/debug/test_server
-    let url = Url::parse("http://127.0.0.1:4546/assets/fixture.json").unwrap();
-    // Dns resolver substitutes `127.0.0.1` with `localhost`
-    let target_url =
-      Url::parse("http://localhost:4545/assets/fixture.json").unwrap();
-    let client = create_test_client();
-    let result = client
-      .fetch_once(FetchOnceArgs {
-        url,
-        maybe_accept: None,
-        maybe_etag: None,
-        maybe_auth_token: None,
-      })
-      .await;
-    if let Ok(FetchOnceResult::Redirect(url, _)) = result {
-      assert_eq!(url, target_url);
-    } else {
-      panic!();
-    }
+    assert_eq!(err.to_string(), "Too many redirects.");
   }
 
   #[test]
@@ -593,274 +420,4 @@ mod tests {
     assert_eq!(new_uri.host_str().unwrap(), "deno.land");
     assert_eq!(new_uri.path(), "/z");
   }
-
-  #[tokio::test]
-  async fn test_fetch_with_cafile_string() {
-    let _http_server_guard = test_util::http_server();
-    // Relies on external http server. See target/debug/test_server
-    let url = Url::parse("https://localhost:5545/assets/fixture.json").unwrap();
-
-    let client = HttpClient::from_client(
-      create_http_client(
-        version::get_user_agent(),
-        None,
-        vec![read(
-          test_util::testdata_path()
-            .join("tls/RootCA.pem")
-            .to_str()
-            .unwrap(),
-        )
-        .unwrap()],
-        None,
-        None,
-        None,
-      )
-      .unwrap(),
-    );
-    let result = client
-      .fetch_once(FetchOnceArgs {
-        url,
-        maybe_accept: None,
-        maybe_etag: None,
-        maybe_auth_token: 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("etag"), None);
-      assert_eq!(headers.get("x-typescript-types"), None);
-    } else {
-      panic!();
-    }
-  }
-
-  #[tokio::test]
-  async fn test_fetch_with_default_certificate_store() {
-    let _http_server_guard = test_util::http_server();
-    // Relies on external http server with a valid mozilla root CA cert.
-    let url = Url::parse("https://deno.land").unwrap();
-    let client = HttpClient::from_client(
-      create_http_client(
-        version::get_user_agent(),
-        None, // This will load mozilla certs by default
-        vec![],
-        None,
-        None,
-        None,
-      )
-      .unwrap(),
-    );
-
-    let result = client
-      .fetch_once(FetchOnceArgs {
-        url,
-        maybe_accept: None,
-        maybe_etag: None,
-        maybe_auth_token: None,
-      })
-      .await;
-
-    println!("{:?}", result);
-    if let Ok(FetchOnceResult::Code(body, _headers)) = result {
-      assert!(!body.is_empty());
-    } else {
-      panic!();
-    }
-  }
-
-  // TODO(@justinmchase): Windows should verify certs too and fail to make this request without ca certs
-  #[cfg(not(windows))]
-  #[tokio::test]
-  #[ignore] // https://github.com/denoland/deno/issues/12561
-  async fn test_fetch_with_empty_certificate_store() {
-    use deno_runtime::deno_tls::rustls::RootCertStore;
-
-    let _http_server_guard = test_util::http_server();
-    // Relies on external http server with a valid mozilla root CA cert.
-    let url = Url::parse("https://deno.land").unwrap();
-    let client = HttpClient::new(
-      Some(RootCertStore::empty()), // no certs loaded at all
-      None,
-    )
-    .unwrap();
-
-    let result = client
-      .fetch_once(FetchOnceArgs {
-        url,
-        maybe_accept: None,
-        maybe_etag: None,
-        maybe_auth_token: None,
-      })
-      .await;
-
-    if let Ok(FetchOnceResult::Code(_body, _headers)) = result {
-      // This test is expected to fail since to CA certs have been loaded
-      panic!();
-    }
-  }
-
-  #[tokio::test]
-  async fn test_fetch_with_cafile_gzip() {
-    let _http_server_guard = test_util::http_server();
-    // Relies on external http server. See target/debug/test_server
-    let url =
-      Url::parse("https://localhost:5545/run/import_compression/gziped")
-        .unwrap();
-    let client = HttpClient::from_client(
-      create_http_client(
-        version::get_user_agent(),
-        None,
-        vec![read(
-          test_util::testdata_path()
-            .join("tls/RootCA.pem")
-            .to_str()
-            .unwrap(),
-        )
-        .unwrap()],
-        None,
-        None,
-        None,
-      )
-      .unwrap(),
-    );
-    let result = client
-      .fetch_once(FetchOnceArgs {
-        url,
-        maybe_accept: None,
-        maybe_etag: None,
-        maybe_auth_token: None,
-      })
-      .await;
-    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(),
-        "application/javascript"
-      );
-      assert_eq!(headers.get("etag"), None);
-      assert_eq!(headers.get("x-typescript-types"), None);
-    } else {
-      panic!();
-    }
-  }
-
-  #[tokio::test]
-  async fn test_fetch_with_cafile_with_etag() {
-    let _http_server_guard = test_util::http_server();
-    let url = Url::parse("https://localhost:5545/etag_script.ts").unwrap();
-    let client = HttpClient::from_client(
-      create_http_client(
-        version::get_user_agent(),
-        None,
-        vec![read(
-          test_util::testdata_path()
-            .join("tls/RootCA.pem")
-            .to_str()
-            .unwrap(),
-        )
-        .unwrap()],
-        None,
-        None,
-        None,
-      )
-      .unwrap(),
-    );
-    let result = client
-      .fetch_once(FetchOnceArgs {
-        url: url.clone(),
-        maybe_accept: None,
-        maybe_etag: None,
-        maybe_auth_token: None,
-      })
-      .await;
-    if let Ok(FetchOnceResult::Code(body, headers)) = result {
-      assert!(!body.is_empty());
-      assert_eq!(String::from_utf8(body).unwrap(), "console.log('etag')");
-      assert_eq!(
-        headers.get("content-type").unwrap(),
-        "application/typescript"
-      );
-      assert_eq!(headers.get("etag").unwrap(), "33a64df551425fcc55e");
-      assert_eq!(headers.get("x-typescript-types"), None);
-    } else {
-      panic!();
-    }
-
-    let res = client
-      .fetch_once(FetchOnceArgs {
-        url,
-        maybe_accept: None,
-        maybe_etag: Some("33a64df551425fcc55e".to_string()),
-        maybe_auth_token: None,
-      })
-      .await;
-    assert_eq!(res.unwrap(), FetchOnceResult::NotModified);
-  }
-
-  #[tokio::test]
-  async fn test_fetch_with_cafile_brotli() {
-    let _http_server_guard = test_util::http_server();
-    // Relies on external http server. See target/debug/test_server
-    let url =
-      Url::parse("https://localhost:5545/run/import_compression/brotli")
-        .unwrap();
-    let client = HttpClient::from_client(
-      create_http_client(
-        version::get_user_agent(),
-        None,
-        vec![read(
-          test_util::testdata_path()
-            .join("tls/RootCA.pem")
-            .to_str()
-            .unwrap(),
-        )
-        .unwrap()],
-        None,
-        None,
-        None,
-      )
-      .unwrap(),
-    );
-    let result = client
-      .fetch_once(FetchOnceArgs {
-        url,
-        maybe_accept: None,
-        maybe_etag: None,
-        maybe_auth_token: None,
-      })
-      .await;
-    if let Ok(FetchOnceResult::Code(body, headers)) = result {
-      assert!(!body.is_empty());
-      assert_eq!(String::from_utf8(body).unwrap(), "console.log('brotli');");
-      assert_eq!(
-        headers.get("content-type").unwrap(),
-        "application/javascript"
-      );
-      assert_eq!(headers.get("etag"), None);
-      assert_eq!(headers.get("x-typescript-types"), None);
-    } else {
-      panic!();
-    }
-  }
-
-  #[tokio::test]
-  async fn bad_redirect() {
-    let _g = test_util::http_server();
-    let url_str = "http://127.0.0.1:4545/bad_redirect";
-    let url = Url::parse(url_str).unwrap();
-    let client = create_test_client();
-    let result = client
-      .fetch_once(FetchOnceArgs {
-        url,
-        maybe_accept: None,
-        maybe_etag: None,
-        maybe_auth_token: None,
-      })
-      .await;
-    assert!(result.is_err());
-    let err = result.unwrap_err();
-    // Check that the error message contains the original URL
-    assert!(err.to_string().contains(url_str));
-  }
 }

cli/tools/standalone.rs

@@ -9,6 +9,8 @@ use crate::http_util::HttpClient;
 use crate::standalone::Metadata;
 use crate::standalone::MAGIC_TRAILER;
 use crate::util::path::path_has_trailing_slash;
+use crate::util::progress_bar::ProgressBar;
+use crate::util::progress_bar::ProgressBarStyle;
 use crate::ProcState;
 use deno_core::anyhow::bail;
 use deno_core::anyhow::Context;
@@ -122,23 +124,26 @@ async fn download_base_binary(
   binary_path_suffix: &str,
 ) -> Result<(), AnyError> {
   let download_url = format!("https://dl.deno.land/{}", binary_path_suffix);
+  let maybe_bytes = {
+    let progress_bars = ProgressBar::new(ProgressBarStyle::DownloadBars);
+    let progress = progress_bars.update(&download_url);
 
-  log::info!("Checking {}", &download_url);
-
-  let res = client.get(&download_url).send().await?;
-
-  let binary_content = if res.status().is_success() {
-    log::info!("Download has been found");
-    res.bytes().await?.to_vec()
-  } else {
+    client
+      .download_with_progress(download_url, &progress)
+      .await?
+  };
+  let bytes = match maybe_bytes {
+    Some(bytes) => bytes,
+    None => {
       log::info!("Download could not be found, aborting");
       std::process::exit(1)
+    }
   };
 
   std::fs::create_dir_all(output_directory)?;
   let output_path = output_directory.join(binary_path_suffix);
   std::fs::create_dir_all(output_path.parent().unwrap())?;
-  tokio::fs::write(output_path, binary_content).await?;
+  tokio::fs::write(output_path, bytes).await?;
   Ok(())
 }
 

cli/tools/upgrade.rs

@@ -7,7 +7,6 @@ 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;
 use crate::version;
@@ -17,7 +16,6 @@ use deno_core::anyhow::Context;
 use deno_core::error::AnyError;
 use deno_core::futures::future::BoxFuture;
 use deno_core::futures::FutureExt;
-use deno_core::futures::StreamExt;
 use once_cell::sync::Lazy;
 use std::borrow::Cow;
 use std::env;
@@ -353,7 +351,9 @@ pub async fn upgrade(
     )
   };
 
-  let archive_data = download_package(&client, &download_url).await?;
+  let archive_data = download_package(&client, &download_url)
+    .await
+    .with_context(|| format!("Failed downloading {}", download_url))?;
 
   log::info!("Deno is upgrading to version {}", &install_version);
 
@@ -402,22 +402,20 @@ pub async fn upgrade(
 async fn get_latest_release_version(
   client: &HttpClient,
 ) -> Result<String, AnyError> {
-  let res = client
-    .get("https://dl.deno.land/release-latest.txt")
-    .send()
+  let text = client
+    .download_text("https://dl.deno.land/release-latest.txt")
     .await?;
-  let version = res.text().await?.trim().to_string();
+  let version = text.trim().to_string();
   Ok(version.replace('v', ""))
 }
 
 async fn get_latest_canary_version(
   client: &HttpClient,
 ) -> Result<String, AnyError> {
-  let res = client
-    .get("https://dl.deno.land/canary-latest.txt")
-    .send()
+  let text = client
+    .download_text("https://dl.deno.land/canary-latest.txt")
     .await?;
-  let version = res.text().await?.trim().to_string();
+  let version = text.trim().to_string();
   Ok(version)
 }
 
@@ -426,49 +424,23 @@ async fn download_package(
   download_url: &str,
 ) -> Result<Vec<u8>, AnyError> {
   log::info!("Downloading {}", &download_url);
-
-  let res = client.get(download_url).send().await?;
-
-  if res.status().is_success() {
-    let total_size = res.content_length().unwrap();
-    let mut current_size = 0;
-    let mut data = Vec::with_capacity(total_size as usize);
-    let mut stream = res.bytes_stream();
-    let mut skip_print = 0;
+  let maybe_bytes = {
     let progress_bar = ProgressBar::new(ProgressBarStyle::DownloadBars);
+    // provide an empty string here in order to prefer the downloading
+    // text above which will stay alive after the progress bars are complete
     let progress = progress_bar.update("");
-    progress.set_total_size(total_size);
-    while let Some(item) = stream.next().await {
-      let bytes = item?;
-      current_size += bytes.len() as u64;
-      data.extend_from_slice(&bytes);
-      if progress_bar.is_enabled() {
-        progress.set_position(current_size);
-      } else if skip_print == 0 {
-        log::info!(
-          "{} / {} ({:^5.1}%)",
-          human_download_size(current_size, total_size),
-          human_download_size(total_size, total_size),
-          (current_size as f64 / total_size as f64) * 100.0,
-        );
-        skip_print = 10;
-      } else {
-        skip_print -= 1;
-      }
-    }
-    drop(progress);
-    log::info!(
-      "{} / {} (100.0%)",
-      human_download_size(current_size, total_size),
-      human_download_size(total_size, total_size)
-    );
-
-    Ok(data)
-  } else {
+    client
+      .download_with_progress(download_url, &progress)
+      .await?
+  };
+  match maybe_bytes {
+    Some(bytes) => Ok(bytes),
+    None => {
       log::info!("Download could not be found, aborting");
       std::process::exit(1)
     }
   }
+}
 
 pub fn unpack(
   archive_data: Vec<u8>,

cli/util/progress_bar/mod.rs

@@ -76,10 +76,6 @@ impl ProgressBar {
     }
   }
 
-  pub fn is_enabled(&self) -> bool {
-    self.draw_thread.is_some()
-  }
-
   pub fn update(&self, msg: &str) -> UpdateGuard {
     match &self.draw_thread {
       Some(draw_thread) => {