From 1fad6eb2acc993714fad9d333f409495f5b3d6db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 24 Jul 2024 22:22:43 +0100 Subject: [PATCH] fix(ext/fetch): respect authority from URL (#24705) This commit fixes handling of "authority" in the URL by properly sending "Authorization Basic..." header in `fetch` API. This is a regression from https://github.com/denoland/deno/pull/24593 Fixes https://github.com/denoland/deno/issues/24697 CC @seanmonstar --- Cargo.lock | 1 + ext/fetch/Cargo.toml | 1 + ext/fetch/lib.rs | 42 +++++++++++++++++++++++++++++++++++++++- ext/fetch/proxy.rs | 13 ++++++++----- ext/node/ops/http.rs | 10 +++++++++- tests/unit/fetch_test.ts | 18 +++++++++++++++++ 6 files changed, 78 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 341e1cdba7..3329a2eea4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1482,6 +1482,7 @@ dependencies = [ "hyper-rustls", "hyper-util", "ipnet", + "percent-encoding", "rustls-webpki", "serde", "serde_json", diff --git a/ext/fetch/Cargo.toml b/ext/fetch/Cargo.toml index c28d88b240..616956e652 100644 --- a/ext/fetch/Cargo.toml +++ b/ext/fetch/Cargo.toml @@ -27,6 +27,7 @@ hyper.workspace = true hyper-rustls.workspace = true hyper-util.workspace = true ipnet.workspace = true +percent-encoding.workspace = true rustls-webpki.workspace = true serde.workspace = true serde_json.workspace = true diff --git a/ext/fetch/lib.rs b/ext/fetch/lib.rs index 9912ff3072..7c717ccec9 100644 --- a/ext/fetch/lib.rs +++ b/ext/fetch/lib.rs @@ -55,6 +55,7 @@ use http::header::HeaderName; use http::header::HeaderValue; use http::header::ACCEPT; use http::header::ACCEPT_ENCODING; +use http::header::AUTHORIZATION; use http::header::CONTENT_LENGTH; use http::header::HOST; use http::header::PROXY_AUTHORIZATION; @@ -77,6 +78,7 @@ use tower_http::decompression::Decompression; // Re-export data_url pub use data_url; +pub use proxy::basic_auth; pub use fs_fetch_handler::FsFetchHandler; @@ -349,7 +351,7 @@ where }; let method = Method::from_bytes(&method)?; - let url = Url::parse(&url)?; + let mut url = Url::parse(&url)?; // Check scheme before asking for net permission let scheme = url.scheme(); @@ -385,6 +387,7 @@ where let permissions = state.borrow_mut::(); permissions.check_net_url(&url, "fetch()")?; + let maybe_authority = extract_authority(&mut url); let uri = url .as_str() .parse::() @@ -428,6 +431,12 @@ where *request.method_mut() = method.clone(); *request.uri_mut() = uri; + if let Some((username, password)) = maybe_authority { + request.headers_mut().insert( + AUTHORIZATION, + proxy::basic_auth(&username, password.as_deref()), + ); + } if let Some(len) = con_len { request.headers_mut().insert(CONTENT_LENGTH, len.into()); } @@ -1096,3 +1105,34 @@ impl Client { pub type ReqBody = http_body_util::combinators::BoxBody; pub type ResBody = http_body_util::combinators::BoxBody; + +/// Copied from https://github.com/seanmonstar/reqwest/blob/b9d62a0323d96f11672a61a17bf8849baec00275/src/async_impl/request.rs#L572 +/// Check the request URL for a "username:password" type authority, and if +/// found, remove it from the URL and return it. +pub fn extract_authority(url: &mut Url) -> Option<(String, Option)> { + use percent_encoding::percent_decode; + + if url.has_authority() { + let username: String = percent_decode(url.username().as_bytes()) + .decode_utf8() + .ok()? + .into(); + let password = url.password().and_then(|pass| { + percent_decode(pass.as_bytes()) + .decode_utf8() + .ok() + .map(String::from) + }); + if !username.is_empty() || password.is_some() { + url + .set_username("") + .expect("has_authority means set_username shouldn't fail"); + url + .set_password(None) + .expect("has_authority means set_password shouldn't fail"); + return Some((username, password)); + } + } + + None +} diff --git a/ext/fetch/proxy.rs b/ext/fetch/proxy.rs index c8e54d5ec6..bbb40e2f1d 100644 --- a/ext/fetch/proxy.rs +++ b/ext/fetch/proxy.rs @@ -106,7 +106,7 @@ pub(crate) fn from_env() -> Proxies { Proxies { intercepts, no } } -pub(crate) fn basic_auth(user: &str, pass: &str) -> HeaderValue { +pub fn basic_auth(user: &str, pass: Option<&str>) -> HeaderValue { use base64::prelude::BASE64_STANDARD; use base64::write::EncoderWriter; use std::io::Write; @@ -114,7 +114,10 @@ pub(crate) fn basic_auth(user: &str, pass: &str) -> HeaderValue { let mut buf = b"Basic ".to_vec(); { let mut encoder = EncoderWriter::new(&mut buf, &BASE64_STANDARD); - let _ = write!(encoder, "{user}:{pass}"); + let _ = write!(encoder, "{user}:"); + if let Some(password) = pass { + let _ = write!(encoder, "{password}"); + } } let mut header = HeaderValue::from_bytes(&buf).expect("base64 is always valid HeaderValue"); @@ -140,10 +143,10 @@ impl Intercept { pub(crate) fn set_auth(&mut self, user: &str, pass: &str) { match self.target { Target::Http { ref mut auth, .. } => { - *auth = Some(basic_auth(user, pass)); + *auth = Some(basic_auth(user, Some(pass))); } Target::Https { ref mut auth, .. } => { - *auth = Some(basic_auth(user, pass)); + *auth = Some(basic_auth(user, Some(pass))); } Target::Socks { ref mut auth, .. } => { *auth = Some((user.into(), pass.into())); @@ -192,7 +195,7 @@ impl Target { if is_socks { socks_auth = Some((user.into(), pass.into())); } else { - http_auth = Some(basic_auth(user, pass)); + http_auth = Some(basic_auth(user, Some(pass))); } builder = builder.authority(host_port); } else { diff --git a/ext/node/ops/http.rs b/ext/node/ops/http.rs index 89024e3f31..19847820e1 100644 --- a/ext/node/ops/http.rs +++ b/ext/node/ops/http.rs @@ -18,6 +18,7 @@ use deno_fetch::ResourceToBodyAdapter; use http::header::HeaderMap; use http::header::HeaderName; use http::header::HeaderValue; +use http::header::AUTHORIZATION; use http::header::CONTENT_LENGTH; use http::Method; use http_body_util::BodyExt; @@ -43,7 +44,8 @@ where }; let method = Method::from_bytes(&method)?; - let url = Url::parse(&url)?; + let mut url = Url::parse(&url)?; + let maybe_authority = deno_fetch::extract_authority(&mut url); { let permissions = state.borrow_mut::

(); @@ -89,6 +91,12 @@ where .map_err(|_| type_error("Invalid URL"))?; *request.headers_mut() = header_map; + if let Some((username, password)) = maybe_authority { + request.headers_mut().insert( + AUTHORIZATION, + deno_fetch::basic_auth(&username, password.as_deref()), + ); + } if let Some(len) = con_len { request.headers_mut().insert(CONTENT_LENGTH, len.into()); } diff --git a/tests/unit/fetch_test.ts b/tests/unit/fetch_test.ts index bc3822d994..39cadcb440 100644 --- a/tests/unit/fetch_test.ts +++ b/tests/unit/fetch_test.ts @@ -2042,3 +2042,21 @@ Deno.test("Response with subarray TypedArray body", async () => { const expected = new Uint8Array([2, 3, 4, 5]); assertEquals(actual, expected); }); + +// Regression test for https://github.com/denoland/deno/issues/24697 +Deno.test("URL authority is used as 'Authorization' header", async () => { + const deferred = Promise.withResolvers(); + const ac = new AbortController(); + + const server = Deno.serve({ port: 4502, signal: ac.signal }, (req) => { + deferred.resolve(req.headers.get("authorization")); + return new Response("Hello world"); + }); + + const res = await fetch("http://deno:land@localhost:4502"); + await res.text(); + const authHeader = await deferred.promise; + ac.abort(); + await server.finished; + assertEquals(authHeader, "Basic ZGVubzpsYW5k"); +});