Merge branch 'master' into feat/io-uring

.github/workflows/ci.yml

@@ -78,7 +78,7 @@ jobs:
 
       - name: tests
         uses: actions-rs/cargo@v1
-        timeout-minutes: 40
+        timeout-minutes: 60
         with:
           command: ci-test
           args: --skip=test_reading_deflate_encoding_large_random_rustls
@@ -174,5 +174,5 @@ jobs:
 
       - name: doc tests
         uses: actions-rs/cargo@v1
-        timeout-minutes: 40
+        timeout-minutes: 60
         with: { command: ci-doctest }

Cargo.toml

@@ -112,7 +112,7 @@ awc = { version = "3.0.0-beta.10", features = ["openssl"] }
 
 brotli2 = "0.3.2"
 criterion = { version = "0.3", features = ["html_reports"] }
-env_logger = "0.8"
+env_logger = "0.9"
 flate2 = "1.0.13"
 futures-util = { version = "0.3.7", default-features = false, features = ["std"] }
 rand = "0.8"
@@ -120,7 +120,7 @@ rcgen = "0.8"
 rustls-pemfile = "0.2"
 tls-openssl = { package = "openssl", version = "0.10.9" }
 tls-rustls = { package = "rustls", version = "0.20.0" }
-zstd = "0.7"
+zstd = "0.9"
 
 [profile.dev]
 # Disabling debug info speeds up builds a bunch and we don't rely on it for debugging that much.

actix-files/src/path_buf.rs

@@ -8,7 +8,7 @@ use actix_web::{dev::Payload, FromRequest, HttpRequest};
 
 use crate::error::UriSegmentError;
 
