Skip to content

Commit

Permalink
fix(ext/fetch): respect authority from URL (#24705)
Browse files Browse the repository at this point in the history
This commit fixes handling of "authority" in the URL by properly
sending "Authorization Basic..." header in `fetch` API.

This is a regression from #24593
Fixes #24697

CC @seanmonstar
  • Loading branch information
bartlomieju authored and dsherret committed Jul 26, 2024
1 parent 3b98c6e commit a8817fe
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 7 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions ext/fetch/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
42 changes: 41 additions & 1 deletion ext/fetch/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -385,6 +387,7 @@ where
let permissions = state.borrow_mut::<FP>();
permissions.check_net_url(&url, "fetch()")?;

let maybe_authority = extract_authority(&mut url);
let uri = url
.as_str()
.parse::<Uri>()
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -1096,3 +1105,34 @@ impl Client {

pub type ReqBody = http_body_util::combinators::BoxBody<Bytes, Error>;
pub type ResBody = http_body_util::combinators::BoxBody<Bytes, Error>;

/// 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<String>)> {
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
}
13 changes: 8 additions & 5 deletions ext/fetch/proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,18 @@ 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;

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");
Expand All @@ -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()));
Expand Down Expand Up @@ -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 {
Expand Down
10 changes: 9 additions & 1 deletion ext/node/ops/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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::<P>();
Expand Down Expand Up @@ -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());
}
Expand Down
18 changes: 18 additions & 0 deletions tests/unit/fetch_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string | null | undefined>();
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");
});

0 comments on commit a8817fe

Please sign in to comment.