Make status codes for JsonRejection more precise (#868)

* Fix status codes for `JsonRejection` rejections

Fixes #865

* Apply suggestions from code review

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

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

axum/CHANGELOG.md

@@ -71,6 +71,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   be accepted but most likely result in bugs ([#823])
 - **breaking:** `Headers` has been removed. Arrays of tuples directly implement
   `IntoResponseParts` so `([("x-foo", "foo")], response)` now works ([#797])
+- **breaking:** `InvalidJsonBody` has been replaced with `JsonDataError` to clearly signal that the
+  request body was syntactically valid JSON but couldn't be deserialized into the target type
+- **changed:** New `JsonSyntaxError` variant added to `JsonRejection`. This is returned when the
+  request body contains syntactically invalid JSON
 - **fixed:** Set `Allow` header when responding with `405 Method Not Allowed` ([#733])
 - **fixed:** Correctly set the `Content-Length` header for response to `HEAD`
   requests ([#734])

axum/src/docs/extract.md

@@ -209,9 +209,12 @@ async fn create_user(payload: Result<Json<Value>, JsonRejection>) {
             // Request didn't have `Content-Type: application/json`
             // header
         }
-        Err(JsonRejection::InvalidJsonBody(_)) => {
+        Err(JsonRejection::JsonDataError(_)) => {
             // Couldn't deserialize the body into the target type
         }
+        Err(JsonRejection::JsonSyntaxError(_)) => {
+            // Syntax error in the body
+        }
         Err(JsonRejection::BytesRejection(_)) => {
             // Failed to extract the request body
         }
@@ -249,7 +252,7 @@ breaking the public API.
 
 For example that means while [`Json`] is implemented using [`serde_json`] it
 doesn't directly expose the [`serde_json::Error`] thats contained in
-[`JsonRejection::InvalidJsonBody`]. However it is still possible to access via
+[`JsonRejection::JsonDataError`]. However it is still possible to access via
 methods from [`std::error::Error`]:
 
 ```rust
@@ -267,21 +270,11 @@ async fn handler(result: Result<Json<Value>, JsonRejection>) -> impl IntoRespons
         Ok(Json(payload)) => Ok(Json(json!({ "payload": payload }))),
 
         Err(err) => match err {
-            // attempt to extract the inner `serde_json::Error`, if that
-            // succeeds we can provide a more specific error
-            JsonRejection::InvalidJsonBody(err) => {
-                if let Some(serde_json_err) = find_error_source::<serde_json::Error>(&err) {
-                    Err((
-                        StatusCode::BAD_REQUEST,
-                        format!(
-                            "Invalid JSON at line {} column {}",
-                            serde_json_err.line(),
-                            serde_json_err.column()
-                        ),
-                    ))
-                } else {
-                    Err((StatusCode::BAD_REQUEST, "Unknown error".to_string()))
-                }
+            JsonRejection::JsonDataError(err) => {
+                Err(serde_json_error_response(err))
+            }
+            JsonRejection::JsonSyntaxError(err) => {
+                Err(serde_json_error_response(err))
             }
             // handle other rejections from the `Json` extractor
             JsonRejection::MissingJsonContentType(_) => Err((
@@ -302,6 +295,26 @@ async fn handler(result: Result<Json<Value>, JsonRejection>) -> impl IntoRespons
     }
 }
 
+// attempt to extract the inner `serde_json::Error`, if that succeeds we can
+// provide a more specific error
+fn serde_json_error_response<E>(err: E) -> (StatusCode, String)
+where
+    E: Error + 'static,
+{
+    if let Some(serde_json_err) = find_error_source::<serde_json::Error>(&err) {
+        (
+            StatusCode::BAD_REQUEST,
+            format!(
+                "Invalid JSON at line {} column {}",
+                serde_json_err.line(),
+                serde_json_err.column()
+            ),
+        )
+    } else {
+        (StatusCode::BAD_REQUEST, "Unknown error".to_string())
+    }
+}
+
 // attempt to downcast `err` into a `T` and if that fails recursively try and
 // downcast `err`'s source
 fn find_error_source<'a, T>(err: &'a (dyn Error + 'static)) -> Option<&'a T>

axum/src/extract/rejection.rs

@@ -9,10 +9,24 @@ pub use axum_core::extract::rejection::*;
 #[cfg(feature = "json")]
 define_rejection! {
     #[status = UNPROCESSABLE_ENTITY]
+    #[body = "Failed to deserialize the JSON body into the target type"]
+    #[cfg_attr(docsrs, doc(cfg(feature = "json")))]
+    /// Rejection type for [`Json`](super::Json).
+    ///
+    /// This rejection is used if the request body is syntactically valid JSON but couldn't be
+    /// deserialized into the target type.
+    pub struct JsonDataError(Error);
+}
+
+#[cfg(feature = "json")]
+define_rejection! {
+    #[status = BAD_REQUEST]
     #[body = "Failed to parse the request body as JSON"]
     #[cfg_attr(docsrs, doc(cfg(feature = "json")))]
     /// Rejection type for [`Json`](super::Json).
-    pub struct InvalidJsonBody(Error);
+    ///
+    /// This rejection is used if the request body didn't contain syntactically valid JSON.
+    pub struct JsonSyntaxError(Error);
 }
 
 #[cfg(feature = "json")]
@@ -141,7 +155,8 @@ composite_rejection! {
     /// can fail.
     #[cfg_attr(docsrs, doc(cfg(feature = "json")))]
     pub enum JsonRejection {
-        InvalidJsonBody,
+        JsonDataError,
+        JsonSyntaxError,
         MissingJsonContentType,
         BytesRejection,
     }

axum/src/json.rs

@@ -99,7 +99,27 @@ where
         if json_content_type(req) {
             let bytes = Bytes::from_request(req).await?;
 
-            let value = serde_json::from_slice(&bytes).map_err(InvalidJsonBody::from_err)?;
+            let value = match serde_json::from_slice(&bytes) {
+                Ok(value) => value,
+                Err(err) => {
+                    let rejection = match err.classify() {
+                        serde_json::error::Category::Data => JsonDataError::from_err(err).into(),
+                        serde_json::error::Category::Syntax | serde_json::error::Category::Eof => {
+                            JsonSyntaxError::from_err(err).into()
+                        }
+                        serde_json::error::Category::Io => {
+                            if cfg!(debug_assertions) {
+                                // we don't use `serde_json::from_reader` and instead always buffer
+                                // bodies first, so we shouldn't encounter any IO errors
+                                unreachable!()
+                            } else {
+                                JsonSyntaxError::from_err(err).into()
+                            }
+                        }
+                    };
+                    return Err(rejection);
+                }
+            };
 
             Ok(Json(value))
         } else {
@@ -244,4 +264,19 @@ mod tests {
         assert!(valid_json_content_type("application/cloudevents+json").await);
         assert!(!valid_json_content_type("text/json").await);
     }
+
+    #[tokio::test]
+    async fn invalid_json_syntax() {
+        let app = Router::new().route("/", post(|_: Json<serde_json::Value>| async {}));
+
+        let client = TestClient::new(app);
+        let res = client
+            .post("/")
+            .body("{")
+            .header("content-type", "application/json")
+            .send()
+            .await;
+
+        assert_eq!(res.status(), StatusCode::BAD_REQUEST);
+    }
 }

examples/customize-extractor-error/src/main.rs

@@ -69,7 +69,7 @@ where
             Err(rejection) => {
                 // convert the error from `axum::Json` into whatever we want
                 let (status, body): (_, Cow<'_, str>) = match rejection {
-                    JsonRejection::InvalidJsonBody(err) => (
+                    JsonRejection::JsonDataError(err) => (
                         StatusCode::BAD_REQUEST,
                         format!("Invalid JSON request: {}", err).into(),
                     ),