From d06ceffe3420514a563e09df8f717c67e1610659 Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Thu, 13 Nov 2025 20:12:59 +0100 Subject: [PATCH] Fix redirect loop with one-component path for crate without trailing slash --- src/web/extractors/rustdoc.rs | 66 ++++++++++++++++++++++++++++++++--- src/web/rustdoc.rs | 31 +++++++++++++++- 2 files changed, 92 insertions(+), 5 deletions(-) diff --git a/src/web/extractors/rustdoc.rs b/src/web/extractors/rustdoc.rs index c0b0a6f7a..37e2d1a54 100644 --- a/src/web/extractors/rustdoc.rs +++ b/src/web/extractors/rustdoc.rs @@ -723,10 +723,24 @@ fn generate_rustdoc_path_for_url( && !path.is_empty() && path != INDEX_HTML { - // ust juse the given inner to start, if: - // * it's not empty - // * it's not just "index.html" - path.to_string() + if let Some(target_name) = target_name + && path == target_name + { + // Sometimes people add a path that is just the target name. + // This matches our `rustdoc_redirector_handler` route, + // without trailing slash (`/{name}/{version}/{target}`). + // + // Even though the rustdoc html handler would also be capable + // handling this input, we want to redirect to the trailing + // slash version (`/{name}/{version}/{target}/`), + // so they are separate. + format!("{}/", target_name) + } else { + // just use the given inner to start, if: + // * it's not empty + // * it's not just "index.html" + path.to_string() + } } else if let Some(target_name) = target_name { // after having no usable given path, we generate one with the // target name, if we have one. @@ -1600,4 +1614,48 @@ mod tests { assert_eq!(params.doc_target(), Some(DEFAULT_TARGET)); assert_eq!(params.inner_path(), "dummy/struct.Dummy.html"); } + + #[test] + fn test_parse_something() { + // test for https://github.com/rust-lang/docs.rs/issues/2989 + let params = dbg!( + RustdocParams::new(KRATE) + .with_page_kind(PageKind::Rustdoc) + .try_with_original_uri(format!("/{KRATE}/latest/{KRATE}")) + .unwrap() + .with_req_version(ReqVersion::Latest) + .with_doc_target(KRATE) + ); + + assert_eq!(params.rustdoc_url(), "/krate/latest/krate/"); + + let params = dbg!( + params + .with_target_name(KRATE) + .with_default_target(DEFAULT_TARGET) + .with_doc_targets(TARGETS.iter().cloned()) + ); + + assert_eq!(params.rustdoc_url(), "/krate/latest/krate/"); + } + + #[test_case(Some(PageKind::Rustdoc))] + #[test_case(None)] + fn test_only_target_name_without_slash(page_kind: Option) { + // test for https://github.com/rust-lang/docs.rs/issues/2989 + let params = RustdocParams::new(KRATE) + .with_maybe_page_kind(page_kind) + .try_with_original_uri("/dummy/latest/dummy") + .unwrap() + .with_req_version(ReqVersion::Latest) + .with_maybe_doc_target(None::) + .with_inner_path(KRATE) + .with_default_target(DEFAULT_TARGET) + .with_target_name(KRATE) + .with_doc_targets(TARGETS.iter().cloned()); + + dbg!(¶ms); + + assert_eq!(params.rustdoc_url(), "/krate/latest/krate/"); + } } diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index 361ef483f..51d8504d8 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -229,7 +229,7 @@ pub(crate) async fn rustdoc_redirector_handler( url }; - trace!("redirect to doc"); + trace!(%url, ?cache_policy, path_in_crate, "redirect to doc"); Ok(axum_cached_redirect(url, cache_policy)?) } @@ -3341,4 +3341,33 @@ mod test { assert_eq!(response.status(), StatusCode::NOT_FOUND); Ok(()) } + + #[tokio::test(flavor = "multi_thread")] + #[test_case("/dummy/"; "only krate")] + #[test_case("/dummy/latest/"; "with version")] + #[test_case("/dummy/latest/dummy"; "full without trailing slash")] + #[test_case("/dummy/latest/dummy/"; "final target")] + async fn test_full_latest_url_without_trailing_slash(path: &str) -> Result<()> { + // test for https://github.com/rust-lang/docs.rs/issues/2989 + + let env = TestEnvironment::new().await?; + + env.fake_release() + .await + .name("dummy") + .version("1.0.0") + .create() + .await?; + + let web = env.web_app().await; + const TARGET: &str = "/dummy/latest/dummy/"; + if path == TARGET { + web.get(path).await?.status().is_success(); + } else { + web.assert_redirect_unchecked(path, "/dummy/latest/dummy/") + .await?; + } + + Ok(()) + } }