Skip to content

Commit

Permalink
Avoid duplicating authorization header with netrc (#2325)
Browse files Browse the repository at this point in the history
## Summary

The netrc middleware we added in
#2241 has a slight problem. If you
include credentials in your index URL, _and_ in the netrc file, the
crate blindly adds the netrc credentials as a header. And given the
`ReqwestBuilder` API, this means you end up with _two_ `Authorization`
headers, which always leads to an invalid request, though the exact
failure can take different forms.

This PR removes the middleware crate in favor of our own middleware.
Instead of using the `RequestInitialiser` API, we have to use the
`Middleware` API, so that we can remove the header on the request
itself.

Closes #2323.

## Test Plan

- Verified that running against a private index with credentials in the
URL (but no netrc file) worked without error.
- Verified that running against a private index with credentials in the
netrc file (but not the URL) worked without error.
- Verified that running against a private index with a mix of
credentials in both _also_ worked without error.
  • Loading branch information
charliermarsh authored Mar 10, 2024
1 parent 7fc8087 commit 67fb023
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 16 deletions.
13 changes: 2 additions & 11 deletions Cargo.lock

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

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,10 @@ reflink-copy = { version = "0.1.14" }
regex = { version = "1.10.2" }
reqwest = { version = "0.11.23", default-features = false, features = ["json", "gzip", "brotli", "stream", "rustls-tls-native-roots"] }
reqwest-middleware = { version = "0.2.4" }
reqwest-netrc = { version = "0.1.1" }
reqwest-retry = { version = "0.3.0" }
rkyv = { version = "0.7.43", features = ["strict", "validation"] }
rmp-serde = { version = "1.1.2" }
rust-netrc = { version = "0.1.1" }
rustc-hash = { version = "1.1.0" }
same-file = { version = "1.0.6" }
seahash = { version = "4.1.0" }
Expand Down
3 changes: 2 additions & 1 deletion crates/uv-client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,18 @@ anyhow = { workspace = true }
async-trait = { workspace = true }
async_http_range_reader = { workspace = true }
async_zip = { workspace = true, features = ["tokio"] }
base64 = { workspace = true }
chrono = { workspace = true }
fs-err = { workspace = true, features = ["tokio"] }
futures = { workspace = true }
html-escape = { workspace = true }
http = { workspace = true }
reqwest = { workspace = true }
reqwest-middleware = { workspace = true }
reqwest-netrc = { workspace = true }
reqwest-retry = { workspace = true }
rkyv = { workspace = true, features = ["strict", "validation"] }
rmp-serde = { workspace = true }
rust-netrc = { workspace = true }
rustc-hash = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
Expand Down
76 changes: 76 additions & 0 deletions crates/uv-client/src/middleware.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::fmt::Debug;

use http::HeaderValue;
use netrc::{Netrc, Result};
use reqwest::{Request, Response};
use reqwest_middleware::{Middleware, Next};
use task_local_extensions::Extensions;
Expand Down Expand Up @@ -45,3 +47,77 @@ impl Middleware for OfflineMiddleware {
))
}
}

/// A middleware with support for netrc files.
///
/// Based on: <https://github.com/gribouille/netrc>.
pub(crate) struct NetrcMiddleware {
nrc: Netrc,
}

impl NetrcMiddleware {
pub(crate) fn new() -> Result<Self> {
Netrc::new().map(|nrc| NetrcMiddleware { nrc })
}
}

#[async_trait::async_trait]
impl Middleware for NetrcMiddleware {
async fn handle(
&self,
mut req: Request,
_extensions: &mut Extensions,
next: Next<'_>,
) -> reqwest_middleware::Result<Response> {
// If the request already has an authorization header, we don't need to do anything.
// This gives in-URL credentials precedence over the netrc file.
if req.headers().contains_key(reqwest::header::AUTHORIZATION) {
return next.run(req, _extensions).await;
}

if let Some(auth) = req.url().host_str().and_then(|host| {
self.nrc
.hosts
.get(host)
.or_else(|| self.nrc.hosts.get("default"))
}) {
req.headers_mut().insert(
reqwest::header::AUTHORIZATION,
basic_auth(
&auth.login,
if auth.password.is_empty() {
None
} else {
Some(&auth.password)
},
),
);
}
next.run(req, _extensions).await
}
}

/// Create a `HeaderValue` for basic authentication.
///
/// Source: <https://github.com/seanmonstar/reqwest/blob/2c11ef000b151c2eebeed2c18a7b81042220c6b0/src/util.rs#L3>
fn basic_auth<U, P>(username: U, password: Option<P>) -> HeaderValue
where
U: std::fmt::Display,
P: std::fmt::Display,
{
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, "{}:", username);
if let Some(password) = password {
let _ = write!(encoder, "{}", password);
}
}
let mut header = HeaderValue::from_bytes(&buf).expect("base64 is always valid HeaderValue");
header.set_sensitive(true);
header
}
5 changes: 2 additions & 3 deletions crates/uv-client/src/registry_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use async_http_range_reader::AsyncHttpRangeReader;
use futures::{FutureExt, TryStreamExt};
use http::HeaderMap;
use reqwest::{Client, ClientBuilder, Response, StatusCode};
use reqwest_netrc::NetrcMiddleware;
use reqwest_retry::policies::ExponentialBackoff;
use reqwest_retry::RetryTransientMiddleware;
use serde::{Deserialize, Serialize};
Expand All @@ -30,7 +29,7 @@ use uv_warnings::warn_user_once;

use crate::cached_client::CacheControl;
use crate::html::SimpleHtml;
use crate::middleware::OfflineMiddleware;
use crate::middleware::{NetrcMiddleware, OfflineMiddleware};
use crate::remote_metadata::wheel_metadata_from_remote_zip;
use crate::rkyvutil::OwnedArchive;
use crate::{CachedClient, CachedClientError, Error, ErrorKind};
Expand Down Expand Up @@ -133,7 +132,7 @@ impl RegistryClientBuilder {

// Initialize the netrc middleware.
let client = if let Ok(netrc) = NetrcMiddleware::new() {
client.with_init(netrc)
client.with(netrc)
} else {
client
};
Expand Down

0 comments on commit 67fb023

Please sign in to comment.