Disallow nesting at the root of the router (#3051)

This commit is contained in:
Jonas Platte 2024-11-26 20:59:40 +01:00 committed by GitHub
parent fb1b8f1a55
commit 11806fbc61
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 50 additions and 126 deletions

View File

@ -324,14 +324,14 @@ mod tests {
"/{*path}",
any(|req: Request| {
Router::new()
.nest("/", Router::new().route("/foo", get(|| async {})))
.nest("/foo", Router::new().route("/bar", get(|| async {})))
.oneshot(req)
}),
);
let client = TestClient::new(app);
let res = client.get("/foo").await;
let res = client.get("/foo/bar").await;
assert_eq!(res.status(), StatusCode::OK);
}

View File

@ -191,42 +191,6 @@ mod tests {
assert_eq!(res.status(), StatusCode::OK);
}
#[crate::test]
async fn nested_at_root() {
let api = Router::new().route(
"/users",
get(|nested_path: NestedPath| {
assert_eq!(nested_path.as_str(), "/");
async {}
}),
);
let app = Router::new().nest("/", api);
let client = TestClient::new(app);
let res = client.get("/users").await;
assert_eq!(res.status(), StatusCode::OK);
}
#[crate::test]
async fn deeply_nested_from_root() {
let api = Router::new().route(
"/users",
get(|nested_path: NestedPath| {
assert_eq!(nested_path.as_str(), "/api");
async {}
}),
);
let app = Router::new().nest("/", Router::new().nest("/api", api));
let client = TestClient::new(app);
let res = client.get("/api/users").await;
assert_eq!(res.status(), StatusCode::OK);
}
#[crate::test]
async fn in_fallbacks() {
let api = Router::new().fallback(get(|nested_path: NestedPath| {

View File

@ -200,6 +200,10 @@ where
#[doc(alias = "scope")] // Some web frameworks like actix-web use this term
#[track_caller]
pub fn nest(self, path: &str, router: Router<S>) -> Self {
if path.is_empty() || path == "/" {
panic!("Nesting at the root is no longer supported. Use merge instead.");
}
let RouterInner {
path_router,
fallback_router,
@ -227,6 +231,10 @@ where
T::Response: IntoResponse,
T::Future: Send + 'static,
{
if path.is_empty() || path == "/" {
panic!("Nesting at the root is no longer supported. Use fallback_service instead.");
}
tap_inner!(self, mut this => {
panic_on_err!(this.path_router.nest_service(path, service));
})

View File

@ -515,10 +515,8 @@ impl fmt::Debug for Node {
#[track_caller]
fn validate_nest_path(v7_checks: bool, path: &str) -> &str {
if path.is_empty() {
// nesting at `""` and `"/"` should mean the same thing
return "/";
}
assert!(path.starts_with('/'));
assert!(path.len() > 1);
if path.split('/').any(|segment| {
segment.starts_with("{*") && segment.ends_with('}') && !segment.ends_with("}}")

View File

@ -200,13 +200,13 @@ async fn doesnt_panic_if_used_with_nested_router() {
async fn handler() {}
let routes_static =
Router::new().nest_service("/", crate::routing::get_service(handler.into_service()));
Router::new().nest_service("/foo", crate::routing::get_service(handler.into_service()));
let routes_all = Router::new().fallback_service(routes_static);
let client = TestClient::new(routes_all);
let res = client.get("/foobar").await;
let res = client.get("/foo/bar").await;
assert_eq!(res.status(), StatusCode::OK);
}

View File

@ -600,7 +600,7 @@ async fn head_with_middleware_applied() {
let app = Router::new()
.nest(
"/",
"/foo",
Router::new().route("/", get(|| async { "Hello, World!" })),
)
.layer(CompressionLayer::new().compress_when(SizeAbove::new(0)));
@ -608,13 +608,13 @@ async fn head_with_middleware_applied() {
let client = TestClient::new(app);
// send GET request
let res = client.get("/").header("accept-encoding", "gzip").await;
let res = client.get("/foo").header("accept-encoding", "gzip").await;
assert_eq!(res.headers()["transfer-encoding"], "chunked");
// cannot have `transfer-encoding: chunked` and `content-length`
assert!(!res.headers().contains_key("content-length"));
// send HEAD request
let res = client.head("/").header("accept-encoding", "gzip").await;
let res = client.head("/foo").header("accept-encoding", "gzip").await;
// no response body so no `transfer-encoding`
assert!(!res.headers().contains_key("transfer-encoding"));
// no content-length since we cannot know it since the response

View File

@ -60,74 +60,49 @@ async fn nesting_apps() {
#[crate::test]
async fn wrong_method_nest() {
let nested_app = Router::new().route("/", get(|| async {}));
let app = Router::new().nest("/", nested_app);
let app = Router::new().nest("/foo", nested_app);
let client = TestClient::new(app);
let res = client.get("/").await;
let res = client.get("/foo").await;
assert_eq!(res.status(), StatusCode::OK);
let res = client.post("/").await;
let res = client.post("/foo").await;
assert_eq!(res.status(), StatusCode::METHOD_NOT_ALLOWED);
assert_eq!(res.headers()[ALLOW], "GET,HEAD");
let res = client.patch("/foo").await;
let res = client.patch("/foo/bar").await;
assert_eq!(res.status(), StatusCode::NOT_FOUND);
}
#[crate::test]
async fn nesting_router_at_root() {
let nested = Router::new().route("/foo", get(|uri: Uri| async move { uri.to_string() }));
let app = Router::new().nest("/", nested);
let client = TestClient::new(app);
let res = client.get("/").await;
assert_eq!(res.status(), StatusCode::NOT_FOUND);
let res = client.get("/foo").await;
assert_eq!(res.status(), StatusCode::OK);
assert_eq!(res.text().await, "/foo");
let res = client.get("/foo/bar").await;
assert_eq!(res.status(), StatusCode::NOT_FOUND);
#[test]
#[should_panic(expected = "Nesting at the root is no longer supported. Use merge instead.")]
fn nest_router_at_root() {
let nested = Router::new().route("/foo", get(|| async {}));
let _: Router = Router::new().nest("/", nested);
}
#[crate::test]
async fn nesting_router_at_empty_path() {
let nested = Router::new().route("/foo", get(|uri: Uri| async move { uri.to_string() }));
let app = Router::new().nest("", nested);
let client = TestClient::new(app);
let res = client.get("/").await;
assert_eq!(res.status(), StatusCode::NOT_FOUND);
let res = client.get("/foo").await;
assert_eq!(res.status(), StatusCode::OK);
assert_eq!(res.text().await, "/foo");
let res = client.get("/foo/bar").await;
assert_eq!(res.status(), StatusCode::NOT_FOUND);
#[test]
#[should_panic(expected = "Nesting at the root is no longer supported. Use merge instead.")]
fn nest_router_at_empty_path() {
let nested = Router::new().route("/foo", get(|| async {}));
let _: Router = Router::new().nest("", nested);
}
#[crate::test]
async fn nesting_handler_at_root() {
let app = Router::new().nest_service("/", get(|uri: Uri| async move { uri.to_string() }));
#[test]
#[should_panic(
expected = "Nesting at the root is no longer supported. Use fallback_service instead."
)]
fn nest_service_at_root() {
let _: Router = Router::new().nest_service("/", get(|| async {}));
}
let client = TestClient::new(app);
let res = client.get("/").await;
assert_eq!(res.status(), StatusCode::OK);
assert_eq!(res.text().await, "/");
let res = client.get("/foo").await;
assert_eq!(res.status(), StatusCode::OK);
assert_eq!(res.text().await, "/foo");
let res = client.get("/foo/bar").await;
assert_eq!(res.status(), StatusCode::OK);
assert_eq!(res.text().await, "/foo/bar");
#[test]
#[should_panic(
expected = "Nesting at the root is no longer supported. Use fallback_service instead."
)]
fn nest_service_at_empty_path() {
let _: Router = Router::new().nest_service("", get(|| async {}));
}
#[crate::test]
@ -227,21 +202,6 @@ async fn nested_multiple_routes() {
assert_eq!(client.get("/api/teams").await.text().await, "teams");
}
#[test]
#[should_panic = r#"Invalid route "/": Insertion failed due to conflict with previously registered route: /"#]
fn nested_service_at_root_with_other_routes() {
let _: Router = Router::new()
.nest_service("/", Router::new().route("/users", get(|| async {})))
.route("/", get(|| async {}));
}
#[test]
fn nested_at_root_with_other_routes() {
let _: Router = Router::new()
.nest("/", Router::new().route("/users", get(|| async {})))
.route("/", get(|| async {}));
}
#[crate::test]
async fn multiple_top_level_nests() {
let app = Router::new()
@ -405,18 +365,12 @@ macro_rules! nested_route_test {
}
// test cases taken from https://github.com/tokio-rs/axum/issues/714#issuecomment-1058144460
nested_route_test!(nest_1, nest = "", route = "/", expected = "/");
nested_route_test!(nest_2, nest = "", route = "/a", expected = "/a");
nested_route_test!(nest_3, nest = "", route = "/a/", expected = "/a/");
nested_route_test!(nest_4, nest = "/", route = "/", expected = "/");
nested_route_test!(nest_5, nest = "/", route = "/a", expected = "/a");
nested_route_test!(nest_6, nest = "/", route = "/a/", expected = "/a/");
nested_route_test!(nest_7, nest = "/a", route = "/", expected = "/a");
nested_route_test!(nest_8, nest = "/a", route = "/a", expected = "/a/a");
nested_route_test!(nest_9, nest = "/a", route = "/a/", expected = "/a/a/");
nested_route_test!(nest_11, nest = "/a/", route = "/", expected = "/a/");
nested_route_test!(nest_12, nest = "/a/", route = "/a", expected = "/a/a");
nested_route_test!(nest_13, nest = "/a/", route = "/a/", expected = "/a/a/");
nested_route_test!(nest_1, nest = "/a", route = "/", expected = "/a");
nested_route_test!(nest_2, nest = "/a", route = "/a", expected = "/a/a");
nested_route_test!(nest_3, nest = "/a", route = "/a/", expected = "/a/a/");
nested_route_test!(nest_4, nest = "/a/", route = "/", expected = "/a/");
nested_route_test!(nest_5, nest = "/a/", route = "/a", expected = "/a/a");
nested_route_test!(nest_6, nest = "/a/", route = "/a/", expected = "/a/a/");
#[crate::test]
#[should_panic(