From 56215e71bfb1b28cb79155078024f76df2d8b9cf Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Sun, 22 May 2022 10:43:35 +0200 Subject: [PATCH 1/9] Automatically handle `http_body::LengthLimitError` --- axum-core/CHANGELOG.md | 6 ++ axum-core/Cargo.toml | 2 +- axum-core/src/extract/rejection.rs | 73 ++++++++++++++++++++++-- axum/Cargo.toml | 2 +- axum/src/extract/content_length_limit.rs | 5 ++ axum/src/routing/tests/mod.rs | 17 ++++++ 6 files changed, 97 insertions(+), 8 deletions(-) diff --git a/axum-core/CHANGELOG.md b/axum-core/CHANGELOG.md index 94b260945b..1ccd3c3610 100644 --- a/axum-core/CHANGELOG.md +++ b/axum-core/CHANGELOG.md @@ -7,8 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 # Unreleased +- **added:** Automatically handle `http_body::LengthLimitError` in `FailedToBufferBody` and map + such errors to `413 Payload Too Large` ([#1048]) +- **added:** `FailedToBufferBody::is_length_limit_error` to check if the underlying error is + `http_body::LengthLimitError`. Its source error can also be downcast to + `http_body::LengthLimitError` ([#1048]) - **fixed:** Use `impl IntoResponse` less in docs ([#1049]) +[#1048]: https://github.com/tokio-rs/axum/pull/1048 [#1049]: https://github.com/tokio-rs/axum/pull/1049 # 0.2.4 (02. May, 2022) diff --git a/axum-core/Cargo.toml b/axum-core/Cargo.toml index 9ce2a6423a..2b817302c7 100644 --- a/axum-core/Cargo.toml +++ b/axum-core/Cargo.toml @@ -15,7 +15,7 @@ async-trait = "0.1" bytes = "1.0" futures-util = { version = "0.3", default-features = false, features = ["alloc"] } http = "0.2.7" -http-body = "0.4" +http-body = "0.4.5" mime = "0.3.16" [dev-dependencies] diff --git a/axum-core/src/extract/rejection.rs b/axum-core/src/extract/rejection.rs index fc81eda7db..068799e554 100644 --- a/axum-core/src/extract/rejection.rs +++ b/axum-core/src/extract/rejection.rs @@ -2,6 +2,7 @@ use crate::response::{IntoResponse, Response}; use http::StatusCode; +use http_body::LengthLimitError; use std::fmt; /// Rejection type used if you try and extract the request body more than @@ -28,12 +29,72 @@ impl fmt::Display for BodyAlreadyExtracted { impl std::error::Error for BodyAlreadyExtracted {} -define_rejection! { - #[status = BAD_REQUEST] - #[body = "Failed to buffer the request body"] - /// Rejection type for extractors that buffer the request body. Used if the - /// request body cannot be buffered due to an error. - pub struct FailedToBufferBody(Error); +/// Rejection type for extractors that buffer the request body. Used if the +/// request body cannot be buffered due to an error. +// TODO: in next major for axum-core make this a #[non_exhaustive] enum so we don't need the +// additional indirection +#[derive(Debug)] +pub struct FailedToBufferBody(FailedToBufferBodyInner); + +impl FailedToBufferBody { + /// Check if the body failed to be buffered because a length limit was hit. + /// + /// This can _only_ happen when you're using [`tower_http::limit::RequestBodyLimitLayer`] or + /// otherwise wrapping request bodies in [`http_body::Limited`]. + pub fn is_length_limit_error(&self) -> bool { + matches!(self.0, FailedToBufferBodyInner::LengthLimitError(_)) + } +} + +#[derive(Debug)] +enum FailedToBufferBodyInner { + Unknown(crate::Error), + LengthLimitError(LengthLimitError), +} + +impl FailedToBufferBody { + pub(crate) fn from_err(err: E) -> Self + where + E: Into, + { + let err = err.into(); + match err.downcast::() { + Ok(err) => Self(FailedToBufferBodyInner::LengthLimitError(*err)), + Err(err) => Self(FailedToBufferBodyInner::Unknown(crate::Error::new(err))), + } + } +} + +impl crate::response::IntoResponse for FailedToBufferBody { + fn into_response(self) -> crate::response::Response { + match self.0 { + FailedToBufferBodyInner::Unknown(err) => ( + http::StatusCode::BAD_REQUEST, + format!(concat!("Failed to buffer the request body", ": {}"), err), + ) + .into_response(), + FailedToBufferBodyInner::LengthLimitError(err) => ( + StatusCode::PAYLOAD_TOO_LARGE, + format!(concat!("Failed to buffer the request body", ": {}"), err), + ) + .into_response(), + } + } +} + +impl std::fmt::Display for FailedToBufferBody { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Failed to buffer the request body") + } +} + +impl std::error::Error for FailedToBufferBody { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match &self.0 { + FailedToBufferBodyInner::Unknown(err) => Some(err), + FailedToBufferBodyInner::LengthLimitError(err) => Some(err), + } + } } define_rejection! { diff --git a/axum/Cargo.toml b/axum/Cargo.toml index e575543d88..b5b7347c47 100644 --- a/axum/Cargo.toml +++ b/axum/Cargo.toml @@ -81,7 +81,7 @@ features = [ ] [dev-dependencies.tower-http] -version = "0.3.0" +version = "0.3.4" features = ["full"] [package.metadata.docs.rs] diff --git a/axum/src/extract/content_length_limit.rs b/axum/src/extract/content_length_limit.rs index 6412324b2c..bc74f3572e 100644 --- a/axum/src/extract/content_length_limit.rs +++ b/axum/src/extract/content_length_limit.rs @@ -29,6 +29,11 @@ use std::ops::Deref; /// ``` /// /// This requires the request to have a `Content-Length` header. +/// +/// If you want to limit the size of request bodies without requiring a `Content-Length` header +/// consider using [`tower_http::limit::RequestBodyLimitLayer`]. +// TODO(david): change this to apply `http_body::Limited` so it also supports streaming requests. +// Will probably require renaming it. #[derive(Debug, Clone)] pub struct ContentLengthLimit(pub T); diff --git a/axum/src/routing/tests/mod.rs b/axum/src/routing/tests/mod.rs index 768ffe0261..d112ae7af6 100644 --- a/axum/src/routing/tests/mod.rs +++ b/axum/src/routing/tests/mod.rs @@ -699,3 +699,20 @@ async fn routes_must_start_with_slash() { let app = Router::new().route(":foo", get(|| async {})); TestClient::new(app); } + +#[tokio::test] +async fn limited_body() { + const LIMIT: usize = 3; + + let app = Router::new() + .route("/", post(|_: Bytes| async {})) + .layer(tower_http::limit::RequestBodyLimitLayer::new(LIMIT)); + + let client = TestClient::new(app); + + let res = client.post("/").body("a".repeat(LIMIT)).send().await; + assert_eq!(res.status(), StatusCode::OK); + + let res = client.post("/").body("a".repeat(LIMIT * 2)).send().await; + assert_eq!(res.status(), StatusCode::PAYLOAD_TOO_LARGE); +} From 68e6c5998d9dc04d735fbd0a1170ed338b5e7b70 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Mon, 6 Jun 2022 21:04:13 +0200 Subject: [PATCH 2/9] add tower-http dev dep to axum-core --- axum-core/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/axum-core/Cargo.toml b/axum-core/Cargo.toml index 2b817302c7..f5449b8d08 100644 --- a/axum-core/Cargo.toml +++ b/axum-core/Cargo.toml @@ -23,3 +23,4 @@ futures-util = "0.3" axum = { path = "../axum", version = "0.5" } hyper = "0.14" tokio = { version = "1.0", features = ["macros"] } +tower-http = { version = "0.3.4", features = ["limit"] } From e99186c5459f4267a3ca6f0d794102ac9e1ef9fa Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Mon, 6 Jun 2022 22:23:36 +0200 Subject: [PATCH 3/9] just make it a link --- axum-core/Cargo.toml | 1 - axum-core/src/extract/rejection.rs | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/axum-core/Cargo.toml b/axum-core/Cargo.toml index f5449b8d08..2b817302c7 100644 --- a/axum-core/Cargo.toml +++ b/axum-core/Cargo.toml @@ -23,4 +23,3 @@ futures-util = "0.3" axum = { path = "../axum", version = "0.5" } hyper = "0.14" tokio = { version = "1.0", features = ["macros"] } -tower-http = { version = "0.3.4", features = ["limit"] } diff --git a/axum-core/src/extract/rejection.rs b/axum-core/src/extract/rejection.rs index 068799e554..5ec92a0ae7 100644 --- a/axum-core/src/extract/rejection.rs +++ b/axum-core/src/extract/rejection.rs @@ -41,6 +41,8 @@ impl FailedToBufferBody { /// /// This can _only_ happen when you're using [`tower_http::limit::RequestBodyLimitLayer`] or /// otherwise wrapping request bodies in [`http_body::Limited`]. + /// + /// [`tower_http::limit::RequestBodyLimitLayer`]: https://docs.rs/tower-http/latest/tower_http/limit/struct.RequestBodyLimitLayer.html pub fn is_length_limit_error(&self) -> bool { matches!(self.0, FailedToBufferBodyInner::LengthLimitError(_)) } From 58f9e064ae7da74caf903d729a101d64aac2a68c Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Wed, 8 Jun 2022 15:17:18 +0200 Subject: [PATCH 4/9] Make `FailedToBufferBody` an enum --- axum-core/CHANGELOG.md | 3 -- axum-core/src/extract/rejection.rs | 81 +++++++++--------------------- axum-core/src/macros.rs | 1 + 3 files changed, 25 insertions(+), 60 deletions(-) diff --git a/axum-core/CHANGELOG.md b/axum-core/CHANGELOG.md index 1ccd3c3610..091ff85514 100644 --- a/axum-core/CHANGELOG.md +++ b/axum-core/CHANGELOG.md @@ -9,9 +9,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **added:** Automatically handle `http_body::LengthLimitError` in `FailedToBufferBody` and map such errors to `413 Payload Too Large` ([#1048]) -- **added:** `FailedToBufferBody::is_length_limit_error` to check if the underlying error is - `http_body::LengthLimitError`. Its source error can also be downcast to - `http_body::LengthLimitError` ([#1048]) - **fixed:** Use `impl IntoResponse` less in docs ([#1049]) [#1048]: https://github.com/tokio-rs/axum/pull/1048 diff --git a/axum-core/src/extract/rejection.rs b/axum-core/src/extract/rejection.rs index 5ec92a0ae7..e82e098e53 100644 --- a/axum-core/src/extract/rejection.rs +++ b/axum-core/src/extract/rejection.rs @@ -1,8 +1,10 @@ //! Rejection response types. -use crate::response::{IntoResponse, Response}; +use crate::{ + response::{IntoResponse, Response}, + BoxError, +}; use http::StatusCode; -use http_body::LengthLimitError; use std::fmt; /// Rejection type used if you try and extract the request body more than @@ -29,74 +31,39 @@ impl fmt::Display for BodyAlreadyExtracted { impl std::error::Error for BodyAlreadyExtracted {} -/// Rejection type for extractors that buffer the request body. Used if the -/// request body cannot be buffered due to an error. -// TODO: in next major for axum-core make this a #[non_exhaustive] enum so we don't need the -// additional indirection -#[derive(Debug)] -pub struct FailedToBufferBody(FailedToBufferBodyInner); - -impl FailedToBufferBody { - /// Check if the body failed to be buffered because a length limit was hit. - /// - /// This can _only_ happen when you're using [`tower_http::limit::RequestBodyLimitLayer`] or - /// otherwise wrapping request bodies in [`http_body::Limited`]. - /// - /// [`tower_http::limit::RequestBodyLimitLayer`]: https://docs.rs/tower-http/latest/tower_http/limit/struct.RequestBodyLimitLayer.html - pub fn is_length_limit_error(&self) -> bool { - matches!(self.0, FailedToBufferBodyInner::LengthLimitError(_)) +composite_rejection! { + /// Rejection type for extractors that buffer the request body. Used if the + /// request body cannot be buffered due to an error. + pub enum FailedToBufferBody { + LengthLimitError, + UnknownBodyError, } } -#[derive(Debug)] -enum FailedToBufferBodyInner { - Unknown(crate::Error), - LengthLimitError(LengthLimitError), -} - impl FailedToBufferBody { pub(crate) fn from_err(err: E) -> Self where - E: Into, + E: Into, { - let err = err.into(); - match err.downcast::() { - Ok(err) => Self(FailedToBufferBodyInner::LengthLimitError(*err)), - Err(err) => Self(FailedToBufferBodyInner::Unknown(crate::Error::new(err))), + match err.into().downcast::() { + Ok(err) => Self::LengthLimitError(LengthLimitError::from_err(err)), + Err(err) => Self::UnknownBodyError(UnknownBodyError::from_err(err)), } } } -impl crate::response::IntoResponse for FailedToBufferBody { - fn into_response(self) -> crate::response::Response { - match self.0 { - FailedToBufferBodyInner::Unknown(err) => ( - http::StatusCode::BAD_REQUEST, - format!(concat!("Failed to buffer the request body", ": {}"), err), - ) - .into_response(), - FailedToBufferBodyInner::LengthLimitError(err) => ( - StatusCode::PAYLOAD_TOO_LARGE, - format!(concat!("Failed to buffer the request body", ": {}"), err), - ) - .into_response(), - } - } -} - -impl std::fmt::Display for FailedToBufferBody { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "Failed to buffer the request body") - } +define_rejection! { + #[status = PAYLOAD_TOO_LARGE] + #[body = "Failed to buffer the request body"] + /// Encountered some other error when buffering the body. + pub struct LengthLimitError(Error); } -impl std::error::Error for FailedToBufferBody { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match &self.0 { - FailedToBufferBodyInner::Unknown(err) => Some(err), - FailedToBufferBodyInner::LengthLimitError(err) => Some(err), - } - } +define_rejection! { + #[status = BAD_REQUEST] + #[body = "Failed to buffer the request body"] + /// Encountered some other error when buffering the body. + pub struct UnknownBodyError(Error); } define_rejection! { diff --git a/axum-core/src/macros.rs b/axum-core/src/macros.rs index ee0bfd7bf9..9ea64cdcfd 100644 --- a/axum-core/src/macros.rs +++ b/axum-core/src/macros.rs @@ -10,6 +10,7 @@ macro_rules! define_rejection { pub struct $name(pub(crate) crate::Error); impl $name { + #[allow(dead_code)] pub(crate) fn from_err(err: E) -> Self where E: Into, From 3c3f1ce2a3820732544c1eab3aae4b5a73e4c3e4 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Wed, 8 Jun 2022 15:17:27 +0200 Subject: [PATCH 5/9] Fix tests now that tower-http handles `Content-Length` --- axum/src/routing/tests/mod.rs | 47 +++++++++++++++++++++++++--- axum/src/test_helpers/test_client.rs | 1 + 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/axum/src/routing/tests/mod.rs b/axum/src/routing/tests/mod.rs index d112ae7af6..b253bc5282 100644 --- a/axum/src/routing/tests/mod.rs +++ b/axum/src/routing/tests/mod.rs @@ -8,7 +8,7 @@ use crate::{ test_helpers::*, BoxError, Json, Router, }; -use http::{Method, Request, Response, StatusCode, Uri}; +use http::{header::CONTENT_LENGTH, HeaderMap, Method, Request, Response, StatusCode, Uri}; use hyper::Body; use serde::Deserialize; use serde_json::{json, Value}; @@ -20,7 +20,7 @@ use std::{ time::Duration, }; use tower::{service_fn, timeout::TimeoutLayer, ServiceBuilder, ServiceExt}; -use tower_http::auth::RequireAuthorizationLayer; +use tower_http::{auth::RequireAuthorizationLayer, limit::RequestBodyLimitLayer}; use tower_service::Service; mod fallback; @@ -701,12 +701,17 @@ async fn routes_must_start_with_slash() { } #[tokio::test] -async fn limited_body() { +async fn limited_body_with_content_length() { const LIMIT: usize = 3; let app = Router::new() - .route("/", post(|_: Bytes| async {})) - .layer(tower_http::limit::RequestBodyLimitLayer::new(LIMIT)); + .route( + "/", + post(|headers: HeaderMap, _body: Bytes| async move { + assert!(headers.get(CONTENT_LENGTH).is_some()); + }), + ) + .layer(RequestBodyLimitLayer::new(LIMIT)); let client = TestClient::new(app); @@ -716,3 +721,35 @@ async fn limited_body() { let res = client.post("/").body("a".repeat(LIMIT * 2)).send().await; assert_eq!(res.status(), StatusCode::PAYLOAD_TOO_LARGE); } + +#[tokio::test] +async fn limited_body_with_streaming_body() { + const LIMIT: usize = 3; + + let app = Router::new() + .route( + "/", + post(|headers: HeaderMap, _body: Bytes| async move { + assert!(headers.get(CONTENT_LENGTH).is_none()); + }), + ) + .layer(RequestBodyLimitLayer::new(LIMIT)); + + let client = TestClient::new(app); + + let stream = futures_util::stream::iter(vec![Ok::<_, hyper::Error>("a".repeat(LIMIT))]); + let res = client + .post("/") + .body(Body::wrap_stream(stream)) + .send() + .await; + assert_eq!(res.status(), StatusCode::OK); + + let stream = futures_util::stream::iter(vec![Ok::<_, hyper::Error>("a".repeat(LIMIT * 2))]); + let res = client + .post("/") + .body(Body::wrap_stream(stream)) + .send() + .await; + assert_eq!(res.status(), StatusCode::PAYLOAD_TOO_LARGE); +} diff --git a/axum/src/test_helpers/test_client.rs b/axum/src/test_helpers/test_client.rs index 27f4be4a53..34791b45b6 100644 --- a/axum/src/test_helpers/test_client.rs +++ b/axum/src/test_helpers/test_client.rs @@ -118,6 +118,7 @@ impl RequestBuilder { } } +#[derive(Debug)] pub(crate) struct TestResponse { response: reqwest::Response, } From a2da9d559e9e3b10be780ef194f8e3c7d14c4896 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Wed, 8 Jun 2022 15:19:53 +0200 Subject: [PATCH 6/9] Bring back explanation for `LengthLimitError` --- axum-core/src/extract/rejection.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/axum-core/src/extract/rejection.rs b/axum-core/src/extract/rejection.rs index e82e098e53..96c4163bb9 100644 --- a/axum-core/src/extract/rejection.rs +++ b/axum-core/src/extract/rejection.rs @@ -56,6 +56,11 @@ define_rejection! { #[status = PAYLOAD_TOO_LARGE] #[body = "Failed to buffer the request body"] /// Encountered some other error when buffering the body. + /// + /// This can _only_ happen when you're using [`tower_http::limit::RequestBodyLimitLayer`] or + /// otherwise wrapping request bodies in [`http_body::Limited`]. + /// + /// [`tower_http::limit::RequestBodyLimitLayer`]: https://docs.rs/tower-http/0.3/tower_http/limit/struct.RequestBodyLimitLayer.html pub struct LengthLimitError(Error); } From 5dc05af2fce14e1017d7bff7167f11e75020f58c Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Wed, 8 Jun 2022 15:20:55 +0200 Subject: [PATCH 7/9] remove todo we likely can't fix --- axum/src/extract/content_length_limit.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/axum/src/extract/content_length_limit.rs b/axum/src/extract/content_length_limit.rs index bc74f3572e..6b3ecbce2a 100644 --- a/axum/src/extract/content_length_limit.rs +++ b/axum/src/extract/content_length_limit.rs @@ -32,8 +32,6 @@ use std::ops::Deref; /// /// If you want to limit the size of request bodies without requiring a `Content-Length` header /// consider using [`tower_http::limit::RequestBodyLimitLayer`]. -// TODO(david): change this to apply `http_body::Limited` so it also supports streaming requests. -// Will probably require renaming it. #[derive(Debug, Clone)] pub struct ContentLengthLimit(pub T); From 68af349755111b297d1848c4082dd5160b4bac09 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Wed, 8 Jun 2022 15:21:32 +0200 Subject: [PATCH 8/9] improve wording in docs --- axum-core/src/extract/rejection.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/axum-core/src/extract/rejection.rs b/axum-core/src/extract/rejection.rs index 96c4163bb9..e6f53b8224 100644 --- a/axum-core/src/extract/rejection.rs +++ b/axum-core/src/extract/rejection.rs @@ -67,7 +67,7 @@ define_rejection! { define_rejection! { #[status = BAD_REQUEST] #[body = "Failed to buffer the request body"] - /// Encountered some other error when buffering the body. + /// Encountered an unknown error when buffering the body. pub struct UnknownBodyError(Error); } From cb6e252cd2ed4ee923ac47fdb12165ec58398753 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Wed, 8 Jun 2022 15:38:53 +0200 Subject: [PATCH 9/9] Update axum/src/extract/content_length_limit.rs Co-authored-by: Jonas Platte --- axum/src/extract/content_length_limit.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/axum/src/extract/content_length_limit.rs b/axum/src/extract/content_length_limit.rs index 6b3ecbce2a..62148a8ea7 100644 --- a/axum/src/extract/content_length_limit.rs +++ b/axum/src/extract/content_length_limit.rs @@ -30,7 +30,7 @@ use std::ops::Deref; /// /// This requires the request to have a `Content-Length` header. /// -/// If you want to limit the size of request bodies without requiring a `Content-Length` header +/// If you want to limit the size of request bodies without requiring a `Content-Length` header, /// consider using [`tower_http::limit::RequestBodyLimitLayer`]. #[derive(Debug, Clone)] pub struct ContentLengthLimit(pub T);