From cd86f7ec7a8bb2089f8af07c64651572da6c8918 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Fri, 17 Feb 2023 09:59:02 +0100 Subject: [PATCH] Use `422 Unprocessable Entity` for `Form` deserialization errors, except `GET` and `HEAD` requests (#1683) --- .../fail/wrong_return_type.stderr | 2 +- axum/CHANGELOG.md | 31 ++------ axum/src/extract/rejection.rs | 9 +++ axum/src/form.rs | 70 +++++++++++++++---- 4 files changed, 71 insertions(+), 41 deletions(-) diff --git a/axum-macros/tests/debug_handler/fail/wrong_return_type.stderr b/axum-macros/tests/debug_handler/fail/wrong_return_type.stderr index e6a33a62..151c5f84 100644 --- a/axum-macros/tests/debug_handler/fail/wrong_return_type.stderr +++ b/axum-macros/tests/debug_handler/fail/wrong_return_type.stderr @@ -13,7 +13,7 @@ error[E0277]: the trait bound `bool: IntoResponse` is not satisfied (Response<()>, T1, R) (Response<()>, T1, T2, R) (Response<()>, T1, T2, T3, R) - and 124 others + and $N others note: required by a bound in `__axum_macros_check_handler_into_response::{closure#0}::check` --> tests/debug_handler/fail/wrong_return_type.rs:4:23 | diff --git a/axum/CHANGELOG.md b/axum/CHANGELOG.md index 9a6b8eb9..17b7fc23 100644 --- a/axum/CHANGELOG.md +++ b/axum/CHANGELOG.md @@ -7,34 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 # Unreleased -- None. - -# 0.6.6 (12. February, 2023) - -- **fixed:** Enable passing `MethodRouter` to `Router::fallback` ([#1730]) - -[#1730]: https://github.com/tokio-rs/axum/pull/1730 - -# 0.6.5 (11. February, 2023) - -- **fixed:** Fix `#[debug_handler]` sometimes giving wrong borrow related suggestions ([#1710]) -- Document gotchas related to using `impl IntoResponse` as the return type from handler functions ([#1736]) - -[#1710]: https://github.com/tokio-rs/axum/pull/1710 -[#1736]: https://github.com/tokio-rs/axum/pull/1736 - -# 0.6.4 (22. January, 2023) - -- Depend on axum-macros 0.3.2 - -# 0.6.3 (20. January, 2023) - -- **added:** Implement `IntoResponse` for `&'static [u8; N]` and `[u8; N]` ([#1690]) -- **fixed:** Make `Path` support types using `serde::Deserializer::deserialize_any` ([#1693]) -- **added:** Add `RawPathParams` ([#1713]) -- **added:** Implement `Clone` and `Service` for `axum::middleware::Next` ([#1712]) -- **fixed:** Document required tokio features to run "Hello, World!" example ([#1715]) +- **added:** Add `FormRejection::FailedToDeserializeFormBody` which is returned + if the request body couldn't be deserialized into the target type, as opposed + to `FailedToDeserializeForm` which is only for query parameters ([#1683]) +[#1683]: https://github.com/tokio-rs/axum/pull/1683 [#1690]: https://github.com/tokio-rs/axum/pull/1690 [#1693]: https://github.com/tokio-rs/axum/pull/1693 [#1712]: https://github.com/tokio-rs/axum/pull/1712 diff --git a/axum/src/extract/rejection.rs b/axum/src/extract/rejection.rs index 0bb2df04..7285d49e 100644 --- a/axum/src/extract/rejection.rs +++ b/axum/src/extract/rejection.rs @@ -78,6 +78,14 @@ define_rejection! { pub struct FailedToDeserializeForm(Error); } +define_rejection! { + #[status = UNPROCESSABLE_ENTITY] + #[body = "Failed to deserialize form body"] + /// Rejection type used if the [`Form`](super::Form) extractor is unable to + /// deserialize the form body into the target type. + pub struct FailedToDeserializeFormBody(Error); +} + define_rejection! { #[status = BAD_REQUEST] #[body = "Failed to deserialize query string"] @@ -104,6 +112,7 @@ composite_rejection! { pub enum FormRejection { InvalidFormContentType, FailedToDeserializeForm, + FailedToDeserializeFormBody, BytesRejection, } } diff --git a/axum/src/form.rs b/axum/src/form.rs index b4a90a24..11412081 100644 --- a/axum/src/form.rs +++ b/axum/src/form.rs @@ -14,11 +14,13 @@ use std::ops::Deref; /// /// # As extractor /// -/// If used as an extractor `Form` will deserialize `application/x-www-form-urlencoded` request -/// bodies into some target type via [`serde::Deserialize`]. +/// If used as an extractor `Form` will deserialize the query parameters for `GET` and `HEAD` +/// requests and `application/x-www-form-urlencoded` encoded request bodies for other methods. It +/// supports any type that implements [`serde::Deserialize`]. /// -/// Since parsing form data requires consuming the request body, the `Form` extractor must be -/// *last* if there are multiple extractors in a handler. See ["the order of extractors"][order-of-extractors] +/// Since parsing form data might require consuming the request body, the `Form` extractor must be +/// *last* if there are multiple extractors in a handler. See ["the order of +/// extractors"][order-of-extractors] /// /// [order-of-extractors]: crate::extract#the-order-of-extractors /// @@ -73,10 +75,19 @@ where type Rejection = FormRejection; async fn from_request(req: Request, _state: &S) -> Result { + let is_get_or_head = + req.method() == http::Method::GET || req.method() == http::Method::HEAD; + match req.extract().await { Ok(RawForm(bytes)) => { - let value = serde_urlencoded::from_bytes(&bytes) - .map_err(FailedToDeserializeForm::from_err)?; + let value = + serde_urlencoded::from_bytes(&bytes).map_err(|err| -> FormRejection { + if is_get_or_head { + FailedToDeserializeForm::from_err(err).into() + } else { + FailedToDeserializeFormBody::from_err(err).into() + } + })?; Ok(Form(value)) } Err(RawFormRejection::BytesRejection(r)) => Err(FormRejection::BytesRejection(r)), @@ -114,9 +125,15 @@ impl Deref for Form { #[cfg(test)] mod tests { use super::*; - use crate::body::{Empty, Full}; + use crate::{ + body::{Empty, Full}, + routing::{on, MethodFilter}, + test_helpers::TestClient, + Router, + }; use bytes::Bytes; - use http::{Method, Request}; + use http::{header::CONTENT_TYPE, Method, Request}; + use mime::APPLICATION_WWW_FORM_URLENCODED; use serde::{Deserialize, Serialize}; use std::fmt::Debug; @@ -138,10 +155,7 @@ mod tests { let req = Request::builder() .uri("http://example.com/test") .method(Method::POST) - .header( - http::header::CONTENT_TYPE, - mime::APPLICATION_WWW_FORM_URLENCODED.as_ref(), - ) + .header(CONTENT_TYPE, APPLICATION_WWW_FORM_URLENCODED.as_ref()) .body(Full::::new( serde_urlencoded::to_string(&value).unwrap().into(), )) @@ -205,7 +219,7 @@ mod tests { let req = Request::builder() .uri("http://example.com/test") .method(Method::POST) - .header(http::header::CONTENT_TYPE, mime::APPLICATION_JSON.as_ref()) + .header(CONTENT_TYPE, mime::APPLICATION_JSON.as_ref()) .body(Full::::new( serde_urlencoded::to_string(&Pagination { size: Some(10), @@ -222,4 +236,34 @@ mod tests { FormRejection::InvalidFormContentType(InvalidFormContentType) )); } + + #[tokio::test] + async fn deserialize_error_status_codes() { + #[allow(dead_code)] + #[derive(Deserialize)] + struct Payload { + a: i32, + } + + let app = Router::new().route( + "/", + on( + MethodFilter::GET | MethodFilter::POST, + |_: Form| async {}, + ), + ); + + let client = TestClient::new(app); + + let res = client.get("/?a=false").send().await; + assert_eq!(res.status(), StatusCode::BAD_REQUEST); + + let res = client + .post("/") + .header(CONTENT_TYPE, APPLICATION_WWW_FORM_URLENCODED.as_ref()) + .body("a=false") + .send() + .await; + assert_eq!(res.status(), StatusCode::UNPROCESSABLE_ENTITY); + } }