From 285456383726b65773431acbb239695d5006c4a8 Mon Sep 17 00:00:00 2001 From: konstin Date: Tue, 22 Oct 2024 11:26:37 +0200 Subject: [PATCH] Respect allow insecure host in publish 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 --- crates/uv-client/src/base_client.rs | 10 ---------- .../uv-client/tests/it/user_agent_version.rs | 13 ++++++++----- crates/uv-publish/src/lib.rs | 15 ++++++++------- crates/uv-publish/src/tests.rs | 4 ++-- crates/uv-python/src/downloads.rs | 2 +- crates/uv-requirements-txt/src/lib.rs | 18 ++++++++++++++++-- crates/uv/src/commands/project/run.rs | 2 +- crates/uv/src/commands/publish.rs | 4 ++-- 8 files changed, 38 insertions(+), 30 deletions(-) diff --git a/crates/uv-client/src/base_client.rs b/crates/uv-client/src/base_client.rs index 0cc0f403a08a..b2b2829cf5e4 100644 --- a/crates/uv-client/src/base_client.rs +++ b/crates/uv-client/src/base_client.rs @@ -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 diff --git a/crates/uv-client/tests/it/user_agent_version.rs b/crates/uv-client/tests/it/user_agent_version.rs index 4511067b3da4..42eebf3bed22 100644 --- a/crates/uv-client/tests/it/user_agent_version.rs +++ b/crates/uv-client/tests/it/user_agent_version.rs @@ -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; @@ -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?; @@ -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?; diff --git a/crates/uv-publish/src/lib.rs b/crates/uv-publish/src/lib.rs index f832079ab181..d3a68a1c6509 100644 --- a/crates/uv-publish/src/lib.rs +++ b/crates/uv-publish/src/lib.rs @@ -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; @@ -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}; @@ -246,7 +246,7 @@ pub async fn check_trusted_publishing( keyring_provider: KeyringProviderType, trusted_publishing: TrustedPublishing, registry: &Url, - client: &ClientWithMiddleware, + client: &BaseClient, ) -> Result, PublishError> { match trusted_publishing { TrustedPublishing::Automatic => { @@ -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 @@ -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), @@ -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>, @@ -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)], @@ -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. diff --git a/crates/uv-publish/src/tests.rs b/crates/uv-publish/src/tests.rs index aac1762600b1..99a675f105e0 100644 --- a/crates/uv-publish/src/tests.rs +++ b/crates/uv-publish/src/tests.rs @@ -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, @@ -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, diff --git a/crates/uv-python/src/downloads.rs b/crates/uv-python/src/downloads.rs index e9679def0fc0..4e865375da52 100644 --- a/crates/uv-python/src/downloads.rs +++ b/crates/uv-python/src/downloads.rs @@ -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()?; diff --git a/crates/uv-requirements-txt/src/lib.rs b/crates/uv-requirements-txt/src/lib.rs index 83bd2cccfff5..3ebe38b40951 100644 --- a/crates/uv-requirements-txt/src/lib.rs +++ b/crates/uv-requirements-txt/src/lib.rs @@ -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()? @@ -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 { @@ -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}`") + } } } } @@ -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(), } } } @@ -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}`") + } } } } diff --git a/crates/uv/src/commands/project/run.rs b/crates/uv/src/commands/project/run.rs index 38894f4c55de..ed94cc20a8d7 100644 --- a/crates/uv/src/commands/project/run.rs +++ b/crates/uv/src/commands/project/run.rs @@ -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(); diff --git a/crates/uv/src/commands/publish.rs b/crates/uv/src/commands/publish.rs index b850e7b0ddb3..315add475644 100644 --- a/crates/uv/src/commands/publish.rs +++ b/crates/uv/src/commands/publish.rs @@ -67,7 +67,7 @@ pub(crate) async fn publish( keyring_provider, trusted_publishing, &publish_url, - &oidc_client.client(), + &oidc_client, ) .await?; @@ -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(),