From 389ba81e7eceab06e79840e0094a7667dc229544 Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Fri, 15 Dec 2023 06:38:10 +0100 Subject: [PATCH 1/3] parse request version in extractor, refactor match_version --- Cargo.lock | 89 +++++- Cargo.toml | 3 +- clippy.toml | 3 + src/web/build_details.rs | 19 +- src/web/builds.rs | 64 ++-- src/web/crate_details.rs | 198 +++++------- src/web/error.rs | 39 ++- src/web/extractors.rs | 19 ++ src/web/features.rs | 47 ++- src/web/mod.rs | 380 ++++++++++++++++------- src/web/releases.rs | 47 +-- src/web/rustdoc.rs | 215 +++++++------ src/web/sitemap.rs | 8 +- src/web/source.rs | 111 ++++--- src/web/status.rs | 68 ++-- templates/crate/features.html | 6 +- templates/header/package_navigation.html | 2 +- templates/rustdoc/platforms.html | 4 +- templates/rustdoc/topbar.html | 4 +- 19 files changed, 785 insertions(+), 541 deletions(-) create mode 100644 clippy.toml diff --git a/Cargo.lock b/Cargo.lock index 3ac65f6e0..ad3341c3e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -643,6 +643,7 @@ checksum = "1236b4b292f6c4d6dc34604bb5120d85c3fe1d1aa596bd5cc52ca054d13e7b9e" dependencies = [ "async-trait", "axum-core", + "axum-macros", "bytes", "futures-util", "http 1.0.0", @@ -712,6 +713,18 @@ dependencies = [ "tower-service", ] +[[package]] +name = "axum-macros" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "00c055ee2d014ae5981ce1016374e8213682aa14d9bf40e48ab48b5f3ef20eaa" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "syn 2.0.51", +] + [[package]] name = "backtrace" version = "0.3.69" @@ -1341,8 +1354,18 @@ version = "0.14.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7b750cb3417fd1b327431a470f388520309479ab0bf5e323505daf0290cd3850" dependencies = [ - "darling_core", - "darling_macro", + "darling_core 0.14.4", + "darling_macro 0.14.4", +] + +[[package]] +name = "darling" +version = "0.20.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0209d94da627ab5605dcccf08bb18afa5009cfbef48d8a8b7d7bdbc79be25c5e" +dependencies = [ + "darling_core 0.20.3", + "darling_macro 0.20.3", ] [[package]] @@ -1359,17 +1382,42 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "darling_core" +version = "0.20.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "177e3443818124b357d8e76f53be906d60937f0d3a90773a664fa63fa253e621" +dependencies = [ + "fnv", + "ident_case", + "proc-macro2", + "quote", + "strsim 0.10.0", + "syn 2.0.51", +] + [[package]] name = "darling_macro" version = "0.14.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a4aab4dbc9f7611d8b55048a3a16d2d010c2c8334e46304b40ac1cc14bf3b48e" dependencies = [ - "darling_core", + "darling_core 0.14.4", "quote", "syn 1.0.109", ] +[[package]] +name = "darling_macro" +version = "0.20.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "836a9bbc7ad63342d6d6e7b815ccab164bc77a2d95d84bc3117a8c0d5c98e2d5" +dependencies = [ + "darling_core 0.20.3", + "quote", + "syn 2.0.51", +] + [[package]] name = "dashmap" version = "5.5.3" @@ -1421,6 +1469,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b42b6fa04a440b495c8b04d0e71b707c585f83cb9cb28cf8cd0d976c315e31b4" dependencies = [ "powerfmt", + "serde", ] [[package]] @@ -1438,7 +1487,7 @@ version = "0.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c11bdc11a0c47bc7d37d582b5285da6849c96681023680b906673c5707af7b0f" dependencies = [ - "darling", + "darling 0.14.4", "proc-macro2", "quote", "syn 1.0.109", @@ -1561,6 +1610,7 @@ dependencies = [ "sentry-tracing", "serde", "serde_json", + "serde_with", "slug", "sqlx", "string_cache", @@ -3289,6 +3339,7 @@ checksum = "bd070e393353796e801d209ad339e89596eb4c8d430d18ede6a1cced8fafbd99" dependencies = [ "autocfg", "hashbrown 0.12.3", + "serde", ] [[package]] @@ -3299,6 +3350,7 @@ checksum = "233cf39063f058ea2caae4091bf4a3ef70a653afbc026f5c4a4135d114e3c177" dependencies = [ "equivalent", "hashbrown 0.14.3", + "serde", ] [[package]] @@ -5247,6 +5299,35 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_with" +version = "3.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "64cd236ccc1b7a29e7e2739f27c0b2dd199804abc4290e32f59f3b68d6405c23" +dependencies = [ + "base64 0.21.7", + "chrono", + "hex", + "indexmap 1.9.3", + "indexmap 2.2.3", + "serde", + "serde_json", + "serde_with_macros", + "time", +] + +[[package]] +name = "serde_with_macros" +version = "3.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93634eb5f75a2323b16de4748022ac4297f9e76b6dced2be287a099f41b5e788" +dependencies = [ + "darling 0.20.3", + "proc-macro2", + "quote", + "syn 2.0.51", +] + [[package]] name = "servo_arc" version = "0.1.1" diff --git a/Cargo.toml b/Cargo.toml index 72d766c2a..4b66cd0f1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -84,9 +84,10 @@ uuid = { version = "1.1.2", features = ["v4"]} # Data serialization and deserialization serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" +serde_with = "3.4.0" # axum dependencies -axum = "0.7.3" +axum = { version = "0.7.3", features = ["macros"] } axum-extra = { version = "0.9.1", features = ["typed-header"] } hyper = { version = "1.1.0", default-features = false } tower = "0.4.11" diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 000000000..ad8b2692d --- /dev/null +++ b/clippy.toml @@ -0,0 +1,3 @@ +disallowed-types = [ + { path = "axum::extract::Path", reason = "use our own custom web::extractors::Path for a nicer error response" }, +] diff --git a/src/web/build_details.rs b/src/web/build_details.rs index 737152535..2e8e54091 100644 --- a/src/web/build_details.rs +++ b/src/web/build_details.rs @@ -2,17 +2,14 @@ use crate::{ impl_axum_webpage, web::{ error::{AxumNope, AxumResult}, - extractors::DbConnection, + extractors::{DbConnection, Path}, file::File, - MetaData, + MetaData, ReqVersion, }, AsyncStorage, Config, }; use anyhow::Context as _; -use axum::{ - extract::{Extension, Path}, - response::IntoResponse, -}; +use axum::{extract::Extension, response::IntoResponse}; use chrono::{DateTime, Utc}; use serde::Serialize; use std::sync::Arc; @@ -39,12 +36,13 @@ impl_axum_webpage! { } pub(crate) async fn build_details_handler( - Path((name, version, id)): Path<(String, String, String)>, + Path((name, version, id)): Path<(String, ReqVersion, String)>, mut conn: DbConnection, Extension(config): Extension>, Extension(storage): Extension>, ) -> AxumResult { let id: i32 = id.parse().map_err(|_| AxumNope::BuildNotFound)?; + let version = version.assume_exact()?; let row = sqlx::query!( "SELECT @@ -60,7 +58,7 @@ pub(crate) async fn build_details_handler( WHERE builds.id = $1 AND crates.name = $2 AND releases.version = $3", id, name, - version, + version.to_string(), ) .fetch_optional(&mut *conn) .await? @@ -75,7 +73,7 @@ pub(crate) async fn build_details_handler( }; Ok(BuildDetailsPage { - metadata: MetaData::from_crate(&mut conn, &name, &version, &version).await?, + metadata: MetaData::from_crate(&mut conn, &name, &version, None).await?, build_details: BuildDetails { id, rustc_version: row.rustc_version, @@ -147,7 +145,8 @@ mod tests { let attrs = node.attributes.borrow(); let url = attrs.get("href").unwrap(); - let page = kuchikiki::parse_html().one(env.frontend().get(url).send()?.text()?); + let page = kuchikiki::parse_html() + .one(env.frontend().get(url).send()?.error_for_status()?.text()?); let log = page.select("pre").unwrap().next().unwrap().text_contents(); diff --git a/src/web/builds.rs b/src/web/builds.rs index 0c8e16b49..1f4621c0f 100644 --- a/src/web/builds.rs +++ b/src/web/builds.rs @@ -1,18 +1,20 @@ -use super::{cache::CachePolicy, headers::CanonicalUrl, MatchSemver}; +use super::{cache::CachePolicy, error::AxumNope, headers::CanonicalUrl}; use crate::{ docbuilder::Limits, impl_axum_webpage, - web::{error::AxumResult, extractors::DbConnection, match_version, MetaData}, + web::{ + error::AxumResult, + extractors::{DbConnection, Path}, + match_version, MetaData, ReqVersion, + }, Config, }; use anyhow::Result; use axum::{ - extract::{Extension, Path}, - http::header::ACCESS_CONTROL_ALLOW_ORIGIN, - response::IntoResponse, - Json, + extract::Extension, http::header::ACCESS_CONTROL_ALLOW_ORIGIN, response::IntoResponse, Json, }; use chrono::{DateTime, Utc}; +use semver::Version; use serde::Serialize; use std::sync::Arc; @@ -39,28 +41,23 @@ impl_axum_webpage! { } pub(crate) async fn build_list_handler( - Path((name, req_version)): Path<(String, String)>, + Path((name, req_version)): Path<(String, ReqVersion)>, mut conn: DbConnection, Extension(config): Extension>, ) -> AxumResult { - let (version, version_or_latest) = match match_version(&mut conn, &name, Some(&req_version)) + let version = match_version(&mut conn, &name, &req_version) .await? - .exact_name_only()? - { - MatchSemver::Exact((version, _)) => (version.clone(), version), - MatchSemver::Latest((version, _)) => (version, "latest".to_string()), - - MatchSemver::Semver((version, _)) => { - return Ok(super::axum_cached_redirect( - &format!("/crate/{name}/{version}/builds"), + .assume_exact_name()? + .into_canonical_req_version_or_else(|version| { + AxumNope::Redirect( + format!("/crate/{name}/{version}/builds"), CachePolicy::ForeverInCdn, - )? - .into_response()); - } - }; + ) + })? + .into_version(); Ok(BuildsPage { - metadata: MetaData::from_crate(&mut conn, &name, &version, &version_or_latest).await?, + metadata: MetaData::from_crate(&mut conn, &name, &version, Some(req_version)).await?, builds: get_builds(&mut conn, &name, &version).await?, limits: Limits::for_crate(&config, &mut conn, &name).await?, canonical_url: CanonicalUrl::from_path(format!("/crate/{name}/latest/builds")), @@ -70,22 +67,19 @@ pub(crate) async fn build_list_handler( } pub(crate) async fn build_list_json_handler( - Path((name, req_version)): Path<(String, String)>, + Path((name, req_version)): Path<(String, ReqVersion)>, mut conn: DbConnection, ) -> AxumResult { - let version = match match_version(&mut conn, &name, Some(&req_version)) + let version = match_version(&mut conn, &name, &req_version) .await? - .exact_name_only()? - { - MatchSemver::Exact((version, _)) | MatchSemver::Latest((version, _)) => version, - MatchSemver::Semver((version, _)) => { - return Ok(super::axum_cached_redirect( - &format!("/crate/{name}/{version}/builds.json"), + .assume_exact_name()? + .into_canonical_req_version_or_else(|version| { + AxumNope::Redirect( + format!("/crate/{name}/{version}/builds.json"), CachePolicy::ForeverInCdn, - )? - .into_response()); - } - }; + ) + })? + .into_version(); Ok(( Extension(CachePolicy::NoStoreMustRevalidate), @@ -98,7 +92,7 @@ pub(crate) async fn build_list_json_handler( async fn get_builds( conn: &mut sqlx::PgConnection, name: &str, - version: &str, + version: &Version, ) -> Result> { Ok(sqlx::query_as!( Build, @@ -114,7 +108,7 @@ async fn get_builds( WHERE crates.name = $1 AND releases.version = $2 ORDER BY id DESC", name, - version, + version.to_string(), ) .fetch_all(&mut *conn) .await?) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index 78cfae6ea..edb12480d 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -1,6 +1,5 @@ -use super::{markdown, match_version, MatchSemver, MetaData}; +use super::{markdown, match_version, MetaData}; use crate::utils::{get_correct_docsrs_style_file, report_error}; -use crate::web::axum_cached_redirect; use crate::web::rustdoc::RustdocHtmlParams; use crate::{ impl_axum_webpage, @@ -9,30 +8,31 @@ use crate::{ cache::CachePolicy, encode_url_path, error::{AxumNope, AxumResult}, - extractors::DbConnection, + extractors::{DbConnection, Path}, + ReqVersion, }, AsyncStorage, }; use anyhow::{anyhow, Context, Result}; use axum::{ - extract::{Extension, Path}, + extract::Extension, response::{IntoResponse, Response as AxumResponse}, }; use chrono::{DateTime, Utc}; use futures_util::stream::TryStreamExt; use log::warn; +use semver::Version; 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 #[derive(Debug, Clone, PartialEq, Serialize)] pub struct CrateDetails { name: String, - version: String, + version: Version, description: Option, owners: Vec<(String, String)>, dependencies: Option, @@ -100,8 +100,8 @@ impl CrateDetails { pub async fn new( conn: &mut sqlx::PgConnection, name: &str, - version: &str, - version_or_latest: &str, + version: &Version, + req_version: Option, ) -> Result, anyhow::Error> { let krate = match sqlx::query!( r#"SELECT @@ -153,7 +153,7 @@ impl CrateDetails { LEFT JOIN repositories ON releases.repository_id = repositories.id WHERE crates.name = $1 AND releases.version = $2;"#, name, - version + version.to_string(), ) .fetch_optional(&mut *conn) .await? @@ -174,8 +174,8 @@ impl CrateDetails { let metadata = MetaData { name: krate.name.clone(), - version: krate.version.clone(), - version_or_latest: version_or_latest.to_string(), + version: version.clone(), + req_version: req_version.unwrap_or_else(|| ReqVersion::Exact(version.clone())), description: krate.description.clone(), rustdoc_status: krate.rustdoc_status, target_name: Some(krate.target_name.clone()), @@ -187,7 +187,7 @@ impl CrateDetails { let mut crate_details = CrateDetails { name: krate.name, - version: krate.version, + version: version.clone(), description: krate.description, owners: Vec::new(), dependencies: krate.dependencies, @@ -248,7 +248,7 @@ impl CrateDetails { let manifest = match storage .fetch_source_file( &self.name, - &self.version, + &self.version.to_string(), self.latest_build_id.unwrap_or(0), "Cargo.toml", self.archive_storage, @@ -277,7 +277,7 @@ impl CrateDetails { match storage .fetch_source_file( &self.name, - &self.version, + &self.version.to_string(), self.latest_build_id.unwrap_or(0), path, self.archive_storage, @@ -382,7 +382,7 @@ impl_axum_webpage! { #[derive(Deserialize, Clone, Debug)] pub(crate) struct CrateDetailHandlerParams { name: String, - version: Option, + version: Option, } #[tracing::instrument(skip(conn, storage))] @@ -391,34 +391,28 @@ pub(crate) async fn crate_details_handler( Extension(storage): Extension>, mut conn: DbConnection, ) -> AxumResult { - // this handler must always called with a crate name - if params.version.is_none() { - return Ok(super::axum_cached_redirect( - encode_url_path(&format!("/crate/{}/latest", params.name)), + let req_version = params.version.ok_or_else(|| { + AxumNope::Redirect( + format!("/crate/{}/{}", ¶ms.name, ReqVersion::Latest), CachePolicy::ForeverInCdn, - )? - .into_response()); - } - - let found_version = match_version(&mut conn, ¶ms.name, params.version.as_deref()) - .await - .and_then(|m| m.exact_name_only())?; + ) + })?; - let (version, version_or_latest, is_latest_url) = match found_version { - MatchSemver::Exact((version, _)) => (version.clone(), version, false), - MatchSemver::Latest((version, _)) => (version, "latest".to_string(), true), - MatchSemver::Semver((version, _)) => { - return Ok(super::axum_cached_redirect( - &format!("/crate/{}/{}", ¶ms.name, version), + let version = match_version(&mut conn, ¶ms.name, &req_version) + .await? + .assume_exact_name()? + .into_canonical_req_version_or_else(|version| { + AxumNope::Redirect( + format!("/crate/{}/{}", ¶ms.name, version), CachePolicy::ForeverInCdn, - )? - .into_response()); - } - }; + ) + })? + .into_version(); - let mut details = CrateDetails::new(&mut conn, ¶ms.name, &version, &version_or_latest) - .await? - .ok_or(AxumNope::VersionNotFound)?; + let mut details = + CrateDetails::new(&mut conn, ¶ms.name, &version, Some(req_version.clone())) + .await? + .ok_or(AxumNope::VersionNotFound)?; match details.fetch_readme(&storage).await { Ok(readme) => details.readme = readme.or(details.readme), @@ -427,7 +421,7 @@ pub(crate) async fn crate_details_handler( let mut res = CrateDetailsPage { details }.into_response(); res.extensions_mut() - .insert::(if is_latest_url { + .insert::(if req_version.is_latest() { CachePolicy::ForeverInCdn } else { CachePolicy::ForeverInCdnAndStaleInBrowser @@ -457,14 +451,10 @@ pub(crate) async fn get_all_releases( let req_path: String = params.path.clone().unwrap_or_default(); let req_path: Vec<&str> = req_path.split('/').collect(); - let release_found = match_version(&mut conn, ¶ms.name, Some(¶ms.version)).await?; - trace!(?release_found, "found release"); - - let (version, _) = match release_found.version { - MatchSemver::Exact((version, _)) => (version.clone(), version), - MatchSemver::Latest((version, _)) => (version, "latest".to_string()), - MatchSemver::Semver(_) => return Err(AxumNope::VersionNotFound), - }; + let version = match_version(&mut conn, ¶ms.name, ¶ms.version) + .await? + .into_canonical_req_version_or_else(|_| AxumNope::VersionNotFound)? + .into_version(); let row = sqlx::query!( "SELECT @@ -475,7 +465,7 @@ pub(crate) async fn get_all_releases( INNER JOIN releases on crates.id = releases.crate_id WHERE crates.name = $1 and releases.version = $2;", params.name, - &version, + &version.to_string(), ) .fetch_optional(&mut *conn) .await? @@ -528,7 +518,8 @@ pub(crate) async fn get_all_releases( #[derive(Debug, Clone, PartialEq, Serialize)] struct ShortMetadata { name: String, - version_or_latest: String, + version: Version, + req_version: ReqVersion, doc_targets: Vec, } @@ -555,53 +546,31 @@ pub(crate) async fn get_all_platforms_inner( let req_path: String = params.path.unwrap_or_default(); let req_path: Vec<&str> = req_path.split('/').collect(); - let release_found = match_version(&mut conn, ¶ms.name, Some(¶ms.version)).await?; - trace!(?release_found, "found release"); - - // 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 version = match_version(&mut conn, ¶ms.name, ¶ms.version) + .await? + .into_exactly_named_or_else(|corrected_name, req_version| { + AxumNope::Redirect( + encode_url_path(&format!( + "/platforms/{}/{}/{}", + corrected_name, + req_version, + req_path.join("/") + )), + CachePolicy::NoCaching, + ) + })? + .into_canonical_req_version_or_else(|version| { + AxumNope::Redirect( + encode_url_path(&format!( + "/platforms/{}/{}/{}", + ¶ms.name, + version, + req_path.join("/") + )), + CachePolicy::ForeverInCdn, + ) + })? + .into_version(); let krate = sqlx::query!( "SELECT @@ -613,7 +582,7 @@ pub(crate) async fn get_all_platforms_inner( INNER JOIN crates ON releases.crate_id = crates.id WHERE crates.name = $1 AND releases.version = $2;", params.name, - version + version.to_string(), ) .fetch_optional(&mut *conn) .await? @@ -666,7 +635,8 @@ pub(crate) async fn get_all_platforms_inner( let res = PlatformList { metadata: ShortMetadata { name: krate.name, - version_or_latest: version_or_latest.to_string(), + version: version.clone(), + req_version: params.version.clone(), doc_targets, }, inner_path, @@ -701,6 +671,7 @@ mod tests { }; use anyhow::{Context, Error}; use kuchikiki::traits::TendrilSink; + use semver::Version; use std::collections::HashMap; async fn assert_last_successful_build_equals( @@ -710,10 +681,11 @@ mod tests { expected_last_successful_build: Option<&str>, ) -> Result<(), Error> { let mut conn = db.async_conn().await; - let details = CrateDetails::new(&mut conn, package, version, version) - .await - .with_context(|| anyhow::anyhow!("could not fetch crate details"))? - .unwrap(); + let details = + CrateDetails::new(&mut conn, package, &Version::parse(version).unwrap(), None) + .await + .with_context(|| anyhow::anyhow!("could not fetch crate details"))? + .unwrap(); assert_eq!( details.last_successful_build, @@ -878,7 +850,7 @@ mod tests { let details = env.runtime().block_on(async move { let mut conn = db.async_conn().await; - CrateDetails::new(&mut conn, "foo", "0.2.0", "0.2.0") + CrateDetails::new(&mut conn, "foo", &"0.2.0".parse().unwrap(), None) .await .unwrap() .unwrap() @@ -999,7 +971,7 @@ mod tests { for version in &["0.0.1", "0.0.2", "0.0.3"] { let details = env.runtime().block_on(async move { let mut conn = db.async_conn().await; - CrateDetails::new(&mut conn, "foo", version, version) + CrateDetails::new(&mut conn, "foo", &version.parse().unwrap(), None) .await .unwrap() .unwrap() @@ -1029,7 +1001,7 @@ mod tests { for version in &["0.0.1", "0.0.2", "0.0.3-pre.1"] { let details = env.runtime().block_on(async move { let mut conn = db.async_conn().await; - CrateDetails::new(&mut conn, "foo", version, version) + CrateDetails::new(&mut conn, "foo", &version.parse().unwrap(), None) .await .unwrap() .unwrap() @@ -1060,7 +1032,7 @@ mod tests { for version in &["0.0.1", "0.0.2", "0.0.3"] { let details = env.runtime().block_on(async move { let mut conn = db.async_conn().await; - CrateDetails::new(&mut conn, "foo", version, version) + CrateDetails::new(&mut conn, "foo", &(version.parse().unwrap()), None) .await .unwrap() .unwrap() @@ -1099,7 +1071,7 @@ mod tests { for version in &["0.0.1", "0.0.2", "0.0.3"] { let details = env.runtime().block_on(async move { let mut conn = db.async_conn().await; - CrateDetails::new(&mut conn, "foo", version, version) + CrateDetails::new(&mut conn, "foo", &version.parse().unwrap(), None) .await .unwrap() .unwrap() @@ -1159,7 +1131,7 @@ mod tests { let details = env.runtime().block_on(async move { let mut conn = db.async_conn().await; - CrateDetails::new(&mut conn, "foo", "0.0.1", "0.0.1") + CrateDetails::new(&mut conn, "foo", &"0.0.1".parse().unwrap(), None) .await .unwrap() .unwrap() @@ -1185,7 +1157,7 @@ mod tests { let details = env.runtime().block_on(async move { let mut conn = db.async_conn().await; - CrateDetails::new(&mut conn, "foo", "0.0.1", "0.0.1") + CrateDetails::new(&mut conn, "foo", &"0.0.1".parse().unwrap(), None) .await .unwrap() .unwrap() @@ -1212,7 +1184,7 @@ mod tests { let details = env.runtime().block_on(async move { let mut conn = db.async_conn().await; - CrateDetails::new(&mut conn, "foo", "0.0.1", "0.0.1") + CrateDetails::new(&mut conn, "foo", &"0.0.1".parse().unwrap(), None) .await .unwrap() .unwrap() @@ -1234,7 +1206,7 @@ mod tests { let details = env.runtime().block_on(async move { let mut conn = db.async_conn().await; - CrateDetails::new(&mut conn, "foo", "0.0.1", "0.0.1") + CrateDetails::new(&mut conn, "foo", &"0.0.1".parse().unwrap(), None) .await .unwrap() .unwrap() @@ -1681,7 +1653,7 @@ mod tests { let details = env.runtime().block_on(async move { let mut conn = env.async_db().await.async_conn().await; - CrateDetails::new(&mut conn, "dummy", "0.5.0", "0.5.0") + CrateDetails::new(&mut conn, "dummy", &"0.5.0".parse().unwrap(), None) .await .unwrap() .unwrap() diff --git a/src/web/error.rs b/src/web/error.rs index 51f73b437..7759f0522 100644 --- a/src/web/error.rs +++ b/src/web/error.rs @@ -1,7 +1,7 @@ use crate::{ db::PoolError, storage::PathNotFoundError, - web::{releases::Search, AxumErrorPage}, + web::{cache::CachePolicy, releases::Search, AxumErrorPage}, }; use anyhow::anyhow; use axum::{ @@ -11,7 +11,6 @@ use axum::{ use std::borrow::Cow; #[derive(Debug, thiserror::Error)] -#[allow(dead_code)] // FIXME: remove after iron is gone pub enum AxumNope { #[error("Requested resource not found")] ResourceNotFound, @@ -25,12 +24,12 @@ pub enum AxumNope { VersionNotFound, #[error("Search yielded no results")] NoResults, - #[error("Internal server error")] - InternalServerError, #[error("internal error")] InternalError(anyhow::Error), #[error("bad request")] - BadRequest, + BadRequest(anyhow::Error), + #[error("redirect")] + Redirect(String, CachePolicy), } impl IntoResponse for AxumNope { @@ -90,21 +89,12 @@ impl IntoResponse for AxumNope { } .into_response() } - AxumNope::BadRequest => AxumErrorPage { + AxumNope::BadRequest(source) => AxumErrorPage { title: "Bad request", - message: "Bad request".into(), + message: Cow::Owned(source.to_string()), status: StatusCode::BAD_REQUEST, } .into_response(), - AxumNope::InternalServerError => { - // something went wrong, details should have been logged - AxumErrorPage { - title: "Internal server error", - message: "internal server error".into(), - status: StatusCode::INTERNAL_SERVER_ERROR, - } - .into_response() - } AxumNope::InternalError(source) => { let web_error = crate::web::AxumErrorPage { title: "Internal Server Error", @@ -116,6 +106,12 @@ impl IntoResponse for AxumNope { web_error.into_response() } + AxumNope::Redirect(target, cache_policy) => { + match super::axum_cached_redirect(&target, cache_policy) { + Ok(response) => response.into_response(), + Err(err) => AxumNope::InternalError(err).into_response(), + } + } } } } @@ -198,11 +194,14 @@ mod tests { } #[test] - fn check_404_page_content_not_semver_version() { + fn check_400_page_content_not_semver_version() { wrapper(|env| { env.fake_release().name("dummy").create()?; - let page = kuchikiki::parse_html() - .one(env.frontend().get("/dummy/not-semver").send()?.text()?); + + let response = env.frontend().get("/dummy/not-semver").send()?; + assert_eq!(response.status(), 400); + + let page = kuchikiki::parse_html().one(response.text()?); assert_eq!(page.select("#crate-title").unwrap().count(), 1); assert_eq!( page.select("#crate-title") @@ -210,7 +209,7 @@ mod tests { .next() .unwrap() .text_contents(), - "The requested version does not exist", + "Bad request" ); Ok(()) diff --git a/src/web/extractors.rs b/src/web/extractors.rs index 360f24079..90552d1c0 100644 --- a/src/web/extractors.rs +++ b/src/web/extractors.rs @@ -51,4 +51,23 @@ impl DerefMut for DbConnection { } } +/// custom axum `Path` extractor that uses our own AxumNope::BadRequest +/// as error response instead of a plain text "bad request" +#[allow(clippy::disallowed_types)] +mod path_impl { + use super::*; + + #[derive(FromRequestParts)] + #[from_request(via(axum::extract::Path), rejection(AxumNope))] + pub(crate) struct Path(pub T); +} + +pub(crate) use path_impl::Path; + +impl From for AxumNope { + fn from(value: axum::extract::rejection::PathRejection) -> Self { + AxumNope::BadRequest(value.into()) + } +} + // TODO: we will write tests for this when async db tests are working diff --git a/src/web/features.rs b/src/web/features.rs index cb118f00d..c63e75dc2 100644 --- a/src/web/features.rs +++ b/src/web/features.rs @@ -1,14 +1,16 @@ -use super::headers::CanonicalUrl; -use super::MatchSemver; use crate::{ db::types::Feature, impl_axum_webpage, web::{ - cache::CachePolicy, error::AxumResult, extractors::DbConnection, match_version, MetaData, + cache::CachePolicy, + error::{AxumNope, AxumResult}, + extractors::{DbConnection, Path}, + headers::CanonicalUrl, + match_version, MetaData, ReqVersion, }, }; use anyhow::anyhow; -use axum::{extract::Path, response::IntoResponse}; +use axum::response::IntoResponse; use serde::Serialize; use std::collections::{HashMap, VecDeque}; @@ -34,27 +36,22 @@ impl_axum_webpage! { } pub(crate) async fn build_features_handler( - Path((name, req_version)): Path<(String, String)>, + Path((name, req_version)): Path<(String, ReqVersion)>, mut conn: DbConnection, ) -> AxumResult { - let (version, version_or_latest, is_latest_url) = - match match_version(&mut conn, &name, Some(&req_version)) - .await? - .exact_name_only()? - { - MatchSemver::Exact((version, _)) => (version.clone(), version, false), - MatchSemver::Latest((version, _)) => (version, "latest".to_string(), true), - - MatchSemver::Semver((version, _)) => { - return Ok(super::axum_cached_redirect( - &format!("/crate/{}/{}/features", &name, version), - CachePolicy::ForeverInCdn, - )? - .into_response()); - } - }; - - let metadata = MetaData::from_crate(&mut conn, &name, &version, &version_or_latest).await?; + let version = match_version(&mut conn, &name, &req_version) + .await? + .assume_exact_name()? + .into_canonical_req_version_or_else(|version| { + AxumNope::Redirect( + format!("/crate/{}/{}/features", &name, version), + CachePolicy::ForeverInCdn, + ) + })? + .into_version(); + + let metadata = + MetaData::from_crate(&mut conn, &name, &version, Some(req_version.clone())).await?; let row = sqlx::query!( r#" @@ -63,7 +60,7 @@ pub(crate) async fn build_features_handler( INNER JOIN crates ON crates.id = releases.crate_id WHERE crates.name = $1 AND releases.version = $2"#, name, - version + version.to_string(), ) .fetch_optional(&mut *conn) .await? @@ -82,7 +79,7 @@ pub(crate) async fn build_features_handler( metadata, features, default_len, - is_latest_url, + is_latest_url: req_version.is_latest(), canonical_url: CanonicalUrl::from_path(format!("/crate/{}/latest/features", &name)), use_direct_platform_links: true, } diff --git a/src/web/mod.rs b/src/web/mod.rs index 78647e7c9..b6a544c4f 100644 --- a/src/web/mod.rs +++ b/src/web/mod.rs @@ -46,10 +46,12 @@ use page::TemplateData; use percent_encoding::{utf8_percent_encode, AsciiSet, CONTROLS}; use semver::{Version, VersionReq}; use serde::Serialize; -use std::net::{IpAddr, Ipv4Addr}; +use serde_with::{DeserializeFromStr, SerializeDisplay}; use std::{ borrow::{Borrow, Cow}, - net::SocketAddr, + fmt::{self, Display}, + net::{IpAddr, Ipv4Addr, SocketAddr}, + str::FromStr, sync::Arc, }; use tower::ServiceBuilder; @@ -67,54 +69,167 @@ pub(crate) fn encode_url_path(path: &str) -> String { const DEFAULT_BIND: SocketAddr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), 3000); +/// Represents a version identifier in a request in the original state. +/// Can be an exact version, a semver requirement, or the string "latest". +#[derive(Debug, Default, Clone, PartialEq, Eq, SerializeDisplay, DeserializeFromStr)] +pub(crate) enum ReqVersion { + Exact(Version), + Semver(VersionReq), + #[default] + Latest, +} + +impl ReqVersion { + fn assume_exact(self) -> Result { + if let ReqVersion::Exact(version) = self { + Ok(version) + } else { + Err(AxumNope::VersionNotFound) + } + } + + pub(crate) fn is_latest(&self) -> bool { + matches!(self, ReqVersion::Latest) + } +} + +impl Display for ReqVersion { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ReqVersion::Exact(version) => version.fmt(f), + ReqVersion::Semver(version_req) => version_req.fmt(f), + ReqVersion::Latest => write!(f, "latest"), + } + } +} + +impl FromStr for ReqVersion { + type Err = semver::Error; + fn from_str(s: &str) -> Result { + if s == "latest" { + Ok(ReqVersion::Latest) + } else if let Ok(version) = Version::parse(s) { + Ok(ReqVersion::Exact(version)) + } else if s.is_empty() || s == "newest" { + Ok(ReqVersion::Semver(VersionReq::STAR)) + } else { + VersionReq::parse(s).map(ReqVersion::Semver) + } + } +} + #[derive(Debug)] -struct MatchVersion { - /// Represents the crate name that was found when attempting to load a crate release. - /// +struct MatchedRelease { + /// crate name + pub name: String, + + /// The crate name that was found when attempting to load a crate release. /// `match_version` will attempt to match a provided crate name against similar crate names with /// dashes (`-`) replaced with underscores (`_`) and vice versa. pub corrected_name: Option, - pub version: MatchSemver, - pub rustdoc_status: bool, - pub target_name: String, + + /// what kind of version did we get in the request? ("latest", semver, exact) + pub req_version: ReqVersion, + + /// the matched release + pub release: crate_details::Release, } -impl MatchVersion { - /// If the matched version was an exact match to the requested crate name, returns the - /// `MatchSemver` for the query. If the lookup required a dash/underscore conversion, returns - /// `CrateNotFound`. - fn exact_name_only(self) -> Result { +impl MatchedRelease { + fn assume_exact_name(self) -> Result { if self.corrected_name.is_none() { - Ok(self.version) + Ok(self) } else { Err(AxumNope::CrateNotFound) } } -} -/// Represents the possible results of attempting to load a version requirement. -/// The id (i32) of the release is stored to simplify successive queries. -#[derive(Debug, Clone, PartialEq, Eq)] -enum MatchSemver { - /// `match_version` was given an exact version, which matched a saved crate version. - Exact((String, i32)), - /// `match_version` was given a semver version requirement, which matched the given saved crate - /// version. - Semver((String, i32)), - // `match_version` was given the string "latest", which matches the given saved crate version. - Latest((String, i32)), -} + fn into_exactly_named(self) -> Self { + if let Some(corrected_name) = self.corrected_name { + Self { + name: corrected_name.to_owned(), + corrected_name: None, + ..self + } + } else { + self + } + } -impl MatchSemver { - /// Discard information about whether the loaded version was an exact match, and return the - /// matched version string and id. - pub fn into_parts(self) -> (String, i32) { - match self { - MatchSemver::Exact((v, i)) - | MatchSemver::Semver((v, i)) - | MatchSemver::Latest((v, i)) => (v, i), + fn into_exactly_named_or_else(self, f: F) -> Result + where + F: FnOnce(&str, &ReqVersion) -> AxumNope, + { + if let Some(corrected_name) = self.corrected_name { + Err(f(&corrected_name, &self.req_version)) + } else { + Ok(self) } } + + /// Canonicalize the the version from the request + /// + /// Mainly: + /// * "newest"/"*" or empty -> "latest" in the URL + /// * any other semver requirement -> specific version in the URL + fn into_canonical_req_version(self) -> Self { + match self.req_version { + ReqVersion::Exact(_) | ReqVersion::Latest => self, + ReqVersion::Semver(version_req) => { + if version_req == VersionReq::STAR { + Self { + req_version: ReqVersion::Latest, + ..self + } + } else { + Self { + req_version: ReqVersion::Exact(self.release.version.clone()), + ..self + } + } + } + } + } + + /// translate this MatchRelease into a specific semver::Version while canonicalizing the + /// version specification. + fn into_canonical_req_version_or_else(self, f: F) -> Result + where + F: FnOnce(&ReqVersion) -> AxumNope, + { + let original_req_version = self.req_version.clone(); + let canonicalized = self.into_canonical_req_version(); + + if canonicalized.req_version == original_req_version { + Ok(canonicalized) + } else { + Err(f(&canonicalized.req_version)) + } + } + + fn into_version(self) -> Version { + self.release.version + } + + fn id(&self) -> i32 { + self.release.id + } + + fn version(&self) -> &Version { + &self.release.version + } + + fn rustdoc_status(&self) -> bool { + self.release.rustdoc_status + } + + fn target_name(&self) -> &str { + &self.release.target_name + } + + fn is_latest_url(&self) -> bool { + matches!(self.req_version, ReqVersion::Latest) + } } /// Checks the database for crate releases that match the given name and version. @@ -128,8 +243,8 @@ impl MatchSemver { async fn match_version( conn: &mut sqlx::PgConnection, name: &str, - input_version: Option<&str>, -) -> Result { + input_version: &ReqVersion, +) -> Result { let (crate_id, corrected_name) = { let row = sqlx::query!( "SELECT id, name @@ -150,9 +265,8 @@ async fn match_version( }; // first load and parse all versions of this crate, - // skipping and reporting versions that are not semver valid. // `releases_for_crate` is already sorted, newest version first. - let releases = crate_details::releases_for_crate(conn, crate_id) + let mut releases = crate_details::releases_for_crate(conn, crate_id) .await .context("error fetching releases for crate")?; @@ -160,55 +274,45 @@ async fn match_version( return Err(AxumNope::CrateNotFound); } - // version is an Option<&str> from router::Router::get, need to decode first. - // Any encoding errors we treat as _any version_. - let req_version = input_version.unwrap_or("*"); + let req_semver: VersionReq = match input_version { + ReqVersion::Exact(parsed_req_version) => { + if let Some(release) = releases + .iter() + .find(|release| &release.version == parsed_req_version) + { + return Ok(MatchedRelease { + name: name.to_owned(), + corrected_name, + req_version: input_version.clone(), + release: release.clone(), + }); + } - // first check for exact match, we can't expect users to use semver in query - if let Ok(parsed_req_version) = Version::parse(req_version) { - if let Some(release) = releases - .iter() - .find(|release| release.version == parsed_req_version) - { - return Ok(MatchVersion { - corrected_name, - version: MatchSemver::Exact((release.version.to_string(), release.id)), - rustdoc_status: release.rustdoc_status, - target_name: release.target_name.clone(), - }); + if let Ok(version_req) = VersionReq::parse(&parsed_req_version.to_string()) { + // when we don't find a release with exact version, + // we try to interpret it as a semver requirement. + // A normal semver version ("1.2.3") is equivalent to a caret semver requirement. + version_req + } else { + return Err(AxumNope::VersionNotFound); + } } - } - - // Now try to match with semver, treat `newest` and `latest` as `*` - let req_semver = if req_version == "newest" || req_version == "latest" { - VersionReq::STAR - } else { - VersionReq::parse(req_version).map_err(|err| { - info!( - "could not parse version requirement \"{}\": {:?}", - req_version, err - ); - AxumNope::VersionNotFound - })? + ReqVersion::Latest => VersionReq::STAR, + ReqVersion::Semver(version_req) => version_req.clone(), }; - // starting here, we only look at non-yanked releases - let releases: Vec<_> = releases.iter().filter(|r| !r.yanked).collect(); + // when matching semver requirements, we only want to look at non-yanked releases. + releases.retain(|r| !r.yanked); - // try to match the version in all un-yanked releases. if let Some(release) = releases .iter() .find(|release| req_semver.matches(&release.version)) { - return Ok(MatchVersion { + return Ok(MatchedRelease { + name: name.to_owned(), corrected_name, - version: if input_version == Some("latest") { - MatchSemver::Latest((release.version.to_string(), release.id)) - } else { - MatchSemver::Semver((release.version.to_string(), release.id)) - }, - rustdoc_status: release.rustdoc_status, - target_name: release.target_name.clone(), + req_version: input_version.clone(), + release: release.clone(), }); } @@ -218,11 +322,11 @@ async fn match_version( if req_semver == VersionReq::STAR { return releases .first() - .map(|release| MatchVersion { + .map(|release| MatchedRelease { + name: name.to_owned(), corrected_name: corrected_name.clone(), - version: MatchSemver::Semver((release.version.to_string(), release.id)), - rustdoc_status: release.rustdoc_status, - target_name: release.target_name.clone(), + req_version: input_version.clone(), + release: release.clone(), }) .ok_or(AxumNope::VersionNotFound); } @@ -485,11 +589,13 @@ where #[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub(crate) struct MetaData { pub(crate) name: String, - // If we're on a page with /latest/ in the URL, the string "latest". - // Otherwise, the version as a string. - pub(crate) version_or_latest: String, - // The exact version of the crate being shown. Never contains "latest". - pub(crate) version: String, + /// The exact version of the release being shown. + pub(crate) version: Version, + /// The version identifier in the request that was used to request this page. + /// This might be any of the variants of `ReqVersion`, but + /// due to a canonicalization step, it is either an Exact version, or `/latest/` + /// most of the time. + pub(crate) req_version: ReqVersion, pub(crate) description: Option, pub(crate) target_name: Option, pub(crate) rustdoc_status: bool, @@ -505,8 +611,8 @@ impl MetaData { async fn from_crate( conn: &mut sqlx::PgConnection, name: &str, - version: &str, - version_or_latest: &str, + version: &Version, + req_version: Option, ) -> Result { sqlx::query!( "SELECT @@ -523,15 +629,15 @@ impl MetaData { INNER JOIN crates ON crates.id = releases.crate_id WHERE crates.name = $1 AND releases.version = $2", name, - version + version.to_string(), ) .fetch_optional(&mut *conn) .await .context("error fetching crate metadata")? .map(|row| MetaData { name: row.name, - version: row.version, - version_or_latest: version_or_latest.to_string(), + version: version.clone(), + req_version: req_version.unwrap_or_else(|| ReqVersion::Exact(version.clone())), description: row.description, target_name: Some(row.target_name), rustdoc_status: row.rustdoc_status, @@ -592,26 +698,29 @@ mod test { .unwrap() } - async fn version(v: Option<&str>, db: &TestDatabase) -> Option { + async fn version(v: Option<&str>, db: &TestDatabase) -> Option { let mut conn = db.async_conn().await; - let version = match_version(&mut conn, "foo", v) - .await - .ok()? - .exact_name_only() - .ok()? - .into_parts() - .0; + let version = match_version( + &mut conn, + "foo", + &ReqVersion::from_str(v.unwrap_or_default()).unwrap(), + ) + .await + .ok()? + .assume_exact_name() + .ok()? + .into_version(); Some(version) } #[allow(clippy::unnecessary_wraps)] - fn semver(version: &'static str) -> Option { - Some(version.into()) + fn semver(version: &'static str) -> Option { + version.parse().ok() } #[allow(clippy::unnecessary_wraps)] - fn exact(version: &'static str) -> Option { - Some(version.into()) + fn exact(version: &'static str) -> Option { + version.parse().ok() } fn clipboard_is_present_for_path(path: &str, web: &TestFrontend) -> bool { @@ -746,7 +855,7 @@ mod test { } #[test] - fn double_slash_does_redirect_and_remove_slash() { + fn double_slash_does_redirect_to_latest_version() { wrapper(|env| { env.fake_release() .name("bat") @@ -754,8 +863,7 @@ mod test { .create() .unwrap(); let web = env.frontend(); - let response = web.get("/bat//").send()?; - assert_eq!(response.status().as_u16(), StatusCode::NOT_FOUND.as_u16()); + assert_redirect("/bat//", "/bat/latest/bat/", web)?; Ok(()) }) } @@ -933,8 +1041,8 @@ mod test { fn serialize_metadata() { let mut metadata = MetaData { name: "serde".to_string(), - version: "1.0.0".to_string(), - version_or_latest: "1.0.0".to_string(), + version: "1.0.0".parse().unwrap(), + req_version: ReqVersion::Latest, description: Some("serde does stuff".to_string()), target_name: None, rustdoc_status: true, @@ -950,7 +1058,7 @@ mod test { let correct_json = json!({ "name": "serde", "version": "1.0.0", - "version_or_latest": "1.0.0", + "req_version": "latest", "description": "serde does stuff", "target_name": null, "rustdoc_status": true, @@ -969,7 +1077,7 @@ mod test { let correct_json = json!({ "name": "serde", "version": "1.0.0", - "version_or_latest": "1.0.0", + "req_version": "latest", "description": "serde does stuff", "target_name": "serde_lib_name", "rustdoc_status": true, @@ -988,7 +1096,7 @@ mod test { let correct_json = json!({ "name": "serde", "version": "1.0.0", - "version_or_latest": "1.0.0", + "req_version": "latest", "description": null, "target_name": "serde_lib_name", "rustdoc_status": true, @@ -1009,13 +1117,19 @@ mod test { async_wrapper(|env| async move { release("0.1.0", &env).await; let mut conn = env.async_db().await.async_conn().await; - let metadata = MetaData::from_crate(&mut conn, "foo", "0.1.0", "latest").await; + let metadata = MetaData::from_crate( + &mut conn, + "foo", + &"0.1.0".parse().unwrap(), + Some(ReqVersion::Latest), + ) + .await; assert_eq!( metadata.unwrap(), MetaData { name: "foo".to_string(), - version_or_latest: "latest".to_string(), - version: "0.1.0".to_string(), + version: "0.1.0".parse().unwrap(), + req_version: ReqVersion::Latest, description: Some("Fake package".to_string()), target_name: Some("foo".to_string()), rustdoc_status: true, @@ -1082,4 +1196,40 @@ mod test { assert!(axum_redirect(path).is_err()); assert!(axum_cached_redirect(path, cache::CachePolicy::NoCaching).is_err()); } + + #[test] + fn test_parse_req_version_latest() { + let req_version: ReqVersion = "latest".parse().unwrap(); + assert_eq!(req_version, ReqVersion::Latest); + assert_eq!(req_version.to_string(), "latest"); + } + + #[test_case("1.2.3")] + fn test_parse_req_version_exact(input: &str) { + let req_version: ReqVersion = input.parse().unwrap(); + assert_eq!( + req_version, + ReqVersion::Exact(Version::parse(input).unwrap()) + ); + assert_eq!(req_version.to_string(), input); + } + + #[test_case("^1.2.3")] + #[test_case("*")] + fn test_parse_req_version_semver(input: &str) { + let req_version: ReqVersion = input.parse().unwrap(); + assert_eq!( + req_version, + ReqVersion::Semver(VersionReq::parse(input).unwrap()) + ); + assert_eq!(req_version.to_string(), input); + } + + #[test_case("")] + #[test_case("newest")] + fn test_parse_req_version_semver_latest(input: &str) { + let req_version: ReqVersion = input.parse().unwrap(); + assert_eq!(req_version, ReqVersion::Semver(VersionReq::STAR)); + assert_eq!(req_version.to_string(), "*") + } } diff --git a/src/web/releases.rs b/src/web/releases.rs index 16893f994..16988d059 100644 --- a/src/web/releases.rs +++ b/src/web/releases.rs @@ -9,14 +9,14 @@ use crate::{ web::{ axum_parse_uri_with_params, axum_redirect, encode_url_path, error::{AxumNope, AxumResult}, - extractors::DbConnection, - match_version, + extractors::{DbConnection, Path}, + match_version, ReqVersion, }, BuildQueue, Config, InstanceMetrics, }; use anyhow::{anyhow, bail, Context as _, Result}; use axum::{ - extract::{Extension, Path, Query}, + extract::{Extension, Query}, response::{IntoResponse, Response as AxumResponse}, }; use base64::{engine::general_purpose::STANDARD as b64, Engine}; @@ -562,17 +562,25 @@ pub(crate) async fn search_handler( // since we never pass a version into `match_version` here, we'll never get // `MatchVersion::Exact`, so the distinction between `Exact` and `Semver` doesn't // matter - if let Ok(matchver) = match_version(&mut conn, krate, None).await { + if let Ok(matchver) = match_version(&mut conn, krate, &ReqVersion::Latest) + .await + .map(|matched_release| matched_release.into_exactly_named()) + { params.remove("query"); queries.extend(params); - let (version, _) = matchver.version.into_parts(); - let krate = matchver.corrected_name.unwrap_or_else(|| krate.to_string()); - let uri = if matchver.rustdoc_status { - let target_name = matchver.target_name; - axum_parse_uri_with_params(&format!("/{krate}/{version}/{target_name}/"), queries)? + let uri = if matchver.rustdoc_status() { + axum_parse_uri_with_params( + &format!( + "/{}/{}/{}/", + matchver.name, + matchver.version(), + matchver.target_name(), + ), + queries, + )? } else { - format!("/crate/{krate}/{version}") + format!("/crate/{}/{}", matchver.name, matchver.version()) .parse::() .context("could not parse redirect URI")? }; @@ -605,17 +613,14 @@ pub(crate) async fn search_handler( return Err(AxumNope::NoResults); } - let p = form_urlencoded::parse(query_params.as_bytes()); - if let Some(v) = p - .filter_map(|(k, v)| { - if &k == "sort" { - Some(v.to_string()) - } else { - None - } - }) - .next() - { + let mut p = form_urlencoded::parse(query_params.as_bytes()); + if let Some(v) = p.find_map(|(k, v)| { + if &k == "sort" { + Some(v.to_string()) + } else { + None + } + }) { sort_by = v; }; diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index 23f4f1f03..6c3b97b83 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -11,23 +11,24 @@ use crate::{ csp::Csp, encode_url_path, error::{AxumNope, AxumResult}, - extractors::DbConnection, + extractors::{DbConnection, Path}, file::File, match_version, metrics::RenderingTimesRecorder, page::TemplateData, - MatchSemver, MetaData, + MetaData, ReqVersion, }, AsyncStorage, Config, InstanceMetrics, RUSTDOC_STATIC_STORAGE_PREFIX, }; use anyhow::{anyhow, Context as _}; use axum::{ - extract::{Extension, Path, Query}, + extract::{Extension, Query}, http::{StatusCode, Uri}, response::{Html, IntoResponse, Response as AxumResponse}, }; use lol_html::errors::RewritingError; use once_cell::sync::Lazy; +use semver::Version; use serde::{Deserialize, Serialize}; use std::{ collections::{BTreeMap, HashMap}, @@ -51,7 +52,7 @@ static DOC_RUST_LANG_ORG_REDIRECTS: Lazy> = Lazy::new(|| { #[derive(Debug, Clone, Deserialize)] pub(crate) struct RustdocRedirectorParams { name: String, - version: Option, + version: Option, target: Option, } @@ -142,7 +143,7 @@ pub(crate) async fn rustdoc_redirector_handler( } } - let (mut crate_name, path_in_crate) = match params.name.split_once("::") { + let (crate_name, path_in_crate) = match params.name.split_once("::") { Some((krate, path)) => (krate.to_string(), Some(path.to_string())), None => (params.name.to_string(), None), }; @@ -160,21 +161,24 @@ pub(crate) async fn rustdoc_redirector_handler( // it doesn't matter if the version that was given was exact or not, since we're redirecting // anyway rendering_time.step("match version"); - let v = match_version(&mut conn, &crate_name, params.version.as_deref()).await?; - trace!(?v, "matched version"); - if let Some(new_name) = v.corrected_name { - // `match_version` checked against -/_ typos, so if we have a name here we should - // use that instead - crate_name = new_name; - } - let (mut version, id) = v.version.into_parts(); + let matched_release = match_version( + &mut conn, + &crate_name, + ¶ms.version.clone().unwrap_or_default(), + ) + .await? + .into_exactly_named(); + trace!(?matched_release, "matched version"); + let crate_name = matched_release.name.clone(); // we might get requests to crate-specific JS files here. if let Some(ref target) = params.target { if target.ends_with(".js") { // this URL is actually from a crate-internal path, serve it there instead rendering_time.step("serve JS for crate"); - let krate = CrateDetails::new(&mut conn, &crate_name, &version, &version) + let version = matched_release.into_version(); + + let krate = CrateDetails::new(&mut conn, &crate_name, &version, params.version.clone()) .await? .ok_or(AxumNope::ResourceNotFound)?; @@ -183,7 +187,7 @@ pub(crate) async fn rustdoc_redirector_handler( match storage .fetch_rustdoc_file( &crate_name, - &version, + &version.to_string(), krate.latest_build_id.unwrap_or(0), target, krate.archive_storage, @@ -213,9 +217,7 @@ pub(crate) async fn rustdoc_redirector_handler( } } - if let None | Some("latest") = params.version.as_deref() { - version = "latest".to_string() - } + let matched_release = matched_release.into_canonical_req_version(); // get target name and whether it has docs // FIXME: This is a bit inefficient but allowing us to use less code in general @@ -227,7 +229,7 @@ pub(crate) async fn rustdoc_redirector_handler( rustdoc_status FROM releases WHERE releases.id = $1", - id, + matched_release.id(), ) .fetch_one(&mut *conn) .await?; @@ -244,12 +246,18 @@ pub(crate) async fn rustdoc_redirector_handler( rendering_time.step("redirect to doc"); let url_str = if let Some(target) = target { - format!("/{crate_name}/{version}/{target}/{target_name}/") + format!( + "/{crate_name}/{}/{target}/{target_name}/", + matched_release.req_version + ) } else { - format!("/{crate_name}/{version}/{target_name}/") + format!( + "/{crate_name}/{}/{target_name}/", + matched_release.req_version + ) }; - let cache = if version == "latest" { + let cache = if matched_release.is_latest_url() { CachePolicy::ForeverInCdn } else { CachePolicy::ForeverInCdnAndStaleInBrowser @@ -265,7 +273,7 @@ pub(crate) async fn rustdoc_redirector_handler( } else { rendering_time.step("redirect to crate"); Ok(axum_cached_redirect( - format!("/crate/{crate_name}/{version}"), + format!("/crate/{crate_name}/{}", matched_release.req_version), CachePolicy::ForeverInCdn, )? .into_response()) @@ -339,7 +347,7 @@ impl RustdocPage { #[derive(Clone, Deserialize, Debug)] pub(crate) struct RustdocHtmlParams { pub(crate) name: String, - pub(crate) version: String, + pub(crate) version: ReqVersion, // 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. @@ -369,7 +377,7 @@ pub(crate) async fn rustdoc_html_server_handler( // 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)?; + .map_err(|err| AxumNope::BadRequest(err.into()))?; let mut req_path: Vec<&str> = original_path.split('/').collect(); // Remove the empty start, the name and the version from the path @@ -382,7 +390,7 @@ pub(crate) async fn rustdoc_html_server_handler( #[instrument] fn redirect( name: &str, - vers: &str, + vers: &Version, path: &[&str], cache_policy: CachePolicy, ) -> AxumResult { @@ -406,50 +414,50 @@ pub(crate) async fn rustdoc_html_server_handler( // * If both the name and the version are an exact match, return the version of the crate. // * If there is an exact match, but the requested crate name was corrected (dashes vs. underscores), redirect to the corrected name. // * If there is a semver (but not exact) match, redirect to the exact version. - let release_found = match_version(&mut conn, ¶ms.name, Some(¶ms.version)).await?; - trace!(?release_found, "found release"); - - let (version, version_or_latest, is_latest_url) = 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, false) - } - - 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(), true) - } - - // 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 version = match_version(&mut conn, ¶ms.name, ¶ms.version) + .await? + .into_exactly_named_or_else(|corrected_name, req_version| { + AxumNope::Redirect( + encode_url_path(&format!( + "/{}/{}/{}", + corrected_name, + req_version, + req_path.join("/") + )), + CachePolicy::NoCaching, + ) + })? + .into_canonical_req_version_or_else(|version| { + AxumNope::Redirect( + encode_url_path(&format!( + "/{}/{}/{}", + ¶ms.name, + version, + req_path.join("/") + )), + CachePolicy::ForeverInCdn, + ) + })? + .into_version(); trace!("crate details"); rendering_time.step("crate details"); // Get the crate's details from the database // NOTE: we know this crate must exist because we just checked it above (or else `match_version` is buggy) - let krate = CrateDetails::new(&mut conn, ¶ms.name, &version, &version_or_latest) - .await? - .ok_or(AxumNope::ResourceNotFound)?; + let krate = CrateDetails::new( + &mut conn, + ¶ms.name, + &version, + Some(params.version.clone()), + ) + .await? + .ok_or(AxumNope::ResourceNotFound)?; if !krate.rustdoc_status { rendering_time.step("redirect to crate"); return Ok(axum_cached_redirect( - format!("/crate/{}/{}", params.name, version_or_latest), + format!("/crate/{}/{}", params.name, params.version), CachePolicy::ForeverInCdn, )? .into_response()); @@ -460,7 +468,7 @@ pub(crate) async fn rustdoc_html_server_handler( if req_path.first().copied() == Some(&krate.metadata.default_target) { return redirect( ¶ms.name, - &version_or_latest, + &version, &req_path[1..], CachePolicy::ForeverInCdn, ); @@ -484,7 +492,7 @@ pub(crate) async fn rustdoc_html_server_handler( let blob = match storage .fetch_rustdoc_file( ¶ms.name, - &version, + &version.to_string(), krate.latest_build_id.unwrap_or(0), &storage_path, krate.archive_storage, @@ -506,19 +514,14 @@ pub(crate) async fn rustdoc_html_server_handler( return if storage .rustdoc_file_exists( ¶ms.name, - &version, + &version.to_string(), krate.latest_build_id.unwrap_or(0), &storage_path, krate.archive_storage, ) .await? { - redirect( - ¶ms.name, - &version_or_latest, - &req_path, - CachePolicy::ForeverInCdn, - ) + redirect(¶ms.name, &version, &req_path, CachePolicy::ForeverInCdn) } else if req_path.first().map_or(false, |p| p.contains('-')) { // This is a target, not a module; it may not have been built. // Redirect to the default target and show a search page instead of a hard 404. @@ -526,7 +529,7 @@ pub(crate) async fn rustdoc_html_server_handler( encode_url_path(&format!( "/crate/{}/{}/target-redirect/{}", params.name, - version, + params.version, req_path.join("/") )), CachePolicy::ForeverInCdn, @@ -536,7 +539,7 @@ pub(crate) async fn rustdoc_html_server_handler( if storage_path == format!("{}/index.html", krate.target_name) { error!( krate = params.name, - version, + version = version.to_string(), original_path = original_path.as_ref(), storage_path, "Couldn't find crate documentation root on storage. @@ -565,17 +568,9 @@ pub(crate) async fn rustdoc_html_server_handler( let latest_release = krate.latest_release()?; // Get the latest version of the crate - let latest_version = latest_release.version.to_string(); + let latest_version = latest_release.version.clone(); let is_latest_version = latest_version == version; - let is_prerelease = !(semver::Version::parse(&version) - .with_context(|| { - format!( - "invalid semver in database for crate {}: {}", - params.name, &version - ) - })? - .pre - .is_empty()); + let is_prerelease = !(version.pre.is_empty()); // The path within this crate version's rustdoc output let (target, inner_path) = { @@ -648,11 +643,11 @@ pub(crate) async fn rustdoc_html_server_handler( Ok(RustdocPage { latest_path, permalink_path, - latest_version, + latest_version: latest_version.to_string(), target, inner_path, is_latest_version, - is_latest_url, + is_latest_url: params.version.is_latest(), is_prerelease, metadata, krate, @@ -737,19 +732,16 @@ fn path_for_version( } pub(crate) async fn target_redirect_handler( - Path((name, version, req_path)): Path<(String, String, String)>, + Path((name, req_version, req_path)): Path<(String, ReqVersion, String)>, mut conn: DbConnection, Extension(storage): Extension>, ) -> AxumResult { - let release_found = match_version(&mut conn, &name, Some(&version)).await?; - let (version, version_or_latest, is_latest_url) = match release_found.version { - MatchSemver::Exact((version, _)) => (version.clone(), version, false), - MatchSemver::Latest((version, _)) => (version, "latest".to_string(), true), - // semver matching not supported here - MatchSemver::Semver(_) => return Err(AxumNope::VersionNotFound), - }; + let version = match_version(&mut conn, &name, &req_version) + .await? + .into_canonical_req_version_or_else(|_| AxumNope::VersionNotFound)? + .into_version(); - let crate_details = CrateDetails::new(&mut conn, &name, &version, &version_or_latest) + let crate_details = CrateDetails::new(&mut conn, &name, &version, Some(req_version.clone())) .await? .ok_or(AxumNope::VersionNotFound)?; @@ -781,7 +773,7 @@ pub(crate) async fn target_redirect_handler( let (redirect_path, query_args) = if storage .rustdoc_file_exists( &name, - &version, + &version.to_string(), crate_details.latest_build_id.unwrap_or(0), &storage_location_for_path, crate_details.archive_storage, @@ -797,10 +789,10 @@ pub(crate) async fn target_redirect_handler( Ok(axum_cached_redirect( axum_parse_uri_with_params( - &encode_url_path(&format!("/{name}/{version_or_latest}/{redirect_path}")), + &encode_url_path(&format!("/{name}/{}/{redirect_path}", req_version)), query_args, )?, - if is_latest_url { + if req_version.is_latest() { CachePolicy::ForeverInCdn } else { CachePolicy::ForeverInCdnAndStaleInBrowser @@ -810,7 +802,7 @@ pub(crate) async fn target_redirect_handler( #[derive(Deserialize, Debug)] pub(crate) struct BadgeQueryParams { - version: Option, + version: Option, } #[instrument] @@ -818,10 +810,11 @@ pub(crate) async fn badge_handler( Path(name): Path, Query(query): Query, ) -> AxumResult { - let version = query.version.unwrap_or_else(|| "latest".to_string()); - - let url = url::Url::parse(&format!("https://img.shields.io/docsrs/{name}/{version}")) - .context("could not parse URL")?; + let url = url::Url::parse(&format!( + "https://img.shields.io/docsrs/{name}/{}", + query.version.unwrap_or_default(), + )) + .context("could not parse URL")?; Ok(( StatusCode::MOVED_PERMANENTLY, @@ -831,17 +824,17 @@ pub(crate) async fn badge_handler( } pub(crate) async fn download_handler( - Path((name, req_version)): Path<(String, String)>, + Path((name, req_version)): Path<(String, ReqVersion)>, mut conn: DbConnection, Extension(storage): Extension>, Extension(config): Extension>, ) -> AxumResult { - let (version, _) = match_version(&mut conn, &name, Some(&req_version)) + let version = match_version(&mut conn, &name, &req_version) .await? - .exact_name_only()? - .into_parts(); + .assume_exact_name()? + .into_version(); - let archive_path = rustdoc_archive_path(&name, &version); + let archive_path = rustdoc_archive_path(&name, &version.to_string()); // not all archives are set for public access yet, so we check if // the access is set and fix it if needed. @@ -1109,7 +1102,11 @@ mod test { .rustdoc_file("dummy/index.html") .create()?; - let resp = env.frontend().get("/dummy/latest/dummy/").send()?; + let resp = env + .frontend() + .get("/dummy/latest/dummy/") + .send()? + .error_for_status()?; assert_cache_control(&resp, CachePolicy::ForeverInCdn, &env.config()); assert!(resp.url().as_str().ends_with("/dummy/latest/dummy/")); let body = String::from_utf8(resp.bytes().unwrap().to_vec()).unwrap(); @@ -1459,7 +1456,7 @@ mod test { let web = env.frontend(); assert_redirect("/dummy_dash", "/dummy-dash/latest/dummy_dash/", web)?; - assert_redirect("/dummy_dash/*", "/dummy-dash/0.2.0/dummy_dash/", web)?; + assert_redirect("/dummy_dash/*", "/dummy-dash/latest/dummy_dash/", web)?; assert_redirect("/dummy_dash/0.1.0", "/dummy-dash/0.1.0/dummy_dash/", web)?; assert_redirect( "/dummy-underscore", @@ -1468,7 +1465,7 @@ mod test { )?; assert_redirect( "/dummy-underscore/*", - "/dummy_underscore/0.2.0/dummy_underscore/", + "/dummy_underscore/latest/dummy_underscore/", web, )?; assert_redirect( @@ -1483,7 +1480,7 @@ mod test { )?; assert_redirect( "/dummy_mixed_separators/*", - "/dummy_mixed-separators/0.2.0/dummy_mixed_separators/", + "/dummy_mixed-separators/latest/dummy_mixed_separators/", web, )?; assert_redirect( diff --git a/src/web/sitemap.rs b/src/web/sitemap.rs index a50ca1181..bc9927efb 100644 --- a/src/web/sitemap.rs +++ b/src/web/sitemap.rs @@ -5,16 +5,12 @@ use crate::{ utils::{get_config, spawn_blocking, ConfigName}, web::{ error::{AxumNope, AxumResult}, - extractors::DbConnection, + extractors::{DbConnection, Path}, AxumErrorPage, }, Config, }; -use axum::{ - extract::{Extension, Path}, - http::StatusCode, - response::IntoResponse, -}; +use axum::{extract::Extension, http::StatusCode, response::IntoResponse}; use chrono::{TimeZone, Utc}; use futures_util::stream::TryStreamExt; use serde::Serialize; diff --git a/src/web/source.rs b/src/web/source.rs index 5dfc81930..1f14dc659 100644 --- a/src/web/source.rs +++ b/src/web/source.rs @@ -5,14 +5,15 @@ use crate::{ storage::PathNotFoundError, utils::get_correct_docsrs_style_file, web::{ - cache::CachePolicy, error::AxumNope, file::File as DbFile, headers::CanonicalUrl, - MatchSemver, MetaData, + cache::CachePolicy, error::AxumNope, extractors::Path, file::File as DbFile, + headers::CanonicalUrl, MetaData, ReqVersion, }, AsyncStorage, }; use anyhow::{Context as _, Result}; -use axum::{extract::Path, response::IntoResponse, Extension}; +use axum::{response::IntoResponse, Extension}; use axum_extra::headers::HeaderMapExt; +use semver::Version; use serde::{Deserialize, Serialize}; use std::{cmp::Ordering, sync::Arc}; use tracing::instrument; @@ -70,8 +71,8 @@ impl FileList { async fn from_path( conn: &mut sqlx::PgConnection, name: &str, - version: &str, - version_or_latest: &str, + version: &Version, + req_version: Option, folder: &str, ) -> Result> { let row = match sqlx::query!( @@ -89,7 +90,7 @@ impl FileList { INNER JOIN crates ON crates.id = releases.crate_id WHERE crates.name = $1 AND releases.version = $2", name, - version, + version.to_string(), ) .fetch_optional(&mut *conn) .await? @@ -148,8 +149,8 @@ impl FileList { Ok(Some(FileList { metadata: MetaData { name: row.name, - version: row.version, - version_or_latest: version_or_latest.to_string(), + version: version.clone(), + req_version: req_version.unwrap_or_else(|| ReqVersion::Exact(version.clone())), description: row.description, target_name: Some(row.target_name), rustdoc_status: row.rustdoc_status, @@ -191,41 +192,37 @@ impl_axum_webpage! { #[derive(Deserialize, Clone, Debug)] pub(crate) struct SourceBrowserHandlerParams { name: String, - version: String, + version: ReqVersion, #[serde(default)] path: String, } #[instrument(skip(pool, storage))] pub(crate) async fn source_browser_handler( - Path(SourceBrowserHandlerParams { - mut name, - version, - path, - }): Path, + Path(params): Path, Extension(storage): Extension>, Extension(pool): Extension, ) -> AxumResult { let mut conn = pool.get_async().await?; - let v = match_version(&mut conn, &name, Some(&version)).await?; - - if let Some(new_name) = &v.corrected_name { - // `match_version` checked against -/_ typos, so if we have a name here we should - // use that instead - name = new_name.to_string(); - } - let (version, version_or_latest, is_latest_url) = match v.version { - MatchSemver::Latest((version, _)) => (version, "latest".to_string(), true), - MatchSemver::Exact((version, _)) => (version.clone(), version, false), - MatchSemver::Semver((version, _)) => { - return Ok(super::axum_cached_redirect( - &format!("/crate/{name}/{version}/source/{path}"), + let version = match_version(&mut conn, ¶ms.name, ¶ms.version) + .await? + .into_exactly_named_or_else(|corrected_name, req_version| { + AxumNope::Redirect( + format!( + "/crate/{corrected_name}/{req_version}/source/{}", + params.path + ), + CachePolicy::NoCaching, + ) + })? + .into_canonical_req_version_or_else(|version| { + AxumNope::Redirect( + format!("/crate/{}/{version}/source/{}", params.name, params.path), CachePolicy::ForeverInCdn, - )? - .into_response()); - } - }; + ) + })? + .into_version(); let row = sqlx::query!( "SELECT @@ -242,21 +239,21 @@ pub(crate) async fn source_browser_handler( WHERE name = $1 AND version = $2", - name, - version + params.name, + version.to_string() ) .fetch_one(&mut *conn) .await?; // try to get actual file first // skip if request is a directory - let blob = if !path.ends_with('/') { + let blob = if !params.path.ends_with('/') { match storage .fetch_source_file( - &name, - &version, + ¶ms.name, + &version.to_string(), row.latest_build_id.unwrap_or(0), - &path, + ¶ms.path, row.archive_storage, ) .await @@ -275,7 +272,10 @@ pub(crate) async fn source_browser_handler( None }; - let canonical_url = CanonicalUrl::from_path(format!("/crate/{name}/latest/source/{path}")); + let canonical_url = CanonicalUrl::from_path(format!( + "/crate/{}/latest/source/{}", + params.name, params.path + )); let (file, file_content) = if let Some(blob) = blob { let is_text = blob.mime.starts_with("text") || blob.mime == "application/json"; @@ -304,17 +304,17 @@ pub(crate) async fn source_browser_handler( (None, None) }; - let current_folder = if let Some(last_slash_pos) = path.rfind('/') { - &path[..last_slash_pos + 1] + let current_folder = if let Some(last_slash_pos) = params.path.rfind('/') { + ¶ms.path[..last_slash_pos + 1] } else { "" }; let file_list = FileList::from_path( &mut conn, - &name, + ¶ms.name, &version, - &version_or_latest, + Some(params.version.clone()), current_folder, ) .await? @@ -326,7 +326,7 @@ pub(crate) async fn source_browser_handler( file, file_content, canonical_url, - is_latest_url, + is_latest_url: params.version.is_latest(), use_direct_platform_links: true, } .into_response()) @@ -537,7 +537,7 @@ mod tests { #[test_case(true)] #[test_case(false)] - fn semver_handled(archive_storage: bool) { + fn semver_handled_latest(archive_storage: bool) { wrapper(|env| { env.fake_release() .archive_storage(archive_storage) @@ -549,6 +549,29 @@ mod tests { assert_success("/crate/mbedtls/0.2.0/source/", web)?; assert_redirect_cached( "/crate/mbedtls/*/source/", + "/crate/mbedtls/latest/source/", + CachePolicy::ForeverInCdn, + web, + &env.config(), + )?; + Ok(()) + }) + } + + #[test_case(true)] + #[test_case(false)] + fn semver_handled(archive_storage: bool) { + wrapper(|env| { + env.fake_release() + .archive_storage(archive_storage) + .name("mbedtls") + .version("0.2.0") + .source_file("README.md", b"hello") + .create()?; + let web = env.frontend(); + assert_success("/crate/mbedtls/0.2.0/source/", web)?; + assert_redirect_cached( + "/crate/mbedtls/~0.2.0/source/", "/crate/mbedtls/0.2.0/source/", CachePolicy::ForeverInCdn, web, diff --git a/src/web/status.rs b/src/web/status.rs index d4b881fc7..69d188a82 100644 --- a/src/web/status.rs +++ b/src/web/status.rs @@ -1,16 +1,15 @@ -use super::cache::CachePolicy; +use super::{cache::CachePolicy, error::AxumNope}; use crate::web::{ - axum_redirect, error::AxumResult, extractors::DbConnection, match_version, MatchSemver, + error::AxumResult, + extractors::{DbConnection, Path}, + match_version, ReqVersion, }; use axum::{ - extract::{Extension, Path}, - http::header::ACCESS_CONTROL_ALLOW_ORIGIN, - response::IntoResponse, - Json, + extract::Extension, http::header::ACCESS_CONTROL_ALLOW_ORIGIN, response::IntoResponse, Json, }; pub(crate) async fn status_handler( - Path((name, req_version)): Path<(String, String)>, + Path((name, req_version)): Path<(String, ReqVersion)>, mut conn: DbConnection, ) -> impl IntoResponse { ( @@ -19,31 +18,23 @@ pub(crate) async fn status_handler( // We use an async block to emulate a try block so that we can apply the above CORS header // and cache policy to both successful and failed responses async move { - let (version, id) = match match_version(&mut conn, &name, Some(&req_version)) + let matched_release = match_version(&mut conn, &name, &req_version) .await? - .exact_name_only()? - { - MatchSemver::Exact((version, id)) | MatchSemver::Latest((version, id)) => { - (version, id) - } - MatchSemver::Semver((version, _)) => { - let redirect = axum_redirect(format!("/crate/{name}/{version}/status.json"))?; - return Ok(redirect.into_response()); - } - }; - - let rustdoc_status: bool = sqlx::query_scalar!( - "SELECT releases.rustdoc_status - FROM releases - WHERE releases.id = $1 - ", - id - ) - .fetch_one(&mut *conn) - .await?; + .assume_exact_name()?; + + let rustdoc_status = matched_release.rustdoc_status(); + + let version = matched_release + .into_canonical_req_version_or_else(|version| { + AxumNope::Redirect( + format!("/crate/{name}/{version}/status.json"), + CachePolicy::NoCaching, + ) + })? + .into_version(); let json = Json(serde_json::json!({ - "version": version, + "version": version.to_string(), "doc_status": rustdoc_status, })); @@ -91,8 +82,25 @@ mod tests { }); } + #[test] + fn redirect_latest() { + wrapper(|env| { + env.fake_release().name("foo").version("0.1.0").create()?; + + let redirect = assert_redirect( + "/crate/foo/*/status.json", + "/crate/foo/latest/status.json", + env.frontend(), + )?; + assert_cache_control(&redirect, CachePolicy::NoStoreMustRevalidate, &env.config()); + assert_eq!(redirect.headers()["access-control-allow-origin"], "*"); + + Ok(()) + }); + } + #[test_case("0.1")] - #[test_case("*")] + #[test_case("~0.1"; "semver")] fn redirect(version: &str) { wrapper(|env| { env.fake_release().name("foo").version("0.1.0").create()?; diff --git a/templates/crate/features.html b/templates/crate/features.html index c7125ccce..51f0733f7 100644 --- a/templates/crate/features.html +++ b/templates/crate/features.html @@ -56,9 +56,9 @@

