From f1ebae065a4df984888223c415b07d143db113c3 Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Wed, 26 Nov 2025 07:55:32 +0100 Subject: [PATCH 1/2] fix wrong semver redirects with items --- src/web/extractors/rustdoc.rs | 40 ++++++++++++++++++++++++++++++++--- src/web/rustdoc.rs | 26 ++++++++++++++++++++++- 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/src/web/extractors/rustdoc.rs b/src/web/extractors/rustdoc.rs index 6b2a60970..0084b28ce 100644 --- a/src/web/extractors/rustdoc.rs +++ b/src/web/extractors/rustdoc.rs @@ -720,9 +720,24 @@ fn generate_rustdoc_url(name: &str, version: &ReqVersion, path: &str) -> Escaped fn generate_rustdoc_path_for_url( target_name: Option<&str>, default_target: Option<&str>, - doc_target: Option<&str>, - inner_path: Option<&str>, + mut doc_target: Option<&str>, + mut inner_path: Option<&str>, ) -> String { + // if we have an "unparsed" set of params, we might have a part of + // the inner path in `doc_target`. Thing is: + // We don't know if that's a real target, or a part of the path, + // But the "saner" default for this method is to treat it as part + // of the path, not a potential doc target. + let inner_path = if target_name.is_none() + && default_target.is_none() + && let (Some(doc_target), Some(inner_path)) = (doc_target.take(), inner_path.as_mut()) + && !doc_target.is_empty() + { + Some(format!("{doc_target}/{inner_path}")) + } else { + inner_path.map(|s| s.to_string()) + }; + // first validate & fix the inner path to use. let result = if let Some(path) = inner_path && !path.is_empty() @@ -737,7 +752,7 @@ fn generate_rustdoc_path_for_url( // * it's not just "index.html" // * we have a slash in the path. path.to_string() - } else if ROOT_RUSTDOC_HTML_FILES.contains(&path) { + } else if ROOT_RUSTDOC_HTML_FILES.contains(&path.as_str()) { // special case: some files are at the root of the rustdoc output, // without a trailing slash, and the routes are fine with that. // e.g. `/help.html`, `/settings.html`, `/all.html`, ... @@ -1682,4 +1697,23 @@ mod tests { assert_eq!(params.rustdoc_url(), expected_url); } + + #[test] + fn test_item_with_semver_url() { + // https://github.com/rust-lang/docs.rs/iss + + let ver: Version = "0.14.0".parse().unwrap(); + let params = RustdocParams::new(KRATE) + .with_page_kind(PageKind::Rustdoc) + .with_req_version(ReqVersion::Exact(ver)) + .with_doc_target(KRATE) + .with_inner_path("trait.Itertools.html"); + + dbg!(¶ms); + + assert_eq!( + params.rustdoc_url(), + format!("/{KRATE}/0.14.0/{KRATE}/trait.Itertools.html") + ) + } } diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index aef08b6f3..cc4db0056 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -474,7 +474,7 @@ pub(crate) async fn rustdoc_html_server_handler( })? .into_canonical_req_version_or_else(|version| { AxumNope::Redirect( - params.clone().with_req_version(version).rustdoc_url(), + dbg!(params.clone().with_req_version(version)).rustdoc_url(), CachePolicy::ForeverInCdn, ) })?; @@ -3464,4 +3464,28 @@ mod test { Ok(()) } + + #[tokio::test(flavor = "multi_thread")] + async fn test_fetch_item_with_semver_url() -> Result<()> { + // https://github.com/rust-lang/docs.rs/issues/3036 + + let env = TestEnvironment::new().await?; + + env.fake_release() + .await + .name("itertools") + .version("0.14.0") + .rustdoc_file("itertools/trait.Itertools.html") + .create() + .await?; + + let web = env.web_app().await; + web.assert_redirect( + "/itertools/^0.14/itertools/trait.Itertools.html", + "/itertools/0.14.0/itertools/trait.Itertools.html", + ) + .await?; + + Ok(()) + } } From 7538da978548278e018946019d818483200fac67 Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Wed, 26 Nov 2025 12:13:16 +0100 Subject: [PATCH 2/2] add more explaining comments --- src/web/extractors/rustdoc.rs | 5 ++++- src/web/rustdoc.rs | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/web/extractors/rustdoc.rs b/src/web/extractors/rustdoc.rs index 0084b28ce..8ceec8499 100644 --- a/src/web/extractors/rustdoc.rs +++ b/src/web/extractors/rustdoc.rs @@ -1700,7 +1700,10 @@ mod tests { #[test] fn test_item_with_semver_url() { - // https://github.com/rust-lang/docs.rs/iss + // https://github.com/rust-lang/docs.rs/issues/3036 + // This fixes an issue where we mistakenly attached a + // trailing `/` to a rustdoc URL when redirecting + // to the exact version, coming from a semver version. let ver: Version = "0.14.0".parse().unwrap(); let params = RustdocParams::new(KRATE) diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index cc4db0056..08e48d664 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -474,7 +474,7 @@ pub(crate) async fn rustdoc_html_server_handler( })? .into_canonical_req_version_or_else(|version| { AxumNope::Redirect( - dbg!(params.clone().with_req_version(version)).rustdoc_url(), + params.clone().with_req_version(version).rustdoc_url(), CachePolicy::ForeverInCdn, ) })?; @@ -3468,7 +3468,9 @@ mod test { #[tokio::test(flavor = "multi_thread")] async fn test_fetch_item_with_semver_url() -> Result<()> { // https://github.com/rust-lang/docs.rs/issues/3036 - + // This fixes an issue where we mistakenly attached a + // trailing `/` to a rustdoc URL when redirecting + // to the exact version, coming from a semver version. let env = TestEnvironment::new().await?; env.fake_release()