Merge branch 'main' into separate-nesting-opaque-services

axum-extra/CHANGELOG.md

@@ -7,10 +7,13 @@ and this project adheres to [Semantic Versioning].
 
 # Unreleased
 
+- **added:** Add `RouterExt::route_with_tsr` for adding routes with an
+  additional "trailing slash redirect" route ([#1119])
 - **breaking:** `Resource::nest` and `Resource::nest_collection` now only
   accepts `Router`s ([#1086])
 
 [#1086]: https://github.com/tokio-rs/axum/pull/1086
+[#1119]: https://github.com/tokio-rs/axum/pull/1119
 
 # 0.3.5 (27. June, 2022)
 

axum-extra/src/routing/mod.rs

@@ -1,6 +1,13 @@
 //! Additional types for defining routes.
 
-use axum::{handler::Handler, Router};
+use axum::{
+    handler::Handler,
+    http::Request,
+    response::{Redirect, Response},
+    Router,
+};
+use std::{convert::Infallible, future::ready};
+use tower_service::Service;
 
 mod resource;
 
@@ -126,6 +133,37 @@ pub trait RouterExt<B>: sealed::Sealed {
         H: Handler<T, B>,
         T: FirstElementIs<P> + 'static,
         P: TypedPath;
+
+    /// Add another route to the router with an additional "trailing slash redirect" route.
+    ///
+    /// If you add a route _without_ a trailing slash, such as `/foo`, this method will also add a
+    /// route for `/foo/` that redirects to `/foo`.
+    ///
+    /// If you add a route _with_ a trailing slash, such as `/bar/`, this method will also add a
+    /// route for `/bar` that redirects to `/bar/`.
+    ///
+    /// This is similar to what axum 0.5.x did by default, except this explicitly adds another
+    /// route, so trying to add a `/foo/` route after calling `.route_with_tsr("/foo", /* ... */)`
+    /// will result in a panic due to route overlap.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// use axum::{Router, routing::get};
+    /// use axum_extra::routing::RouterExt;
+    ///
+    /// let app = Router::new()
+    ///     // `/foo/` will redirect to `/foo`
+    ///     .route_with_tsr("/foo", get(|| async {}))
+    ///     // `/bar` will redirect to `/bar/`
+    ///     .route_with_tsr("/bar/", get(|| async {}));
+    /// # let _: Router = app;
+    /// ```
+    fn route_with_tsr<T>(self, path: &str, service: T) -> Self
+    where
+        T: Service<Request<B>, Response = Response, Error = Infallible> + Clone + Send + 'static,
+        T::Future: Send + 'static,
+        Self: Sized;
 }
 
 impl<B> RouterExt<B> for Router<B>
@@ -211,9 +249,62 @@ where
     {
         self.route(P::PATH, axum::routing::trace(handler))
     }
+
+    fn route_with_tsr<T>(mut self, path: &str, service: T) -> Self
+    where
+        T: Service<Request<B>, Response = Response, Error = Infallible> + Clone + Send + 'static,
+        T::Future: Send + 'static,
+        Self: Sized,
+    {
+        self = self.route(path, service);
+
+        let redirect = Redirect::permanent(path);
+
+        if let Some(path_without_trailing_slash) = path.strip_suffix('/') {
+            self.route(
+                path_without_trailing_slash,
+                (move || ready(redirect.clone())).into_service(),
+            )
+        } else {
+            self.route(
+                &format!("{}/", path),
+                (move || ready(redirect.clone())).into_service(),
+            )
+        }
+    }
 }
 
 mod sealed {
     pub trait Sealed {}
     impl<B> Sealed for axum::Router<B> {}
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::test_helpers::*;
+    use axum::{http::StatusCode, routing::get};
+
+    #[tokio::test]
+    async fn test_tsr() {
+        let app = Router::new()
+            .route_with_tsr("/foo", get(|| async {}))
+            .route_with_tsr("/bar/", get(|| async {}));
+
+        let client = TestClient::new(app);
+
+        let res = client.get("/foo").send().await;
+        assert_eq!(res.status(), StatusCode::OK);
+
+        let res = client.get("/foo/").send().await;
+        assert_eq!(res.status(), StatusCode::PERMANENT_REDIRECT);
+        assert_eq!(res.headers()["location"], "/foo");
+
+        let res = client.get("/bar/").send().await;
+        assert_eq!(res.status(), StatusCode::OK);
+
+        let res = client.get("/bar").send().await;
+        assert_eq!(res.status(), StatusCode::PERMANENT_REDIRECT);
+        assert_eq!(res.headers()["location"], "/bar/");
+    }
+}

axum/CHANGELOG.md

@@ -22,10 +22,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   `/foo.js` ([#1086])
 - **fixed:** Routes like `/foo` and `/*rest` are no longer considered
   overlapping. `/foo` will take priority ([#1086])
+- **breaking:** Remove trailing slash redirects. Previously if you added a route
+  for `/foo`, axum would redirect calls to `/foo/` to `/foo` (or vice versa for
+  `/foo/`). That is no longer supported and such requests will now be sent to
+  the fallback. Consider using `axum_extra::routing::RouterExt::route_with_tsr`
+  if you want the old behavior ([#1119])
 
 [#1077]: https://github.com/tokio-rs/axum/pull/1077
 [#1086]: https://github.com/tokio-rs/axum/pull/1086
 [#1102]: https://github.com/tokio-rs/axum/pull/1102
+[#1119]: https://github.com/tokio-rs/axum/pull/1119
 [#924]: https://github.com/tokio-rs/axum/pull/924
 
 # 0.5.10 (28. June, 2022)

axum/src/routing/mod.rs

@@ -4,12 +4,12 @@ use self::{future::RouteFuture, not_found::NotFound};
 use crate::{
     body::{boxed, Body, Bytes, HttpBody},
     extract::connect_info::IntoMakeServiceWithConnectInfo,
-    response::{IntoResponse, Redirect, Response},
+    response::Response,
     routing::strip_prefix::StripPrefix,
     util::try_downcast,
     BoxError,
 };
-use http::{Request, Uri};
+use http::Request;
 use matchit::MatchError;
 use std::{
     borrow::Cow,
@@ -484,47 +484,18 @@ where
 
         match self.node.at(&path) {
             Ok(match_) => self.call_route(match_, req),
-            Err(err) => {
-                let mut fallback = match &self.fallback {
-                    Fallback::Default(inner) => inner.clone(),
-                    Fallback::Custom(inner) => inner.clone(),
-                };
-
-                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,
-                };
-
-                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
+                | MatchError::ExtraTrailingSlash
+                | MatchError::MissingTrailingSlash,
+            ) => match &self.fallback {
+                Fallback::Default(inner) => inner.clone().call(req),
+                Fallback::Custom(inner) => inner.clone().call(req),
+            },
         }
     }
 }
 
-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('?');
-        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).ok()
-}
-
 /// Wrapper around `matchit::Router` that supports merging two `Router`s.
 #[derive(Clone, Default)]
 struct Node {
@@ -630,14 +601,3 @@ 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");
-}

axum/src/routing/route.rs

@@ -112,16 +112,6 @@ impl<B, E> RouteFuture<B, E> {
         }
     }
 
-    pub(super) fn from_response(response: Response) -> Self {
-        Self {
-            kind: RouteFutureKind::Response {
-                response: Some(response),
-            },
-            strip_body: false,
-            allow_header: None,
-        }
-    }
-
     pub(crate) fn strip_body(mut self, strip_body: bool) -> Self {
         self.strip_body = strip_body;
         self

axum/src/routing/tests/mod.rs

@@ -315,33 +315,26 @@ async fn middleware_applies_to_routes_above() {
 }
 
 #[tokio::test]
-async fn with_trailing_slash() {
+async fn not_found_for_extra_trailing_slash() {
     let app = Router::new().route("/foo", get(|| async {}));
 
     let client = TestClient::new(app);
 
     let res = client.get("/foo/").send().await;
-    assert_eq!(res.status(), StatusCode::PERMANENT_REDIRECT);
-    assert_eq!(res.headers().get("location").unwrap(), "/foo");
+    assert_eq!(res.status(), StatusCode::NOT_FOUND);
 
-    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");
+    let res = client.get("/foo").send().await;
+    assert_eq!(res.status(), StatusCode::OK);
 }
 
 #[tokio::test]
-async fn without_trailing_slash() {
+async fn not_found_for_missing_trailing_slash() {
     let app = Router::new().route("/foo/", get(|| async {}));
 
     let client = TestClient::new(app);
 
     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");
+    assert_eq!(res.status(), StatusCode::NOT_FOUND);
 }
 
 #[tokio::test]
@@ -361,31 +354,7 @@ async fn with_and_without_trailing_slash() {
     assert_eq!(res.text().await, "without tsr");
 }
 
-// for https://github.com/tokio-rs/axum/issues/681
-#[tokio::test]
-async fn with_trailing_slash_post() {
-    let app = Router::new().route("/foo", post(|| async {}));
-
-    let client = TestClient::new(app);
-
-    // `TestClient` automatically follows redirects
-    let res = client.post("/foo/").send().await;
-    assert_eq!(res.status(), StatusCode::PERMANENT_REDIRECT);
-    assert_eq!(res.headers().get("location").unwrap(), "/foo");
-}
-
-// for https://github.com/tokio-rs/axum/issues/681
-#[tokio::test]
-async fn without_trailing_slash_post() {
-    let app = Router::new().route("/foo/", post(|| async {}));
-
-    let client = TestClient::new(app);
-
-    let res = client.post("/foo").send().await;
-    assert_eq!(res.status(), StatusCode::PERMANENT_REDIRECT);
-    assert_eq!(res.headers().get("location").unwrap(), "/foo/");
-}
-
+// for https://github.com/tokio-rs/axum/issues/420
 #[tokio::test]
 async fn wildcard_doesnt_match_just_trailing_slash() {
     let app = Router::new().route(