From 83e1a15040bc9db30fbf90fbcf5c7a8d6adf5649 Mon Sep 17 00:00:00 2001 From: David Calavera Date: Sun, 17 Apr 2022 14:12:52 -0700 Subject: [PATCH] Fix trailing redirection with query parameters (#936) * Fix trailing redirection with query parameters When the request URI matches a route that need a trailing slash, or has an extra trailing slash, the redirect URI is not generated correctly. This change adds or removes a trailing slash to the path part of the URI, instead of the full URI, preserving query parameters during redirection. Signed-off-by: David Calavera * Make trailing slash logic safer Extract parts from Uri and recreate it, so it doesn't bump into corner cases with string manipulation. Signed-off-by: David Calavera * Remove extra assignment. Signed-off-by: David Calavera * Update axum/src/routing/mod.rs Co-authored-by: David Pedersen * changelog Co-authored-by: David Pedersen --- axum/CHANGELOG.md | 2 ++ axum/src/routing/mod.rs | 36 +++++++++++++++++++++++++++-------- axum/src/routing/tests/mod.rs | 8 ++++++++ 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/axum/CHANGELOG.md b/axum/CHANGELOG.md index c87064f4..533f7ffe 100644 --- a/axum/CHANGELOG.md +++ b/axum/CHANGELOG.md @@ -9,8 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **added:** Add `axum::extract::multipart::Field::chunk` method for streaming a single chunk from the field ([#901]) +- **fixed:** Fix trailing slash redirection with query parameters ([#936]) [#901]: https://github.com/tokio-rs/axum/pull/901 +[#936]: https://github.com/tokio-rs/axum/pull/936 # 0.5.1 (03. April, 2022) diff --git a/axum/src/routing/mod.rs b/axum/src/routing/mod.rs index 81a02700..581e9241 100644 --- a/axum/src/routing/mod.rs +++ b/axum/src/routing/mod.rs @@ -9,7 +9,7 @@ use crate::{ util::try_downcast, BoxError, }; -use http::Request; +use http::{Request, Uri}; use matchit::MatchError; use std::{ borrow::Cow, @@ -497,13 +497,20 @@ where match self.node.at(&path) { Ok(match_) => self.call_route(match_, req), - Err(MatchError::MissingTrailingSlash) => RouteFuture::from_response( - Redirect::permanent(&format!("{}/", req.uri())).into_response(), - ), - Err(MatchError::ExtraTrailingSlash) => RouteFuture::from_response( - Redirect::permanent(req.uri().to_string().strip_suffix('/').unwrap()) - .into_response(), - ), + Err(MatchError::MissingTrailingSlash) => { + let new_uri = replace_trailing_slash(req.uri(), &format!("{}/", &path)); + + RouteFuture::from_response( + Redirect::permanent(&new_uri.to_string()).into_response(), + ) + } + Err(MatchError::ExtraTrailingSlash) => { + let new_uri = replace_trailing_slash(req.uri(), &path.strip_suffix('/').unwrap()); + + RouteFuture::from_response( + Redirect::permanent(&new_uri.to_string()).into_response(), + ) + } Err(MatchError::NotFound) => match &self.fallback { Fallback::Default(inner) => inner.clone().call(req), Fallback::Custom(inner) => inner.clone().call(req), @@ -512,6 +519,19 @@ where } } +fn replace_trailing_slash(uri: &Uri, new_path: &str) -> Uri { + let mut new_path_and_query = new_path.to_string(); + if let Some(query) = uri.query() { + new_path_and_query.push('?'); + new_path_and_query.push_str(query); + } + + let mut parts = uri.clone().into_parts(); + parts.path_and_query = Some(new_path_and_query.parse().unwrap()); + + Uri::from_parts(parts).unwrap() +} + /// Wrapper around `matchit::Router` that supports merging two `Router`s. #[derive(Clone, Default)] struct Node { diff --git a/axum/src/routing/tests/mod.rs b/axum/src/routing/tests/mod.rs index 4052c653..768ffe02 100644 --- a/axum/src/routing/tests/mod.rs +++ b/axum/src/routing/tests/mod.rs @@ -324,6 +324,10 @@ async fn with_trailing_slash() { let res = client.get("/foo/").send().await; assert_eq!(res.status(), StatusCode::PERMANENT_REDIRECT); assert_eq!(res.headers().get("location").unwrap(), "/foo"); + + let res = client.get("/foo/?bar=baz").send().await; + assert_eq!(res.status(), StatusCode::PERMANENT_REDIRECT); + assert_eq!(res.headers().get("location").unwrap(), "/foo?bar=baz"); } #[tokio::test] @@ -335,6 +339,10 @@ async fn without_trailing_slash() { let res = client.get("/foo").send().await; assert_eq!(res.status(), StatusCode::PERMANENT_REDIRECT); assert_eq!(res.headers().get("location").unwrap(), "/foo/"); + + let res = client.get("/foo?bar=baz").send().await; + assert_eq!(res.status(), StatusCode::PERMANENT_REDIRECT); + assert_eq!(res.headers().get("location").unwrap(), "/foo/?bar=baz"); } #[tokio::test]