Skip to content

Commit

Permalink
Ensure authentication is passed from the index url to distribution files
Browse files Browse the repository at this point in the history
  • Loading branch information
zanieb committed Feb 22, 2024
1 parent 8382f71 commit e801f64
Show file tree
Hide file tree
Showing 10 changed files with 52 additions and 21 deletions.
10 changes: 10 additions & 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 crates/distribution-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ distribution-filename = { path = "../distribution-filename", features = ["serde"
pep440_rs = { path = "../pep440-rs" }
pep508_rs = { path = "../pep508-rs" }
platform-tags = { path = "../platform-tags" }
uv-auth = { path = "../uv-auth" }
uv-fs = { path = "../uv-fs" }
uv-git = { path = "../uv-git", features = ["vendored-openssl"] }
uv-normalize = { path = "../uv-normalize" }
Expand Down
11 changes: 9 additions & 2 deletions crates/distribution-types/src/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ use thiserror::Error;

use pep440_rs::{VersionSpecifiers, VersionSpecifiersParseError};
use pypi_types::{DistInfoMetadata, Hashes, Yanked};
use url::Url;
use uv_auth::safe_copy_url_auth;

/// Error converting [`pypi_types::File`] to [`distribution_type::File`].
#[derive(Debug, Error)]
Expand Down Expand Up @@ -39,7 +41,7 @@ pub struct File {

impl File {
/// `TryFrom` instead of `From` to filter out files with invalid requires python version specifiers
pub fn try_from(file: pypi_types::File, base: &str) -> Result<Self, FileConversionError> {
pub fn try_from(file: pypi_types::File, base: &Url) -> Result<Self, FileConversionError> {
Ok(Self {
dist_info_metadata: file.dist_info_metadata,
filename: file.filename,
Expand All @@ -51,7 +53,12 @@ impl File {
size: file.size,
upload_time_utc_ms: file.upload_time.map(|dt| dt.timestamp_millis()),
url: if file.url.contains("://") {
FileLocation::AbsoluteUrl(file.url)
let url = safe_copy_url_auth(
base,
Url::parse(&file.url)
.map_err(|err| FileConversionError::Url(file.url.clone(), err))?,
);
FileLocation::AbsoluteUrl(url.to_string())
} else {
FileLocation::RelativeUrl(base.to_string(), file.url)
},
Expand Down
6 changes: 6 additions & 0 deletions crates/pypi-types/src/base_url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ impl BaseUrl {
pub fn as_url(&self) -> &Url {
&self.0
}

/// Convert to the underlying [`Url`].
#[must_use]
pub fn into_url(self) -> Url {
self.0
}
}

impl From<Url> for BaseUrl {
Expand Down
8 changes: 8 additions & 0 deletions crates/uv-auth/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[package]
name = "uv-auth"
version = "0.0.1"
edition = "2021"

[dependencies]
url = { workspace = true }
tracing = { workspace = true }
7 changes: 3 additions & 4 deletions crates/uv-client/src/auth.rs → crates/uv-auth/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
/// HTTP authentication utilities.
use tracing::warn;
use url::Url;

/// HTTP authentication utilities.

/// Copy authentication from one URL to another URL if applicable.
///
/// See [`should_retain_auth`] for details on when authentication is retained.
#[must_use]
pub(crate) fn safe_copy_auth(request_url: &Url, mut response_url: Url) -> Url {
pub fn safe_copy_url_auth(request_url: &Url, mut response_url: Url) -> Url {
if should_retain_auth(request_url, &response_url) {
response_url
.set_username(request_url.username())
Expand Down Expand Up @@ -61,7 +60,7 @@ fn should_retain_auth(request_url: &Url, response_url: &Url) -> bool {
mod tests {
use url::{ParseError, Url};

use crate::auth::should_retain_auth;
use crate::should_retain_auth;

#[test]
fn test_should_retain_auth() -> Result<(), ParseError> {
Expand Down
1 change: 1 addition & 0 deletions crates/uv-client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ install-wheel-rs = { path = "../install-wheel-rs" }
pep440_rs = { path = "../pep440-rs" }
pep508_rs = { path = "../pep508-rs" }
platform-tags = { path = "../platform-tags" }
uv-auth = { path = "../uv-auth" }
uv-cache = { path = "../uv-cache" }
uv-fs = { path = "../uv-fs", features = ["tokio"] }
uv-normalize = { path = "../uv-normalize" }
Expand Down
7 changes: 4 additions & 3 deletions crates/uv-client/src/flat_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ use distribution_types::{
use pep440_rs::Version;
use platform_tags::Tags;
use pypi_types::{Hashes, Yanked};
use uv_auth::safe_copy_url_auth;
use uv_cache::{Cache, CacheBucket};
use uv_normalize::PackageName;

use crate::auth::safe_copy_auth;
use crate::cached_client::{CacheControl, CachedClientError};
use crate::html::SimpleHtml;
use crate::{Connectivity, Error, ErrorKind, RegistryClient};
Expand Down Expand Up @@ -156,16 +156,17 @@ impl<'a> FlatIndexClient<'a> {
async {
// Use the response URL, rather than the request URL, as the base for relative URLs.
// This ensures that we handle redirects and other URL transformations correctly.
let url = safe_copy_auth(url, response.url().clone());
let url = safe_copy_url_auth(url, response.url().clone());

let text = response.text().await.map_err(ErrorKind::RequestError)?;
let SimpleHtml { base, files } = SimpleHtml::parse(&text, &url)
.map_err(|err| Error::from_html_err(err, url.clone()))?;

let base = safe_copy_url_auth(&url, base.into_url());
let files: Vec<File> = files
.into_iter()
.filter_map(|file| {
match File::try_from(file, base.as_url().as_str()) {
match File::try_from(file, &base) {
Ok(file) => Some(file),
Err(err) => {
// Ignore files with unparsable version specifiers.
Expand Down
1 change: 0 additions & 1 deletion crates/uv-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ pub use registry_client::{
};
pub use rkyvutil::OwnedArchive;

mod auth;
mod cached_client;
mod error;
mod flat_index;
Expand Down
21 changes: 10 additions & 11 deletions crates/uv-client/src/registry_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ use distribution_types::{BuiltDist, File, FileLocation, IndexUrl, IndexUrls, Nam
use install_wheel_rs::{find_dist_info, is_metadata_entry};
use pep440_rs::Version;
use pypi_types::{Metadata21, SimpleJson};
use uv_auth::safe_copy_url_auth;
use uv_cache::{Cache, CacheBucket, WheelCache};
use uv_normalize::PackageName;
use uv_warnings::warn_user_once;

use crate::auth::safe_copy_auth;
use crate::cached_client::CacheControl;
use crate::html::SimpleHtml;
use crate::middleware::OfflineMiddleware;
Expand Down Expand Up @@ -249,7 +249,7 @@ impl RegistryClient {
async {
// Use the response URL, rather than the request URL, as the base for relative URLs.
// This ensures that we handle redirects and other URL transformations correctly.
let url = safe_copy_auth(&url, response.url().clone());
let url = safe_copy_url_auth(&url, response.url().clone());

let content_type = response
.headers()
Expand All @@ -271,17 +271,16 @@ impl RegistryClient {
let bytes = response.bytes().await.map_err(ErrorKind::RequestError)?;
let data: SimpleJson = serde_json::from_slice(bytes.as_ref())
.map_err(|err| Error::from_json_err(err, url.clone()))?;
let metadata =
SimpleMetadata::from_files(data.files, package_name, url.as_str());
metadata

SimpleMetadata::from_files(data.files, package_name, &url)
}
MediaType::Html => {
let text = response.text().await.map_err(ErrorKind::RequestError)?;
let SimpleHtml { base, files } = SimpleHtml::parse(&text, &url)
.map_err(|err| Error::from_html_err(err, url.clone()))?;
let metadata =
SimpleMetadata::from_files(files, package_name, base.as_url().as_str());
metadata
let base = safe_copy_url_auth(&url, base.into_url());

SimpleMetadata::from_files(files, package_name, &base)
}
};
OwnedArchive::from_unarchived(&unarchived)
Expand Down Expand Up @@ -670,7 +669,7 @@ impl SimpleMetadata {
self.0.iter()
}

fn from_files(files: Vec<pypi_types::File>, package_name: &PackageName, base: &str) -> Self {
fn from_files(files: Vec<pypi_types::File>, package_name: &PackageName, base: &Url) -> Self {
let mut map: BTreeMap<Version, VersionFiles> = BTreeMap::default();

// Group the distributions by version and kind
Expand Down Expand Up @@ -810,11 +809,11 @@ mod tests {
}
"#;
let data: SimpleJson = serde_json::from_str(response).unwrap();
let base = "https://pypi.org/simple/pyflyby/";
let base = Url::parse("https://pypi.org/simple/pyflyby/").unwrap();
let simple_metadata = SimpleMetadata::from_files(
data.files,
&PackageName::from_str("pyflyby").unwrap(),
base,
&base,
);
let versions: Vec<String> = simple_metadata
.iter()
Expand Down

0 comments on commit e801f64

Please sign in to comment.