From d31e6ad7c7fca31a5640f6cdc3f0870eef2c390c Mon Sep 17 00:00:00 2001 From: John Mumm Date: Mon, 7 Jul 2025 12:51:21 +0200 Subject: [PATCH] Move fragment preservation test to directly test our redirect handling logic (#14480) When [updating](https://github.com/astral-sh/uv/pull/14475) to the latest `reqwest` version, our fragment propagation test broke. That test was partially testing the `reqwest` behavior, so this PR moves the fragment test to directly test our logic for constructing redirect requests. --- crates/uv-client/src/base_client.rs | 39 +++++++++++++++++++++++++ crates/uv-client/src/registry_client.rs | 38 ------------------------ 2 files changed, 39 insertions(+), 38 deletions(-) diff --git a/crates/uv-client/src/base_client.rs b/crates/uv-client/src/base_client.rs index d62621863..e11845adb 100644 --- a/crates/uv-client/src/base_client.rs +++ b/crates/uv-client/src/base_client.rs @@ -982,6 +982,45 @@ mod tests { Ok(()) } + #[tokio::test] + async fn test_redirect_preserves_fragment() -> Result<()> { + for status in &[301, 302, 303, 307, 308] { + let server = MockServer::start().await; + Mock::given(method("GET")) + .respond_with( + ResponseTemplate::new(*status) + .insert_header("location", format!("{}/redirect", server.uri())), + ) + .mount(&server) + .await; + + let request = Client::new() + .get(format!("{}#fragment", server.uri())) + .build() + .unwrap(); + + let response = Client::builder() + .redirect(reqwest::redirect::Policy::none()) + .build() + .unwrap() + .execute(request.try_clone().unwrap()) + .await + .unwrap(); + + let redirect_request = + request_into_redirect(request, &response, CrossOriginCredentialsPolicy::Secure)? + .unwrap(); + assert!( + redirect_request + .url() + .fragment() + .is_some_and(|fragment| fragment == "fragment") + ); + } + + Ok(()) + } + #[tokio::test] async fn test_redirect_removes_authorization_header_on_cross_origin() -> Result<()> { for status in &[301, 302, 303, 307, 308] { diff --git a/crates/uv-client/src/registry_client.rs b/crates/uv-client/src/registry_client.rs index b53e1ed9a..5788ea56c 100644 --- a/crates/uv-client/src/registry_client.rs +++ b/crates/uv-client/src/registry_client.rs @@ -1416,44 +1416,6 @@ mod tests { Ok(()) } - #[tokio::test] - async fn test_redirect_preserve_fragment() -> Result<(), Error> { - let redirect_server = MockServer::start().await; - - // Configure the redirect server to respond with a 307 with a relative URL. - Mock::given(method("GET")) - .respond_with(ResponseTemplate::new(307).insert_header("Location", "/foo".to_string())) - .mount(&redirect_server) - .await; - - Mock::given(method("GET")) - .and(path_regex("/foo")) - .respond_with(ResponseTemplate::new(200)) - .mount(&redirect_server) - .await; - - let cache = Cache::temp()?; - let registry_client = RegistryClientBuilder::new(cache).build(); - let client = registry_client.cached_client().uncached(); - - let mut url = DisplaySafeUrl::parse(&redirect_server.uri())?; - url.set_fragment(Some("fragment")); - - assert_eq!( - client - .for_host(&url) - .get(Url::from(url.clone())) - .send() - .await? - .url() - .to_string(), - format!("{}/foo#fragment", redirect_server.uri()), - "Requests should preserve fragment" - ); - - Ok(()) - } - #[test] fn ignore_failing_files() { // 1.7.7 has an invalid requires-python field (double comma), 1.7.8 is valid