From 4e321595bcf4a450efcfe5655e68b7c917c38fa8 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Wed, 2 Sep 2020 22:12:07 +0100 Subject: [PATCH] extract more config types from Data as well (#1641) --- CHANGES.md | 9 +++++-- src/types/form.rs | 38 +++++++++++++++++++++++------ src/types/json.rs | 58 +++++++++++++++++++++++++++++++++----------- src/types/payload.rs | 19 ++++++--------- 4 files changed, 89 insertions(+), 35 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 0d731174..82c562f5 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -3,11 +3,16 @@ ## Unreleased - 2020-xx-xx ### Added * `middleware::NormalizePath` now has configurable behaviour for either always having a trailing - slash, or as the new addition, always trimming trailing slashes. + slash, or as the new addition, always trimming trailing slashes. [#1639] ### Changed -* Update actix-codec and actix-utils dependencies. +* Update actix-codec and actix-utils dependencies. [#1634] +* `FormConfig` and `JsonConfig` configurations are now also considered when set + using `App::data`. [#1641] +[#1639]: https://github.com/actix/actix-web/pull/1639 +[#1641]: https://github.com/actix/actix-web/pull/1641 +[#1634]: https://github.com/actix/actix-web/pull/1634 ## 3.0.0-beta.3 - 2020-08-17 ### Changed diff --git a/src/types/form.rs b/src/types/form.rs index ea061d55..de88c2a9 100644 --- a/src/types/form.rs +++ b/src/types/form.rs @@ -23,7 +23,7 @@ use crate::http::{ StatusCode, }; use crate::request::HttpRequest; -use crate::responder::Responder; +use crate::{responder::Responder, web}; /// Form data helper (`application/x-www-form-urlencoded`) /// @@ -121,8 +121,12 @@ where fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future { let req2 = req.clone(); let (limit, err) = req - .app_data::() - .map(|c| (c.limit, c.ehandler.clone())) + .app_data::() + .or_else(|| { + req.app_data::>() + .map(|d| d.as_ref()) + }) + .map(|c| (c.limit, c.err_handler.clone())) .unwrap_or((16384, None)); UrlEncoded::new(req, payload) @@ -200,7 +204,7 @@ impl Responder for Form { #[derive(Clone)] pub struct FormConfig { limit: usize, - ehandler: Option Error>>, + err_handler: Option Error>>, } impl FormConfig { @@ -215,7 +219,7 @@ impl FormConfig { where F: Fn(UrlencodedError, &HttpRequest) -> Error + 'static, { - self.ehandler = Some(Rc::new(f)); + self.err_handler = Some(Rc::new(f)); self } } @@ -223,8 +227,8 @@ impl FormConfig { impl Default for FormConfig { fn default() -> Self { FormConfig { - limit: 16384, - ehandler: None, + limit: 16_384, // 2^14 bytes (~16kB) + err_handler: None, } } } @@ -378,7 +382,7 @@ mod tests { use serde::{Deserialize, Serialize}; use super::*; - use crate::http::header::{HeaderValue, CONTENT_TYPE}; + use crate::http::header::{HeaderValue, CONTENT_LENGTH, CONTENT_TYPE}; use crate::test::TestRequest; #[derive(Deserialize, Serialize, Debug, PartialEq)] @@ -499,4 +503,22 @@ mod tests { use crate::responder::tests::BodyTest; assert_eq!(resp.body().bin_ref(), b"hello=world&counter=123"); } + + #[actix_rt::test] + async fn test_with_config_in_data_wrapper() { + let ctype = HeaderValue::from_static("application/x-www-form-urlencoded"); + + let (req, mut pl) = TestRequest::default() + .header(CONTENT_TYPE, ctype) + .header(CONTENT_LENGTH, HeaderValue::from_static("20")) + .set_payload(Bytes::from_static(b"hello=test&counter=4")) + .app_data(web::Data::new(FormConfig::default().limit(10))) + .to_http_parts(); + + let s = Form::::from_request(&req, &mut pl).await; + assert!(s.is_err()); + + let err_str = s.err().unwrap().to_string(); + assert!(err_str.contains("Urlencoded payload size is bigger")); + } } diff --git a/src/types/json.rs b/src/types/json.rs index 527b4b61..ab7978df 100644 --- a/src/types/json.rs +++ b/src/types/json.rs @@ -20,7 +20,7 @@ use crate::dev::Decompress; use crate::error::{Error, JsonPayloadError}; use crate::extract::FromRequest; use crate::request::HttpRequest; -use crate::responder::Responder; +use crate::{responder::Responder, web}; /// Json helper /// @@ -179,10 +179,11 @@ where #[inline] fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future { let req2 = req.clone(); - let (limit, err, ctype) = req - .app_data::() - .map(|c| (c.limit, c.ehandler.clone(), c.content_type.clone())) - .unwrap_or((32768, None, None)); + let config = JsonConfig::from_req(req); + + let limit = config.limit; + let ctype = config.content_type.clone(); + let err_handler = config.err_handler.clone(); JsonBody::new(req, payload, ctype) .limit(limit) @@ -193,7 +194,8 @@ where Request path: {}", req2.path() ); - if let Some(err) = err { + + if let Some(err) = err_handler { Err((*err)(e, &req2)) } else { Err(e.into()) @@ -255,7 +257,8 @@ where #[derive(Clone)] pub struct JsonConfig { limit: usize, - ehandler: Option Error + Send + Sync>>, + err_handler: + Option Error + Send + Sync>>, content_type: Option bool + Send + Sync>>, } @@ -271,7 +274,7 @@ impl JsonConfig { where F: Fn(JsonPayloadError, &HttpRequest) -> Error + Send + Sync + 'static, { - self.ehandler = Some(Arc::new(f)); + self.err_handler = Some(Arc::new(f)); self } @@ -283,15 +286,26 @@ impl JsonConfig { self.content_type = Some(Arc::new(predicate)); self } + + /// Extract payload config from app data. Check both `T` and `Data`, in that order, and fall + /// back to the default payload config. + fn from_req(req: &HttpRequest) -> &Self { + req.app_data::() + .or_else(|| req.app_data::>().map(|d| d.as_ref())) + .unwrap_or_else(|| &DEFAULT_CONFIG) + } } +// Allow shared refs to default. +const DEFAULT_CONFIG: JsonConfig = JsonConfig { + limit: 32_768, // 2^15 bytes, (~32kB) + err_handler: None, + content_type: None, +}; + impl Default for JsonConfig { fn default() -> Self { - JsonConfig { - limit: 32768, - ehandler: None, - content_type: None, - } + DEFAULT_CONFIG.clone() } } @@ -422,7 +436,7 @@ mod tests { use super::*; use crate::error::InternalError; - use crate::http::header; + use crate::http::header::{self, HeaderValue, CONTENT_LENGTH, CONTENT_TYPE}; use crate::test::{load_stream, TestRequest}; use crate::HttpResponse; @@ -659,4 +673,20 @@ mod tests { let s = Json::::from_request(&req, &mut pl).await; assert!(s.is_err()) } + + #[actix_rt::test] + async fn test_with_config_in_data_wrapper() { + let (req, mut pl) = TestRequest::default() + .header(CONTENT_TYPE, HeaderValue::from_static("application/json")) + .header(CONTENT_LENGTH, HeaderValue::from_static("16")) + .set_payload(Bytes::from_static(b"{\"name\": \"test\"}")) + .app_data(web::Data::new(JsonConfig::default().limit(10))) + .to_http_parts(); + + let s = Json::::from_request(&req, &mut pl).await; + assert!(s.is_err()); + + let err_str = s.err().unwrap().to_string(); + assert!(err_str.contains("Json payload size is bigger than allowed")); + } } diff --git a/src/types/payload.rs b/src/types/payload.rs index 653abf08..bbdd8952 100644 --- a/src/types/payload.rs +++ b/src/types/payload.rs @@ -279,27 +279,24 @@ impl PayloadConfig { Ok(()) } - /// Allow payload config extraction from app data checking both `T` and `Data`, in that - /// order, and falling back to the default payload config. - fn from_req(req: &HttpRequest) -> &PayloadConfig { - req.app_data::() - .or_else(|| { - req.app_data::>() - .map(|d| d.as_ref()) - }) - .unwrap_or_else(|| &DEFAULT_PAYLOAD_CONFIG) + /// Extract payload config from app data. Check both `T` and `Data`, in that order, and fall + /// back to the default payload config. + fn from_req(req: &HttpRequest) -> &Self { + req.app_data::() + .or_else(|| req.app_data::>().map(|d| d.as_ref())) + .unwrap_or_else(|| &DEFAULT_CONFIG) } } // Allow shared refs to default. -static DEFAULT_PAYLOAD_CONFIG: PayloadConfig = PayloadConfig { +const DEFAULT_CONFIG: PayloadConfig = PayloadConfig { limit: 262_144, // 2^18 bytes (~256kB) mimetype: None, }; impl Default for PayloadConfig { fn default() -> Self { - DEFAULT_PAYLOAD_CONFIG.clone() + DEFAULT_CONFIG.clone() } }