Fix possible panic when doing trailing slash redirect (#1124)

* Fix possible panic when doing trailing slash redirect

Fixes https://github.com/tokio-rs/axum/issues/1122

* changelog link

* Update axum/src/routing/mod.rs

Co-authored-by: Jonas Platte <jplatte+git@posteo.de>

Co-authored-by: Jonas Platte <jplatte+git@posteo.de>

axum/CHANGELOG.md

@@ -8,8 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 # Unreleased
 
 - **fixed:** Make `Router` cheaper to clone ([#1123])
+- **fixed:** Fix possible panic when doing trailing slash redirect ([#1124])
 
 [#1123]: https://github.com/tokio-rs/axum/pull/1123
+[#1124]: https://github.com/tokio-rs/axum/pull/1124
 
 # 0.5.9 (20. June, 2022)
 

axum/src/routing/mod.rs

@@ -500,29 +500,35 @@ where
 
         match self.node.at(&path) {
             Ok(match_) => self.call_route(match_, req),
-            Err(MatchError::MissingTrailingSlash) => {
-                let new_uri = replace_trailing_slash(req.uri(), &format!("{}/", &path));
+            Err(err) => {
+                let mut fallback = match &self.fallback {
+                    Fallback::Default(inner) => inner.clone(),
+                    Fallback::Custom(inner) => inner.clone(),
+                };
 
-                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());
+                let new_uri = match err {
+                    MatchError::MissingTrailingSlash => {
+                        replace_path(req.uri(), &format!("{}/", &path))
+                    }
+                    MatchError::ExtraTrailingSlash => {
+                        replace_path(req.uri(), path.strip_suffix('/').unwrap())
+                    }
+                    MatchError::NotFound => None,
+                };
 
-                RouteFuture::from_response(
-                    Redirect::permanent(&new_uri.to_string()).into_response(),
-                )
+                if let Some(new_uri) = new_uri {
+                    RouteFuture::from_response(
+                        Redirect::permanent(&new_uri.to_string()).into_response(),
+                    )
+                } else {
+                    fallback.call(req)
+                }
             }
-            Err(MatchError::NotFound) => match &self.fallback {
-                Fallback::Default(inner) => inner.clone().call(req),
-                Fallback::Custom(inner) => inner.clone().call(req),
-            },
         }
     }
 }
 
-fn replace_trailing_slash(uri: &Uri, new_path: &str) -> Uri {
+fn replace_path(uri: &Uri, new_path: &str) -> Option<Uri> {
     let mut new_path_and_query = new_path.to_owned();
     if let Some(query) = uri.query() {
         new_path_and_query.push('?');
@@ -532,7 +538,7 @@ fn replace_trailing_slash(uri: &Uri, new_path: &str) -> Uri {
     let mut parts = uri.clone().into_parts();
     parts.path_and_query = Some(new_path_and_query.parse().unwrap());
 
-    Uri::from_parts(parts).unwrap()
+    Uri::from_parts(parts).ok()
 }
 
 /// Wrapper around `matchit::Router` that supports merging two `Router`s.
@@ -640,3 +646,14 @@ fn traits() {
     use crate::test_helpers::*;
     assert_send::<Router<()>>();
 }
+
+// https://github.com/tokio-rs/axum/issues/1122
+#[test]
+fn test_replace_trailing_slash() {
+    let uri = "api.ipify.org:80".parse::<Uri>().unwrap();
+    assert!(uri.scheme().is_none());
+    assert_eq!(uri.authority(), Some(&"api.ipify.org:80".parse().unwrap()));
+    assert!(uri.path_and_query().is_none());
+
+    replace_path(&uri, "/foo");
+}