From 22931688f73e5e524f3648aa6d548fece335c008 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Fri, 19 Nov 2021 21:15:41 +0100 Subject: [PATCH] Fix `Router::merge` for overlapping routes same different methods (#543) Discovered while working on something else that we didn't properly handle this: ```rust let one = Router::new().route("/", get(|| async {})); let two = Router::new().route("/", post(|| async {})); let app = one.merge(two); ``` --- axum/src/routing/mod.rs | 20 ++++++++------------ axum/src/routing/tests/mod.rs | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/axum/src/routing/mod.rs b/axum/src/routing/mod.rs index 7448b25b..f557980c 100644 --- a/axum/src/routing/mod.rs +++ b/axum/src/routing/mod.rs @@ -244,12 +244,15 @@ where nested_at_root, } = other; - if let Err(err) = self.node.merge(node) { - self.panic_on_matchit_error(err); - } - for (id, route) in routes { - assert!(self.routes.insert(id, route).is_none()); + let path = node + .route_id_to_path + .get(&id) + .expect("no path for route id. This is a bug in axum. Please file an issue"); + self = match route { + Endpoint::MethodRouter(route) => self.route(path, route), + Endpoint::Route(route) => self.route(path, route), + }; } self.fallback = match (self.fallback, fallback) { @@ -603,13 +606,6 @@ impl Node { Ok(()) } - fn merge(&mut self, other: Node) -> Result<(), matchit::InsertError> { - for (id, path) in other.route_id_to_path { - self.insert(&*path, id)?; - } - Ok(()) - } - fn at<'n, 'p>( &'n self, path: &'p str, diff --git a/axum/src/routing/tests/mod.rs b/axum/src/routing/tests/mod.rs index 66892899..ecdd0ab8 100644 --- a/axum/src/routing/tests/mod.rs +++ b/axum/src/routing/tests/mod.rs @@ -584,3 +584,19 @@ async fn nesting_router_with_fallbacks_panics() { let app = Router::new().nest("/", one); TestClient::new(app); } + +#[tokio::test] +async fn merging_routers_with_same_paths_but_different_methods() { + let one = Router::new().route("/", get(|| async { "GET" })); + let two = Router::new().route("/", post(|| async { "POST" })); + + let client = TestClient::new(one.merge(two)); + + let res = client.get("/").send().await; + let body = res.text().await; + assert_eq!(body, "GET"); + + let res = client.post("/").send().await; + let body = res.text().await; + assert_eq!(body, "POST"); +}