extract more config types from Data<T> as well (#1641)

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

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::<FormConfig>()
+            .app_data::<Self::Config>()
-            .map(|c| (c.limit, c.ehandler.clone()))
+            .or_else(|| {
+                req.app_data::<web::Data<Self::Config>>()
+                    .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<T: Serialize> Responder for Form<T> {
 #[derive(Clone)]
 pub struct FormConfig {
     limit: usize,
-    ehandler: Option<Rc<dyn Fn(UrlencodedError, &HttpRequest) -> Error>>,
+    err_handler: Option<Rc<dyn Fn(UrlencodedError, &HttpRequest) -> 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,
+            limit: 16_384, // 2^14 bytes (~16kB)
-            ehandler: None,
+            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::<Info>::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"));
+    }
 }

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
+        let config = JsonConfig::from_req(req);
-            .app_data::<Self::Config>()
+
-            .map(|c| (c.limit, c.ehandler.clone(), c.content_type.clone()))
+        let limit = config.limit;
-            .unwrap_or((32768, None, None));
+        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<Arc<dyn Fn(JsonPayloadError, &HttpRequest) -> Error + Send + Sync>>,
+    err_handler:
+        Option<Arc<dyn Fn(JsonPayloadError, &HttpRequest) -> Error + Send + Sync>>,
     content_type: Option<Arc<dyn Fn(mime::Mime) -> 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<T>`, in that order, and fall
+    /// back to the default payload config.
+    fn from_req(req: &HttpRequest) -> &Self {
+        req.app_data::<Self>()
+            .or_else(|| req.app_data::<web::Data<Self>>().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 {
+        DEFAULT_CONFIG.clone()
-            limit: 32768,
-            ehandler: None,
-            content_type: None,
-        }
     }
 }
 
@@ -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::<MyObject>::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::<MyObject>::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"));
+    }
 }

src/types/payload.rs

@@ -279,27 +279,24 @@ impl PayloadConfig {
         Ok(())
     }
 
-    /// Allow payload config extraction from app data checking both `T` and `Data<T>`, in that
+    /// Extract payload config from app data. Check both `T` and `Data<T>`, in that order, and fall
-    /// order, and falling back to the default payload config.
+    /// back to the default payload config.
-    fn from_req(req: &HttpRequest) -> &PayloadConfig {
+    fn from_req(req: &HttpRequest) -> &Self {
-        req.app_data::<PayloadConfig>()
+        req.app_data::<Self>()
-            .or_else(|| {
+            .or_else(|| req.app_data::<web::Data<Self>>().map(|d| d.as_ref()))
-                req.app_data::<web::Data<PayloadConfig>>()
+            .unwrap_or_else(|| &DEFAULT_CONFIG)
-                    .map(|d| d.as_ref())
-            })
-            .unwrap_or_else(|| &DEFAULT_PAYLOAD_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()
     }
 }