Skip to content

Commit

Permalink
fix path URL-encoding for redirects on crate-details pages
Browse files Browse the repository at this point in the history
  • Loading branch information
syphar committed Mar 8, 2024
1 parent a1928f7 commit b83591a
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 4 deletions.
18 changes: 18 additions & 0 deletions src/web/crate_details.rs
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,7 @@ mod tests {
};
use anyhow::Error;
use kuchikiki::traits::TendrilSink;
use reqwest::StatusCode;
use semver::Version;
use std::collections::HashMap;

Expand Down Expand Up @@ -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(())
})
}
}
17 changes: 14 additions & 3 deletions src/web/error.rs
Original file line number Diff line number Diff line change
@@ -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::{
Expand Down Expand Up @@ -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(),
}
Expand Down Expand Up @@ -144,9 +144,20 @@ pub(crate) type AxumResult<T> = Result<T, AxumNope>;

#[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| {
Expand Down
7 changes: 7 additions & 0 deletions src/web/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
2 changes: 1 addition & 1 deletion src/web/rustdoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<RustdocRedirectorParams>,
Extension(storage): Extension<Arc<AsyncStorage>>,
Expand Down

0 comments on commit b83591a

Please sign in to comment.