Skip to content

Commit

Permalink
Respect allow insecure host in publish
Browse files Browse the repository at this point in the history
Switch to respecting the allow insecure host setting in all places where we use the client.

Fixes #8423
Part of #6985
Doesn't touch #6983
  • Loading branch information
konstin committed Oct 22, 2024
1 parent 0dd4d01 commit 2854563
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 30 deletions.
10 changes: 0 additions & 10 deletions crates/uv-client/src/base_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,16 +350,6 @@ enum Security {
}

impl BaseClient {
/// The underlying [`ClientWithMiddleware`] for secure requests.
pub fn client(&self) -> ClientWithMiddleware {
self.client.clone()
}

/// The underlying [`Client`] without middleware.
pub fn raw_client(&self) -> Client {
self.raw_client.clone()
}

/// Selects the appropriate client based on the host's trustworthiness.
pub fn for_host(&self, url: &Url) -> &ClientWithMiddleware {
if self
Expand Down
13 changes: 8 additions & 5 deletions crates/uv-client/tests/it/user_agent_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ use hyper::service::service_fn;
use hyper::{Request, Response};
use hyper_util::rt::TokioIo;
use insta::{assert_json_snapshot, assert_snapshot, with_settings};
use std::str::FromStr;
use tokio::net::TcpListener;

use url::Url;
use uv_cache::Cache;
use uv_client::LineHaul;
use uv_client::RegistryClientBuilder;
Expand Down Expand Up @@ -53,11 +54,12 @@ async fn test_user_agent_has_version() -> Result<()> {
let client = RegistryClientBuilder::new(cache).build();

// Send request to our dummy server
let url = Url::from_str(&format!("http://{addr}"))?;
let res = client
.cached_client()
.uncached()
.client()
.get(format!("http://{addr}"))
.for_host(&url)
.get(url)
.send()
.await?;

Expand Down Expand Up @@ -149,11 +151,12 @@ async fn test_user_agent_has_linehaul() -> Result<()> {
let client = builder.build();

// Send request to our dummy server
let url = Url::from_str(&format!("http://{addr}"))?;
let res = client
.cached_client()
.uncached()
.client()
.get(format!("http://{addr}"))
.for_host(&url)
.get(url)
.send()
.await?;

Expand Down
15 changes: 8 additions & 7 deletions crates/uv-publish/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use itertools::Itertools;
use reqwest::header::AUTHORIZATION;
use reqwest::multipart::Part;
use reqwest::{Body, Response, StatusCode};
use reqwest_middleware::{ClientWithMiddleware, RequestBuilder};
use reqwest_middleware::RequestBuilder;
use reqwest_retry::{Retryable, RetryableStrategy};
use rustc_hash::FxHashSet;
use serde::Deserialize;
Expand All @@ -24,7 +24,7 @@ use tokio::io::AsyncReadExt;
use tokio_util::io::ReaderStream;
use tracing::{debug, enabled, trace, Level};
use url::Url;
use uv_client::UvRetryableStrategy;
use uv_client::{BaseClient, UvRetryableStrategy};
use uv_configuration::{KeyringProviderType, TrustedPublishing};
use uv_distribution_filename::{DistFilename, SourceDistExtension, SourceDistFilename};
use uv_fs::{ProgressReader, Simplified};
Expand Down Expand Up @@ -246,7 +246,7 @@ pub async fn check_trusted_publishing(
keyring_provider: KeyringProviderType,
trusted_publishing: TrustedPublishing,
registry: &Url,
client: &ClientWithMiddleware,
client: &BaseClient,
) -> Result<Option<TrustedPublishingToken>, PublishError> {
match trusted_publishing {
TrustedPublishing::Automatic => {
Expand All @@ -264,7 +264,7 @@ pub async fn check_trusted_publishing(
// We could check for credentials from the keyring or netrc the auth middleware first, but
// given that we are in GitHub Actions we check for trusted publishing first.
debug!("Running on GitHub Actions without explicit credentials, checking for trusted publishing");
match trusted_publishing::get_token(registry, client).await {
match trusted_publishing::get_token(registry, client.for_host(registry)).await {
Ok(token) => Ok(Some(token)),
Err(err) => {
// TODO(konsti): It would be useful if we could differentiate between actual errors
Expand All @@ -283,7 +283,7 @@ pub async fn check_trusted_publishing(
);
}

let token = trusted_publishing::get_token(registry, client).await?;
let token = trusted_publishing::get_token(registry, client.for_host(registry)).await?;
Ok(Some(token))
}
TrustedPublishing::Never => Ok(None),
Expand All @@ -300,7 +300,7 @@ pub async fn upload(
raw_filename: &str,
filename: &DistFilename,
registry: &Url,
client: &ClientWithMiddleware,
client: &BaseClient,
retries: u32,
username: Option<&str>,
password: Option<&str>,
Expand Down Expand Up @@ -504,7 +504,7 @@ async fn build_request(
raw_filename: &str,
filename: &DistFilename,
registry: &Url,
client: &ClientWithMiddleware,
client: &BaseClient,
username: Option<&str>,
password: Option<&str>,
form_metadata: &[(&'static str, String)],
Expand Down Expand Up @@ -543,6 +543,7 @@ async fn build_request(
};

let mut request = client
.for_host(&url)
.post(url)
.multipart(form)
// Ask PyPI for a structured error messages instead of HTML-markup error messages.
Expand Down
4 changes: 2 additions & 2 deletions crates/uv-publish/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ async fn upload_request_source_dist() {
raw_filename,
&filename,
&Url::parse("https://example.org/upload").unwrap(),
&BaseClientBuilder::new().build().client(),
&BaseClientBuilder::new().build(),
Some("ferris"),
Some("F3RR!S"),
&form_metadata,
Expand Down Expand Up @@ -229,7 +229,7 @@ async fn upload_request_wheel() {
raw_filename,
&filename,
&Url::parse("https://example.org/upload").unwrap(),
&BaseClientBuilder::new().build().client(),
&BaseClientBuilder::new().build(),
Some("ferris"),
Some("F3RR!S"),
&form_metadata,
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-python/src/downloads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ async fn read_url(

Ok((Either::Left(reader), Some(size)))
} else {
let response = client.client().get(url.clone()).send().await?;
let response = client.for_host(url).get(url.clone()).send().await?;

// Ensure the request was successful.
response.error_for_status_ref()?;
Expand Down
18 changes: 16 additions & 2 deletions crates/uv-requirements-txt/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -798,9 +798,11 @@ async fn read_url_to_string(
url: path.as_ref().to_owned(),
})?;

let url = Url::from_str(path_utf8)
.map_err(|err| RequirementsTxtParserError::InvalidUrl(path_utf8.to_string(), err))?;
Ok(client
.client()
.get(path_utf8)
.for_host(&url)
.get(url)
.send()
.await?
.error_for_status()?
Expand Down Expand Up @@ -890,6 +892,8 @@ pub enum RequirementsTxtParserError {
},
#[cfg(feature = "http")]
Reqwest(reqwest_middleware::Error),
#[cfg(feature = "http")]
InvalidUrl(String, url::ParseError),
}

impl Display for RequirementsTxtParserError {
Expand Down Expand Up @@ -956,6 +960,10 @@ impl Display for RequirementsTxtParserError {
Self::Reqwest(err) => {
write!(f, "Error while accessing remote requirements file {err}")
}
#[cfg(feature = "http")]
Self::InvalidUrl(url, err) => {
write!(f, "Not a valid URL, {err}: `{url}`")
}
}
}
}
Expand All @@ -982,6 +990,8 @@ impl std::error::Error for RequirementsTxtParserError {
Self::NonUnicodeUrl { .. } => None,
#[cfg(feature = "http")]
Self::Reqwest(err) => err.source(),
#[cfg(feature = "http")]
Self::InvalidUrl(_, err) => err.source(),
}
}
}
Expand Down Expand Up @@ -1114,6 +1124,10 @@ impl Display for RequirementsTxtFileError {
self.file.user_display(),
)
}
#[cfg(feature = "http")]
RequirementsTxtParserError::InvalidUrl(url, err) => {
write!(f, "Not a valid URL, {err}: `{url}`")
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/uv/src/commands/project/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1185,7 +1185,7 @@ impl RunCommand {
.connectivity(connectivity)
.native_tls(native_tls)
.build();
let response = client.client().get(url.clone()).send().await?;
let response = client.for_host(&url).get(url.clone()).send().await?;

// Stream the response to the file.
let mut writer = file.as_file();
Expand Down
4 changes: 2 additions & 2 deletions crates/uv/src/commands/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ pub(crate) async fn publish(
keyring_provider,
trusted_publishing,
&publish_url,
&oidc_client.client(),
&oidc_client,
)
.await?;

Expand Down Expand Up @@ -104,7 +104,7 @@ pub(crate) async fn publish(
&raw_filename,
&filename,
&publish_url,
&upload_client.client(),
&upload_client,
DEFAULT_RETRIES,
username.as_deref(),
password.as_deref(),
Expand Down

0 comments on commit 2854563

Please sign in to comment.