diff --git a/axum-extra/CHANGELOG.md b/axum-extra/CHANGELOG.md index 5e721961..86c14814 100644 --- a/axum-extra/CHANGELOG.md +++ b/axum-extra/CHANGELOG.md @@ -9,8 +9,20 @@ and this project adheres to [Semantic Versioning]. - **breaking:** Remove the deprecated `Host`, `Scheme` and `OptionalPath` extractors ([#3599]) +- **breaking:** Change `routing::RouterExt::route_with_tsr` to only redirect + the HTTP methods that the supplied `MethodRouter` handles. This allows the + following pattern which lead to a panic before because the two + `route_with_tsr` calls would both attempt to register a method-independent + redirect ([#3586]): + + ```rust + Router::new() + .route_with_tsr("/path", get(/* handler */)) + .route_with_tsr("/path", post(/* handler */)) + ``` [#3599]: https://github.com/tokio-rs/axum/pull/3599 +[#3586]: https://github.com/tokio-rs/axum/pull/3586 # 0.12.5 diff --git a/axum-extra/src/routing/mod.rs b/axum-extra/src/routing/mod.rs index 45cb180e..3c5c2383 100644 --- a/axum-extra/src/routing/mod.rs +++ b/axum-extra/src/routing/mod.rs @@ -3,7 +3,7 @@ use axum::{ extract::{OriginalUri, Request}, response::{IntoResponse, Redirect, Response}, - routing::{any, MethodRouter}, + routing::{any, on, MethodFilter, MethodRouter}, Router, }; use http::{uri::PathAndQuery, StatusCode, Uri}; @@ -336,8 +336,9 @@ where Self: Sized, { validate_tsr_path(path); + let method_filter = method_router.method_filter(); self = self.route(path, method_router); - add_tsr_redirect_route(self, path) + add_tsr_redirect_route(self, path, method_filter) } #[track_caller] @@ -350,7 +351,7 @@ where { validate_tsr_path(path); self = self.route_service(path, service); - add_tsr_redirect_route(self, path) + add_tsr_redirect_route(self, path, None) } } @@ -361,7 +362,11 @@ fn validate_tsr_path(path: &str) { } } -fn add_tsr_redirect_route(router: Router, path: &str) -> Router +fn add_tsr_redirect_route( + router: Router, + path: &str, + method_filter: Option, +) -> Router where S: Clone + Send + Sync + 'static, { @@ -379,11 +384,21 @@ where } } - if let Some(path_without_trailing_slash) = path.strip_suffix('/') { - router.route(path_without_trailing_slash, any(redirect_handler)) + let _slot; + let redirect_path = if let Some(without_slash) = path.strip_suffix('/') { + without_slash } else { - router.route(&format!("{path}/"), any(redirect_handler)) - } + // FIXME: Can return `&format!(...)` directly when MSRV is updated + _slot = format!("{path}/"); + &_slot + }; + + let method_router = match method_filter { + Some(f) => on(f, redirect_handler), + None => any(redirect_handler), + }; + + router.route(redirect_path, method_router) } /// Map the path of a `Uri`. @@ -417,7 +432,10 @@ mod sealed { mod tests { use super::*; use crate::test_helpers::*; - use axum::{extract::Path, routing::get}; + use axum::{ + extract::Path, + routing::{get, post}, + }; #[tokio::test] async fn test_tsr() { @@ -500,6 +518,13 @@ mod tests { assert_eq!(res.headers()["location"], "/neko/nyan/"); } + #[test] + fn tsr_independent_route_registration() { + let _: Router = Router::new() + .route_with_tsr("/x", get(|| async {})) + .route_with_tsr("/x", post(|| async {})); + } + #[test] #[should_panic = "Cannot add a trailing slash redirect route for `/`"] fn tsr_at_root() { diff --git a/axum/CHANGELOG.md b/axum/CHANGELOG.md index 2a8cceef..2cb2042f 100644 --- a/axum/CHANGELOG.md +++ b/axum/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 (never returned `Err`) and be an uninhabited type if `with_graceful_shutdown` is not used (because it was already never terminating if that method wasn't used) ([#3601]) - **added:** New `ListenerExt::limit_connections` allows limiting concurrent `axum::serve` connections ([#3489]) +- **added:** `MethodRouter::method_filter` ([#3586]) - **changed:** `serve` has an additional generic argument and can now work with any response body type, not just `axum::body::Body` ([#3205]) @@ -24,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#3478]: https://github.com/tokio-rs/axum/pull/3478 [#3601]: https://github.com/tokio-rs/axum/pull/3601 [#3489]: https://github.com/tokio-rs/axum/pull/3489 +[#3586]: https://github.com/tokio-rs/axum/pull/3586 # 0.8.8 diff --git a/axum/src/routing/method_routing.rs b/axum/src/routing/method_routing.rs index 5ac3d2b9..c274eb43 100644 --- a/axum/src/routing/method_routing.rs +++ b/axum/src/routing/method_routing.rs @@ -660,6 +660,54 @@ where self } + /// Get a [`MethodFilter`] for the methods that this `MethodRouter` has + /// custom code for. + /// + /// Note that `MethodRouter`'s [`Service`] implementation never fails (it + /// always creates an HTTP response) based on which HTTP method was used. + /// However, the information which methods have the default behavior of + /// returning HTTP 405 is stored, and can be queried with this method. + /// + /// Returns `None` if the `MethodRouter` was constructed with [`any`] or + /// has had a [`fallback`][Self::fallback] set. + pub fn method_filter(&self) -> Option { + let Self { + get, + head, + delete, + options, + patch, + post, + put, + trace, + connect, + fallback, + allow_header: _, + } = self; + + if !fallback.is_default() { + return None; + } + + let filter = [ + (get, MethodFilter::GET), + (head, MethodFilter::HEAD), + (delete, MethodFilter::DELETE), + (options, MethodFilter::OPTIONS), + (patch, MethodFilter::PATCH), + (post, MethodFilter::POST), + (put, MethodFilter::PUT), + (trace, MethodFilter::TRACE), + (connect, MethodFilter::CONNECT), + ] + .into_iter() + .filter_map(|(ep, f)| ep.is_some().then_some(f)) + .reduce(MethodFilter::or) + .expect("can't create a MethodRouter with all-default handlers"); + + Some(filter) + } + /// Add a fallback [`Handler`] if no custom one has been provided. pub(crate) fn default_fallback(self, handler: H) -> Self where @@ -839,7 +887,7 @@ where panic!( "Overlapping method route. Cannot add two method routes that both handle \ `{method_name}`", - ) + ); } *out = endpoint.clone(); for method in methods { @@ -1614,6 +1662,25 @@ mod tests { assert_eq!(text, "state"); } + #[test] + fn method_filter() { + let router: MethodRouter = get(|| async {}); + assert_eq!(router.method_filter(), Some(MethodFilter::GET)); + + let router: MethodRouter = get(|| async {}).head(|| async {}).post(|| async {}); + assert_eq!( + router.method_filter(), + Some( + MethodFilter::GET + .or(MethodFilter::HEAD) + .or(MethodFilter::POST) + ) + ); + + let router: MethodRouter = any(|| async {}); + assert_eq!(router.method_filter(), None); + } + async fn call(method: Method, svc: &mut S) -> (StatusCode, HeaderMap, String) where S: Service, diff --git a/axum/src/routing/mod.rs b/axum/src/routing/mod.rs index b1225b07..f9978333 100644 --- a/axum/src/routing/mod.rs +++ b/axum/src/routing/mod.rs @@ -740,6 +740,10 @@ where } } } + + fn is_default(&self) -> bool { + matches!(self, Self::Default(..)) + } } impl Clone for Fallback {