Use 422 Unprocessable Entity for form deserialization errors

[connec]

Fixes #1680.

Motivation

Per #1680, we would prefer to use 422 Unprocessable Entity responses for non-syntactic deserialization errors from the Form extractor. This is now trivial since the Form and Query rejection types were separated in #1496.

Solution

• Update the status for axum::extract::rejection::FailedToDeserializeForm from BAD_REQUEST to UNPROCESSABLE_ENTITY.
• The same change has been applied to axum_extra::extract::FormRejection::FailedToDeserializeForm for consistency.

[davidpdrsn]

Nice! Wanna add a test and changelog entry as well?

[connec]

Sure, I was a bit lazy and left it out since nothing was presently testing it, but happy to add one. I'll have to sort it a little bit later though.

[davidpdrsn]

That's alright. I think having a test would be good. Then its harder to break by accident.

[connec]

Ah, it looks like I spoke too soon with my assumption that the Form and Query extractors were fully separated. You can presently use the Form extractor with GET requests and it will read from the URL query instead, due to the use of RawForm internally:

axum/axum/src/extract/raw_form.rs

Lines 47 to 64 in 211de92

	async fn from_request(req: Request<B>, state: &S) -> Result<Self, Self::Rejection> {
	if req.method() == Method::GET {
	let mut bytes = BytesMut::new();
	if let Some(query) = req.uri().query() {
	bytes.extend(query.as_bytes());
	}
	Ok(Self(bytes.freeze()))
	} else {
	if !has_content_type(req.headers(), &mime::APPLICATION_WWW_FORM_URLENCODED) {
	return Err(InvalidFormContentType.into());
	}
	Ok(Self(Bytes::from_request(req, state).await?))
	}
	}
	}

As such, with this change, a GET request with semantically invalid Form data in the URL would return a 422, which #1378 establishes is invalid.

I'm not sure what would be the preferred solution to this? Some options I can think of:

1. Remove the special-casing for GET requests (and perhaps direct people to Query in the documentation?). This seems like it would be a breaking change to RawForm though.

2. Pipe through some information about the source of the form data, and use separate composite_rejection variants for semantically invalid queries vs. bodies. Adding a separate variant would not be breaking, but RawForm is a public type with public fields, so adding any more info to the struct would also be a breaking change for RawForm.

3. The FromRequest implementations of RawForm and Form could be fully decoupled (i.e. copy the RawForm body implementation into the Form implementation). This would functionally be a breaking change to Form, but the documentation currently suggests that only request bodies are processed, so it could be argued to be a bug fix 😅

   axum/axum/src/form.rs

   Lines 15 to 21 in 211de92

   	/// # As extractor
   	///
   	/// If used as an extractor `Form` will deserialize `application/x-www-form-urlencoded` request
   	/// bodies into some target type via [`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]

Perhaps there are other options?

[davidpdrsn]

Good catch! Another reason testing is important. I think adding another variant to FormRejection is the simplest approach. I went ahead and implemented it in #1683.

[davidpdrsn]

Thanks for working on this but I think we'll move forward with #1683

[connec]

Of course, sorry I didn't have to follow it through!

[davidpdrsn]

No worries! You're welcome to give #1683 a look though :)