Merge branch 'master' into fix-1611-pr

.github/PULL_REQUEST_TEMPLATE.md

@@ -1,6 +1,8 @@
-## PR Type
+<!-- Thanks for considering contributing actix! -->
-What kind of change does this PR make?
+<!-- Please fill out the following to make our reviews easy. -->
 
+## PR Type
+<!-- What kind of change does this PR make? -->
 <!-- Bug Fix / Feature / Refactor / Code Style / Other -->
 INSERT_PR_TYPE
 

.github/workflows/linux.yml

@@ -8,7 +8,7 @@ jobs:
       fail-fast: false
       matrix:
         version:
-          - 1.41.1 # MSRV
+          - 1.42.0 # MSRV
           - stable
           - nightly
 

CHANGES.md

@@ -1,10 +1,19 @@
 # Changes
 
 ## Unreleased - 2020-xx-xx
+### Changed
+* `PayloadConfig` is now also considered in `Bytes` and `String` extractors when set
+  using `App::data`. [#1610]
+* `web::Path` now has a public representation: `web::Path(pub T)` that enables
+  destructuring. [#1594]
+* MSRV is now 1.42.0.
+
 ### Fixed
 * Memory leak of app data in pooled requests. [#1609]
 
+[#1594]: https://github.com/actix/actix-web/pull/1594
 [#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

MIGRATION.md

@@ -12,6 +12,26 @@
 * `BodySize::Sized64` variant has been removed. `BodySize::Sized` now receives a
   `u64` instead of a `usize`.
 
+* Code that was using `path.<index>` to access a `web::Path<(A, B, C)>`s elements now needs to use
+  destructuring or `.into_inner()`. For example:
+
+  ```rust
+  // Previously:
+  async fn some_route(path: web::Path<(String, String)>) -> String {
+    format!("Hello, {} {}", path.0, path.1)
+  }
+
+  // Now (this also worked before):
+  async fn some_route(path: web::Path<(String, String)>) -> String {
+    let (first_name, last_name) = path.into_inner();
+    format!("Hello, {} {}", first_name, last_name)
+  }
+  // Or (this wasn't previously supported):
+  async fn some_route(web::Path((first_name, last_name)): web::Path<(String, String)>) -> String {
+    format!("Hello, {} {}", first_name, last_name)
+  }
+  ```
+
 ## 2.0.0
 
 * `HttpServer::start()` renamed to `HttpServer::run()`. It also possible to

README.md

@@ -7,7 +7,7 @@
 
 [![crates.io](https://meritbadge.herokuapp.com/actix-web)](https://crates.io/crates/actix-web)
 [![Documentation](https://docs.rs/actix-web/badge.svg)](https://docs.rs/actix-web)
-[![Version](https://img.shields.io/badge/rustc-1.41+-lightgray.svg)](https://blog.rust-lang.org/2020/02/27/Rust-1.41.1.html)
+[![Version](https://img.shields.io/badge/rustc-1.42+-ab6000.svg)](https://blog.rust-lang.org/2020/03/12/Rust-1.42.html)
 ![License](https://img.shields.io/crates/l/actix-web.svg)
 <br />
 [![Build Status](https://travis-ci.org/actix/actix-web.svg?branch=master)](https://travis-ci.org/actix/actix-web) 
@@ -32,7 +32,7 @@
 * Middlewares ([Logger, Session, CORS, etc](https://actix.rs/docs/middleware/))
 * Includes an async [HTTP client](https://actix.rs/actix-web/actix_web/client/index.html)
 * Supports [Actix actor framework](https://github.com/actix/actix)
-* Runs on stable Rust 1.41+
+* Runs on stable Rust 1.42+
 
 ## Documentation
 
@@ -61,8 +61,8 @@ Code:
 use actix_web::{get, web, App, HttpServer, Responder};
 
 #[get("/{id}/{name}/index.html")]
-async fn index(info: web::Path<(u32, String)>) -> impl Responder {
+async fn index(web::Path((id, name)): web::Path<(u32, String)>) -> impl Responder {
-    format!("Hello {}! id:{}", info.1, info.0)
+    format!("Hello {}! id:{}", name, id)
 }
 
 #[actix_web::main]

actix-http/src/body.rs

@@ -192,14 +192,8 @@ impl MessageBody for Body {
 impl PartialEq for Body {
     fn eq(&self, other: &Body) -> bool {
         match *self {
-            Body::None => match *other {
+            Body::None => matches!(*other, Body::None),
-                Body::None => true,
+            Body::Empty => matches!(*other, Body::Empty),
-                _ => false,
-            },
-            Body::Empty => match *other {
-                Body::Empty => true,
-                _ => false,
-            },
             Body::Bytes(ref b) => match *other {
                 Body::Bytes(ref b2) => b == b2,
                 _ => false,

actix-http/src/client/h2proto.rs

@@ -37,10 +37,7 @@ where
     trace!("Sending client request: {:?} {:?}", head, body.size());
     let head_req = head.as_ref().method == Method::HEAD;
     let length = body.size();
-    let eof = match length {
+    let eof = matches!(length, BodySize::None | BodySize::Empty | BodySize::Sized(0));
-        BodySize::None | BodySize::Empty | BodySize::Sized(0) => true,
-        _ => false,
-    };
 
     let mut req = Request::new(());
     *req.uri_mut() = head.as_ref().uri.clone();

actix-http/src/h1/decoder.rs

@@ -652,10 +652,7 @@ mod tests {
         }
 
         fn is_unhandled(&self) -> bool {
-            match self {
+            matches!(self, PayloadType::Stream(_))
-                PayloadType::Stream(_) => true,
-                _ => false,
-            }
         }
     }
 
@@ -667,10 +664,7 @@ mod tests {
             }
         }
         fn eof(&self) -> bool {
-            match *self {
+            matches!(*self, PayloadItem::Eof)
-                PayloadItem::Eof => true,
-                _ => false,
-            }
         }
     }
 

actix-http/src/h1/dispatcher.rs

@@ -156,14 +156,8 @@ enum PollResponse {
 impl PartialEq for PollResponse {
     fn eq(&self, other: &PollResponse) -> bool {
         match self {
-            PollResponse::DrainWriteBuf => match other {
+            PollResponse::DrainWriteBuf => matches!(other, PollResponse::DrainWriteBuf),
-                PollResponse::DrainWriteBuf => true,
+            PollResponse::DoNothing => matches!(other, PollResponse::DoNothing),
-                _ => false,
-            },
-            PollResponse::DoNothing => match other {
-                PollResponse::DoNothing => true,
-                _ => false,
-            },
             _ => false,
         }
     }

actix-http/src/header/common/content_disposition.rs

@@ -387,26 +387,17 @@ impl ContentDisposition {
 
     /// Returns `true` if it is [`Inline`](DispositionType::Inline).
     pub fn is_inline(&self) -> bool {
-        match self.disposition {
+        matches!(self.disposition, DispositionType::Inline)
-            DispositionType::Inline => true,
-            _ => false,
-        }
     }
 
     /// Returns `true` if it is [`Attachment`](DispositionType::Attachment).
     pub fn is_attachment(&self) -> bool {
-        match self.disposition {
+        matches!(self.disposition, DispositionType::Attachment)
-            DispositionType::Attachment => true,
-            _ => false,
-        }
     }
 
     /// Returns `true` if it is [`FormData`](DispositionType::FormData).
     pub fn is_form_data(&self) -> bool {
-        match self.disposition {
+        matches!(self.disposition, DispositionType::FormData)
-            DispositionType::FormData => true,
-            _ => false,
-        }
     }
 
     /// Returns `true` if it is [`Ext`](DispositionType::Ext) and the `disp_type` matches.

actix-http/src/header/mod.rs

@@ -148,10 +148,7 @@ impl ContentEncoding {
     #[inline]
     /// Is the content compressed?
     pub fn is_compression(self) -> bool {
-        match self {
+        matches!(self, ContentEncoding::Identity | ContentEncoding::Auto)
-            ContentEncoding::Identity | ContentEncoding::Auto => false,
-            _ => true,
-        }
     }
 
     #[inline]

actix-http/src/ws/frame.rs

@@ -229,10 +229,7 @@ mod tests {
     fn is_none(
         frm: &Result<Option<(bool, OpCode, Option<BytesMut>)>, ProtocolError>,
     ) -> bool {
-        match *frm {
+        matches!(*frm, Ok(None))
-            Ok(None) => true,
-            _ => false,
-        }
     }
 
     fn extract(

awc/src/response.rs

@@ -402,14 +402,9 @@ mod tests {
 
     fn json_eq(err: JsonPayloadError, other: JsonPayloadError) -> bool {
         match err {
-            JsonPayloadError::Payload(PayloadError::Overflow) => match other {
+            JsonPayloadError::Payload(PayloadError::Overflow) =>
-                JsonPayloadError::Payload(PayloadError::Overflow) => true,
+                matches!(other, JsonPayloadError::Payload(PayloadError::Overflow)),
-                _ => false,
+            JsonPayloadError::ContentType => matches!(other, JsonPayloadError::ContentType),
-            },
-            JsonPayloadError::ContentType => match other {
-                JsonPayloadError::ContentType => true,
-                _ => false,
-            },
             _ => false,
         }
     }

rust-toolchain

@@ -1 +1 @@
-1.41.1
+1.42.0

src/lib.rs

@@ -9,8 +9,8 @@
 //! use actix_web::{get, web, App, HttpServer, Responder};
 //!
 //! #[get("/{id}/{name}/index.html")]
-//! async fn index(info: web::Path<(u32, String)>) -> impl Responder {
+//! async fn index(web::Path((id, name)): web::Path<(u32, String)>) -> impl Responder {
-//!     format!("Hello {}! id:{}", info.1, info.0)
+//!     format!("Hello {}! id:{}", name, id)
 //! }
 //!
 //! #[actix_web::main]

src/types/form.rs

@@ -407,18 +407,9 @@ mod tests {
 
     fn eq(err: UrlencodedError, other: UrlencodedError) -> bool {
         match err {
-            UrlencodedError::Overflow { .. } => match other {
+            UrlencodedError::Overflow { .. } => matches!(other, UrlencodedError::Overflow { .. }),
-                UrlencodedError::Overflow { .. } => true,
+            UrlencodedError::UnknownLength => matches!(other, UrlencodedError::UnknownLength),
-                _ => false,
+            UrlencodedError::ContentType => matches!(other, UrlencodedError::ContentType),
-            },
-            UrlencodedError::UnknownLength => match other {
-                UrlencodedError::UnknownLength => true,
-                _ => false,
-            },
-            UrlencodedError::ContentType => match other {
-                UrlencodedError::ContentType => true,
-                _ => false,
-            },
             _ => false,
         }
     }

src/types/json.rs

@@ -433,14 +433,8 @@ mod tests {
 
     fn json_eq(err: JsonPayloadError, other: JsonPayloadError) -> bool {
         match err {
-            JsonPayloadError::Overflow => match other {
+            JsonPayloadError::Overflow => matches!(other, JsonPayloadError::Overflow),
-                JsonPayloadError::Overflow => true,
+            JsonPayloadError::ContentType => matches!(other, JsonPayloadError::ContentType),
-                _ => false,
-            },
-            JsonPayloadError::ContentType => match other {
-                JsonPayloadError::ContentType => true,
-                _ => false,
-            },
             _ => false,
         }
     }

src/types/path.rs

@@ -25,8 +25,8 @@ use crate::FromRequest;
 /// /// extract path info from "/{username}/{count}/index.html" url
 /// /// {username} - deserializes to a String
 /// /// {count} -  - deserializes to a u32
-/// async fn index(info: web::Path<(String, u32)>) -> String {
+/// async fn index(web::Path((username, count)): web::Path<(String, u32)>) -> String {
-///     format!("Welcome {}! {}", info.0, info.1)
+///     format!("Welcome {}! {}", username, count)
 /// }
 ///
 /// fn main() {
@@ -61,20 +61,18 @@ use crate::FromRequest;
 ///     );
 /// }
 /// ```
-pub struct Path<T> {
+pub struct Path<T>(pub T);
-    inner: T,
-}
 
 impl<T> Path<T> {
     /// Deconstruct to an inner value
     pub fn into_inner(self) -> T {
-        self.inner
+        self.0
     }
 }
 
 impl<T> AsRef<T> for Path<T> {
     fn as_ref(&self) -> &T {
-        &self.inner
+        &self.0
     }
 }
 
@@ -82,31 +80,31 @@ impl<T> ops::Deref for Path<T> {
     type Target = T;
 
     fn deref(&self) -> &T {
-        &self.inner
+        &self.0
     }
 }
 
 impl<T> ops::DerefMut for Path<T> {
     fn deref_mut(&mut self) -> &mut T {
-        &mut self.inner
+        &mut self.0
     }
 }
 
 impl<T> From<T> for Path<T> {
     fn from(inner: T) -> Path<T> {
-        Path { inner }
+        Path(inner)
     }
 }
 
 impl<T: fmt::Debug> fmt::Debug for Path<T> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.inner.fmt(f)
+        self.0.fmt(f)
     }
 }
 
 impl<T: fmt::Display> fmt::Display for Path<T> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.inner.fmt(f)
+        self.0.fmt(f)
     }
 }
 
@@ -120,8 +118,8 @@ impl<T: fmt::Display> fmt::Display for Path<T> {
 /// /// extract path info from "/{username}/{count}/index.html" url
 /// /// {username} - deserializes to a String
 /// /// {count} -  - deserializes to a u32
-/// async fn index(info: web::Path<(String, u32)>) -> String {
+/// async fn index(web::Path((username, count)): web::Path<(String, u32)>) -> String {
-///     format!("Welcome {}! {}", info.0, info.1)
+///     format!("Welcome {}! {}", username, count)
 /// }
 ///
 /// fn main() {
@@ -173,7 +171,7 @@ where
 
         ready(
             de::Deserialize::deserialize(PathDeserializer::new(req.match_info()))
-                .map(|inner| Path { inner })
+                .map(Path)
                 .map_err(move |e| {
                     log::debug!(
                         "Failed during Path extractor deserialization. \
@@ -290,21 +288,22 @@ mod tests {
         resource.match_path(req.match_info_mut());
 
         let (req, mut pl) = req.into_parts();
-        let res = <(Path<(String, String)>,)>::from_request(&req, &mut pl)
+        let (Path(res),) = <(Path<(String, String)>,)>::from_request(&req, &mut pl)
             .await
             .unwrap();
-        assert_eq!((res.0).0, "name");
+        assert_eq!(res.0, "name");
-        assert_eq!((res.0).1, "user1");
+        assert_eq!(res.1, "user1");
 
-        let res = <(Path<(String, String)>, Path<(String, String)>)>::from_request(
+        let (Path(a), Path(b)) =
+            <(Path<(String, String)>, Path<(String, String)>)>::from_request(
                 &req, &mut pl,
             )
             .await
             .unwrap();
-        assert_eq!((res.0).0, "name");
+        assert_eq!(a.0, "name");
-        assert_eq!((res.0).1, "user1");
+        assert_eq!(a.1, "user1");
-        assert_eq!((res.1).0, "name");
+        assert_eq!(b.0, "name");
-        assert_eq!((res.1).1, "user1");
+        assert_eq!(b.1, "user1");
 
         let () = <()>::from_request(&req, &mut pl).await.unwrap();
     }
@@ -329,7 +328,7 @@ mod tests {
         let s = s.into_inner();
         assert_eq!(s.value, "user2");
 
-        let s = Path::<(String, String)>::from_request(&req, &mut pl)
+        let Path(s) = Path::<(String, String)>::from_request(&req, &mut pl)
             .await
             .unwrap();
         assert_eq!(s.0, "name");
@@ -344,7 +343,7 @@ mod tests {
         assert_eq!(s.as_ref().key, "name");
         assert_eq!(s.value, 32);
 
-        let s = Path::<(String, u8)>::from_request(&req, &mut pl)
+        let Path(s) = Path::<(String, u8)>::from_request(&req, &mut pl)
             .await
             .unwrap();
         assert_eq!(s.0, "name");

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")