diff --git a/CHANGELOG.md b/CHANGELOG.md index f952be59..7947ec8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add support for WebSocket protocol negotiation. ([#83](https://github.com/tokio-rs/axum/pull/83)) - Use `pin-project-lite` instead of `pin-project`. ([#95](https://github.com/tokio-rs/axum/pull/95)) - Re-export `http` crate and `hyper::Server`. ([#110](https://github.com/tokio-rs/axum/pull/110)) +- Fix `Query` and `Form` extractors giving bad request error when query string is empty. ([#117](https://github.com/tokio-rs/axum/pull/117)) ## Breaking changes diff --git a/src/extract/form.rs b/src/extract/form.rs index e951bcd8..0aed6856 100644 --- a/src/extract/form.rs +++ b/src/extract/form.rs @@ -50,20 +50,20 @@ where #[allow(warnings)] async fn from_request(req: &mut RequestParts) -> Result { - if !has_content_type(&req, "application/x-www-form-urlencoded")? { - Err(InvalidFormContentType)?; - } - if req.method().ok_or(MethodAlreadyExtracted)? == Method::GET { let query = req .uri() .ok_or(UriAlreadyExtracted)? .query() - .ok_or(QueryStringMissing)?; + .unwrap_or_default(); let value = serde_urlencoded::from_str(query) .map_err(FailedToDeserializeQueryString::new::)?; Ok(Form(value)) } else { + if !has_content_type(&req, "application/x-www-form-urlencoded")? { + Err(InvalidFormContentType)?; + } + let body = take_body(req)?; let chunks = hyper::body::aggregate(body) .await @@ -83,3 +83,121 @@ impl Deref for Form { &self.0 } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::extract::RequestParts; + use http::Request; + use serde::{Deserialize, Serialize}; + use std::fmt::Debug; + + #[derive(Debug, PartialEq, Serialize, Deserialize)] + struct Pagination { + size: Option, + page: Option, + } + + async fn check_query(uri: impl AsRef, value: T) { + let mut req = RequestParts::new( + Request::builder() + .uri(uri.as_ref()) + .body(http_body::Empty::::new()) + .unwrap(), + ); + assert_eq!(Form::::from_request(&mut req).await.unwrap().0, value); + } + + async fn check_body(value: T) { + let mut req = RequestParts::new( + Request::builder() + .uri("http://example.com/test") + .method(Method::POST) + .header( + http::header::CONTENT_TYPE, + "application/x-www-form-urlencoded", + ) + .body(http_body::Full::::new( + serde_urlencoded::to_string(&value).unwrap().into(), + )) + .unwrap(), + ); + assert_eq!(Form::::from_request(&mut req).await.unwrap().0, value); + } + + #[tokio::test] + async fn test_form_query() { + check_query( + "http://example.com/test", + Pagination { + size: None, + page: None, + }, + ) + .await; + + check_query( + "http://example.com/test?size=10", + Pagination { + size: Some(10), + page: None, + }, + ) + .await; + + check_query( + "http://example.com/test?size=10&page=20", + Pagination { + size: Some(10), + page: Some(20), + }, + ) + .await; + } + + #[tokio::test] + async fn test_form_body() { + check_body(Pagination { + size: None, + page: None, + }) + .await; + + check_body(Pagination { + size: Some(10), + page: None, + }) + .await; + + check_body(Pagination { + size: Some(10), + page: Some(20), + }) + .await; + } + + #[tokio::test] + async fn test_incorrect_content_type() { + let mut req = RequestParts::new( + Request::builder() + .uri("http://example.com/test") + .method(Method::POST) + .header(http::header::CONTENT_TYPE, "application/json") + .body(http_body::Full::::new( + serde_urlencoded::to_string(&Pagination { + size: Some(10), + page: None, + }) + .unwrap() + .into(), + )) + .unwrap(), + ); + assert!(matches!( + Form::::from_request(&mut req) + .await + .unwrap_err(), + FormRejection::InvalidFormContentType(InvalidFormContentType) + )); + } +} diff --git a/src/extract/query.rs b/src/extract/query.rs index ed86ff11..fd3786ba 100644 --- a/src/extract/query.rs +++ b/src/extract/query.rs @@ -51,7 +51,7 @@ where .uri() .ok_or(UriAlreadyExtracted)? .query() - .ok_or(QueryStringMissing)?; + .unwrap_or_default(); let value = serde_urlencoded::from_str(query) .map_err(FailedToDeserializeQueryString::new::)?; Ok(Query(value)) @@ -65,3 +65,53 @@ impl Deref for Query { &self.0 } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::extract::RequestParts; + use http::Request; + use serde::Deserialize; + use std::fmt::Debug; + + async fn check(uri: impl AsRef, value: T) { + let mut req = RequestParts::new(Request::builder().uri(uri.as_ref()).body(()).unwrap()); + assert_eq!(Query::::from_request(&mut req).await.unwrap().0, value); + } + + #[tokio::test] + async fn test_query() { + #[derive(Debug, PartialEq, Deserialize)] + struct Pagination { + size: Option, + page: Option, + } + + check( + "http://example.com/test", + Pagination { + size: None, + page: None, + }, + ) + .await; + + check( + "http://example.com/test?size=10", + Pagination { + size: Some(10), + page: None, + }, + ) + .await; + + check( + "http://example.com/test?size=10&page=20", + Pagination { + size: Some(10), + page: Some(20), + }, + ) + .await; + } +}