Merge branch 'master' into router050

actix-http/src/error.rs

@@ -108,8 +108,10 @@ pub(crate) enum Kind {
 
 impl fmt::Debug for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        // TODO: more detail
+        f.debug_struct("actix_http::Error")
-        f.write_str("actix_http::Error")
+            .field("kind", &self.inner.kind)
+            .field("cause", &self.inner.cause)
+            .finish()
     }
 }
 
@@ -386,7 +388,6 @@ pub enum DispatchError {
 impl StdError for DispatchError {
     fn source(&self) -> Option<&(dyn StdError + 'static)> {
         match self {
-            // TODO: error source extraction?
             DispatchError::Service(_res) => None,
             DispatchError::Body(err) => Some(&**err),
             DispatchError::Io(err) => Some(err),

actix-http/src/h2/dispatcher.rs

@@ -25,7 +25,9 @@ use pin_project_lite::pin_project;
 use crate::{
     body::{BodySize, BoxBody, MessageBody},
     config::ServiceConfig,
-    header::{HeaderValue, CONNECTION, CONTENT_LENGTH, DATE, TRANSFER_ENCODING},
+    header::{
+        HeaderName, HeaderValue, CONNECTION, CONTENT_LENGTH, DATE, TRANSFER_ENCODING, UPGRADE,
+    },
     service::HttpFlow,
     Extensions, OnConnectData, Payload, Request, Response, ResponseHead,
 };
@@ -306,13 +308,22 @@ fn prepare_response(
 
     // copy headers
     for (key, value) in head.headers.iter() {
-        match *key {
+        match key {
-            // TODO: consider skipping other headers according to:
+            // omit HTTP/1.x only headers according to:
-            //       https://datatracker.ietf.org/doc/html/rfc7540#section-8.1.2.2
+            // https://datatracker.ietf.org/doc/html/rfc7540#section-8.1.2.2
-            // omit HTTP/1.x only headers
+            &CONNECTION | &TRANSFER_ENCODING | &UPGRADE => continue,
-            CONNECTION | TRANSFER_ENCODING => continue,
+
-            CONTENT_LENGTH if skip_len => continue,
+            &CONTENT_LENGTH if skip_len => continue,
-            DATE => has_date = true,
+            &DATE => has_date = true,
+
+            // omit HTTP/1.x only headers according to:
+            // https://datatracker.ietf.org/doc/html/rfc7540#section-8.1.2.2
+            hdr if hdr == HeaderName::from_static("keep-alive")
+                || hdr == HeaderName::from_static("proxy-connection") =>
+            {
+                continue
+            }
+
             _ => {}
         }
 

actix-test/src/lib.rs

@@ -43,7 +43,7 @@ pub use actix_http_test::unused_addr;
 use actix_service::{map_config, IntoServiceFactory, ServiceFactory, ServiceFactoryExt as _};
 pub use actix_web::test::{
     call_and_read_body, call_and_read_body_json, call_service, init_service, ok_service,
-    read_body, read_body_json, simple_service, TestRequest,
+    read_body, read_body_json, status_service, TestRequest,
 };
 use actix_web::{
     body::MessageBody,

actix-web/CHANGES.md

@@ -1,6 +1,9 @@
 # Changes
 
 ## Unreleased - 2021-xx-xx
+- Rename `test::{simple_service => status_service}`. [#2659]
+
+[#2659]: https://github.com/actix/actix-web/pull/2659
 
 
 ## 4.0.0-rc.3 - 2022-02-08

actix-web/Cargo.toml

@@ -77,6 +77,7 @@ actix-web-codegen = { version = "0.5.0-rc.2", optional = true }
 
 ahash = "0.7"
 bytes = "1"
+bytestring = "1"
 cfg-if = "1"
 cookie = { version = "0.16", features = ["percent-encode"], optional = true }
 derive_more = "0.99.5"

actix-web/MIGRATION-4.0.md

@@ -3,8 +3,7 @@
 This guide walks you through the process of migrating from v3.x.y to v4.x.y.  
 If you are migrating to v4.x.y from an older version of Actix Web (v2.x.y or earlier), check out the other historical migration notes in this folder.
 
-This document is not designed to be exhaustive - it focuses on the most significant changes coming in v4.  
+This document is not designed to be exhaustive—it focuses on the most significant changes coming in v4. You can find an exhaustive changelog in [CHANGES.md](./CHANGES.md), complete of PR links. If you think that some of the changes that we omitted deserve to be called out in this document, please open an issue or submit a PR.
-You can find an exhaustive changelog in [CHANGES.md](./CHANGES.md), complete of PR links. If you think that some of the changes that we omitted deserve to be called out in this document, please open an issue or submit a PR. 
 
 Headings marked with :warning: are **breaking behavioral changes**. They will probably not surface as compile-time errors though automated tests _might_ detect their effects on your app.
 
@@ -39,8 +38,9 @@ The MSRV of Actix Web has been raised from 1.42 to 1.54.
 
 ## Tokio v1 Ecosystem
 
-Actix Web v4 is now underpinned by `tokio`'s v1 ecosystem.  
+Actix Web v4 is now underpinned by `tokio`'s v1 ecosystem.
-`cargo` supports having multiple versions of the same crate within the same dependency tree, but `tokio` v1 does not interoperate transparently with its previous versions (v0.2, v0.1). Some of your dependencies might rely on `tokio`, either directly or indirectly - if they are using an older version of `tokio`, check if an update is available.  
+
+`cargo` supports having multiple versions of the same crate within the same dependency tree, but `tokio` v1 does not interoperate transparently with its previous versions (v0.2, v0.1). Some of your dependencies might rely on `tokio`, either directly or indirectly—if they are using an older version of `tokio`, check if an update is available.  
 The following command can help you to identify these dependencies:
 
 ```sh
@@ -59,8 +59,8 @@ Lots of modules have been re-organized in this release. If a compile error refer
 
 ## `NormalizePath` Middleware :warning:
 
-The default `NormalizePath` behavior now strips trailing slashes by default.  
+The default `NormalizePath` behavior now strips trailing slashes by default. This was the _documented_ behaviour in Actix Web v3, but the _actual_ behaviour differed. The discrepancy has now been resolved.
-This was the _documented_ behaviour in Actix Web v3, but the _actual_ behaviour differed - the discrepancy has now been fixed.  
+
 As a consequence of this change, routes defined with trailing slashes will become inaccessible when using `NormalizePath::default()`. Calling `NormalizePath::default()` will log a warning. We suggest to use `new` or `trim`.
 
 ```diff
@@ -103,8 +103,7 @@ The `compress` feature flag has been split into more granular feature flags, one
 
 ## `web::Path`
 
-The inner field for `web::Path` is now private.  
+The inner field for `web::Path` is now private. It was causing ambiguity when trying to use tuple indexing due to its `Deref` implementation.
-It was causing too many issues when used with inner tuple types due to its `Deref` implementation.
 
 ```diff
 - async fn handler(web::Path((foo, bar)): web::Path<(String, String)>) {
@@ -118,7 +117,7 @@ Actix Web now depends on version 0.20 of `rustls`. As a result, the server confi
 
 ## Removed `awc` Client Re-export
 
-Actix Web's sister crate `awc` is no longer re-exported through the `client` module. This allows `awc` to have its own release cadence - its breaking changes are no longer blocked by Actix Web's (more conservative) release schedule.
+Actix Web's sister crate `awc` is no longer re-exported through the `client` module. This allows `awc` to have its own release cadence—its breaking changes are no longer blocked by Actix Web's (more conservative) release schedule.
 
 ```diff
 - use actix_web::client::Client;
@@ -134,11 +133,11 @@ Actix Web's sister crate `awc` is no longer re-exported through the `client` mod
 + use use actix_test::start;
 ```
 
-`TestServer` previously lived in `actix_web::test`, but it depends on `awc` which is no longer part of Actix Web's public API (see above). 
+`TestServer` previously lived in `actix_web::test`, but it depends on `awc` which is no longer part of Actix Web's public API (see above).
 
 ## Header APIs
 
-Header related APIs have been standardized across all `actix-*` crates. The terminology now better matches the underlying `HeaderMap` naming conventions. 
+Header related APIs have been standardized across all `actix-*` crates. The terminology now better matches the underlying `HeaderMap` naming conventions.
 
 In short, "insert" always indicates that any existing headers with the same name are overridden, while "append" is used for adding with no removal (e.g. multi-valued headers).
 
@@ -155,7 +154,7 @@ For request and response builder APIs, the new methods provide a unified interfa
 + .insert_header(ContentType::json())
 ```
 
-We chose to deprecate most of the old methods instead of removing them immediately - the warning notes will guide you on how to update. 
+We chose to deprecate most of the old methods instead of removing them immediately—the warning notes will guide you on how to update.
 
 ## Response Body Types
 
@@ -178,16 +177,65 @@ We have boosted the quality and completeness of the documentation for all items
 
 ### `BoxBody`
 
-`BoxBody` is a new type-erased body type. It's used for all error response bodies.  
+`BoxBody` is a new type-erased body type.
-Creating a boxed body is best done by calling [`.boxed()`](https://docs.rs/actix-web/4/actix_web/body/trait.MessageBody.html#method.boxed) on a `MessageBody` type.
+
+It can be useful when writing handlers, responders, and middleware when you want to trade a (very) small amount of performance for a simpler type.
+
+Creating a boxed body is done most efficiently by calling [`.boxed()`](https://docs.rs/actix-web/4/actix_web/body/trait.MessageBody.html#method.boxed) on a `MessageBody` type.
 
 ### `EitherBody`
 
-`EitherBody` is a new "either" type that is particularly useful in middlewares that can bail early, returning their own response plus body type.
+`EitherBody` is a new "either" type that implements `MessageBody`
+
+It is particularly useful in middleware that can bail early, returning their own response plus body type. By default the "right" variant is `BoxBody` (i.e., `EitherBody<B>` === `EitherBody<B, BoxBody>`) but it can be anything that implements `MessageBody`.
+
+For example, it will be common among middleware which value performance of the hot path to use:
+
+```rust
+type Response = Result<ServiceResponse<EitherBody<B>>, Error>
+```
+
+This can be read (ignoring the `Result`) as "resolves with a `ServiceResponse` that is either the inner service's `B` body type or a boxed body type from elsewhere, likely constructed within the middleware itself". Of course, if your middleware contains only simple string other/error responses, it's possible to use them without boxes at the cost of a less simple implementation:
+
+```rust
+type Response = Result<ServiceResponse<EitherBody<B, String>>, Error>
+```
 
 ### Error Handlers
 
-TODO In particular, folks seem to be struggling with the `ErrorHandlers` middleware because of this change and the obscured nature of `EitherBody` within its types.
+`ErrorHandlers` is a commonly used middleware that has changed in design slightly due to the other body type changes.
+
+In particular, an implicit `EitherBody` is used in the `ErrorHandlerResponse<B>` type. An `ErrorHandlerResponse<B>` now expects a `ServiceResponse<EitherBody<B>>` to be returned within response variants. The following is a migration for an error handler that **only modifies** the response argument (left body).
+
+```diff
+  fn add_error_header<B>(mut res: ServiceResponse<B>) -> Result<ErrorHandlerResponse<B>, Error> {
+      res.response_mut().headers_mut().insert(
+          header::CONTENT_TYPE,
+          header::HeaderValue::from_static("Error"),
+      );
+-     Ok(ErrorHandlerResponse::Response(res))
++     Ok(ErrorHandlerResponse::Response(res.map_into_left_body()))
+  }
+```
+
+The following is a migration for an error handler that creates a new response instead (right body).
+
+```diff
+  fn error_handler<B>(res: ServiceResponse<B>) -> Result<ErrorHandlerResponse<B>, Error> {
+-     let req = res.request().clone();
++     let (req, _res) = res.into_parts();
+
+      let res = actix_files::NamedFile::open("./templates/404.html")?
+          .set_status_code(StatusCode::NOT_FOUND)
+-         .into_response(&req)?
+-         .into_body();
++         .into_response(&req);
+
+-     let res = ServiceResponse::new(req, res);
++     let res = ServiceResponse::new(req, res).map_into_right_body();
+      Ok(ErrorHandlerResponse::Response(res))
+  }
+```
 
 ## Middleware Trait APIs
 
@@ -251,7 +299,7 @@ You may need to review the [guidance on shared mutable state](https://docs.rs/ac
 Improvements to module management and re-exports have resulted in not needing direct dependencies on these underlying crates for the vast majority of cases. In particular:
 
 - all traits necessary for creating middlewares are now re-exported through the `dev` modules;
-- `#[actix_web::test]` now exists for async test definitions. 
+- `#[actix_web::test]` now exists for async test definitions.
 
 Relying on these re-exports will ease the transition to future versions of Actix Web.
 

actix-web/src/middleware/err_handlers.rs

@@ -185,6 +185,7 @@ mod tests {
 
     use super::*;
     use crate::{
+        body,
         http::{
             header::{HeaderValue, CONTENT_TYPE},
             StatusCode,
@@ -203,7 +204,7 @@ mod tests {
             Ok(ErrorHandlerResponse::Response(res.map_into_left_body()))
         }
 
-        let srv = test::simple_service(StatusCode::INTERNAL_SERVER_ERROR);
+        let srv = test::status_service(StatusCode::INTERNAL_SERVER_ERROR);
 
         let mw = ErrorHandlers::new()
             .handler(StatusCode::INTERNAL_SERVER_ERROR, error_handler)
@@ -230,7 +231,7 @@ mod tests {
             ))
         }
 
-        let srv = test::simple_service(StatusCode::INTERNAL_SERVER_ERROR);
+        let srv = test::status_service(StatusCode::INTERNAL_SERVER_ERROR);
 
         let mw = ErrorHandlers::new()
             .handler(StatusCode::INTERNAL_SERVER_ERROR, error_handler)
@@ -245,9 +246,7 @@ mod tests {
     #[actix_rt::test]
     async fn changes_body_type() {
         #[allow(clippy::unnecessary_wraps)]
-        fn error_handler<B: 'static>(
+        fn error_handler<B>(res: ServiceResponse<B>) -> Result<ErrorHandlerResponse<B>> {
-            res: ServiceResponse<B>,
-        ) -> Result<ErrorHandlerResponse<B>> {
             let (req, res) = res.into_parts();
             let res = res.set_body(Bytes::from("sorry, that's no bueno"));
 
@@ -258,7 +257,7 @@ mod tests {
             Ok(ErrorHandlerResponse::Response(res))
         }
 
-        let srv = test::simple_service(StatusCode::INTERNAL_SERVER_ERROR);
+        let srv = test::status_service(StatusCode::INTERNAL_SERVER_ERROR);
 
         let mw = ErrorHandlers::new()
             .handler(StatusCode::INTERNAL_SERVER_ERROR, error_handler)
@@ -270,5 +269,33 @@ mod tests {
         assert_eq!(test::read_body(res).await, "sorry, that's no bueno");
     }
 
-    // TODO: test where error is thrown
+    #[actix_rt::test]
+    async fn error_thrown() {
+        #[allow(clippy::unnecessary_wraps)]
+        fn error_handler<B>(_res: ServiceResponse<B>) -> Result<ErrorHandlerResponse<B>> {
+            Err(crate::error::ErrorInternalServerError(
+                "error in error handler",
+            ))
+        }
+
+        let srv = test::status_service(StatusCode::BAD_REQUEST);
+
+        let mw = ErrorHandlers::new()
+            .handler(StatusCode::BAD_REQUEST, error_handler)
+            .new_transform(srv.into_service())
+            .await
+            .unwrap();
+
+        let err = mw
+            .call(TestRequest::default().to_srv_request())
+            .await
+            .unwrap_err();
+        let res = err.error_response();
+
+        assert_eq!(res.status(), StatusCode::INTERNAL_SERVER_ERROR);
+        assert_eq!(
+            body::to_bytes(res.into_body()).await.unwrap(),
+            "error in error handler"
+        );
+    }
 }

actix-web/src/response/responder.rs

@@ -7,14 +7,35 @@ use actix_http::{
 };
 use bytes::{Bytes, BytesMut};
 
-use crate::{Error, HttpRequest, HttpResponse};
-
 use super::CustomizeResponder;
+use crate::{Error, HttpRequest, HttpResponse};
 
 /// Trait implemented by types that can be converted to an HTTP response.
 ///
-/// Any types that implement this trait can be used in the return type of a handler.
+/// Any types that implement this trait can be used in the return type of a handler. Since handlers
-// # TODO: more about implementation notes and foreign impls
+/// will only have one return type, it is idiomatic to use opaque return types `-> impl Responder`.
+///
+/// # Implementations
+/// It is often not required to implement `Responder` for your own types due to a broad base of
+/// built-in implementations:
+/// - `HttpResponse` and `HttpResponseBuilder`
+/// - `Option<R>` where `R: Responder`
+/// - `Result<R, E>` where `R: Responder` and [`E: ResponseError`](crate::ResponseError)
+/// - `(R, StatusCode) where `R: Responder`
+/// - `&'static str`, `String`, `&'_ String`, `Cow<'_, str>`, [`ByteString`](bytestring::ByteString)
+/// - `&'static [u8]`, `Vec<u8>`, `Bytes`, `BytesMut`
+/// - [`Json<T>`](crate::web::Json) and [`Form<T>`](crate::web::Form) where `T: Serialize`
+/// - [`Either<L, R>`](crate::web::Either) where `L: Serialize` and `R: Serialize`
+/// - [`CustomizeResponder<R>`]
+/// - [`actix_files::NamedFile`](https://docs.rs/actix-files/latest/actix_files/struct.NamedFile.html)
+/// - [Experimental responders from `actix-web-lab`](https://docs.rs/actix-web-lab/latest/actix_web_lab/respond/index.html)
+/// - Third party integrations may also have implemented `Responder` where appropriate. For example,
+///   HTML templating engines.
+///
+/// # Customizing Responder Output
+/// Calling [`.customize()`](Responder::customize) on any responder type will wrap it in a
+/// [`CustomizeResponder`] capable of overriding various parts of the response such as the status
+/// code and header map.
 pub trait Responder {
     type Body: MessageBody + 'static;
 
@@ -23,7 +44,7 @@ pub trait Responder {
 
     /// Wraps responder to allow alteration of its response.
     ///
-    /// See [`CustomizeResponder`] docs for its capabilities.
+    /// See [`CustomizeResponder`] docs for more details on its capabilities.
     ///
     /// # Examples
     /// ```
@@ -84,11 +105,8 @@ impl Responder for actix_http::ResponseBuilder {
     }
 }
 
-impl<T> Responder for Option<T>
+impl<R: Responder> Responder for Option<R> {
-where
+    type Body = EitherBody<R::Body>;
-    T: Responder,
-{
-    type Body = EitherBody<T::Body>;
 
     fn respond_to(self, req: &HttpRequest) -> HttpResponse<Self::Body> {
         match self {
@@ -98,12 +116,12 @@ where
     }
 }
 
-impl<T, E> Responder for Result<T, E>
+impl<R, E> Responder for Result<R, E>
 where
-    T: Responder,
+    R: Responder,
     E: Into<Error>,
 {
-    type Body = EitherBody<T::Body>;
+    type Body = EitherBody<R::Body>;
 
     fn respond_to(self, req: &HttpRequest) -> HttpResponse<Self::Body> {
         match self {
@@ -113,8 +131,8 @@ where
     }
 }
 
-impl<T: Responder> Responder for (T, StatusCode) {
+impl<R: Responder> Responder for (R, StatusCode) {
-    type Body = T::Body;
+    type Body = R::Body;
 
     fn respond_to(self, req: &HttpRequest) -> HttpResponse<Self::Body> {
         let mut res = self.0.respond_to(req);
@@ -147,6 +165,7 @@ impl_responder_by_forward_into_base_response!(BytesMut);
 
 impl_responder_by_forward_into_base_response!(&'static str);
 impl_responder_by_forward_into_base_response!(String);
+impl_responder_by_forward_into_base_response!(bytestring::ByteString);
 
 macro_rules! impl_into_string_responder {
     ($res:ty) => {

actix-web/src/response/response.rs

@@ -323,12 +323,6 @@ impl From<Error> for HttpResponse {
 impl<B> From<HttpResponse<B>> for Response<B> {
     fn from(res: HttpResponse<B>) -> Self {
         // this impl will always be called as part of dispatcher
-
-        // TODO: expose cause somewhere?
-        // if let Some(err) = res.error {
-        //     return Response::from_error(err);
-        // }
-
         res.res
     }
 }

actix-web/src/test/mod.rs

@@ -5,7 +5,7 @@
 //!
 //! # Off-The-Shelf Test Services
 //! - [`ok_service`]
-//! - [`simple_service`]
+//! - [`status_service`]
 //!
 //! # Calling Test Service
 //! - [`TestRequest`]
@@ -27,7 +27,7 @@ mod test_utils;
 
 pub use self::test_request::TestRequest;
 #[allow(deprecated)]
-pub use self::test_services::{default_service, ok_service, simple_service};
+pub use self::test_services::{default_service, ok_service, simple_service, status_service};
 #[allow(deprecated)]
 pub use self::test_utils::{
     call_and_read_body, call_and_read_body_json, call_service, init_service, read_body,

actix-web/src/test/test_services.rs

@@ -10,11 +10,11 @@ use crate::{
 /// Creates service that always responds with `200 OK` and no body.
 pub fn ok_service(
 ) -> impl Service<ServiceRequest, Response = ServiceResponse<BoxBody>, Error = Error> {
-    simple_service(StatusCode::OK)
+    status_service(StatusCode::OK)
 }
 
 /// Creates service that always responds with given status code and no body.
-pub fn simple_service(
+pub fn status_service(
     status_code: StatusCode,
 ) -> impl Service<ServiceRequest, Response = ServiceResponse<BoxBody>, Error = Error> {
     fn_service(move |req: ServiceRequest| {
@@ -23,9 +23,17 @@ pub fn simple_service(
 }
 
 #[doc(hidden)]
-#[deprecated(since = "4.0.0", note = "Renamed to `simple_service`.")]
+#[deprecated(since = "4.0.0", note = "Renamed to `status_service`.")]
+pub fn simple_service(
+    status_code: StatusCode,
+) -> impl Service<ServiceRequest, Response = ServiceResponse<BoxBody>, Error = Error> {
+    status_service(status_code)
+}
+
+#[doc(hidden)]
+#[deprecated(since = "4.0.0", note = "Renamed to `status_service`.")]
 pub fn default_service(
     status_code: StatusCode,
 ) -> impl Service<ServiceRequest, Response = ServiceResponse<BoxBody>, Error = Error> {
-    simple_service(status_code)
+    status_service(status_code)
 }

actix-web/src/test/test_utils.rs

@@ -89,7 +89,7 @@ where
 /// ```
 ///
 /// # Panics
-/// Panics if service call returns error.
+/// Panics if service call returns error. To handle errors use `app.call(req)`.
 pub async fn call_service<S, R, B, E>(app: &S, req: R) -> S::Response
 where
     S: Service<R, Response = ServiceResponse<B>, Error = E>,

awc/src/client/pool.rs

@@ -232,7 +232,7 @@ where
                 None => {
                     let (io, proto) = connector.call(req).await?;
 
-                    // TODO: remove when http3 is added in support.
+                    // NOTE: remove when http3 is added in support.
                     assert!(proto != Protocol::Http3);
 
                     if proto == Protocol::Http1 {