diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index 5d71671ef..6d45bf6d7 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -681,6 +681,7 @@ mod tests { }; use anyhow::Error; use kuchikiki::traits::TendrilSink; + use reqwest::StatusCode; use semver::Version; use std::collections::HashMap; @@ -1667,4 +1668,21 @@ mod tests { Ok(()) }); } + + #[test] + fn test_crate_name_with_other_uri_chars() { + wrapper(|env| { + env.fake_release().name("dummy").version("1.0.0").create()?; + + assert_eq!( + env.frontend() + .get_no_redirect("/crate/dummy%3E") + .send()? + .status(), + StatusCode::FOUND + ); + + Ok(()) + }) + } } diff --git a/src/web/error.rs b/src/web/error.rs index 7759f0522..63d5ebdc4 100644 --- a/src/web/error.rs +++ b/src/web/error.rs @@ -1,7 +1,7 @@ use crate::{ db::PoolError, storage::PathNotFoundError, - web::{cache::CachePolicy, releases::Search, AxumErrorPage}, + web::{cache::CachePolicy, encode_url_path, releases::Search, AxumErrorPage}, }; use anyhow::anyhow; use axum::{ @@ -107,7 +107,7 @@ impl IntoResponse for AxumNope { web_error.into_response() } AxumNope::Redirect(target, cache_policy) => { - match super::axum_cached_redirect(&target, cache_policy) { + match super::axum_cached_redirect(&encode_url_path(&target), cache_policy) { Ok(response) => response.into_response(), Err(err) => AxumNope::InternalError(err).into_response(), } @@ -144,9 +144,20 @@ pub(crate) type AxumResult = Result; #[cfg(test)] mod tests { - use crate::test::wrapper; + use super::{AxumNope, IntoResponse}; + use crate::{test::wrapper, web::cache::CachePolicy}; use kuchikiki::traits::TendrilSink; + #[test] + fn test_redirect_error_encodes_url_path() { + let response = + AxumNope::Redirect("/something>".into(), CachePolicy::ForeverInCdnAndBrowser) + .into_response(); + + assert_eq!(response.status(), 302); + assert_eq!(response.headers().get("Location").unwrap(), "/something%3E"); + } + #[test] fn check_404_page_content_crate() { wrapper(|env| { diff --git a/src/web/mod.rs b/src/web/mod.rs index 0e94290b5..8d5b9c3d5 100644 --- a/src/web/mod.rs +++ b/src/web/mod.rs @@ -1248,4 +1248,11 @@ mod test { assert_eq!(req_version, ReqVersion::Semver(VersionReq::STAR)); assert_eq!(req_version.to_string(), "*") } + + #[test_case("/something/", "/something/")] // already valid path + #[test_case("/something>", "/something%3E")] // something to encode + #[test_case("/something%3E", "/something%3E")] // re-running doesn't change anything + fn test_encode_url_path(input: &str, expected: &str) { + assert_eq!(encode_url_path(input), expected); + } } diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index c7a8c0d31..25cb6f2a2 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -86,7 +86,7 @@ async fn try_serve_legacy_toolchain_asset( /// Handler called for `/:crate` and `/:crate/:version` URLs. Automatically redirects to the docs /// or crate details page based on whether the given crate version was successfully built. -#[instrument(skip_all)] +#[instrument(skip(storage, config, conn))] pub(crate) async fn rustdoc_redirector_handler( Path(params): Path, Extension(storage): Extension>,