From 8906853fd9206f486933c8f991d99c89222ab36f Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Thu, 9 Oct 2025 20:51:50 +0200 Subject: [PATCH] fix wrong target-redifect links in topbar release list partial --- src/web/crate_details.rs | 77 ++++++++++++++-------------------------- src/web/routes.rs | 12 ------- src/web/rustdoc.rs | 7 ++-- 3 files changed, 31 insertions(+), 65 deletions(-) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index 7f4d9ef43..85a5b8706 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -584,8 +584,9 @@ pub(crate) async fn get_all_releases( Path(params): Path, mut conn: DbConnection, ) -> AxumResult { - let req_path: String = params.path.clone().unwrap_or_default(); - let req_path: Vec<&str> = req_path.split('/').collect(); + // NOTE: we're getting RustDocHtmlParams here, where both target and path are optional. + // Due to how this handler is used in the `releases_list` macro, we always get both values. + // both values (when used in the topbar). let matched_release = match_version(&mut conn, ¶ms.name, ¶ms.version) .await? @@ -599,61 +600,26 @@ pub(crate) async fn get_all_releases( return Err(AxumNope::CrateNotFound); } - let doc_targets = sqlx::query_scalar!( - "SELECT - releases.doc_targets - FROM releases - WHERE releases.id = $1;", - matched_release.id().0, - ) - .fetch_optional(&mut *conn) - .await? - .ok_or(AxumNope::CrateNotFound)? - .map(MetaData::parse_doc_targets) - .ok_or_else(|| anyhow!("empty doc targets for successful release"))?; - - let inner; - let (target, inner_path) = { - let mut inner_path = req_path.clone(); - - let target = if inner_path.len() > 1 - && doc_targets - .iter() - .any(|s| Some(s) == params.target.as_ref()) - { - inner_path.remove(0); - params.target.as_ref().unwrap() - } else { - "" - }; - - inner = inner_path.join("/"); - (target, inner.trim_end_matches('/')) - }; - - let target_name = matched_release - .target_name() - .ok_or_else(|| anyhow!("empty target name for succesful release"))?; - - let inner_path = if inner_path.is_empty() { - format!("{target_name}/index.html") + // NOTE: we don't check if the target exists here. + // If the target doesn't exist, the target-redirect will think + // it's part of the `inner_path`, don't find the file in storage, + // and redirect to a search. + let target = if let Some(req_target) = params.target { + format!("{req_target}/") } else { - format!("{target_name}/{inner_path}") - }; - - let target = if target.is_empty() { String::new() - } else { - format!("{target}/") }; - let res = ReleaseList { + let inner_path = params.path.unwrap_or_default(); + let inner_path = inner_path.trim_end_matches('/'); + + Ok(ReleaseList { releases: matched_release.all_releases, target, - inner_path, + inner_path: inner_path.to_string(), crate_name: params.name, - }; - Ok(res.into_response()) + } + .into_response()) } #[derive(Debug, Clone, PartialEq)] @@ -2034,6 +2000,17 @@ mod tests { .create() .await?; + check_links( + // https://github.com/rust-lang/docs.rs/issues/2922 + &env, + "/crate/dummy-ba/0.5.0/menus/releases/x86_64-unknown-linux-gnu/src/dummy_ba/de.rs.html", + vec![ + "/crate/dummy-ba/0.5.0/target-redirect/x86_64-unknown-linux-gnu/src/dummy_ba/de.rs.html".to_string(), + "/crate/dummy-ba/0.4.0/target-redirect/x86_64-unknown-linux-gnu/src/dummy_ba/de.rs.html".to_string(), + ], + ) + .await; + check_links( &env, "/crate/dummy-ba/latest/menus/releases/dummy_ba/index.html", diff --git a/src/web/routes.rs b/src/web/routes.rs index c6782b4cd..324b26587 100644 --- a/src/web/routes.rs +++ b/src/web/routes.rs @@ -269,22 +269,10 @@ pub(super) fn build_axum_routes() -> AxumRouter { "/crate/{name}/{version}/menus/platforms", get_internal(super::crate_details::get_all_platforms_root), ) - .route( - "/crate/{name}/{version}/menus/releases/{target}", - get_internal(super::crate_details::get_all_releases), - ) .route( "/crate/{name}/{version}/menus/releases/{target}/{*path}", get_internal(super::crate_details::get_all_releases), ) - .route( - "/crate/{name}/{version}/menus/releases", - get_internal(super::crate_details::get_all_releases), - ) - .route( - "/crate/{name}/{version}/menus/releases/{target}/", - get_internal(super::crate_details::get_all_releases), - ) .route( "/-/rustdoc.static/{*path}", get_internal(super::rustdoc::static_asset_handler), diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index 9959b1210..72a5ce191 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -1105,6 +1105,7 @@ mod test { use anyhow::Context; use chrono::{NaiveDate, Utc}; use kuchikiki::traits::TendrilSink; + use pretty_assertions::assert_eq; use reqwest::StatusCode; use std::collections::BTreeMap; use test_case::test_case; @@ -2687,15 +2688,15 @@ mod test { let releases_response = env .web_app() .await - .get("/crate/hexponent/0.3.1/menus/releases") + .get("/crate/hexponent/0.3.1/menus/releases/x86_64-unknown-linux-gnu/hexponent/index.html") .await?; assert!(releases_response.status().is_success()); releases_response.assert_cache_control(CachePolicy::ForeverInCdn, &env.config()); assert_eq!( parse_release_links_from_menu(&releases_response.text().await?), vec![ - "/crate/hexponent/0.3.1/target-redirect/hexponent/index.html".to_owned(), - "/crate/hexponent/0.3.0/target-redirect/hexponent/index.html".to_owned(), + "/crate/hexponent/0.3.1/target-redirect/x86_64-unknown-linux-gnu/hexponent/index.html".to_owned(), + "/crate/hexponent/0.3.0/target-redirect/x86_64-unknown-linux-gnu/hexponent/index.html".to_owned(), ] );