dnrops
/
axum
mirror of https://github.com/tokio-rs/axum
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove trailing slash redirects (#1119) 

* Prepare axum-next branch

* Remove deprecated `extractor_middleware` function (#1077)

* Allow `Error: Into<Infallible>` for `Route::{layer, route_layer}` (#948)

* Allow `Error: Into<Infallible>` for `Route::{layer, route_layer}` (#924)

* Allow `Error: Into<Infallible>` for `Route::{layer, route_layer}`

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

* changelog

* fixup changelog

* Panic on overlapping routes in `MethodRouter` (#1102)

* Panic on overlapping routes in `MethodRouter`

* changelog link

* add test to ensure `head` and `get` don't overlap

* Fix changelog

* Prepare axum-next branch

* Remove trailing slash redirects

* changelog link

* Fix changelog

* remove asserting to make make the test more clear

* remove tsr related feature

* Add `RouterExt::route_with_tsr`

* Apply suggestions from code review

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

* Update axum-extra/src/routing/mod.rs

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

* fix typos in docs

* Update axum/CHANGELOG.md

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

* mention `RouterExt::route_with_tsr` in the changelog

Co-authored-by: Jonas Platte <jplatte+git@posteo.de>
This commit is contained in:
David Pedersen 2022-06-30 00:22:43 +02:00 committed by GitHub
parent 23808f72a2
commit a4c820420d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 134 additions and 116 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
axum-extra/CHANGELOG.md
View File

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

93
axum-extra/src/routing/mod.rs
View File

@ -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/");
}
}

6
axum/CHANGELOG.md
View File

@ -11,9 +11,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
Use `axum::middleware::from_extractor` instead ([#1077])
- **breaking:** Allow `Error: Into<Infallible>` for `Route::{layer, route_layer}` ([#924])
- **breaking:** `MethodRouter` now panics on overlapping routes ([#1102])
- **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
[#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)

60
axum/src/routing/mod.rs
View File

@ -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,
@ -499,47 +499,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 {
@ -645,14 +616,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");
}

10
axum/src/routing/route.rs
View File

@ -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

47
axum/src/routing/tests/mod.rs
View File

@ -316,33 +316,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]
@ -362,31 +355,6 @@ 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_with_trailing_slash() {
@ -414,8 +382,7 @@ async fn wildcard_with_trailing_slash() {
)
.await
.unwrap();
assert_eq!(res.status(), StatusCode::PERMANENT_REDIRECT);
assert_eq!(res.headers()["location"], "/user1/repo1/tree/");
assert_eq!(res.status(), StatusCode::NOT_FOUND);
// check that the params are deserialized correctly
let client = TestClient::new(app);

29
axum/src/routing/tests/nest.rs
View File

@ -434,20 +434,21 @@ 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/");
// This case is different for opaque services.
//
// The internal route becomes `/a/*__private__axum_nest_tail_param` which, according to matchit
// doesn't match `/a`. However matchit detects that a route for `/a/` exists and so it issues a
// redirect to `/a/`, which ends up calling the inner route as expected.
//
// So while the behavior isn't identical, the outcome is the same
nested_route_test!(
nest_7,
nest = "/a",
route = "/",
expected = "/a",
opaque_redirect = true,
);
// TODO(david): we'll revisit this in https://github.com/tokio-rs/axum/pull/1086
//// This case is different for opaque services.
////
//// The internal route becomes `/a/*__private__axum_nest_tail_param` which, according to matchit
//// doesn't match `/a`. However matchit detects that a route for `/a/` exists and so it issues a
//// redirect to `/a/`, which ends up calling the inner route as expected.
////
//// So while the behavior isn't identical, the outcome is the same
//nested_route_test!(
// nest_7,
// nest = "/a",
// route = "/",
// expected = "/a",
// opaque_redirect = true,
//);
nested_route_test!(nest_8, nest = "/a", route = "/a", expected = "/a/a");
nested_route_test!(nest_9, nest = "/a", route = "/a/", expected = "/a/a/");