Merge branch 'master' into header-from-request

CHANGES.md

@@ -4,11 +4,16 @@
 ### Fixed
 * Double ampersand in Logger format is escaped correctly. [#2067]
 
+### Changed
+* `CustomResponder` would return error as `HttpResponse` when `CustomResponder::with_header` failed instead of skipping.
+  (Only the first error is kept when multiple error occur) [#2093]
+
 ### Removed
 * The `client` mod was removed. Clients should now use `awc` directly.
   [871ca5e4](https://github.com/actix/actix-web/commit/871ca5e4ae2bdc22d1ea02701c2992fa8d04aed7)
 
 [#2067]: https://github.com/actix/actix-web/pull/2067
+[#2093]: https://github.com/actix/actix-web/pull/2093
 
 
 ## 4.0.0-beta.4 - 2021-03-09

awc/src/response.rs

@@ -228,12 +228,13 @@ impl<S> fmt::Debug for ClientResponse<S> {
     }
 }
 
+const DEFAULT_BODY_LIMIT: usize = 2 * 1024 * 1024;
+
 /// Future that resolves to a complete HTTP message body.
 pub struct MessageBody<S> {
     length: Option<usize>,
-    err: Option<PayloadError>,
     timeout: ResponseTimeout,
-    fut: Option<ReadBody<S>>,
+    body: Result<ReadBody<S>, Option<PayloadError>>,
 }
 
 impl<S> MessageBody<S>
@@ -242,41 +243,38 @@ where
 {
     /// Create `MessageBody` for request.
     pub fn new(res: &mut ClientResponse<S>) -> MessageBody<S> {
-        let mut len = None;
+        let length = match res.headers().get(&header::CONTENT_LENGTH) {
-        if let Some(l) = res.headers().get(&header::CONTENT_LENGTH) {
+            Some(value) => {
-            if let Ok(s) = l.to_str() {
+                let len = value.to_str().ok().and_then(|s| s.parse::<usize>().ok());
-                if let Ok(l) = s.parse::<usize>() {
+
-                    len = Some(l)
+                match len {
-                } else {
+                    None => return Self::err(PayloadError::UnknownLength),
-                    return Self::err(PayloadError::UnknownLength);
+                    len => len,
                 }
-            } else {
-                return Self::err(PayloadError::UnknownLength);
             }
-        }
+            None => None,
+        };
 
         MessageBody {
-            length: len,
+            length,
-            err: None,
             timeout: std::mem::take(&mut res.timeout),
-            fut: Some(ReadBody::new(res.take_payload(), 262_144)),
+            body: Ok(ReadBody::new(res.take_payload(), DEFAULT_BODY_LIMIT)),
         }
     }
 
-    /// Change max size of payload. By default max size is 256kB
+    /// Change max size of payload. By default max size is 2048kB
     pub fn limit(mut self, limit: usize) -> Self {
-        if let Some(ref mut fut) = self.fut {
+        if let Ok(ref mut body) = self.body {
-            fut.limit = limit;
+            body.limit = limit;
         }
         self
     }
 
     fn err(e: PayloadError) -> Self {
         MessageBody {
-            fut: None,
-            err: Some(e),
             length: None,
             timeout: ResponseTimeout::default(),
+            body: Err(Some(e)),
         }
     }
 }
@@ -290,19 +288,20 @@ where
     fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
         let this = self.get_mut();
 
-        if let Some(err) = this.err.take() {
+        match this.body {
-            return Poll::Ready(Err(err));
+            Err(ref mut err) => Poll::Ready(Err(err.take().unwrap())),
-        }
+            Ok(ref mut body) => {
+                if let Some(len) = this.length.take() {
+                    if len > body.limit {
+                        return Poll::Ready(Err(PayloadError::Overflow));
+                    }
+                }
 
-        if let Some(len) = this.length.take() {
+                this.timeout.poll_timeout(cx)?;
-            if len > this.fut.as_ref().unwrap().limit {
+
-                return Poll::Ready(Err(PayloadError::Overflow));
+                Pin::new(body).poll(cx)
             }
         }
-
-        this.timeout.poll_timeout(cx)?;
-
-        Pin::new(&mut this.fut.as_mut().unwrap()).poll(cx)
     }
 }
 
@@ -415,7 +414,7 @@ impl<S> ReadBody<S> {
     fn new(stream: Payload<S>, limit: usize) -> Self {
         Self {
             stream,
-            buf: BytesMut::with_capacity(std::cmp::min(limit, 32768)),
+            buf: BytesMut::new(),
             limit,
         }
     }
@@ -430,20 +429,14 @@ where
     fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
         let this = self.get_mut();
 
-        loop {
+        while let Some(chunk) = ready!(Pin::new(&mut this.stream).poll_next(cx)?) {
-            return match Pin::new(&mut this.stream).poll_next(cx)? {
+            if (this.buf.len() + chunk.len()) > this.limit {
-                Poll::Ready(Some(chunk)) => {
+                return Poll::Ready(Err(PayloadError::Overflow));
-                    if (this.buf.len() + chunk.len()) > this.limit {
+            }
-                        Poll::Ready(Err(PayloadError::Overflow))
+            this.buf.extend_from_slice(&chunk);
-                    } else {
-                        this.buf.extend_from_slice(&chunk);
-                        continue;
-                    }
-                }
-                Poll::Ready(None) => Poll::Ready(Ok(this.buf.split().freeze())),
-                Poll::Pending => Poll::Pending,
-            };
         }
+
+        Poll::Ready(Ok(this.buf.split().freeze()))
     }
 }
 
@@ -462,7 +455,7 @@ mod tests {
             _ => unreachable!("error"),
         }
 
-        let mut req = TestResponse::with_header(header::CONTENT_LENGTH, "1000000").finish();
+        let mut req = TestResponse::with_header(header::CONTENT_LENGTH, "10000000").finish();
         match req.body().await.err().unwrap() {
             PayloadError::Overflow => {}
             _ => unreachable!("error"),

src/handler.rs

@@ -28,17 +28,6 @@ where
     fn call(&self, param: T) -> R;
 }
 
-impl<F, R> Handler<(), R> for F
-where
-    F: Fn() -> R + Clone + 'static,
-    R: Future,
-    R::Output: Responder,
-{
-    fn call(&self, _: ()) -> R {
-        (self)()
-    }
-}
-
 #[doc(hidden)]
 /// Extract arguments from request, run factory function and make response.
 pub struct HandlerService<F, T, R>
@@ -177,30 +166,29 @@ where
 }
 
 /// FromRequest trait impl for tuples
-macro_rules! factory_tuple ({ $(($n:tt, $T:ident)),+} => {
+macro_rules! factory_tuple ({ $($param:ident)* } => {
-    impl<Func, $($T,)+ Res> Handler<($($T,)+), Res> for Func
+    impl<Func, $($param,)* Res> Handler<($($param,)*), Res> for Func
-    where Func: Fn($($T,)+) -> Res + Clone + 'static,
+    where Func: Fn($($param),*) -> Res + Clone + 'static,
           Res: Future,
           Res::Output: Responder,
     {
-        fn call(&self, param: ($($T,)+)) -> Res {
+        #[allow(non_snake_case)]
-            (self)($(param.$n,)+)
+        fn call(&self, ($($param,)*): ($($param,)*)) -> Res {
+            (self)($($param,)*)
         }
     }
 });
 
-#[rustfmt::skip]
+factory_tuple! {}
-mod m {
+factory_tuple! { A }
-    use super::*;
+factory_tuple! { A B }
-
+factory_tuple! { A B C }
-    factory_tuple!((0, A));
+factory_tuple! { A B C D }
-    factory_tuple!((0, A), (1, B));
+factory_tuple! { A B C D E }
-    factory_tuple!((0, A), (1, B), (2, C));
+factory_tuple! { A B C D E F }
-    factory_tuple!((0, A), (1, B), (2, C), (3, D));
+factory_tuple! { A B C D E F G }
-    factory_tuple!((0, A), (1, B), (2, C), (3, D), (4, E));
+factory_tuple! { A B C D E F G H }
-    factory_tuple!((0, A), (1, B), (2, C), (3, D), (4, E), (5, F));
+factory_tuple! { A B C D E F G H I }
-    factory_tuple!((0, A), (1, B), (2, C), (3, D), (4, E), (5, F), (6, G));
+factory_tuple! { A B C D E F G H I J }
-    factory_tuple!((0, A), (1, B), (2, C), (3, D), (4, E), (5, F), (6, G), (7, H));
+factory_tuple! { A B C D E F G H I J K }
-    factory_tuple!((0, A), (1, B), (2, C), (3, D), (4, E), (5, F), (6, G), (7, H), (8, I));
+factory_tuple! { A B C D E F G H I J K L }
-    factory_tuple!((0, A), (1, B), (2, C), (3, D), (4, E), (5, F), (6, G), (7, H), (8, I), (9, J));
-}

src/responder.rs

@@ -155,8 +155,7 @@ impl Responder for BytesMut {
 pub struct CustomResponder<T> {
     responder: T,
     status: Option<StatusCode>,
-    headers: Option<HeaderMap>,
+    headers: Result<HeaderMap, HttpError>,
-    error: Option<HttpError>,
 }
 
 impl<T: Responder> CustomResponder<T> {
@@ -164,8 +163,7 @@ impl<T: Responder> CustomResponder<T> {
         CustomResponder {
             responder,
             status: None,
-            headers: None,
+            headers: Ok(HeaderMap::new()),
-            error: None,
         }
     }
 
@@ -206,32 +204,33 @@ impl<T: Responder> CustomResponder<T> {
     where
         H: IntoHeaderPair,
     {
-        if self.headers.is_none() {
+        if let Ok(ref mut headers) = self.headers {
-            self.headers = Some(HeaderMap::new());
+            match header.try_into_header_pair() {
+                Ok((key, value)) => headers.append(key, value),
+                Err(e) => self.headers = Err(e.into()),
+            };
         }
 
-        match header.try_into_header_pair() {
-            Ok((key, value)) => self.headers.as_mut().unwrap().append(key, value),
-            Err(e) => self.error = Some(e.into()),
-        };
-
         self
     }
 }
 
 impl<T: Responder> Responder for CustomResponder<T> {
     fn respond_to(self, req: &HttpRequest) -> HttpResponse {
+        let headers = match self.headers {
+            Ok(headers) => headers,
+            Err(err) => return HttpResponse::from_error(Error::from(err)),
+        };
+
         let mut res = self.responder.respond_to(req);
 
         if let Some(status) = self.status {
             *res.status_mut() = status;
         }
 
-        if let Some(ref headers) = self.headers {
+        for (k, v) in headers {
-            for (k, v) in headers {
+            // TODO: before v4, decide if this should be append instead
-                // TODO: before v4, decide if this should be append instead
+            res.headers_mut().insert(k, v);
-                res.headers_mut().insert(k.clone(), v.clone());
-            }
         }
 
         res

src/service.rs

@@ -573,31 +573,28 @@ macro_rules! services {
 }
 
 /// HttpServiceFactory trait impl for tuples
-macro_rules! service_tuple ({ $(($n:tt, $T:ident)),+} => {
+macro_rules! service_tuple ({ $($T:ident)+ } => {
     impl<$($T: HttpServiceFactory),+> HttpServiceFactory for ($($T,)+) {
+        #[allow(non_snake_case)]
         fn register(self, config: &mut AppService) {
-            $(self.$n.register(config);)+
+            let ($($T,)*) = self;
+            $($T.register(config);)+
         }
     }
 });
 
-#[rustfmt::skip]
+service_tuple! { A }
-mod m {
+service_tuple! { A B }
-    use super::*;
+service_tuple! { A B C }
-
+service_tuple! { A B C D }
-    service_tuple!((0, A));
+service_tuple! { A B C D E }
-    service_tuple!((0, A), (1, B));
+service_tuple! { A B C D E F }
-    service_tuple!((0, A), (1, B), (2, C));
+service_tuple! { A B C D E F G }
-    service_tuple!((0, A), (1, B), (2, C), (3, D));
+service_tuple! { A B C D E F G H }
-    service_tuple!((0, A), (1, B), (2, C), (3, D), (4, E));
+service_tuple! { A B C D E F G H I }
-    service_tuple!((0, A), (1, B), (2, C), (3, D), (4, E), (5, F));
+service_tuple! { A B C D E F G H I J }
-    service_tuple!((0, A), (1, B), (2, C), (3, D), (4, E), (5, F), (6, G));
+service_tuple! { A B C D E F G H I J K }
-    service_tuple!((0, A), (1, B), (2, C), (3, D), (4, E), (5, F), (6, G), (7, H));
+service_tuple! { A B C D E F G H I J K L }
-    service_tuple!((0, A), (1, B), (2, C), (3, D), (4, E), (5, F), (6, G), (7, H), (8, I));
-    service_tuple!((0, A), (1, B), (2, C), (3, D), (4, E), (5, F), (6, G), (7, H), (8, I), (9, J));
-    service_tuple!((0, A), (1, B), (2, C), (3, D), (4, E), (5, F), (6, G), (7, H), (8, I), (9, J), (10, K));
-    service_tuple!((0, A), (1, B), (2, C), (3, D), (4, E), (5, F), (6, G), (7, H), (8, I), (9, J), (10, K), (11, L));
-}
 
 #[cfg(test)]
 mod tests {