From 20a14d422db5beba2550daea0acbd9ce18f58923 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 31 Jul 2023 00:43:34 +0200 Subject: [PATCH 01/14] Only put platforms list is less than 6, otherwise load with AJAX --- src/web/crate_details.rs | 165 +++++++++++++++++++++++++++++++ src/web/routes.rs | 10 +- src/web/rustdoc.rs | 10 +- static/menu.js | 35 +++++-- templates/rustdoc/platforms.html | 25 +++++ templates/rustdoc/topbar.html | 30 +----- 6 files changed, 234 insertions(+), 41 deletions(-) create mode 100644 templates/rustdoc/platforms.html diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index 13ef493b2..27fc862a4 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -1,5 +1,7 @@ use super::{markdown, match_version, MatchSemver, MetaData}; use crate::utils::{get_correct_docsrs_style_file, report_error, spawn_blocking}; +use crate::web::rustdoc::RustdocHtmlParams; +use crate::web::{axum_cached_redirect, match_version_axum}; use crate::{ db::Pool, impl_axum_webpage, @@ -15,6 +17,7 @@ use crate::{ use anyhow::{Context, Result}; use axum::{ extract::{Extension, Path}, + http::Uri, response::{IntoResponse, Response as AxumResponse}, }; use chrono::{DateTime, Utc}; @@ -23,6 +26,7 @@ use serde::Deserialize; use serde::{ser::Serializer, Serialize}; use serde_json::Value; use std::sync::Arc; +use tracing::{instrument, trace}; // TODO: Add target name and versions @@ -466,6 +470,167 @@ pub(crate) async fn get_all_releases( Ok(res.into_response()) } +#[derive(Debug, Clone, PartialEq, Serialize)] +struct ShortMetadata { + name: String, + version_or_latest: String, + doc_targets: Vec, +} + +#[derive(Debug, Clone, PartialEq, Serialize)] +struct PlatformList { + metadata: ShortMetadata, + inner_path: String, + use_direct_platform_links: bool, + current_target: String, +} + +impl_axum_webpage! { + PlatformList = "rustdoc/platforms.html", + cpu_intensive_rendering = true, +} + +#[tracing::instrument] +pub(crate) async fn get_all_platforms( + Path(params): Path, + Extension(pool): Extension, + uri: Uri, +) -> AxumResult { + // since we directly use the Uri-path and not the extracted params from the router, + // we have to percent-decode the string here. + let original_path = percent_encoding::percent_decode(uri.path().as_bytes()) + .decode_utf8() + .map_err(|_| AxumNope::BadRequest)?; + let mut req_path: Vec<&str> = original_path.split('/').collect(); + + let release_found = match_version_axum(&pool, ¶ms.name, Some(¶ms.version)).await?; + trace!(?release_found, "found release"); + + // Remove the empty start, "releases", the name and the version from the path + req_path.drain(..4).for_each(drop); + + // Convenience function to allow for easy redirection + #[instrument] + fn redirect( + name: &str, + vers: &str, + path: &[&str], + cache_policy: CachePolicy, + ) -> AxumResult { + trace!("redirect"); + // Format and parse the redirect url + Ok(axum_cached_redirect( + encode_url_path(&format!("/platforms/{}/{}/{}", name, vers, path.join("/"))), + cache_policy, + )? + .into_response()) + } + + let (version, version_or_latest) = match release_found.version { + MatchSemver::Exact((version, _)) => { + // Redirect when the requested crate name isn't correct + if let Some(name) = release_found.corrected_name { + return redirect(&name, &version, &req_path, CachePolicy::NoCaching); + } + + (version.clone(), version) + } + + MatchSemver::Latest((version, _)) => { + // Redirect when the requested crate name isn't correct + if let Some(name) = release_found.corrected_name { + return redirect(&name, "latest", &req_path, CachePolicy::NoCaching); + } + + (version, "latest".to_string()) + } + + // Redirect when the requested version isn't correct + MatchSemver::Semver((v, _)) => { + // to prevent cloudfront caching the wrong artifacts on URLs with loose semver + // versions, redirect the browser to the returned version instead of loading it + // immediately + return redirect(¶ms.name, &v, &req_path, CachePolicy::ForeverInCdn); + } + }; + + let (name, doc_targets, releases, default_target): (String, Vec, Vec, String) = + spawn_blocking({ + let pool = pool.clone(); + move || { + let mut conn = pool.get()?; + let query = " + SELECT + crates.id, + crates.name, + releases.default_target, + releases.doc_targets + FROM releases + INNER JOIN crates ON releases.crate_id = crates.id + WHERE crates.name = $1 AND releases.version = $2;"; + + let rows = conn.query(query, &[¶ms.name, &version])?; + + let krate = if rows.is_empty() { + return Err(AxumNope::CrateNotFound.into()); + } else { + &rows[0] + }; + + // get releases, sorted by semver + let releases = releases_for_crate(&mut *conn, krate.get("id"))?; + + Ok(( + krate.get("name"), + MetaData::parse_doc_targets(krate.get("doc_targets")), + releases, + krate.get("default_target"), + )) + } + }) + .await?; + + let latest_release = releases + .iter() + .find(|release| release.version.pre.is_empty() && !release.yanked) + .unwrap_or(&releases[0]); + + // The path within this crate version's rustdoc output + let (target, inner_path) = { + let mut inner_path = req_path.clone(); + + let target = if inner_path.len() > 1 && doc_targets.iter().any(|s| s == inner_path[0]) { + inner_path.remove(0) + } else { + "" + }; + + (target, inner_path.join("/")) + }; + + let current_target = if latest_release.build_status { + if target.is_empty() { + default_target + } else { + target.to_owned() + } + } else { + String::new() + }; + + let res = PlatformList { + metadata: ShortMetadata { + name, + version_or_latest: version_or_latest.to_string(), + doc_targets, + }, + inner_path, + use_direct_platform_links: true, + current_target, + }; + Ok(res.into_response()) +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/web/routes.rs b/src/web/routes.rs index 7577cbf57..47a533190 100644 --- a/src/web/routes.rs +++ b/src/web/routes.rs @@ -181,7 +181,15 @@ pub(super) fn build_axum_routes() -> AxumRouter { get_internal(super::crate_details::crate_details_handler), ) .route( - "/:name/releases", + "/platforms/:name/:version/:target/", + get_internal(super::crate_details::get_all_platforms), + ) + .route( + "/platforms/:name/:version/:target/*path", + get_internal(super::crate_details::get_all_platforms), + ) + .route( + "/releases/list/:name", get_internal(super::crate_details::get_all_releases), ) .route_with_tsr( diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index d3e1d4438..f4f18840e 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -349,17 +349,17 @@ impl RustdocPage { } } -#[derive(Clone, Deserialize)] +#[derive(Clone, Deserialize, Debug)] pub(crate) struct RustdocHtmlParams { - name: String, - version: String, + pub(crate) name: String, + pub(crate) version: String, // both target and path are only used for matching the route. // The actual path is read from the request `Uri` because // we have some static filenames directly in the routes. #[allow(dead_code)] - target: Option, + pub(crate) target: Option, #[allow(dead_code)] - path: Option, + pub(crate) path: Option, } /// Serves documentation generated by rustdoc. diff --git a/static/menu.js b/static/menu.js index 8f0c8dc24..86119b253 100644 --- a/static/menu.js +++ b/static/menu.js @@ -5,13 +5,17 @@ const updateMenuPositionForSubMenu = (currentMenuSupplier) => { subMenu?.style.setProperty('--menu-x', `${currentMenu.getBoundingClientRect().x}px`); } -function generateReleaseList(data, crateName) { -} +const loadedMenus = new Set(); -let loadReleases = function() { - const releaseListElem = document.getElementById('releases-list'); - // To prevent reloading the list unnecessarily. - loadReleases = function() {}; +function loadReleases(menu, id, msg, path, extra) { + if (loadedMenus.has(id)) { + return; + } + loadedMenus.add(id); + if (!menu.querySelector(".rotate")) { + return; + } + const releaseListElem = document.getElementById(id); if (!releaseListElem) { // We're not in a documentation page, so no need to do anything. return; @@ -25,11 +29,11 @@ let loadReleases = function() { if (xhttp.status === 200) { releaseListElem.innerHTML = xhttp.responseText; } else { - console.error(`Failed to load release list: [${xhttp.status}] ${xhttp.responseText}`); - document.getElementById('releases-list').innerHTML = "Failed to load release list"; + console.error(`Failed to load ${msg}: [${xhttp.status}] ${xhttp.responseText}`); + document.getElementById(id).innerHTML = `Failed to load ${msg}`; } }; - xhttp.open("GET", `/${crateName}/releases`, true); + xhttp.open("GET", `/${path}/${crateName}${extra}`, true); xhttp.send(); }; @@ -81,7 +85,18 @@ let loadReleases = function() { currentMenu = newMenu; newMenu.className += " pure-menu-active"; backdrop.style.display = "block"; - loadReleases(); + if (newMenu.querySelector("#releases-list")) { + loadReleases(newMenu, "releases-list", "release list", "releases/list", ""); + } else if (newMenu.querySelector("#platforms")) { + loadReleases( + newMenu, + "platforms", + "platforms list", + "platforms", + // We get everything except the first crate name. + "/" + window.location.pathname.split("/").slice(2).join("/") + ); + } } function menuOnClick(e) { if (this.getAttribute("href") != "#") { diff --git a/templates/rustdoc/platforms.html b/templates/rustdoc/platforms.html new file mode 100644 index 000000000..67e0cb167 --- /dev/null +++ b/templates/rustdoc/platforms.html @@ -0,0 +1,25 @@ +{%- for target in metadata.doc_targets -%} + {# + The crate-detail page is the only page where we want to allow google to follow + the target-links. On that page we also don't have to use `/target-redirect/` + because the documentation root page is guaranteed to exist for all targets. + #} + {%- if use_direct_platform_links -%} + {%- set target_url = "/" ~ metadata.name ~ "/" ~ metadata.version_or_latest ~ "/" ~ target ~ "/" ~ inner_path -%} + {%- set target_no_follow = "" -%} + {%- else -%} + {%- set target_url = "/crate/" ~ metadata.name ~ "/" ~ metadata.version_or_latest ~ "/target-redirect/" ~ target ~ "/" ~ inner_path -%} + {%- set target_no_follow = "nofollow" -%} + {%- endif -%} + {%- if current_target is defined and current_target == target -%} + {%- set current = " current" -%} + {%- else -%} + {%- set current = "" -%} + {%- endif -%} + +
  • + + {{- target -}} + +
    • +{%- endfor -%} diff --git a/templates/rustdoc/topbar.html b/templates/rustdoc/topbar.html index 004c4c834..98dc8227a 100644 --- a/templates/rustdoc/topbar.html +++ b/templates/rustdoc/topbar.html @@ -209,31 +209,11 @@ {# Build the dropdown list showing available targets #}
      - {%- for target in metadata.doc_targets -%} - {# - The crate-detail page is the only page where we want to allow google to follow - the target-links. On that page we also don't have to use `/target-redirect/` - because the documentation root page is guaranteed to exist for all targets. - #} - {%- if use_direct_platform_links -%} - {%- set target_url = "/" ~ metadata.name ~ "/" ~ metadata.version_or_latest ~ "/" ~ target ~ "/" ~ inner_path -%} - {%- set target_no_follow = "" -%} - {%- else -%} - {%- set target_url = "/crate/" ~ metadata.name ~ "/" ~ metadata.version_or_latest ~ "/target-redirect/" ~ target ~ "/" ~ inner_path -%} - {%- set target_no_follow = "nofollow" -%} - {%- endif -%} - {%- if current_target is defined and current_target == target -%} - {%- set current = " current" -%} - {%- else -%} - {%- set current = "" -%} - {%- endif -%} - -
    • - - {{- target -}} - -
      • - {%- endfor -%} + {%- if metadata.doc_targets|length < 6 -%} + {%- include "rustdoc/targets.tml" -%} + {%- else -%} + {{ "spinner" | fas }} + {%- endif -%}
    {# Display the features available in current build From 0e30925695f684ce2cd9d8dd82e06671881ceb1f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 8 Aug 2023 20:07:36 +0200 Subject: [PATCH 02/14] Create DEFAULT_MAX_TARGETS constant --- src/docbuilder/limits.rs | 2 +- src/lib.rs | 3 +++ src/web/page/web_page.rs | 8 ++++++-- src/web/rustdoc.rs | 3 ++- templates/rustdoc/topbar.html | 4 ++-- 5 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/docbuilder/limits.rs b/src/docbuilder/limits.rs index 028bdf192..b123c5238 100644 --- a/src/docbuilder/limits.rs +++ b/src/docbuilder/limits.rs @@ -18,7 +18,7 @@ impl Default for Limits { Self { memory: 3 * 1024 * 1024 * 1024, // 3 GB timeout: Duration::from_secs(15 * 60), // 15 minutes - targets: 10, + targets: crate::DEFAULT_MAX_TARGETS, networking: false, max_log_size: 100 * 1024, // 100 KB } diff --git a/src/lib.rs b/src/lib.rs index d53fd32b3..c046fdb9f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -62,3 +62,6 @@ pub const BUILD_VERSION: &str = concat!( /// Example: /// `s3://rust-docs-rs//rustdoc-static/something.css` pub const RUSTDOC_STATIC_STORAGE_PREFIX: &str = "/rustdoc-static/"; + +/// Maximum number of targets allowed for a crate to be documented on. +pub const DEFAULT_MAX_TARGETS: usize = 10; diff --git a/src/web/page/web_page.rs b/src/web/page/web_page.rs index 511a6a4f5..9885d1ead 100644 --- a/src/web/page/web_page.rs +++ b/src/web/page/web_page.rs @@ -93,8 +93,12 @@ macro_rules! impl_axum_webpage { response.extensions_mut().insert($crate::web::page::web_page::DelayedTemplateRender { - context: ::tera::Context::from_serialize(&self) - .expect("could not create tera context from web-page"), + context: { + let mut c = ::tera::Context::from_serialize(&self) + .expect("could not create tera context from web-page"); + c.insert("DEFAULT_MAX_TARGETS", &$crate::DEFAULT_MAX_TARGETS); + c + }, template: { let template: fn(&Self) -> ::std::borrow::Cow<'static, str> = $template; template(&self).to_string() diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index f4f18840e..31ea51f9e 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -317,7 +317,8 @@ impl RustdocPage { let is_latest_url = self.is_latest_url; // Build the page of documentation - let ctx = tera::Context::from_serialize(self).context("error creating tera context")?; + let mut ctx = tera::Context::from_serialize(self).context("error creating tera context")?; + ctx.insert("DEFAULT_MAX_TARGETS", &crate::DEFAULT_MAX_TARGETS); // Extract the head and body of the rustdoc file so that we can insert it into our own html // while logging OOM errors from html rewriting diff --git a/templates/rustdoc/topbar.html b/templates/rustdoc/topbar.html index 98dc8227a..10e9fb4da 100644 --- a/templates/rustdoc/topbar.html +++ b/templates/rustdoc/topbar.html @@ -209,8 +209,8 @@ {# Build the dropdown list showing available targets #}
      - {%- if metadata.doc_targets|length < 6 -%} - {%- include "rustdoc/targets.tml" -%} + {%- if metadata.doc_targets|length < DEFAULT_MAX_TARGETS -%} + {%- include "rustdoc/platforms.html" -%} {%- else -%} {{ "spinner" | fas }} {%- endif -%} From b951f77c653d59ad49e1f74bc45dbbab4722646b Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 8 Aug 2023 20:14:47 +0200 Subject: [PATCH 03/14] Improve code readability --- static/menu.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/static/menu.js b/static/menu.js index 86119b253..671d5ba24 100644 --- a/static/menu.js +++ b/static/menu.js @@ -7,7 +7,7 @@ const updateMenuPositionForSubMenu = (currentMenuSupplier) => { const loadedMenus = new Set(); -function loadReleases(menu, id, msg, path, extra) { +function loadAjaxMenu(menu, id, msg, path, extra) { if (loadedMenus.has(id)) { return; } @@ -86,9 +86,9 @@ function loadReleases(menu, id, msg, path, extra) { newMenu.className += " pure-menu-active"; backdrop.style.display = "block"; if (newMenu.querySelector("#releases-list")) { - loadReleases(newMenu, "releases-list", "release list", "releases/list", ""); + loadAjaxMenu(newMenu, "releases-list", "release list", "releases/list", ""); } else if (newMenu.querySelector("#platforms")) { - loadReleases( + loadAjaxMenu( newMenu, "platforms", "platforms list", From 43cd5804e165a2ef6899a737e6b4a048937e68c1 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 8 Aug 2023 20:18:47 +0200 Subject: [PATCH 04/14] Improve routes for AJAX resources --- src/web/routes.rs | 28 ++++++++++++++++------------ static/menu.js | 4 ++-- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/web/routes.rs b/src/web/routes.rs index 47a533190..cab30605b 100644 --- a/src/web/routes.rs +++ b/src/web/routes.rs @@ -180,18 +180,6 @@ pub(super) fn build_axum_routes() -> AxumRouter { "/crate/:name", get_internal(super::crate_details::crate_details_handler), ) - .route( - "/platforms/:name/:version/:target/", - get_internal(super::crate_details::get_all_platforms), - ) - .route( - "/platforms/:name/:version/:target/*path", - get_internal(super::crate_details::get_all_platforms), - ) - .route( - "/releases/list/:name", - get_internal(super::crate_details::get_all_releases), - ) .route_with_tsr( "/crate/:name/:version", get_internal(super::crate_details::crate_details_handler), @@ -248,6 +236,22 @@ pub(super) fn build_axum_routes() -> AxumRouter { "/crate/:name/:version/source/*path", get_internal(super::source::source_browser_handler), ) + .route( + "/menus/platforms/:name/:version/:target", + get_internal(super::crate_details::get_all_platforms), + ) + .route( + "/menus/platforms/:name/:version/:target/", + get_internal(super::crate_details::get_all_platforms), + ) + .route( + "/menus/platforms/:name/:version/:target/*path", + get_internal(super::crate_details::get_all_platforms), + ) + .route( + "/menus/releases/:name", + get_internal(super::crate_details::get_all_releases), + ) .route( "/-/rustdoc.static/*path", get_internal(super::rustdoc::static_asset_handler), diff --git a/static/menu.js b/static/menu.js index 671d5ba24..3d532918c 100644 --- a/static/menu.js +++ b/static/menu.js @@ -33,7 +33,7 @@ function loadAjaxMenu(menu, id, msg, path, extra) { document.getElementById(id).innerHTML = `Failed to load ${msg}`; } }; - xhttp.open("GET", `/${path}/${crateName}${extra}`, true); + xhttp.open("GET", `/menus/${path}/${crateName}${extra}`, true); xhttp.send(); }; @@ -86,7 +86,7 @@ function loadAjaxMenu(menu, id, msg, path, extra) { newMenu.className += " pure-menu-active"; backdrop.style.display = "block"; if (newMenu.querySelector("#releases-list")) { - loadAjaxMenu(newMenu, "releases-list", "release list", "releases/list", ""); + loadAjaxMenu(newMenu, "releases-list", "release list", "releases", ""); } else if (newMenu.querySelector("#platforms")) { loadAjaxMenu( newMenu, From 127f936f13bb8b059083552e02ca12b151bf14cb Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 8 Aug 2023 20:44:46 +0200 Subject: [PATCH 05/14] Directly use `params.path` instead of using original URI --- src/web/crate_details.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index 27fc862a4..942fde551 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -496,19 +496,12 @@ pub(crate) async fn get_all_platforms( Extension(pool): Extension, uri: Uri, ) -> AxumResult { - // since we directly use the Uri-path and not the extracted params from the router, - // we have to percent-decode the string here. - let original_path = percent_encoding::percent_decode(uri.path().as_bytes()) - .decode_utf8() - .map_err(|_| AxumNope::BadRequest)?; - let mut req_path: Vec<&str> = original_path.split('/').collect(); + let req_path: String = params.path.unwrap_or_default(); + let req_path: Vec<&str> = req_path.split('/').collect(); let release_found = match_version_axum(&pool, ¶ms.name, Some(¶ms.version)).await?; trace!(?release_found, "found release"); - // Remove the empty start, "releases", the name and the version from the path - req_path.drain(..4).for_each(drop); - // Convenience function to allow for easy redirection #[instrument] fn redirect( From f2e302dda20d5b1862358d15040f3a80c5e302e8 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 8 Aug 2023 20:58:40 +0200 Subject: [PATCH 06/14] Fix failing tests --- src/web/crate_details.rs | 7 +++++-- src/web/rustdoc.rs | 8 ++++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index 942fde551..b10643cf8 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -1245,12 +1245,15 @@ mod tests { .add_target("x86_64-pc-windows-msvc") .create()?; - let response = env.frontend().get("/crate/dummy/0.4.0").send()?; + let response = env + .frontend() + .get("/menus/platforms/dummy/0.4.0/x86_64-pc-windows-msvc") + .send()?; assert!(response.status().is_success()); let platform_links: Vec<(String, String)> = kuchikiki::parse_html() .one(response.text()?) - .select(r#"a[aria-label="Platform"] + ul li a"#) + .select(r#"li a"#) .expect("invalid selector") .map(|el| { let attributes = el.attributes.borrow(); diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index 31ea51f9e..8b5450d5d 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -2259,8 +2259,12 @@ mod test { .create()?; // test rustdoc pages stay on the documentation - let page = kuchikiki::parse_html() - .one(env.frontend().get("/hexponent/releases").send()?.text()?); + let page = kuchikiki::parse_html().one( + env.frontend() + .get("/menus/releases/hexponent") + .send()? + .text()?, + ); let selector = r#"ul > li a[href="/crate/hexponent/0.3.1/target-redirect/hexponent/index.html"]"# .to_string(); From 75541b6c5a2ab724a1d430f2554a091fdd3f604f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 8 Aug 2023 22:37:04 +0200 Subject: [PATCH 07/14] Add test for target list --- src/web/crate_details.rs | 42 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index b10643cf8..cf21ea04d 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -1274,6 +1274,48 @@ mod tests { }); } + // Ensure that if there are more than a given number of targets, it will not generate them in + // the HTML directly (they will be loaded by AJAX if the user opens the menu). + #[test] + #[allow(clippy::assertions_on_constants)] + fn platform_menu_ajax() { + assert!(crate::DEFAULT_MAX_TARGETS > 2); + + fn check_count(nb_targets: usize, expected: usize) { + wrapper(|env| { + let mut rel = env + .fake_release() + .name("dummy") + .version("0.4.0") + .rustdoc_file("dummy/index.html") + .rustdoc_file("x86_64-pc-windows-msvc/dummy/index.html") + .default_target("x86_64-unknown-linux-gnu"); + + for nb in 0..nb_targets - 1 { + rel = rel.add_target(&format!("x86_64-pc-windows-msvc{nb}")); + } + rel.create()?; + + let response = env.frontend().get("/crate/dummy/0.4.0").send()?; + assert!(response.status().is_success()); + + let nb_li = kuchikiki::parse_html() + .one(response.text()?) + .select(r#"#platforms li a"#) + .expect("invalid selector") + .count(); + assert_eq!(nb_li, expected); + Ok(()) + }); + } + + // First we check that with 2 releases, the platforms list should be in the HTML. + check_count(2, 2); + // Then we check the same thing but with number of targets equal + // to `DEFAULT_MAX_TARGETS`. + check_count(crate::DEFAULT_MAX_TARGETS, 0); + } + #[test] fn latest_url() { wrapper(|env| { From 26cfbea7e8749efd8af4d6012240e59f01ad2554 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 9 Aug 2023 13:56:20 +0200 Subject: [PATCH 08/14] Remove blacklisting for URIs starting with `/-/` and move `/menus/` routes to `/-/menus/` --- src/web/crate_details.rs | 2 +- src/web/routes.rs | 8 ++++---- src/web/rustdoc.rs | 2 +- static/menu.js | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index cf21ea04d..019db1725 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -1247,7 +1247,7 @@ mod tests { let response = env .frontend() - .get("/menus/platforms/dummy/0.4.0/x86_64-pc-windows-msvc") + .get("/-/menus/platforms/dummy/0.4.0/x86_64-pc-windows-msvc") .send()?; assert!(response.status().is_success()); diff --git a/src/web/routes.rs b/src/web/routes.rs index cab30605b..d7230e1c1 100644 --- a/src/web/routes.rs +++ b/src/web/routes.rs @@ -237,19 +237,19 @@ pub(super) fn build_axum_routes() -> AxumRouter { get_internal(super::source::source_browser_handler), ) .route( - "/menus/platforms/:name/:version/:target", + "/-/menus/platforms/:name/:version/:target", get_internal(super::crate_details::get_all_platforms), ) .route( - "/menus/platforms/:name/:version/:target/", + "/-/menus/platforms/:name/:version/:target/", get_internal(super::crate_details::get_all_platforms), ) .route( - "/menus/platforms/:name/:version/:target/*path", + "/-/menus/platforms/:name/:version/:target/*path", get_internal(super::crate_details::get_all_platforms), ) .route( - "/menus/releases/:name", + "/-/menus/releases/:name", get_internal(super::crate_details::get_all_releases), ) .route( diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index 8b5450d5d..5c0e158e1 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -2261,7 +2261,7 @@ mod test { // test rustdoc pages stay on the documentation let page = kuchikiki::parse_html().one( env.frontend() - .get("/menus/releases/hexponent") + .get("/-/menus/releases/hexponent") .send()? .text()?, ); diff --git a/static/menu.js b/static/menu.js index 3d532918c..686f75bee 100644 --- a/static/menu.js +++ b/static/menu.js @@ -33,7 +33,7 @@ function loadAjaxMenu(menu, id, msg, path, extra) { document.getElementById(id).innerHTML = `Failed to load ${msg}`; } }; - xhttp.open("GET", `/menus/${path}/${crateName}${extra}`, true); + xhttp.open("GET", `/-/menus/${path}/${crateName}${extra}`, true); xhttp.send(); }; From 237a3ab167b32c3bf481be2b0257d1f28c608219 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 9 Aug 2023 17:07:40 +0200 Subject: [PATCH 09/14] Correctly set `use_direct_platform_links` field value depending if it is a crate root or not --- src/web/crate_details.rs | 87 +++++++++++++++++++++++++++++++--------- 1 file changed, 68 insertions(+), 19 deletions(-) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index 019db1725..e9bd4a115 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -496,6 +496,11 @@ pub(crate) async fn get_all_platforms( Extension(pool): Extension, uri: Uri, ) -> AxumResult { + let is_crate_root = params + .path + .as_ref() + .map(|path| path == "index.html") + .unwrap_or(true); let req_path: String = params.path.unwrap_or_default(); let req_path: Vec<&str> = req_path.split('/').collect(); @@ -618,7 +623,7 @@ pub(crate) async fn get_all_platforms( doc_targets, }, inner_path, - use_direct_platform_links: true, + use_direct_platform_links: is_crate_root, current_target, }; Ok(res.into_response()) @@ -1235,40 +1240,84 @@ mod tests { #[test] fn platform_links_are_direct_and_without_nofollow() { + fn check_links(response_text: String, ajax: bool, should_contain_redirect: bool) { + let platform_links: Vec<(String, String)> = kuchikiki::parse_html() + .one(response_text) + .select(&format!(r#"{}li a"#, if ajax { "" } else { "#platforms " })) + .expect("invalid selector") + .map(|el| { + let attributes = el.attributes.borrow(); + let url = attributes.get("href").expect("href").to_string(); + let rel = attributes.get("rel").unwrap_or("").to_string(); + (url, rel) + }) + .collect(); + + assert_eq!(platform_links.len(), 2); + + for (url, rel) in platform_links { + assert_eq!( + url.contains("/target-redirect/"), + should_contain_redirect, + "ajax: {ajax:?}, should_contain_redirect: {should_contain_redirect:?}", + ); + if !should_contain_redirect { + assert_eq!(rel, ""); + } else { + assert_eq!(rel, "nofollow"); + } + } + } + wrapper(|env| { env.fake_release() .name("dummy") .version("0.4.0") .rustdoc_file("dummy/index.html") .rustdoc_file("x86_64-pc-windows-msvc/dummy/index.html") + .rustdoc_file("x86_64-pc-windows-msvc/dummy/struct.A.html") .default_target("x86_64-unknown-linux-gnu") .add_target("x86_64-pc-windows-msvc") .create()?; + let response = env.frontend().get("/dummy/latest/dummy").send()?; + assert!(response.status().is_success()); + check_links(response.text()?, false, true); + // Same test with AJAX endpoint. let response = env .frontend() - .get("/-/menus/platforms/dummy/0.4.0/x86_64-pc-windows-msvc") + .get("/-/menus/platforms/dummy/latest/dummy") .send()?; assert!(response.status().is_success()); + check_links(response.text()?, true, false); - let platform_links: Vec<(String, String)> = kuchikiki::parse_html() - .one(response.text()?) - .select(r#"li a"#) - .expect("invalid selector") - .map(|el| { - let attributes = el.attributes.borrow(); - let url = attributes.get("href").expect("href").to_string(); - let rel = attributes.get("rel").unwrap_or("").to_string(); - (url, rel) - }) - .collect(); - - assert_eq!(platform_links.len(), 2); + let response = env + .frontend() + .get("/dummy/0.4.0/x86_64-pc-windows-msvc/dummy") + .send()?; + assert!(response.status().is_success()); + check_links(response.text()?, false, true); + // Same test with AJAX endpoint. + let response = env + .frontend() + .get("/-/menus/platforms/dummy/0.4.0/x86_64-pc-windows-msvc/dummy") + .send()?; + assert!(response.status().is_success()); + check_links(response.text()?, true, true); - for (url, rel) in platform_links { - assert!(!url.contains("/target-redirect/")); - assert_eq!(rel, ""); - } + let response = env + .frontend() + .get("/dummy/0.4.0/x86_64-pc-windows-msvc/dummy/struct.A.html") + .send()?; + assert!(response.status().is_success()); + check_links(response.text()?, false, true); + // Same test with AJAX endpoint. + let response = env + .frontend() + .get("/-/menus/platforms/dummy/0.4.0/x86_64-pc-windows-msvc/dummy/struct.A.html") + .send()?; + assert!(response.status().is_success()); + check_links(response.text()?, true, true); Ok(()) }); From f3d8e3e744f5a542a061cc24cf05cbe3d4e8b65b Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 19 Aug 2023 22:33:20 +0200 Subject: [PATCH 10/14] Add missing test for docs.rs crate page --- src/web/crate_details.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index e9bd4a115..1ae5bc33c 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -496,11 +496,7 @@ pub(crate) async fn get_all_platforms( Extension(pool): Extension, uri: Uri, ) -> AxumResult { - let is_crate_root = params - .path - .as_ref() - .map(|path| path == "index.html") - .unwrap_or(true); + let is_crate_root = uri.path().starts_with("/crate/"); let req_path: String = params.path.unwrap_or_default(); let req_path: Vec<&str> = req_path.split('/').collect(); @@ -1280,6 +1276,17 @@ mod tests { .add_target("x86_64-pc-windows-msvc") .create()?; + let response = env.frontend().get("/crate/dummy/0.4.0").send()?; + assert!(response.status().is_success()); + check_links(response.text()?, false, false); + // Same test with AJAX endpoint. + let response = env + .frontend() + .get("/-/menus/platforms/crate/dummy/0.4.0") + .send()?; + assert!(response.status().is_success()); + check_links(response.text()?, true, false); + let response = env.frontend().get("/dummy/latest/dummy").send()?; assert!(response.status().is_success()); check_links(response.text()?, false, true); @@ -1289,7 +1296,7 @@ mod tests { .get("/-/menus/platforms/dummy/latest/dummy") .send()?; assert!(response.status().is_success()); - check_links(response.text()?, true, false); + check_links(response.text()?, true, true); let response = env .frontend() From c3d8050a1c104dc2d02ff6ab7550e4ae027e515f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 25 Aug 2023 15:28:00 +0200 Subject: [PATCH 11/14] Allow platforms async load to work on docs.rs crates pages --- src/web/crate_details.rs | 11 ++++++++--- src/web/csp.rs | 5 ++++- src/web/routes.rs | 14 +++++++++++++- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index 1ae5bc33c..babdd1b2b 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -496,7 +496,7 @@ pub(crate) async fn get_all_platforms( Extension(pool): Extension, uri: Uri, ) -> AxumResult { - let is_crate_root = uri.path().starts_with("/crate/"); + let is_crate_root = uri.path().starts_with("/-/menus/platforms/crate/"); let req_path: String = params.path.unwrap_or_default(); let req_path: Vec<&str> = req_path.split('/').collect(); @@ -601,6 +601,11 @@ pub(crate) async fn get_all_platforms( (target, inner_path.join("/")) }; + let inner_path = if inner_path.is_empty() { + format!("{name}/index.html") + } else { + format!("{name}/{inner_path}") + }; let current_target = if latest_release.build_status { if target.is_empty() { @@ -1293,7 +1298,7 @@ mod tests { // Same test with AJAX endpoint. let response = env .frontend() - .get("/-/menus/platforms/dummy/latest/dummy") + .get("/-/menus/platforms/dummy/latest/dummy/") .send()?; assert!(response.status().is_success()); check_links(response.text()?, true, true); @@ -1307,7 +1312,7 @@ mod tests { // Same test with AJAX endpoint. let response = env .frontend() - .get("/-/menus/platforms/dummy/0.4.0/x86_64-pc-windows-msvc/dummy") + .get("/-/menus/platforms/dummy/0.4.0/x86_64-pc-windows-msvc/dummy/") .send()?; assert!(response.status().is_success()); check_links(response.text()?, true, true); diff --git a/src/web/csp.rs b/src/web/csp.rs index f21eb24c0..edfb19d9d 100644 --- a/src/web/csp.rs +++ b/src/web/csp.rs @@ -72,6 +72,9 @@ impl Csp { // Allow loading any font from the current origin. result.push_str("; font-src 'self'"); + // Allow XHR. + result.push_str("; connect-src 'self'"); + // Only allow scripts with the random nonce attached to them. // // We can't just allow 'self' here, as users can upload arbitrary .js files as part of @@ -190,7 +193,7 @@ mod tests { assert_eq!( Some(format!( "default-src 'none'; base-uri 'none'; img-src 'self' https:; \ - style-src 'self'; font-src 'self'; script-src 'nonce-{}'", + style-src 'self'; font-src 'self'; connect-src 'self'; script-src 'nonce-{}'", csp.nonce() )), csp.render(ContentType::Html) diff --git a/src/web/routes.rs b/src/web/routes.rs index d7230e1c1..01b1d8449 100644 --- a/src/web/routes.rs +++ b/src/web/routes.rs @@ -237,7 +237,19 @@ pub(super) fn build_axum_routes() -> AxumRouter { get_internal(super::source::source_browser_handler), ) .route( - "/-/menus/platforms/:name/:version/:target", + "/-/menus/platforms/crate/:name/:version", + get_internal(super::crate_details::get_all_platforms), + ) + .route( + "/-/menus/platforms/crate/:name/:version/:target", + get_internal(super::crate_details::get_all_platforms), + ) + .route( + "/-/menus/platforms/crate/:name/:version/:target/*path", + get_internal(super::crate_details::get_all_platforms), + ) + .route( + "/-/menus/platforms/:name/:version/", get_internal(super::crate_details::get_all_platforms), ) .route( From a8be4b0ea7311dc9569b1869fabebdc08340094b Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 25 Aug 2023 15:35:49 +0200 Subject: [PATCH 12/14] Improve `platform_links_are_direct_and_without_nofollow` readability --- src/web/crate_details.rs | 78 +++++++++++++++------------------------- 1 file changed, 29 insertions(+), 49 deletions(-) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index babdd1b2b..6fddba3a1 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -636,6 +636,7 @@ mod tests { use crate::index::api::CrateOwner; use crate::test::{ assert_cache_control, assert_redirect, assert_redirect_cached, wrapper, TestDatabase, + TestEnvironment, }; use anyhow::{Context, Error}; use kuchikiki::traits::TendrilSink; @@ -1270,6 +1271,25 @@ mod tests { } } + fn run_check_links( + env: &TestEnvironment, + url: &str, + extra: &str, + should_contain_redirect: bool, + ) { + let response = env.frontend().get(url).send().unwrap(); + assert!(response.status().is_success()); + check_links(response.text().unwrap(), false, should_contain_redirect); + // Same test with AJAX endpoint. + let response = env + .frontend() + .get(&format!("/-/menus/platforms{url}{extra}")) + .send() + .unwrap(); + assert!(response.status().is_success()); + check_links(response.text().unwrap(), true, should_contain_redirect); + } + wrapper(|env| { env.fake_release() .name("dummy") @@ -1281,55 +1301,15 @@ mod tests { .add_target("x86_64-pc-windows-msvc") .create()?; - let response = env.frontend().get("/crate/dummy/0.4.0").send()?; - assert!(response.status().is_success()); - check_links(response.text()?, false, false); - // Same test with AJAX endpoint. - let response = env - .frontend() - .get("/-/menus/platforms/crate/dummy/0.4.0") - .send()?; - assert!(response.status().is_success()); - check_links(response.text()?, true, false); - - let response = env.frontend().get("/dummy/latest/dummy").send()?; - assert!(response.status().is_success()); - check_links(response.text()?, false, true); - // Same test with AJAX endpoint. - let response = env - .frontend() - .get("/-/menus/platforms/dummy/latest/dummy/") - .send()?; - assert!(response.status().is_success()); - check_links(response.text()?, true, true); - - let response = env - .frontend() - .get("/dummy/0.4.0/x86_64-pc-windows-msvc/dummy") - .send()?; - assert!(response.status().is_success()); - check_links(response.text()?, false, true); - // Same test with AJAX endpoint. - let response = env - .frontend() - .get("/-/menus/platforms/dummy/0.4.0/x86_64-pc-windows-msvc/dummy/") - .send()?; - assert!(response.status().is_success()); - check_links(response.text()?, true, true); - - let response = env - .frontend() - .get("/dummy/0.4.0/x86_64-pc-windows-msvc/dummy/struct.A.html") - .send()?; - assert!(response.status().is_success()); - check_links(response.text()?, false, true); - // Same test with AJAX endpoint. - let response = env - .frontend() - .get("/-/menus/platforms/dummy/0.4.0/x86_64-pc-windows-msvc/dummy/struct.A.html") - .send()?; - assert!(response.status().is_success()); - check_links(response.text()?, true, true); + run_check_links(env, "/crate/dummy/0.4.0", "", false); + run_check_links(env, "/dummy/latest/dummy", "/", true); + run_check_links(env, "/dummy/0.4.0/x86_64-pc-windows-msvc/dummy", "/", true); + run_check_links( + env, + "/dummy/0.4.0/x86_64-pc-windows-msvc/dummy/struct.A.html", + "/", + true, + ); Ok(()) }); From 529aa3383e32ce68b32ff0ad641cbec477797823 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 26 Aug 2023 23:00:56 +0200 Subject: [PATCH 13/14] Fix crate pages handling target-redirect --- src/web/crate_details.rs | 86 ++++++++++++++++++++++++++++++++++------ src/web/routes.rs | 22 +++++++++- src/web/rustdoc.rs | 2 - 3 files changed, 94 insertions(+), 16 deletions(-) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index 6fddba3a1..992d62ad0 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -17,7 +17,6 @@ use crate::{ use anyhow::{Context, Result}; use axum::{ extract::{Extension, Path}, - http::Uri, response::{IntoResponse, Response as AxumResponse}, }; use chrono::{DateTime, Utc}; @@ -491,12 +490,11 @@ impl_axum_webpage! { } #[tracing::instrument] -pub(crate) async fn get_all_platforms( +pub(crate) async fn get_all_platforms_inner( Path(params): Path, Extension(pool): Extension, - uri: Uri, + is_crate_root: bool, ) -> AxumResult { - let is_crate_root = uri.path().starts_with("/-/menus/platforms/crate/"); let req_path: String = params.path.unwrap_or_default(); let req_path: Vec<&str> = req_path.split('/').collect(); @@ -590,16 +588,23 @@ pub(crate) async fn get_all_platforms( .unwrap_or(&releases[0]); // The path within this crate version's rustdoc output + 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| s == inner_path[0]) { - inner_path.remove(0) + 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 { "" }; - (target, inner_path.join("/")) + inner = inner_path.join("/"); + (target, inner.trim_end_matches('/')) }; let inner_path = if inner_path.is_empty() { format!("{name}/index.html") @@ -630,6 +635,21 @@ pub(crate) async fn get_all_platforms( Ok(res.into_response()) } +pub(crate) async fn get_all_platforms_root( + Path(mut params): Path, + pool: Extension, +) -> AxumResult { + params.path = None; + get_all_platforms_inner(Path(params), pool, true).await +} + +pub(crate) async fn get_all_platforms( + params: Path, + pool: Extension, +) -> AxumResult { + get_all_platforms_inner(params, pool, false).await +} + #[cfg(test)] mod tests { use super::*; @@ -1242,8 +1262,12 @@ mod tests { #[test] fn platform_links_are_direct_and_without_nofollow() { - fn check_links(response_text: String, ajax: bool, should_contain_redirect: bool) { - let platform_links: Vec<(String, String)> = kuchikiki::parse_html() + fn check_links( + response_text: String, + ajax: bool, + should_contain_redirect: bool, + ) -> Vec<(String, String, String)> { + let platform_links: Vec<(String, String, String)> = kuchikiki::parse_html() .one(response_text) .select(&format!(r#"{}li a"#, if ajax { "" } else { "#platforms " })) .expect("invalid selector") @@ -1251,13 +1275,13 @@ mod tests { let attributes = el.attributes.borrow(); let url = attributes.get("href").expect("href").to_string(); let rel = attributes.get("rel").unwrap_or("").to_string(); - (url, rel) + (el.text_contents(), url, rel) }) .collect(); assert_eq!(platform_links.len(), 2); - for (url, rel) in platform_links { + for (_, url, rel) in &platform_links { assert_eq!( url.contains("/target-redirect/"), should_contain_redirect, @@ -1269,6 +1293,7 @@ mod tests { assert_eq!(rel, "nofollow"); } } + platform_links } fn run_check_links( @@ -1276,10 +1301,26 @@ mod tests { url: &str, extra: &str, should_contain_redirect: bool, + ) { + run_check_links_redir( + env, + url, + extra, + should_contain_redirect, + should_contain_redirect, + ) + } + + fn run_check_links_redir( + env: &TestEnvironment, + url: &str, + extra: &str, + should_contain_redirect: bool, + ajax_should_contain_redirect: bool, ) { let response = env.frontend().get(url).send().unwrap(); assert!(response.status().is_success()); - check_links(response.text().unwrap(), false, should_contain_redirect); + let list1 = check_links(response.text().unwrap(), false, should_contain_redirect); // Same test with AJAX endpoint. let response = env .frontend() @@ -1287,7 +1328,18 @@ mod tests { .send() .unwrap(); assert!(response.status().is_success()); - check_links(response.text().unwrap(), true, should_contain_redirect); + let list2 = check_links(response.text().unwrap(), true, ajax_should_contain_redirect); + if should_contain_redirect == ajax_should_contain_redirect { + assert_eq!(list1, list2); + } else { + // If redirects differ, we only check platform names. + assert!( + list1.iter().zip(&list2).all(|(a, b)| a.0 == b.0), + "{:?} != {:?}", + list1, + list2, + ); + } } wrapper(|env| { @@ -1299,8 +1351,16 @@ mod tests { .rustdoc_file("x86_64-pc-windows-msvc/dummy/struct.A.html") .default_target("x86_64-unknown-linux-gnu") .add_target("x86_64-pc-windows-msvc") + .source_file("README.md", b"storage readme") .create()?; + // FIXME: For some reason, there are target-redirects on non-AJAX lists on docs.rs + // crate pages other than the "default" one. + run_check_links_redir(env, "/crate/dummy/0.4.0/features", "", true, false); + run_check_links_redir(env, "/crate/dummy/0.4.0/builds", "", true, false); + run_check_links_redir(env, "/crate/dummy/0.4.0/source/", "", true, false); + run_check_links_redir(env, "/crate/dummy/0.4.0/source/README.md", "", true, false); + run_check_links(env, "/crate/dummy/0.4.0", "", false); run_check_links(env, "/dummy/latest/dummy", "/", true); run_check_links(env, "/dummy/0.4.0/x86_64-pc-windows-msvc/dummy", "/", true); diff --git a/src/web/routes.rs b/src/web/routes.rs index 01b1d8449..a2ef1db5b 100644 --- a/src/web/routes.rs +++ b/src/web/routes.rs @@ -238,7 +238,27 @@ pub(super) fn build_axum_routes() -> AxumRouter { ) .route( "/-/menus/platforms/crate/:name/:version", - get_internal(super::crate_details::get_all_platforms), + get_internal(super::crate_details::get_all_platforms_root), + ) + .route( + "/-/menus/platforms/crate/:name/:version/features", + get_internal(super::crate_details::get_all_platforms_root), + ) + .route( + "/-/menus/platforms/crate/:name/:version/builds", + get_internal(super::crate_details::get_all_platforms_root), + ) + .route( + "/-/menus/platforms/crate/:name/:version/builds/*path", + get_internal(super::crate_details::get_all_platforms_root), + ) + .route( + "/-/menus/platforms/crate/:name/:version/source/", + get_internal(super::crate_details::get_all_platforms_root), + ) + .route( + "/-/menus/platforms/crate/:name/:version/source/*path", + get_internal(super::crate_details::get_all_platforms_root), ) .route( "/-/menus/platforms/crate/:name/:version/:target", diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index 5c0e158e1..19ecc7e15 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -357,9 +357,7 @@ pub(crate) struct RustdocHtmlParams { // both target and path are only used for matching the route. // The actual path is read from the request `Uri` because // we have some static filenames directly in the routes. - #[allow(dead_code)] pub(crate) target: Option, - #[allow(dead_code)] pub(crate) path: Option, } From 686fb37c9846508401c33fa5948b11055ffc1557 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 30 Aug 2023 14:47:45 +0200 Subject: [PATCH 14/14] Migrate menus URLs under `/crate/` --- src/web/crate_details.rs | 56 ++++++++++++++++++++++++++++++---------- src/web/routes.rs | 26 ++++++++----------- src/web/rustdoc.rs | 2 +- static/menu.js | 18 ++++++++++--- 4 files changed, 68 insertions(+), 34 deletions(-) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index 992d62ad0..66ed10843 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -1298,13 +1298,15 @@ mod tests { fn run_check_links( env: &TestEnvironment, - url: &str, + url_start: &str, + url_end: &str, extra: &str, should_contain_redirect: bool, ) { run_check_links_redir( env, - url, + url_start, + url_end, extra, should_contain_redirect, should_contain_redirect, @@ -1313,18 +1315,30 @@ mod tests { fn run_check_links_redir( env: &TestEnvironment, - url: &str, + url_start: &str, + url_end: &str, extra: &str, should_contain_redirect: bool, ajax_should_contain_redirect: bool, ) { - let response = env.frontend().get(url).send().unwrap(); + let response = env + .frontend() + .get(&format!("{url_start}{url_end}")) + .send() + .unwrap(); assert!(response.status().is_success()); let list1 = check_links(response.text().unwrap(), false, should_contain_redirect); // Same test with AJAX endpoint. + let (start, extra_name) = if url_start.starts_with("/crate/") { + ("", "/crate") + } else { + ("/crate", "") + }; let response = env .frontend() - .get(&format!("/-/menus/platforms{url}{extra}")) + .get(&format!( + "{start}{url_start}/menus/platforms{extra_name}{url_end}{extra}" + )) .send() .unwrap(); assert!(response.status().is_success()); @@ -1356,17 +1370,31 @@ mod tests { // FIXME: For some reason, there are target-redirects on non-AJAX lists on docs.rs // crate pages other than the "default" one. - run_check_links_redir(env, "/crate/dummy/0.4.0/features", "", true, false); - run_check_links_redir(env, "/crate/dummy/0.4.0/builds", "", true, false); - run_check_links_redir(env, "/crate/dummy/0.4.0/source/", "", true, false); - run_check_links_redir(env, "/crate/dummy/0.4.0/source/README.md", "", true, false); - - run_check_links(env, "/crate/dummy/0.4.0", "", false); - run_check_links(env, "/dummy/latest/dummy", "/", true); - run_check_links(env, "/dummy/0.4.0/x86_64-pc-windows-msvc/dummy", "/", true); + run_check_links_redir(env, "/crate/dummy/0.4.0", "/features", "", true, false); + run_check_links_redir(env, "/crate/dummy/0.4.0", "/builds", "", true, false); + run_check_links_redir(env, "/crate/dummy/0.4.0", "/source/", "", true, false); + run_check_links_redir( + env, + "/crate/dummy/0.4.0", + "/source/README.md", + "", + true, + false, + ); + + run_check_links(env, "/crate/dummy/0.4.0", "", "/", false); + run_check_links(env, "/dummy/latest", "/dummy", "/", true); + run_check_links( + env, + "/dummy/0.4.0", + "/x86_64-pc-windows-msvc/dummy", + "/", + true, + ); run_check_links( env, - "/dummy/0.4.0/x86_64-pc-windows-msvc/dummy/struct.A.html", + "/dummy/0.4.0", + "/x86_64-pc-windows-msvc/dummy/struct.A.html", "/", true, ); diff --git a/src/web/routes.rs b/src/web/routes.rs index a2ef1db5b..a5b4485c1 100644 --- a/src/web/routes.rs +++ b/src/web/routes.rs @@ -237,51 +237,47 @@ pub(super) fn build_axum_routes() -> AxumRouter { get_internal(super::source::source_browser_handler), ) .route( - "/-/menus/platforms/crate/:name/:version", + "/crate/:name/:version/menus/platforms/crate/", get_internal(super::crate_details::get_all_platforms_root), ) .route( - "/-/menus/platforms/crate/:name/:version/features", + "/crate/:name/:version/menus/platforms/crate/features", get_internal(super::crate_details::get_all_platforms_root), ) .route( - "/-/menus/platforms/crate/:name/:version/builds", + "/crate/:name/:version/menus/platforms/crate/builds", get_internal(super::crate_details::get_all_platforms_root), ) .route( - "/-/menus/platforms/crate/:name/:version/builds/*path", + "/crate/:name/:version/menus/platforms/crate/builds/*path", get_internal(super::crate_details::get_all_platforms_root), ) .route( - "/-/menus/platforms/crate/:name/:version/source/", + "/crate/:name/:version/menus/platforms/crate/source/", get_internal(super::crate_details::get_all_platforms_root), ) .route( - "/-/menus/platforms/crate/:name/:version/source/*path", + "/crate/:name/:version/menus/platforms/crate/source/*path", get_internal(super::crate_details::get_all_platforms_root), ) .route( - "/-/menus/platforms/crate/:name/:version/:target", + "/crate/:name/:version/menus/platforms/:target", get_internal(super::crate_details::get_all_platforms), ) .route( - "/-/menus/platforms/crate/:name/:version/:target/*path", + "/crate/:name/:version/menus/platforms/:target/*path", get_internal(super::crate_details::get_all_platforms), ) .route( - "/-/menus/platforms/:name/:version/", + "/crate/:name/:version/menus/platforms/", get_internal(super::crate_details::get_all_platforms), ) .route( - "/-/menus/platforms/:name/:version/:target/", + "/crate/:name/:version/menus/platforms/:target/", get_internal(super::crate_details::get_all_platforms), ) .route( - "/-/menus/platforms/:name/:version/:target/*path", - get_internal(super::crate_details::get_all_platforms), - ) - .route( - "/-/menus/releases/:name", + "/crate/:name/:version/menus/releases", get_internal(super::crate_details::get_all_releases), ) .route( diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index 19ecc7e15..fc0174d64 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -2259,7 +2259,7 @@ mod test { // test rustdoc pages stay on the documentation let page = kuchikiki::parse_html().one( env.frontend() - .get("/-/menus/releases/hexponent") + .get("/crate/hexponent/0.3.1/menus/releases") .send()? .text()?, ); diff --git a/static/menu.js b/static/menu.js index 686f75bee..c84e0eb3c 100644 --- a/static/menu.js +++ b/static/menu.js @@ -20,7 +20,14 @@ function loadAjaxMenu(menu, id, msg, path, extra) { // We're not in a documentation page, so no need to do anything. return; } - const crateName = window.location.pathname.split('/')[1]; + const parts = window.location.pathname.split("/"); + let crateName = parts[1]; + let version = parts[2]; + if (crateName === "crate") { + crateName = parts[2]; + version = parts[3]; + path += "/crate"; + } const xhttp = new XMLHttpRequest(); xhttp.onreadystatechange = function() { if (xhttp.readyState !== XMLHttpRequest.DONE) { @@ -33,7 +40,8 @@ function loadAjaxMenu(menu, id, msg, path, extra) { document.getElementById(id).innerHTML = `Failed to load ${msg}`; } }; - xhttp.open("GET", `/-/menus/${path}/${crateName}${extra}`, true); + console.log(extra, path); + xhttp.open("GET", `/crate/${crateName}/${version}/menus/${path}${extra}`, true); xhttp.send(); }; @@ -88,13 +96,15 @@ function loadAjaxMenu(menu, id, msg, path, extra) { if (newMenu.querySelector("#releases-list")) { loadAjaxMenu(newMenu, "releases-list", "release list", "releases", ""); } else if (newMenu.querySelector("#platforms")) { + const parts = window.location.pathname.split("/"); + const startFrom = parts[1] === "crate" ? 4 : 3; loadAjaxMenu( newMenu, "platforms", "platforms list", "platforms", - // We get everything except the first crate name. - "/" + window.location.pathname.split("/").slice(2).join("/") + // We get everything except the first crate name and the version. + "/" + parts.slice(startFrom).join("/") ); } }