From 99a3c40aa0a320e4f0eb731f70680b088fe345e3 Mon Sep 17 00:00:00 2001 From: Luca Palmieri Date: Tue, 24 Dec 2019 19:10:46 +0000 Subject: [PATCH] Change the behaviour of `.query`: if the caller has not specified query parameters, try to deserialise in any case from an empty string. This allows successful deserialisation of structs which do not have mandatory fields (e.g. all fields are Option). Return detailed error message when deserialisation fails, to help the caller debug what was wrong with their request. --- src/request.rs | 16 +++-- tests/querystring.rs | 140 +++++++++++++++++++++++++++---------------- 2 files changed, 97 insertions(+), 59 deletions(-) diff --git a/src/request.rs b/src/request.rs index 4b04b6767..723fc3a0b 100644 --- a/src/request.rs +++ b/src/request.rs @@ -267,12 +267,16 @@ impl Request { /// Get the URL querystring. pub fn query<'de, T: Deserialize<'de>>(&'de self) -> Result { - let query = self.uri().query(); - if query.is_none() { - return Err(crate::Error::from(http::StatusCode::BAD_REQUEST)); - } - Ok(serde_qs::from_str(query.unwrap()) - .map_err(|_| crate::Error::from(http::StatusCode::BAD_REQUEST))?) + // Default to an empty query string if no query parameter has been specified. + // This allows successful deserialisation of structs where all fields are optional + // when none of those fields has actually been passed by the caller. + let query = self.uri().query().unwrap_or(""); + serde_qs::from_str(query).map_err(|e| { + // Return the displayable version of the deserialisation error to the caller + // for easier debugging. + let response = crate::Response::new(400).body_string(format!("{}", e)); + crate::Error::from(response) + }) } /// Parse the request body as a form. diff --git a/tests/querystring.rs b/tests/querystring.rs index 86d4874d2..fd899e2e0 100644 --- a/tests/querystring.rs +++ b/tests/querystring.rs @@ -1,53 +1,87 @@ -// use futures::executor::block_on; -// use http_service::Body; -// use http_service_mock::make_server; -// use tide::*; - -// #[derive(Deserialize)] -// struct Params { -// msg: String, -// } - -// async fn handler(cx: crate::Request<()>) -> Result { -// let p = cx.url_query::()?; -// Ok(p.msg) -// } - -// fn app() -> crate::Server<()> { -// let mut app = crate::Server::new(); -// app.at("/").get(handler); -// app -// } - -// #[test] -// fn successfully_deserialize_query() { -// let app = app(); -// let mut server = make_server(app.into_http_service()).unwrap(); -// let req = http::Request::get("/?msg=Hello") -// .body(Body::empty()) -// .unwrap(); -// let res = server.simulate(req).unwrap(); -// assert_eq!(res.status(), 200); -// let body = block_on(res.into_body().into_vec()).unwrap(); -// assert_eq!(&*body, &*b"Hello"); -// } - -// #[test] -// fn unsuccessfully_deserialize_query() { -// let app = app(); -// let mut server = make_server(app.into_http_service()).unwrap(); -// let req = http::Request::get("/").body(Body::empty()).unwrap(); -// let res = server.simulate(req).unwrap(); -// assert_eq!(res.status(), 400); -// } - -// #[test] -// fn malformatted_query() { -// let app = app(); -// let mut server = make_server(app.into_http_service()).unwrap(); -// let req = http::Request::get("/?error=should_fail") -// .body(Body::empty()) -// .unwrap(); -// let res = server.simulate(req).unwrap(); -// assert_eq!(res.status(), 400); -// } +use async_std::io::prelude::ReadExt; +use futures::executor::block_on; +use http_service::Body; +use http_service_mock::{make_server, TestBackend}; +use serde::Deserialize; +use tide::{server::Service, IntoResponse, Request, Response, Server}; + +#[derive(Deserialize)] +struct Params { + msg: String, +} + +#[derive(Deserialize)] +struct OptionalParams { + _msg: Option, + _time: Option, +} + +async fn handler(cx: Request<()>) -> Response { + let p = cx.query::(); + match p { + Ok(params) => params.msg.into_response(), + Err(error) => error.into_response(), + } +} + +async fn optional_handler(cx: Request<()>) -> Response { + let p = cx.query::(); + match p { + Ok(_) => Response::new(200), + Err(error) => error.into_response(), + } +} + +fn get_server() -> TestBackend> { + let mut app = Server::new(); + app.at("/").get(handler); + app.at("/optional").get(optional_handler); + make_server(app.into_http_service()).unwrap() +} + +#[test] +fn successfully_deserialize_query() { + let mut server = get_server(); + let req = http::Request::get("/?msg=Hello") + .body(Body::empty()) + .unwrap(); + let res = server.simulate(req).unwrap(); + assert_eq!(res.status(), 200); + let mut body = String::new(); + block_on(res.into_body().read_to_string(&mut body)).unwrap(); + assert_eq!(body, "Hello"); +} + +#[test] +fn unsuccessfully_deserialize_query() { + let mut server = get_server(); + let req = http::Request::get("/").body(Body::empty()).unwrap(); + let res = server.simulate(req).unwrap(); + assert_eq!(res.status(), 400); + + let mut body = String::new(); + block_on(res.into_body().read_to_string(&mut body)).unwrap(); + assert_eq!(body, "failed with reason: missing field `msg`"); +} + +#[test] +fn malformatted_query() { + let mut server = get_server(); + let req = http::Request::get("/?error=should_fail") + .body(Body::empty()) + .unwrap(); + let res = server.simulate(req).unwrap(); + assert_eq!(res.status(), 400); + + let mut body = String::new(); + block_on(res.into_body().read_to_string(&mut body)).unwrap(); + assert_eq!(body, "failed with reason: missing field `msg`"); +} + +#[test] +fn empty_query_string_for_struct_with_no_required_fields() { + let mut server = get_server(); + let req = http::Request::get("/optional").body(Body::empty()).unwrap(); + let res = server.simulate(req).unwrap(); + assert_eq!(res.status(), 200); +}