-#[derive(Debug)]
+#[derive(Debug, PartialEq, Eq)]
 pub(crate) struct PathBufWrap(PathBuf);
 
 impl FromStr for PathBufWrap {
@@ -21,6 +21,8 @@ impl FromStr for PathBufWrap {
 
 impl PathBufWrap {
     /// Parse a path, giving the choice of allowing hidden files to be considered valid segments.
+    ///
+    /// Path traversal is guarded by this method.
     pub fn parse_path(path: &str, hidden_files: bool) -> Result<Self, UriSegmentError> {
         let mut buf = PathBuf::new();
 
@@ -115,4 +117,24 @@ mod tests {
             PathBuf::from_iter(vec!["test", ".tt"])
         );
     }
+
+    #[test]
+    fn path_traversal() {
+        assert_eq!(
+            PathBufWrap::parse_path("/../README.md", false).unwrap().0,
+            PathBuf::from_iter(vec!["README.md"])
+        );
+
+        assert_eq!(
+            PathBufWrap::parse_path("/../README.md", true).unwrap().0,
+            PathBuf::from_iter(vec!["README.md"])
+        );
+
+        assert_eq!(
+            PathBufWrap::parse_path("/../../../../../../../../../../etc/passwd", false)
+                .unwrap()
+                .0,
+            PathBuf::from_iter(vec!["etc/passwd"])
+        );
+    }
 }

actix-files/tests/traversal.rs

@@ -0,0 +1,27 @@
+use actix_files::Files;
+use actix_web::{
+    http::StatusCode,
+    test::{self, TestRequest},
+    App,
+};
+
+#[actix_rt::test]
+async fn test_directory_traversal_prevention() {
+    let srv = test::init_service(App::new().service(Files::new("/", "./tests"))).await;
+
+    let req =
+        TestRequest::with_uri("/../../../../../../../../../../../etc/passwd").to_request();
+    let res = test::call_service(&srv, req).await;
+    assert_eq!(res.status(), StatusCode::NOT_FOUND);
+
+    let req = TestRequest::with_uri(
+        "/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/etc/passwd",
+    )
+    .to_request();
+    let res = test::call_service(&srv, req).await;
+    assert_eq!(res.status(), StatusCode::NOT_FOUND);
+
+    let req = TestRequest::with_uri("/%00/etc/passwd%00").to_request();
+    let res = test::call_service(&srv, req).await;
+    assert_eq!(res.status(), StatusCode::NOT_FOUND);
+}

actix-http/Cargo.toml

@@ -78,7 +78,7 @@ actix-tls = { version = "3.0.0-beta.7", default-features = false, optional = tru
 # compression
 brotli2 = { version="0.3.2", optional = true }
 flate2 = { version = "1.0.13", optional = true }
-zstd = { version = "0.7", optional = true }
+zstd = { version = "0.9", optional = true }
 
 [dev-dependencies]
 actix-server = "2.0.0-beta.9"
@@ -86,7 +86,7 @@ actix-http-test = { version = "3.0.0-beta.6", features = ["openssl"] }
 actix-tls = { version = "3.0.0-beta.7", features = ["openssl"] }
 async-stream = "0.3"
 criterion = { version = "0.3", features = ["html_reports"] }
-env_logger = "0.8"
+env_logger = "0.9"
 rcgen = "0.8"
 regex = "1.3"
 rustls-pemfile = "0.2"

actix-http/src/body/body.rs

@@ -139,56 +139,56 @@ impl<S: fmt::Debug> fmt::Debug for AnyBody<S> {
     }
 }
 
-impl From<&'static str> for AnyBody {
+impl<B> From<&'static str> for AnyBody<B> {
-    fn from(string: &'static str) -> AnyBody {
+    fn from(string: &'static str) -> Self {
-        AnyBody::Bytes(Bytes::from_static(string.as_ref()))
+        Self::Bytes(Bytes::from_static(string.as_ref()))
     }
 }
 
-impl From<&'static [u8]> for AnyBody {
+impl<B> From<&'static [u8]> for AnyBody<B> {
-    fn from(bytes: &'static [u8]) -> AnyBody {
+    fn from(bytes: &'static [u8]) -> Self {
-        AnyBody::Bytes(Bytes::from_static(bytes))
+        Self::Bytes(Bytes::from_static(bytes))
     }
 }
 
-impl From<Vec<u8>> for AnyBody {
+impl<B> From<Vec<u8>> for AnyBody<B> {
-    fn from(vec: Vec<u8>) -> AnyBody {
+    fn from(vec: Vec<u8>) -> Self {
-        AnyBody::Bytes(Bytes::from(vec))
+        Self::Bytes(Bytes::from(vec))
     }
 }
 
-impl From<String> for AnyBody {
+impl<B> From<String> for AnyBody<B> {
-    fn from(string: String) -> AnyBody {
+    fn from(string: String) -> Self {
-        string.into_bytes().into()
+        Self::Bytes(Bytes::from(string))
     }
 }
 
-impl From<&'_ String> for AnyBody {
+impl<B> From<&'_ String> for AnyBody<B> {
-    fn from(string: &String) -> AnyBody {
+    fn from(string: &String) -> Self {
-        AnyBody::Bytes(Bytes::copy_from_slice(AsRef::<[u8]>::as_ref(&string)))
+        Self::Bytes(Bytes::copy_from_slice(AsRef::<[u8]>::as_ref(&string)))
     }
 }
 
-impl From<Cow<'_, str>> for AnyBody {
+impl<B> From<Cow<'_, str>> for AnyBody<B> {
-    fn from(string: Cow<'_, str>) -> AnyBody {
+    fn from(string: Cow<'_, str>) -> Self {
         match string {
-            Cow::Owned(s) => AnyBody::from(s),
+            Cow::Owned(s) => Self::from(s),
             Cow::Borrowed(s) => {
-                AnyBody::Bytes(Bytes::copy_from_slice(AsRef::<[u8]>::as_ref(s)))
+                Self::Bytes(Bytes::copy_from_slice(AsRef::<[u8]>::as_ref(s)))
             }
         }
     }
 }
 
-impl From<Bytes> for AnyBody {
+impl<B> From<Bytes> for AnyBody<B> {
     fn from(bytes: Bytes) -> Self {
-        AnyBody::Bytes(bytes)
+        Self::Bytes(bytes)
     }
 }
 
-impl From<BytesMut> for AnyBody {
+impl<B> From<BytesMut> for AnyBody<B> {
     fn from(bytes: BytesMut) -> Self {
-        AnyBody::Bytes(bytes.freeze())
+        Self::Bytes(bytes.freeze())
     }
 }
 

actix-http/src/body/mod.rs

@@ -86,11 +86,14 @@ mod tests {
         }
     }
 
+    /// AnyBody alias because rustc does not (can not?) infer the default type parameter.
+    type TestBody = AnyBody;
+
     #[actix_rt::test]
     async fn test_static_str() {
-        assert_eq!(AnyBody::from("").size(), BodySize::Sized(0));
+        assert_eq!(TestBody::from("").size(), BodySize::Sized(0));
-        assert_eq!(AnyBody::from("test").size(), BodySize::Sized(4));
+        assert_eq!(TestBody::from("test").size(), BodySize::Sized(4));
-        assert_eq!(AnyBody::from("test").get_ref(), b"test");
+        assert_eq!(TestBody::from("test").get_ref(), b"test");
 
         assert_eq!("test".size(), BodySize::Sized(4));
         assert_eq!(
@@ -104,14 +107,14 @@ mod tests {
 
     #[actix_rt::test]
     async fn test_static_bytes() {
-        assert_eq!(AnyBody::from(b"test".as_ref()).size(), BodySize::Sized(4));
+        assert_eq!(TestBody::from(b"test".as_ref()).size(), BodySize::Sized(4));
-        assert_eq!(AnyBody::from(b"test".as_ref()).get_ref(), b"test");
+        assert_eq!(TestBody::from(b"test".as_ref()).get_ref(), b"test");
         assert_eq!(
-            AnyBody::copy_from_slice(b"test".as_ref()).size(),
+            TestBody::copy_from_slice(b"test".as_ref()).size(),
             BodySize::Sized(4)
         );
         assert_eq!(
-            AnyBody::copy_from_slice(b"test".as_ref()).get_ref(),
+            TestBody::copy_from_slice(b"test".as_ref()).get_ref(),
             b"test"
         );
         let sb = Bytes::from(&b"test"[..]);
@@ -126,8 +129,8 @@ mod tests {
 
     #[actix_rt::test]
     async fn test_vec() {
-        assert_eq!(AnyBody::from(Vec::from("test")).size(), BodySize::Sized(4));
+        assert_eq!(TestBody::from(Vec::from("test")).size(), BodySize::Sized(4));
-        assert_eq!(AnyBody::from(Vec::from("test")).get_ref(), b"test");
+        assert_eq!(TestBody::from(Vec::from("test")).get_ref(), b"test");
         let test_vec = Vec::from("test");
         pin!(test_vec);
 
@@ -144,8 +147,8 @@ mod tests {
     #[actix_rt::test]
     async fn test_bytes() {
         let b = Bytes::from("test");
-        assert_eq!(AnyBody::from(b.clone()).size(), BodySize::Sized(4));
+        assert_eq!(TestBody::from(b.clone()).size(), BodySize::Sized(4));
-        assert_eq!(AnyBody::from(b.clone()).get_ref(), b"test");
+        assert_eq!(TestBody::from(b.clone()).get_ref(), b"test");
         pin!(b);
 
         assert_eq!(b.size(), BodySize::Sized(4));
@@ -158,8 +161,8 @@ mod tests {
     #[actix_rt::test]
     async fn test_bytes_mut() {
         let b = BytesMut::from("test");
-        assert_eq!(AnyBody::from(b.clone()).size(), BodySize::Sized(4));
+        assert_eq!(TestBody::from(b.clone()).size(), BodySize::Sized(4));
-        assert_eq!(AnyBody::from(b.clone()).get_ref(), b"test");
+        assert_eq!(TestBody::from(b.clone()).get_ref(), b"test");
         pin!(b);
 
         assert_eq!(b.size(), BodySize::Sized(4));
@@ -172,10 +175,10 @@ mod tests {
     #[actix_rt::test]
     async fn test_string() {
         let b = "test".to_owned();
-        assert_eq!(AnyBody::from(b.clone()).size(), BodySize::Sized(4));
+        assert_eq!(TestBody::from(b.clone()).size(), BodySize::Sized(4));
-        assert_eq!(AnyBody::from(b.clone()).get_ref(), b"test");
+        assert_eq!(TestBody::from(b.clone()).get_ref(), b"test");
-        assert_eq!(AnyBody::from(&b).size(), BodySize::Sized(4));
+        assert_eq!(TestBody::from(&b).size(), BodySize::Sized(4));
-        assert_eq!(AnyBody::from(&b).get_ref(), b"test");
+        assert_eq!(TestBody::from(&b).get_ref(), b"test");
         pin!(b);
 
         assert_eq!(b.size(), BodySize::Sized(4));
@@ -216,22 +219,22 @@ mod tests {
 
     #[actix_rt::test]
     async fn test_body_debug() {
-        assert!(format!("{:?}", AnyBody::<BoxBody>::None).contains("Body::None"));
+        assert!(format!("{:?}", TestBody::None).contains("Body::None"));
-        assert!(format!("{:?}", AnyBody::from(Bytes::from_static(b"1"))).contains('1'));
+        assert!(format!("{:?}", TestBody::from(Bytes::from_static(b"1"))).contains('1'));
     }
 
     #[actix_rt::test]
     async fn test_serde_json() {
         use serde_json::{json, Value};
         assert_eq!(
-            AnyBody::from(
+            TestBody::from(
                 serde_json::to_vec(&Value::String("test".to_owned())).unwrap()
             )
             .size(),
             BodySize::Sized(6)
         );
         assert_eq!(
-            AnyBody::from(
+            TestBody::from(
                 serde_json::to_vec(&json!({"test-key":"test-value"})).unwrap()
             )
             .size(),

actix-http/src/error.rs

@@ -66,7 +66,7 @@ impl Error {
     }
 }
 
-impl From<Error> for Response<AnyBody> {
+impl<B> From<Error> for Response<AnyBody<B>> {
     fn from(err: Error) -> Self {
         let status_code = match err.inner.kind {
             Kind::Parse => StatusCode::BAD_REQUEST,

actix-multipart/CHANGES.md

@@ -1,6 +1,14 @@
 # Changes
 
 ## Unreleased - 2021-xx-xx
+* Ensure a correct Content-Disposition header is included in every part of a multipart message. [#2451]
+* Added `MultipartError::NoContentDisposition` variant. [#2451]
+* Since Content-Disposition is now ensured, `Field::content_disposition` is now infallible. [#2451]
+* Added `Field::name` method for getting the field name. [#2451]
+* `MultipartError` now marks variants with inner errors as the source. [#2451]
+* `MultipartError` is now marked as non-exhaustive. [#2451]
+
+[#2451]: https://github.com/actix/actix-web/pull/2451
 
 
 ## 0.4.0-beta.7 - 2021-10-20

actix-multipart/src/error.rs

@@ -2,39 +2,52 @@
 use actix_web::error::{ParseError, PayloadError};
 use actix_web::http::StatusCode;
 use actix_web::ResponseError;
-use derive_more::{Display, From};
+use derive_more::{Display, Error, From};
 
 /// A set of errors that can occur during parsing multipart streams
-#[derive(Debug, Display, From)]
+#[non_exhaustive]
+#[derive(Debug, Display, From, Error)]
 pub enum MultipartError {
+    /// Content-Disposition header is not found or is not equal to "form-data".
+    ///
+    /// According to [RFC 7578](https://tools.ietf.org/html/rfc7578#section-4.2) a
+    /// Content-Disposition header must always be present and equal to "form-data".
+    #[display(fmt = "No Content-Disposition `form-data` header")]
+    NoContentDisposition,
+
     /// Content-Type header is not found
-    #[display(fmt = "No Content-type header found")]
+    #[display(fmt = "No Content-Type header found")]
     NoContentType,
+
     /// Can not parse Content-Type header
     #[display(fmt = "Can not parse Content-Type header")]
     ParseContentType,
+
     /// Multipart boundary is not found
     #[display(fmt = "Multipart boundary is not found")]
     Boundary,
+
     /// Nested multipart is not supported
     #[display(fmt = "Nested multipart is not supported")]
     Nested,
+
     /// Multipart stream is incomplete
     #[display(fmt = "Multipart stream is incomplete")]
     Incomplete,
+
     /// Error during field parsing
     #[display(fmt = "{}", _0)]
     Parse(ParseError),
+
     /// Payload error
     #[display(fmt = "{}", _0)]
     Payload(PayloadError),
+
     /// Not consumed
     #[display(fmt = "Multipart stream is not consumed")]
     NotConsumed,
 }
 
-impl std::error::Error for MultipartError {}
-
 /// Return `BadRequest` for `MultipartError`
 impl ResponseError for MultipartError {
     fn status_code(&self) -> StatusCode {

actix-multipart/src/server.rs

@@ -1,15 +1,20 @@
 //! Multipart response payload support.
 
-use std::cell::{Cell, RefCell, RefMut};
+use std::{
-use std::convert::TryFrom;
+    cell::{Cell, RefCell, RefMut},
-use std::marker::PhantomData;
+    cmp,
-use std::pin::Pin;
+    convert::TryFrom,
-use std::rc::Rc;
+    fmt,
-use std::task::{Context, Poll};
+    marker::PhantomData,
-use std::{cmp, fmt};
+    pin::Pin,
+    rc::Rc,
+    task::{Context, Poll},
+};
 
-use actix_web::error::{ParseError, PayloadError};
+use actix_web::{
-use actix_web::http::header::{self, ContentDisposition, HeaderMap, HeaderName, HeaderValue};
+    error::{ParseError, PayloadError},
+    http::header::{self, ContentDisposition, HeaderMap, HeaderName, HeaderValue},
+};
 use bytes::{Bytes, BytesMut};
 use futures_core::stream::{LocalBoxStream, Stream};
 use futures_util::stream::StreamExt as _;
@@ -40,10 +45,13 @@ enum InnerMultipartItem {
 enum InnerState {
     /// Stream eof
     Eof,
+
     /// Skip data until first boundary
     FirstBoundary,
+
     /// Reading boundary
     Boundary,
+
     /// Reading Headers,
     Headers,
 }
@@ -332,31 +340,55 @@ impl InnerMultipart {
                 return Poll::Pending;
             };
 
-            // content type
+            // According to [RFC 7578](https://tools.ietf.org/html/rfc7578#section-4.2) a
-            let mut mt = mime::APPLICATION_OCTET_STREAM;
+            // Content-Disposition header must always be present and set to "form-data".
-            if let Some(content_type) = headers.get(&header::CONTENT_TYPE) {
+
-                if let Ok(content_type) = content_type.to_str() {
+            let content_disposition = headers
-                    if let Ok(ct) = content_type.parse::<mime::Mime>() {
+                .get(&header::CONTENT_DISPOSITION)
-                        mt = ct;
+                .and_then(|cd| ContentDisposition::from_raw(cd).ok())
-                    }
+                .filter(|content_disposition| {
-                }
+                    let is_form_data =
-            }
+                        content_disposition.disposition == header::DispositionType::FormData;
+
+                    let has_field_name = content_disposition
+                        .parameters
+                        .iter()
+                        .any(|param| matches!(param, header::DispositionParam::Name(_)));
+
+                    is_form_data && has_field_name
+                });
+
+            let cd = if let Some(content_disposition) = content_disposition {
+                content_disposition
+            } else {
+                return Poll::Ready(Some(Err(MultipartError::NoContentDisposition)));
+            };
+
+            let ct: mime::Mime = headers
+                .get(&header::CONTENT_TYPE)
+                .and_then(|ct| ct.to_str().ok())
+                .and_then(|ct| ct.parse().ok())
+                .unwrap_or(mime::APPLICATION_OCTET_STREAM);
 
             self.state = InnerState::Boundary;
 
-            // nested multipart stream
+            // nested multipart stream is not supported
-            if mt.type_() == mime::MULTIPART {
+            if ct.type_() == mime::MULTIPART {
-                Poll::Ready(Some(Err(MultipartError::Nested)))
+                return Poll::Ready(Some(Err(MultipartError::Nested)));
-            } else {
+            }
-                let field = Rc::new(RefCell::new(InnerField::new(
+
-                    self.payload.clone(),
+            let field =
-                    self.boundary.clone(),
+                InnerField::new_in_rc(self.payload.clone(), self.boundary.clone(), &headers)?;
-                    &headers,
+
-                )?));
             self.item = InnerMultipartItem::Field(Rc::clone(&field));
 
-                Poll::Ready(Some(Ok(Field::new(safety.clone(cx), headers, mt, field))))
+            Poll::Ready(Some(Ok(Field::new(
-            }
+                safety.clone(cx),
+                headers,
+                ct,
+                cd,
+                field,
+            ))))
         }
     }
 }
@@ -371,6 +403,7 @@ impl Drop for InnerMultipart {
 /// A single field in a multipart stream
 pub struct Field {
     ct: mime::Mime,
+    cd: ContentDisposition,
     headers: HeaderMap,
     inner: Rc<RefCell<InnerField>>,
     safety: Safety,
@@ -381,35 +414,51 @@ impl Field {
         safety: Safety,
         headers: HeaderMap,
         ct: mime::Mime,
+        cd: ContentDisposition,
         inner: Rc<RefCell<InnerField>>,
     ) -> Self {
         Field {
             ct,
+            cd,
             headers,
             inner,
             safety,
         }
     }
 
-    /// Get a map of headers
+    /// Returns a reference to the field's header map.
     pub fn headers(&self) -> &HeaderMap {
         &self.headers
     }
 
-    /// Get the content type of the field
+    /// Returns a reference to the field's content (mime) type.
     pub fn content_type(&self) -> &mime::Mime {
         &self.ct
     }
 
-    /// Get the content disposition of the field, if it exists
+    /// Returns the field's Content-Disposition.
-    pub fn content_disposition(&self) -> Option<ContentDisposition> {
+    ///
-        // RFC 7578: 'Each part MUST contain a Content-Disposition header field
+    /// Per [RFC 7578 §4.2]: 'Each part MUST contain a Content-Disposition header field where the
-        // where the disposition type is "form-data".'
+    /// disposition type is "form-data". The Content-Disposition header field MUST also contain an
-        if let Some(content_disposition) = self.headers.get(&header::CONTENT_DISPOSITION) {
+    /// additional parameter of "name"; the value of the "name" parameter is the original field name
-            ContentDisposition::from_raw(content_disposition).ok()
+    /// from the form.'
-        } else {
+    ///
-            None
+    /// This crate validates that it exists before returning a `Field`. As such, it is safe to
+    /// unwrap `.content_disposition().get_name()`. The [name](Self::name) method is provided as
+    /// a convenience.
+    ///
+    /// [RFC 7578 §4.2]: https://datatracker.ietf.org/doc/html/rfc7578#section-4.2
+    pub fn content_disposition(&self) -> &ContentDisposition {
+        &self.cd
     }
+
+    /// Returns the field's name.
+    ///
+    /// See [content_disposition] regarding guarantees about
+    pub fn name(&self) -> &str {
+        self.content_disposition()
+            .get_name()
+            .expect("field name should be guaranteed to exist in multipart form-data")
     }
 }
 
@@ -451,20 +500,23 @@ struct InnerField {
 }
 
 impl InnerField {
+    fn new_in_rc(
+        payload: PayloadRef,
+        boundary: String,
+        headers: &HeaderMap,
+    ) -> Result<Rc<RefCell<InnerField>>, PayloadError> {
+        Self::new(payload, boundary, headers).map(|this| Rc::new(RefCell::new(this)))
+    }
+
     fn new(
         payload: PayloadRef,
         boundary: String,
         headers: &HeaderMap,
     ) -> Result<InnerField, PayloadError> {
         let len = if let Some(len) = headers.get(&header::CONTENT_LENGTH) {
-            if let Ok(s) = len.to_str() {
+            match len.to_str().ok().and_then(|len| len.parse::<u64>().ok()) {
-                if let Ok(len) = s.parse::<u64>() {
+                Some(len) => Some(len),
-                    Some(len)
+                None => return Err(PayloadError::Incomplete(None)),
-                } else {
-                    return Err(PayloadError::Incomplete(None));
-                }
-            } else {
-                return Err(PayloadError::Incomplete(None));
             }
         } else {
             None
@@ -658,9 +710,8 @@ impl Clone for PayloadRef {
     }
 }
 
-/// Counter. It tracks of number of clones of payloads and give access to
+/// Counter. It tracks of number of clones of payloads and give access to payload only to top most
-/// payload only to top most task panics if Safety get destroyed and it not top
+/// task panics if Safety get destroyed and it not top most task.
-/// most task.
 #[derive(Debug)]
 struct Safety {
     task: LocalWaker,
@@ -707,11 +758,12 @@ impl Drop for Safety {
         if Rc::strong_count(&self.payload) != self.level {
             self.clean.set(true);
         }
+
         self.task.wake();
     }
 }
 
-/// Payload buffer
+/// Payload buffer.
 struct PayloadBuffer {
     eof: bool,
     buf: BytesMut,
@@ -719,7 +771,7 @@ struct PayloadBuffer {
 }
 
 impl PayloadBuffer {
-    /// Create new `PayloadBuffer` instance
+    /// Constructs new `PayloadBuffer` instance.
     fn new<S>(stream: S) -> Self
     where
         S: Stream<Item = Result<Bytes, PayloadError>> + 'static,
@@ -767,7 +819,7 @@ impl PayloadBuffer {
     }
 
     /// Read until specified ending
-    pub fn read_until(&mut self, line: &[u8]) -> Result<Option<Bytes>, MultipartError> {
+    fn read_until(&mut self, line: &[u8]) -> Result<Option<Bytes>, MultipartError> {
         let res = twoway::find_bytes(&self.buf, line)
             .map(|idx| self.buf.split_to(idx + line.len()).freeze());
 
@@ -779,12 +831,12 @@ impl PayloadBuffer {
     }
 
     /// Read bytes until new line delimiter
-    pub fn readline(&mut self) -> Result<Option<Bytes>, MultipartError> {
+    fn readline(&mut self) -> Result<Option<Bytes>, MultipartError> {
         self.read_until(b"\n")
     }
 
     /// Read bytes until new line delimiter or eof
-    pub fn readline_or_eof(&mut self) -> Result<Option<Bytes>, MultipartError> {
+    fn readline_or_eof(&mut self) -> Result<Option<Bytes>, MultipartError> {
         match self.readline() {
             Err(MultipartError::Incomplete) if self.eof => Ok(Some(self.buf.split().freeze())),
             line => line,
@@ -792,7 +844,7 @@ impl PayloadBuffer {
     }
 
     /// Put unprocessed data back to the buffer
-    pub fn unprocessed(&mut self, data: Bytes) {
+    fn unprocessed(&mut self, data: Bytes) {
         let buf = BytesMut::from(data.as_ref());
         let buf = std::mem::replace(&mut self.buf, buf);
         self.buf.extend_from_slice(&buf);
@@ -914,6 +966,7 @@ mod tests {
              Content-Type: text/plain; charset=utf-8\r\nContent-Length: 4\r\n\r\n\
              test\r\n\
              --abbc761f78ff4d7cb7573b5a23f96ef0\r\n\
+             Content-Disposition: form-data; name=\"file\"; filename=\"fn.txt\"\r\n\
              Content-Type: text/plain; charset=utf-8\r\nContent-Length: 4\r\n\r\n\
              data\r\n\
              --abbc761f78ff4d7cb7573b5a23f96ef0--\r\n",
@@ -965,7 +1018,7 @@ mod tests {
         let mut multipart = Multipart::new(&headers, payload);
         match multipart.next().await {
             Some(Ok(mut field)) => {
-                let cd = field.content_disposition().unwrap();
+                let cd = field.content_disposition();
                 assert_eq!(cd.disposition, DispositionType::FormData);
                 assert_eq!(cd.parameters[0], DispositionParam::Name("file".into()));
 
@@ -1027,7 +1080,7 @@ mod tests {
         let mut multipart = Multipart::new(&headers, payload);
         match multipart.next().await.unwrap() {
             Ok(mut field) => {
-                let cd = field.content_disposition().unwrap();
+                let cd = field.content_disposition();
                 assert_eq!(cd.disposition, DispositionType::FormData);
                 assert_eq!(cd.parameters[0], DispositionParam::Name("file".into()));
 
@@ -1182,4 +1235,59 @@ mod tests {
             _ => unreachable!(),
         }
     }
+
+    #[actix_rt::test]
+    async fn no_content_disposition() {
+        let bytes = Bytes::from(
+            "testasdadsad\r\n\
+             --abbc761f78ff4d7cb7573b5a23f96ef0\r\n\
+             Content-Type: text/plain; charset=utf-8\r\nContent-Length: 4\r\n\r\n\
+             test\r\n\
+             --abbc761f78ff4d7cb7573b5a23f96ef0\r\n",
+        );
+        let mut headers = HeaderMap::new();
+        headers.insert(
+            header::CONTENT_TYPE,
+            header::HeaderValue::from_static(
+                "multipart/mixed; boundary=\"abbc761f78ff4d7cb7573b5a23f96ef0\"",
+            ),
+        );
+        let payload = SlowStream::new(bytes);
+
+        let mut multipart = Multipart::new(&headers, payload);
+        let res = multipart.next().await.unwrap();
+        assert!(res.is_err());
+        assert!(matches!(
+            res.unwrap_err(),
+            MultipartError::NoContentDisposition,
+        ));
+    }
+
+    #[actix_rt::test]
+    async fn no_name_in_content_disposition() {
+        let bytes = Bytes::from(
+            "testasdadsad\r\n\
+             --abbc761f78ff4d7cb7573b5a23f96ef0\r\n\
+             Content-Disposition: form-data; filename=\"fn.txt\"\r\n\
+             Content-Type: text/plain; charset=utf-8\r\nContent-Length: 4\r\n\r\n\
+             test\r\n\
+             --abbc761f78ff4d7cb7573b5a23f96ef0\r\n",
+        );
+        let mut headers = HeaderMap::new();
+        headers.insert(
+            header::CONTENT_TYPE,
+            header::HeaderValue::from_static(
+                "multipart/mixed; boundary=\"abbc761f78ff4d7cb7573b5a23f96ef0\"",
+            ),
+        );
+        let payload = SlowStream::new(bytes);
+
+        let mut multipart = Multipart::new(&headers, payload);
+        let res = multipart.next().await.unwrap();
+        assert!(res.is_err());
+        assert!(matches!(
+            res.unwrap_err(),
+            MultipartError::NoContentDisposition,
+        ));
+    }
 }

actix-web-actors/Cargo.toml

@@ -30,5 +30,5 @@ actix-rt = "2.2"
 actix-test = "0.1.0-beta.6"
 
 awc = { version = "3.0.0-beta.10", default-features = false }
-env_logger = "0.8"
+env_logger = "0.9"
 futures-util = { version = "0.3.7", default-features = false }

awc/Cargo.toml

@@ -97,7 +97,7 @@ actix-tls = { version = "3.0.0-beta.7", features = ["openssl", "rustls"] }
 actix-test = { version = "0.1.0-beta.6", features = ["openssl", "rustls"] }
 
 brotli2 = "0.3.2"
-env_logger = "0.8"
+env_logger = "0.9"
 flate2 = "1.0.13"
 futures-util = { version = "0.3.7", default-features = false }
 rcgen = "0.8"

src/handler.rs

@@ -1,16 +1,13 @@
 use std::future::Future;
-use std::marker::PhantomData;
-use std::pin::Pin;
-use std::task::{Context, Poll};
 
-use actix_service::{Service, ServiceFactory};
+use actix_service::{
-use actix_utils::future::{ready, Ready};
+    boxed::{self, BoxServiceFactory},
-use futures_core::ready;
+    fn_service,
-use pin_project::pin_project;
+};
 
 use crate::{
     service::{ServiceRequest, ServiceResponse},
-    Error, FromRequest, HttpRequest, HttpResponse, Responder,
+    Error, FromRequest, HttpResponse, Responder,
 };
 
 /// A request handler is an async function that accepts zero or more parameters that can be
@@ -27,139 +24,26 @@ where
     fn call(&self, param: T) -> R;
 }
 
-#[doc(hidden)]
+pub fn handler_service<F, T, R>(
-/// Extract arguments from request, run factory function and make response.
+    handler: F,
-pub struct HandlerService<F, T, R>
+) -> BoxServiceFactory<(), ServiceRequest, ServiceResponse, Error, ()>
 where
     F: Handler<T, R>,
     T: FromRequest,
     R: Future,
     R::Output: Responder,
 {
-    hnd: F,
+    boxed::factory(fn_service(move |req: ServiceRequest| {
-    _phantom: PhantomData<(T, R)>,
+        let handler = handler.clone();
-}
+        async move {
-
-impl<F, T, R> HandlerService<F, T, R>
-where
-    F: Handler<T, R>,
-    T: FromRequest,
-    R: Future,
-    R::Output: Responder,
-{
-    pub fn new(hnd: F) -> Self {
-        Self {
-            hnd,
-            _phantom: PhantomData,
-        }
-    }
-}
-
-impl<F, T, R> Clone for HandlerService<F, T, R>
-where
-    F: Handler<T, R>,
-    T: FromRequest,
-    R: Future,
-    R::Output: Responder,
-{
-    fn clone(&self) -> Self {
-        Self {
-            hnd: self.hnd.clone(),
-            _phantom: PhantomData,
-        }
-    }
-}
-
-impl<F, T, R> ServiceFactory<ServiceRequest> for HandlerService<F, T, R>
-where
-    F: Handler<T, R>,
-    T: FromRequest,
-    R: Future,
-    R::Output: Responder,
-{
-    type Response = ServiceResponse;
-    type Error = Error;
-    type Config = ();
-    type Service = Self;
-    type InitError = ();
-    type Future = Ready<Result<Self::Service, ()>>;
-
-    fn new_service(&self, _: ()) -> Self::Future {
-        ready(Ok(self.clone()))
-    }
-}
-
-/// HandlerService is both it's ServiceFactory and Service Type.
-impl<F, T, R> Service<ServiceRequest> for HandlerService<F, T, R>
-where
-    F: Handler<T, R>,
-    T: FromRequest,
-    R: Future,
-    R::Output: Responder,
-{
-    type Response = ServiceResponse;
-    type Error = Error;
-    type Future = HandlerServiceFuture<F, T, R>;
-
-    actix_service::always_ready!();
-
-    fn call(&self, req: ServiceRequest) -> Self::Future {
             let (req, mut payload) = req.into_parts();
-        let fut = T::from_request(&req, &mut payload);
+            let res = match T::from_request(&req, &mut payload).await {
-        HandlerServiceFuture::Extract(fut, Some(req), self.hnd.clone())
+                Err(err) => HttpResponse::from_error(err),
-    }
+                Ok(data) => handler.call(data).await.respond_to(&req),
-}
-
-#[doc(hidden)]
-#[pin_project(project = HandlerProj)]
-pub enum HandlerServiceFuture<F, T, R>
-where
-    F: Handler<T, R>,
-    T: FromRequest,
-    R: Future,
-    R::Output: Responder,
-{
-    Extract(#[pin] T::Future, Option<HttpRequest>, F),
-    Handle(#[pin] R, Option<HttpRequest>),
-}
-
-impl<F, T, R> Future for HandlerServiceFuture<F, T, R>
-where
-    F: Handler<T, R>,
-    T: FromRequest,
-    R: Future,
-    R::Output: Responder,
-{
-    // Error type in this future is a placeholder type.
-    // all instances of error must be converted to ServiceResponse and return in Ok.
-    type Output = Result<ServiceResponse, Error>;
-
-    fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
-        loop {
-            match self.as_mut().project() {
-                HandlerProj::Extract(fut, req, handle) => {
-                    match ready!(fut.poll(cx)) {
-                        Ok(item) => {
-                            let fut = handle.call(item);
-                            let state = HandlerServiceFuture::Handle(fut, req.take());
-                            self.as_mut().set(state);
-                        }
-                        Err(err) => {
-                            let req = req.take().unwrap();
-                            let res = HttpResponse::from_error(err.into());
-                            return Poll::Ready(Ok(ServiceResponse::new(req, res)));
-                        }
             };
+            Ok(ServiceResponse::new(req, res))
         }
-                HandlerProj::Handle(fut, req) => {
+    }))
-                    let res = ready!(fut.poll(cx));
-                    let req = req.take().unwrap();
-                    let res = res.respond_to(&req);
-                    return Poll::Ready(Ok(ServiceResponse::new(req, res)));
-                }
-            }
-        }
-    }
 }
 
 /// FromRequest trait impl for tuples

src/http/header/content_disposition.rs

@@ -34,15 +34,18 @@ fn split_once_and_trim(haystack: &str, needle: char) -> (&str, &str) {
 /// The implied disposition of the content of the HTTP body.
 #[derive(Clone, Debug, PartialEq)]
 pub enum DispositionType {
-    /// Inline implies default processing
+    /// Inline implies default processing.
     Inline,
+
     /// Attachment implies that the recipient should prompt the user to save the response locally,
     /// rather than process it normally (as per its media type).
     Attachment,
-    /// Used in *multipart/form-data* as defined in
+
-    /// [RFC7578](https://tools.ietf.org/html/rfc7578) to carry the field name and the file name.
+    /// Used in *multipart/form-data* as defined in [RFC7578](https://tools.ietf.org/html/rfc7578)
+    /// to carry the field name and optional filename.
     FormData,
-    /// Extension type. Should be handled by recipients the same way as Attachment
+
+    /// Extension type. Should be handled by recipients the same way as Attachment.
     Ext(String),
 }
 
@@ -76,6 +79,7 @@ pub enum DispositionParam {
     /// For [`DispositionType::FormData`] (i.e. *multipart/form-data*), the name of an field from
     /// the form.
     Name(String),
+
     /// A plain file name.
     ///
     /// It is [not supposed](https://tools.ietf.org/html/rfc6266#appendix-D) to contain any
@@ -83,14 +87,17 @@ pub enum DispositionParam {
     /// [`FilenameExt`](DispositionParam::FilenameExt) with charset UTF-8 may be used instead
     /// in case there are Unicode characters in file names.
     Filename(String),
+
     /// An extended file name. It must not exist for `ContentType::Formdata` according to
     /// [RFC7578 Section 4.2](https://tools.ietf.org/html/rfc7578#section-4.2).
     FilenameExt(ExtendedValue),
+
     /// An unrecognized regular parameter as defined in
     /// [RFC5987](https://tools.ietf.org/html/rfc5987) as *reg-parameter*, in
     /// [RFC6266](https://tools.ietf.org/html/rfc6266) as *token "=" value*. Recipients should
     /// ignore unrecognizable parameters.
     Unknown(String, String),
+
     /// An unrecognized extended parameter as defined in
     /// [RFC5987](https://tools.ietf.org/html/rfc5987) as *ext-parameter*, in
     /// [RFC6266](https://tools.ietf.org/html/rfc6266) as *ext-token "=" ext-value*. The single
@@ -205,7 +212,6 @@ impl DispositionParam {
 /// itself, *Content-Disposition* has no effect.
 ///
 /// # ABNF
-
 /// ```text
 /// content-disposition = "Content-Disposition" ":"
 ///                       disposition-type *( ";" disposition-parm )
@@ -289,10 +295,12 @@ impl DispositionParam {
 /// If "filename" parameter is supplied, do not use the file name blindly, check and possibly
 /// change to match local file system conventions if applicable, and do not use directory path
 /// information that may be present. See [RFC2183](https://tools.ietf.org/html/rfc2183#section-2.3).
+// TODO: private fields and use smallvec
 #[derive(Clone, Debug, PartialEq)]
 pub struct ContentDisposition {
     /// The disposition type
     pub disposition: DispositionType,
+
     /// Disposition parameters
     pub parameters: Vec<DispositionParam>,
 }
@@ -509,22 +517,28 @@ impl fmt::Display for DispositionParam {
         //
         //
         // See also comments in test_from_raw_unnecessary_percent_decode.
+
         static RE: Lazy<Regex> =
             Lazy::new(|| Regex::new("[\x00-\x08\x10-\x1F\x7F\"\\\\]").unwrap());
+
         match self {
             DispositionParam::Name(ref value) => write!(f, "name={}", value),
+
             DispositionParam::Filename(ref value) => {
                 write!(f, "filename=\"{}\"", RE.replace_all(value, "\\$0").as_ref())
             }
+
             DispositionParam::Unknown(ref name, ref value) => write!(
                 f,
                 "{}=\"{}\"",
                 name,
                 &RE.replace_all(value, "\\$0").as_ref()
             ),
+
             DispositionParam::FilenameExt(ref ext_value) => {
                 write!(f, "filename*={}", ext_value)
             }
+
             DispositionParam::UnknownExt(ref name, ref ext_value) => {
                 write!(f, "{}*={}", name, ext_value)
             }

src/route.rs

@@ -11,7 +11,7 @@ use futures_core::future::LocalBoxFuture;
 
 use crate::{
     guard::{self, Guard},
-    handler::{Handler, HandlerService},
+    handler::{handler_service, Handler},
     service::{ServiceRequest, ServiceResponse},
     Error, FromRequest, HttpResponse, Responder,
 };
@@ -30,7 +30,7 @@ impl Route {
     #[allow(clippy::new_without_default)]
     pub fn new() -> Route {
         Route {
-            service: boxed::factory(HandlerService::new(HttpResponse::NotFound)),
+            service: handler_service(HttpResponse::NotFound),
             guards: Rc::new(Vec::new()),
         }
     }
@@ -182,7 +182,7 @@ impl Route {
         R: Future + 'static,
         R::Output: Responder + 'static,
     {
-        self.service = boxed::factory(HandlerService::new(handler));
+        self.service = handler_service(handler);
         self
     }