also try extracting payload config as Data<T> (#1610)

CHANGES.md

@@ -1,10 +1,15 @@
 # Changes
 
 ## Unreleased - 2020-xx-xx
+### Changed
+* `PayloadConfig` is now also considered in `Bytes` and `String` extractors when set
+  using `App::data`. [#1610]
+
 ### Fixed
 * Memory leak of app data in pooled requests. [#1609]
 
 [#1609]: https://github.com/actix/actix-web/pull/1609
+[#1610]: https://github.com/actix/actix-web/pull/1610
 
 
 ## 3.0.0-beta.1 - 2020-07-13

src/types/payload.rs

@@ -13,10 +13,10 @@ use futures_util::future::{err, ok, Either, FutureExt, LocalBoxFuture, Ready};
 use futures_util::StreamExt;
 use mime::Mime;
 
-use crate::dev;
 use crate::extract::FromRequest;
 use crate::http::header;
 use crate::request::HttpRequest;
+use crate::{dev, web};
 
 /// Payload extractor returns request 's payload stream.
 ///
@@ -142,13 +142,8 @@ impl FromRequest for Bytes {
 
     #[inline]
     fn from_request(req: &HttpRequest, payload: &mut dev::Payload) -> Self::Future {
-        let tmp;
+        // allow both Config and Data<Config>
-        let cfg = if let Some(cfg) = req.app_data::<PayloadConfig>() {
+        let cfg = PayloadConfig::from_req(req);
-            cfg
-        } else {
-            tmp = PayloadConfig::default();
-            &tmp
-        };
 
         if let Err(e) = cfg.check_mimetype(req) {
             return Either::Right(err(e));
@@ -197,13 +192,7 @@ impl FromRequest for String {
 
     #[inline]
     fn from_request(req: &HttpRequest, payload: &mut dev::Payload) -> Self::Future {
-        let tmp;
+        let cfg = PayloadConfig::from_req(req);
-        let cfg = if let Some(cfg) = req.app_data::<PayloadConfig>() {
-            cfg
-        } else {
-            tmp = PayloadConfig::default();
-            &tmp
-        };
 
         // check content-type
         if let Err(e) = cfg.check_mimetype(req) {
@@ -237,7 +226,12 @@ impl FromRequest for String {
         )
     }
 }
-/// Payload configuration for request's payload.
+
+/// Configuration for request's payload.
+///
+/// Applies to the built-in `Bytes` and `String` extractors. Note that the Payload extractor does
+/// not automatically check conformance with this configuration to allow more flexibility when
+/// building extractors on top of `Payload`.
 #[derive(Clone)]
 pub struct PayloadConfig {
     limit: usize,
@@ -284,14 +278,28 @@ impl PayloadConfig {
         }
         Ok(())
     }
+
+    /// Allow payload config extraction from app data checking both `T` and `Data<T>`, in that
+    /// order, and falling back to the default payload config.
+    fn from_req(req: &HttpRequest) -> &PayloadConfig {
+        req.app_data::<PayloadConfig>()
+            .or_else(|| {
+                req.app_data::<web::Data<PayloadConfig>>()
+                    .map(|d| d.as_ref())
+            })
+            .unwrap_or_else(|| &DEFAULT_PAYLOAD_CONFIG)
+    }
 }
 
+// Allow shared refs to default.
+static DEFAULT_PAYLOAD_CONFIG: PayloadConfig = PayloadConfig {
+    limit: 262_144, // 2^18 bytes (~256kB)
+    mimetype: None,
+};
+
 impl Default for PayloadConfig {
     fn default() -> Self {
-        PayloadConfig {
+        DEFAULT_PAYLOAD_CONFIG.clone()
-            limit: 262_144,
-            mimetype: None,
-        }
     }
 }
 
@@ -407,8 +415,9 @@ mod tests {
     use bytes::Bytes;
 
     use super::*;
-    use crate::http::header;
+    use crate::http::{header, StatusCode};
-    use crate::test::TestRequest;
+    use crate::test::{call_service, init_service, TestRequest};
+    use crate::{web, App, Responder};
 
     #[actix_rt::test]
     async fn test_payload_config() {
@@ -428,6 +437,86 @@ mod tests {
         assert!(cfg.check_mimetype(&req).is_ok());
     }
 
+    #[actix_rt::test]
+    async fn test_config_recall_locations() {
+        async fn bytes_handler(_: Bytes) -> impl Responder {
+            "payload is probably json bytes"
+        }
+
+        async fn string_handler(_: String) -> impl Responder {
+            "payload is probably json string"
+        }
+
+        let mut srv = init_service(
+            App::new()
+                .service(
+                    web::resource("/bytes-app-data")
+                        .app_data(
+                            PayloadConfig::default().mimetype(mime::APPLICATION_JSON),
+                        )
+                        .route(web::get().to(bytes_handler)),
+                )
+                .service(
+                    web::resource("/bytes-data")
+                        .data(PayloadConfig::default().mimetype(mime::APPLICATION_JSON))
+                        .route(web::get().to(bytes_handler)),
+                )
+                .service(
+                    web::resource("/string-app-data")
+                        .app_data(
+                            PayloadConfig::default().mimetype(mime::APPLICATION_JSON),
+                        )
+                        .route(web::get().to(string_handler)),
+                )
+                .service(
+                    web::resource("/string-data")
+                        .data(PayloadConfig::default().mimetype(mime::APPLICATION_JSON))
+                        .route(web::get().to(string_handler)),
+                ),
+        )
+        .await;
+
+        let req = TestRequest::with_uri("/bytes-app-data").to_request();
+        let resp = call_service(&mut srv, req).await;
+        assert_eq!(resp.status(), StatusCode::BAD_REQUEST);
+
+        let req = TestRequest::with_uri("/bytes-data").to_request();
+        let resp = call_service(&mut srv, req).await;
+        assert_eq!(resp.status(), StatusCode::BAD_REQUEST);
+
+        let req = TestRequest::with_uri("/string-app-data").to_request();
+        let resp = call_service(&mut srv, req).await;
+        assert_eq!(resp.status(), StatusCode::BAD_REQUEST);
+
+        let req = TestRequest::with_uri("/string-data").to_request();
+        let resp = call_service(&mut srv, req).await;
+        assert_eq!(resp.status(), StatusCode::BAD_REQUEST);
+
+        let req = TestRequest::with_uri("/bytes-app-data")
+            .header(header::CONTENT_TYPE, mime::APPLICATION_JSON)
+            .to_request();
+        let resp = call_service(&mut srv, req).await;
+        assert_eq!(resp.status(), StatusCode::OK);
+
+        let req = TestRequest::with_uri("/bytes-data")
+            .header(header::CONTENT_TYPE, mime::APPLICATION_JSON)
+            .to_request();
+        let resp = call_service(&mut srv, req).await;
+        assert_eq!(resp.status(), StatusCode::OK);
+
+        let req = TestRequest::with_uri("/string-app-data")
+            .header(header::CONTENT_TYPE, mime::APPLICATION_JSON)
+            .to_request();
+        let resp = call_service(&mut srv, req).await;
+        assert_eq!(resp.status(), StatusCode::OK);
+
+        let req = TestRequest::with_uri("/string-data")
+            .header(header::CONTENT_TYPE, mime::APPLICATION_JSON)
+            .to_request();
+        let resp = call_service(&mut srv, req).await;
+        assert_eq!(resp.status(), StatusCode::OK);
+    }
+
     #[actix_rt::test]
     async fn test_bytes() {
         let (req, mut pl) = TestRequest::with_header(header::CONTENT_LENGTH, "11")