{{ metadata.name }}

There is very little structured metadata to build this page from currently. You should check the - main library docs, - readme, or - Cargo.toml + main library docs, + readme, or + Cargo.toml in case the author documented the features in them.
{%- if features -%} diff --git a/templates/header/package_navigation.html b/templates/header/package_navigation.html index 5a119a96f..134d5152a 100644 --- a/templates/header/package_navigation.html +++ b/templates/header/package_navigation.html @@ -17,7 +17,7 @@
{# The partial path of the crate, `:name/:release` #} - {%- set crate_path = metadata.name ~ "/" ~ metadata.version_or_latest -%} + {%- set crate_path = metadata.name ~ "/" ~ metadata.req_version -%} {# If docs are built, show a button for them #} diff --git a/templates/rustdoc/platforms.html b/templates/rustdoc/platforms.html index 67e0cb167..1342d779e 100644 --- a/templates/rustdoc/platforms.html +++ b/templates/rustdoc/platforms.html @@ -5,10 +5,10 @@ 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_url = "/" ~ metadata.name ~ "/" ~ metadata.req_version ~ "/" ~ 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_url = "/crate/" ~ metadata.name ~ "/" ~ metadata.req_version ~ "/target-redirect/" ~ target ~ "/" ~ inner_path -%} {%- set target_no_follow = "nofollow" -%} {%- endif -%} {%- if current_target is defined and current_target == target -%} diff --git a/templates/rustdoc/topbar.html b/templates/rustdoc/topbar.html index d87efb17a..cac38371a 100644 --- a/templates/rustdoc/topbar.html +++ b/templates/rustdoc/topbar.html @@ -1,7 +1,7 @@ {%- import "macros.html" as macros -%} {# The url of the current release, `/crate/:name/:version` #} -{%- set crate_url = "/crate/" ~ metadata.name ~ "/" ~ metadata.version_or_latest -%} +{%- set crate_url = "/crate/" ~ metadata.name ~ "/" ~ metadata.req_version -%} {%- include "header/topbar_begin.html" -%}{# extra whitespace unremovable, need to use html tags unaffacted by whitespace T_T @@ -22,7 +22,7 @@ {%- include "clipboard.svg" -%} - {%- if metadata.version_or_latest == "latest" -%} + {%- if metadata.req_version == "latest" -%}