diff --git a/axum/CHANGELOG.md b/axum/CHANGELOG.md index f7b32d26..43891cc2 100644 --- a/axum/CHANGELOG.md +++ b/axum/CHANGELOG.md @@ -9,8 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **fixed:** Fix using incorrect path prefix when nesting `Router`s at `/` ([#691]) - **fixed:** Make `nest("", service)` work and mean the same as `nest("/", service)` ([#691]) +- **fixed:** Replace response code `301` with `308` for trailing slash redirects. Also deprecates + `Redirect::found` (`302`) in favor of `Redirect::temporary` (`307`) or `Redirect::to` (`303`). + This is to prevent clients from changing non-`GET` requests to `GET` requests ([#682]) [#691]: https://github.com/tokio-rs/axum/pull/691 +[#682]: https://github.com/tokio-rs/axum/pull/682 # 0.4.3 (21. December, 2021) diff --git a/axum/src/response/redirect.rs b/axum/src/response/redirect.rs index 482e4606..c79bf4c3 100644 --- a/axum/src/response/redirect.rs +++ b/axum/src/response/redirect.rs @@ -33,7 +33,8 @@ impl Redirect { /// This redirect instructs the client to change the method to GET for the subsequent request /// to the given `uri`, which is useful after successful form submission, file upload or when /// you generally don't want the redirected-to page to observe the original request method and - /// body (if non-empty). + /// body (if non-empty). If you want to preserve the request method and body, + /// [`Redirect::temporary`] should be used instead. /// /// # Panics /// @@ -71,16 +72,21 @@ impl Redirect { /// Create a new [`Redirect`] that uses a [`302 Found`][mdn] status code. /// - /// This is the same as [`Redirect::temporary`], except the status code is older and thus - /// supported by some legacy applications that doesn't understand the newer one, but some of - /// those applications wrongly apply [`Redirect::to`] (`303 See Other`) semantics for this - /// status code. It should be avoided where possible. + /// This is the same as [`Redirect::temporary`] ([`307 Temporary Redirect`][mdn307]) except + /// this status code is older and thus supported by some legacy clients that don't understand + /// the newer one. Many clients wrongly apply [`Redirect::to`] ([`303 See Other`][mdn303]) + /// semantics for this status code, so it should be avoided where possible. /// /// # Panics /// /// If `uri` isn't a valid [`HeaderValue`]. /// /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/302 + /// [mdn307]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/307 + /// [mdn303]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/303 + #[deprecated( + note = "This results in different behavior between clients, so Redirect::temporary or Redirect::to should be used instead" + )] pub fn found(uri: Uri) -> Self { Self::with_status_code(StatusCode::FOUND, uri) } diff --git a/axum/src/routing/mod.rs b/axum/src/routing/mod.rs index eb5b0b94..08c0c37b 100644 --- a/axum/src/routing/mod.rs +++ b/axum/src/routing/mod.rs @@ -7,12 +7,14 @@ use crate::{ connect_info::{Connected, IntoMakeServiceWithConnectInfo}, MatchedPath, OriginalUri, }, + response::IntoResponse, + response::Redirect, response::Response, routing::strip_prefix::StripPrefix, util::{try_downcast, ByteStr, PercentDecodedByteStr}, BoxError, }; -use http::{Request, StatusCode, Uri}; +use http::{Request, Uri}; use std::{ borrow::Cow, collections::HashMap, @@ -490,12 +492,8 @@ where } else { with_path(req.uri(), &format!("{}/", path)) }; - let res = Response::builder() - .status(StatusCode::MOVED_PERMANENTLY) - .header(http::header::LOCATION, redirect_to.to_string()) - .body(crate::body::empty()) - .unwrap(); - RouterFuture::from_response(res) + let res = Redirect::permanent(redirect_to); + RouterFuture::from_response(res.into_response()) } else { match &self.fallback { Fallback::Default(inner) => { diff --git a/axum/src/routing/tests/mod.rs b/axum/src/routing/tests/mod.rs index 3577387b..0e32cf8c 100644 --- a/axum/src/routing/tests/mod.rs +++ b/axum/src/routing/tests/mod.rs @@ -354,6 +354,30 @@ 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::OK); +} + +// 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); + + // `TestClient` automatically follows redirects + let res = client.post("/foo").send().await; + assert_eq!(res.status(), StatusCode::OK); +} + // for https://github.com/tokio-rs/axum/issues/420 #[tokio::test] async fn wildcard_with_trailing_slash() { @@ -381,7 +405,7 @@ async fn wildcard_with_trailing_slash() { ) .await .unwrap(); - assert_eq!(res.status(), StatusCode::MOVED_PERMANENTLY); + assert_eq!(res.status(), StatusCode::PERMANENT_REDIRECT); assert_eq!(res.headers()["location"], "/user1/repo1/tree/"); // check that the params are deserialized correctly diff --git a/examples/oauth/Cargo.toml b/examples/oauth/Cargo.toml index 5b1d3c30..a9926f51 100644 --- a/examples/oauth/Cargo.toml +++ b/examples/oauth/Cargo.toml @@ -15,3 +15,4 @@ serde = { version = "1.0", features = ["derive"] } # Use Rustls because it makes it easier to cross-compile on CI reqwest = { version = "0.11", default-features = false, features = ["rustls-tls", "json"] } headers = "0.3" +http = "0.2" diff --git a/examples/oauth/src/main.rs b/examples/oauth/src/main.rs index a6a70f19..b469c123 100644 --- a/examples/oauth/src/main.rs +++ b/examples/oauth/src/main.rs @@ -1,20 +1,26 @@ //! Example OAuth (Discord) implementation. //! -//! Run with -//! +//! 1) Create a new application at +//! 2) Visit the OAuth2 tab to get your CLIENT_ID and CLIENT_SECRET +//! 3) Add a new redirect URI (for this example: `http://127.0.0.1:3000/auth/authorized`) +//! 4) Run with the following (replacing values appropriately): //! ```not_rust -//! CLIENT_ID=123 CLIENT_SECRET=secret cargo run -p example-oauth +//! CLIENT_ID=REPLACE_ME CLIENT_SECRET=REPLACE_ME cargo run -p example-oauth //! ``` use async_session::{MemoryStore, Session, SessionStore}; use axum::{ async_trait, - extract::{Extension, FromRequest, Query, RequestParts, TypedHeader}, + extract::{ + rejection::TypedHeaderRejectionReason, Extension, FromRequest, Query, RequestParts, + TypedHeader, + }, http::{header::SET_COOKIE, HeaderMap}, response::{IntoResponse, Redirect, Response}, routing::get, AddExtensionLayer, Router, }; +use http::header; use oauth2::{ basic::BasicClient, reqwest::async_http_client, AuthUrl, AuthorizationCode, ClientId, ClientSecret, CsrfToken, RedirectUrl, Scope, TokenResponse, TokenUrl, @@ -22,13 +28,6 @@ use oauth2::{ use serde::{Deserialize, Serialize}; use std::{env, net::SocketAddr}; -// Quick instructions: -// 1) create a new application at https://discord.com/developers/applications -// 2) visit the OAuth2 tab to get your CLIENT_ID and CLIENT_SECRET -// 3) add a new redirect URI (For this example: http://localhost:3000/auth/authorized) -// 4) AUTH_URL and TOKEN_URL may stay the same for discord. -// More information: https://discord.com/developers/applications/792730475856527411/oauth2 - static COOKIE_NAME: &str = "SESSION"; #[tokio::main] @@ -39,7 +38,7 @@ async fn main() { } tracing_subscriber::fmt::init(); - // `MemoryStore` just used as an example. Don't use this in production. + // `MemoryStore` is just used as an example. Don't use this in production. let store = MemoryStore::new(); let oauth_client = oauth_client(); @@ -64,8 +63,8 @@ async fn main() { fn oauth_client() -> BasicClient { // Environment variables (* = required): - // *"CLIENT_ID" "123456789123456789"; - // *"CLIENT_SECRET" "rAn60Mch4ra-CTErsSf-r04utHcLienT"; + // *"CLIENT_ID" "REPLACE_ME"; + // *"CLIENT_SECRET" "REPLACE_ME"; // "REDIRECT_URL" "http://127.0.0.1:3000/auth/authorized"; // "AUTH_URL" "https://discord.com/api/oauth2/authorize?response_type=code"; // "TOKEN_URL" "https://discord.com/api/oauth2/token"; @@ -119,7 +118,7 @@ async fn discord_auth(Extension(client): Extension) -> impl IntoRes .url(); // Redirect to Discord's oauth service - Redirect::found(auth_url.to_string().parse().unwrap()) + Redirect::to(auth_url.to_string().parse().unwrap()) } // Valid user session required. If there is none, redirect to the auth page @@ -138,12 +137,12 @@ async fn logout( let session = match store.load_session(cookie.to_string()).await.unwrap() { Some(s) => s, // No session active, just redirect - None => return Redirect::found("/".parse().unwrap()), + None => return Redirect::to("/".parse().unwrap()), }; store.destroy_session(session).await.unwrap(); - Redirect::found("/".parse().unwrap()) + Redirect::to("/".parse().unwrap()) } #[derive(Debug, Deserialize)] @@ -192,14 +191,14 @@ async fn login_authorized( let mut headers = HeaderMap::new(); headers.insert(SET_COOKIE, cookie.parse().unwrap()); - (headers, Redirect::found("/".parse().unwrap())) + (headers, Redirect::to("/".parse().unwrap())) } struct AuthRedirect; impl IntoResponse for AuthRedirect { fn into_response(self) -> Response { - Redirect::found("/auth/discord".parse().unwrap()).into_response() + Redirect::temporary("/auth/discord".parse().unwrap()).into_response() } } @@ -218,8 +217,13 @@ where let cookies = TypedHeader::::from_request(req) .await - .expect("could not get cookies"); - + .map_err(|e| match *e.name() { + header::COOKIE => match e.reason() { + TypedHeaderRejectionReason::Missing => AuthRedirect, + _ => panic!("unexpected error getting Cookie header(s): {}", e), + }, + _ => panic!("unexpected error getting cookies: {}", e), + })?; let session_cookie = cookies.get(COOKIE_NAME).ok_or(AuthRedirect)?; let session = store