diff --git a/CHANGES.md b/CHANGES.md index 3e0b12d9e..15b76d8f1 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,13 @@ # Changes ## Unreleased - 2021-xx-xx +### Added +* Method on `Responder` trait (`customize`) for customizing responders and `CustomizeResponder` struct. [#????] + +### Changed +* Align `DefaultHeader` method terminology, deprecating previous methods. [#????] + +[#????]: https://github.com/actix/actix-web/pull/???? ## 4.0.0-beta.14 - 2021-12-11 diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index fe47902f7..4f2a42aea 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -1,6 +1,10 @@ # Changes ## Unreleased - 2021-xx-xx +### Changed +* Rename trait `IntoHeaderPair => TryIntoHeaderPair`. [#????] + +[#????]: https://github.com/actix/actix-web/pull/???? ## 3.0.0-beta.15 - 2021-12-11 @@ -260,7 +264,7 @@ ## 3.0.0-beta.2 - 2021-02-10 ### Added -* `IntoHeaderPair` trait that allows using typed and untyped headers in the same methods. [#1869] +* `TryIntoHeaderPair` trait that allows using typed and untyped headers in the same methods. [#1869] * `ResponseBuilder::insert_header` method which allows using typed headers. [#1869] * `ResponseBuilder::append_header` method which allows using typed headers. [#1869] * `TestRequest::insert_header` method which allows using typed headers. [#1869] diff --git a/actix-http/src/h2/service.rs b/actix-http/src/h2/service.rs index f5821370a..469648054 100644 --- a/actix-http/src/h2/service.rs +++ b/actix-http/src/h2/service.rs @@ -270,10 +270,10 @@ where type Future = H2ServiceHandlerResponse; fn poll_ready(&self, cx: &mut Context<'_>) -> Poll> { - self.flow.service.poll_ready(cx).map_err(|e| { - let e = e.into(); - error!("Service readiness error: {:?}", e); - DispatchError::Service(e) + self.flow.service.poll_ready(cx).map_err(|err| { + let err = err.into(); + error!("Service readiness error: {:?}", err); + DispatchError::Service(err) }) } @@ -297,7 +297,6 @@ where T: AsyncRead + AsyncWrite + Unpin, S::Future: 'static, { - Incoming(Dispatcher), Handshake( Option>>, Option, @@ -305,6 +304,7 @@ where OnConnectData, HandshakeWithTimeout, ), + Established(Dispatcher), } pub struct H2ServiceHandlerResponse @@ -332,7 +332,6 @@ where fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { match self.state { - State::Incoming(ref mut disp) => Pin::new(disp).poll(cx), State::Handshake( ref mut srv, ref mut config, @@ -343,7 +342,7 @@ where Ok((conn, timer)) => { let on_connect_data = mem::take(conn_data); - self.state = State::Incoming(Dispatcher::new( + self.state = State::Established(Dispatcher::new( conn, srv.take().unwrap(), config.take().unwrap(), @@ -360,6 +359,8 @@ where Poll::Ready(Err(err)) } }, + + State::Established(ref mut disp) => Pin::new(disp).poll(cx), } } } diff --git a/actix-http/src/header/into_pair.rs b/actix-http/src/header/into_pair.rs index b4250e06e..6e9288f01 100644 --- a/actix-http/src/header/into_pair.rs +++ b/actix-http/src/header/into_pair.rs @@ -1,19 +1,17 @@ -//! [`IntoHeaderPair`] trait and implementations. +//! [`TryIntoHeaderPair`] trait and implementations. use std::convert::TryFrom as _; -use http::{ - header::{HeaderName, InvalidHeaderName, InvalidHeaderValue}, - Error as HttpError, HeaderValue, +use super::{ + Header, HeaderName, HeaderValue, IntoHeaderValue, InvalidHeaderName, InvalidHeaderValue, }; +use crate::error::HttpError; -use super::{Header, IntoHeaderValue}; - -/// An interface for types that can be converted into a [`HeaderName`]/[`HeaderValue`] pair for +/// An interface for types that can be converted into a [`HeaderName`] + [`HeaderValue`] pair for /// insertion into a [`HeaderMap`]. /// /// [`HeaderMap`]: super::HeaderMap -pub trait IntoHeaderPair: Sized { +pub trait TryIntoHeaderPair: Sized { type Error: Into; fn try_into_header_pair(self) -> Result<(HeaderName, HeaderValue), Self::Error>; @@ -34,7 +32,7 @@ impl From for HttpError { } } -impl IntoHeaderPair for (HeaderName, V) +impl TryIntoHeaderPair for (HeaderName, V) where V: IntoHeaderValue, V::Error: Into, @@ -50,7 +48,7 @@ where } } -impl IntoHeaderPair for (&HeaderName, V) +impl TryIntoHeaderPair for (&HeaderName, V) where V: IntoHeaderValue, V::Error: Into, @@ -66,7 +64,7 @@ where } } -impl IntoHeaderPair for (&[u8], V) +impl TryIntoHeaderPair for (&[u8], V) where V: IntoHeaderValue, V::Error: Into, @@ -83,7 +81,7 @@ where } } -impl IntoHeaderPair for (&str, V) +impl TryIntoHeaderPair for (&str, V) where V: IntoHeaderValue, V::Error: Into, @@ -100,22 +98,24 @@ where } } -impl IntoHeaderPair for (String, V) +impl TryIntoHeaderPair for (String, V) where V: IntoHeaderValue, V::Error: Into, { type Error = InvalidHeaderPart; + #[inline] fn try_into_header_pair(self) -> Result<(HeaderName, HeaderValue), Self::Error> { let (name, value) = self; (name.as_str(), value).try_into_header_pair() } } -impl IntoHeaderPair for T { +impl TryIntoHeaderPair for T { type Error = ::Error; + #[inline] fn try_into_header_pair(self) -> Result<(HeaderName, HeaderValue), Self::Error> { Ok((T::name(), self.try_into_value()?)) } diff --git a/actix-http/src/header/mod.rs b/actix-http/src/header/mod.rs index 5fe76381b..bac8a3509 100644 --- a/actix-http/src/header/mod.rs +++ b/actix-http/src/header/mod.rs @@ -37,7 +37,7 @@ mod shared; mod utils; pub use self::as_name::AsHeaderName; -pub use self::into_pair::IntoHeaderPair; +pub use self::into_pair::TryIntoHeaderPair; pub use self::into_value::IntoHeaderValue; pub use self::map::HeaderMap; pub use self::shared::{ diff --git a/actix-http/src/response_builder.rs b/actix-http/src/response_builder.rs index dfc2612fb..451025e53 100644 --- a/actix-http/src/response_builder.rs +++ b/actix-http/src/response_builder.rs @@ -8,7 +8,7 @@ use std::{ use crate::{ body::{EitherBody, MessageBody}, error::{Error, HttpError}, - header::{self, IntoHeaderPair, IntoHeaderValue}, + header::{self, IntoHeaderValue, TryIntoHeaderPair}, message::{BoxedResponseHead, ConnectionType, ResponseHead}, Extensions, Response, StatusCode, }; @@ -90,10 +90,7 @@ impl ResponseBuilder { /// assert!(res.headers().contains_key("content-type")); /// assert!(res.headers().contains_key("x-test")); /// ``` - pub fn insert_header(&mut self, header: H) -> &mut Self - where - H: IntoHeaderPair, - { + pub fn insert_header(&mut self, header: impl TryIntoHeaderPair) -> &mut Self { if let Some(parts) = self.inner() { match header.try_into_header_pair() { Ok((key, value)) => { @@ -121,10 +118,7 @@ impl ResponseBuilder { /// assert_eq!(res.headers().get_all("content-type").count(), 1); /// assert_eq!(res.headers().get_all("x-test").count(), 2); /// ``` - pub fn append_header(&mut self, header: H) -> &mut Self - where - H: IntoHeaderPair, - { + pub fn append_header(&mut self, header: impl TryIntoHeaderPair) -> &mut Self { if let Some(parts) = self.inner() { match header.try_into_header_pair() { Ok((key, value)) => parts.headers.append(key, value), diff --git a/actix-http/src/test.rs b/actix-http/src/test.rs index ec781743d..066e615c9 100644 --- a/actix-http/src/test.rs +++ b/actix-http/src/test.rs @@ -14,7 +14,7 @@ use bytes::{Bytes, BytesMut}; use http::{Method, Uri, Version}; use crate::{ - header::{HeaderMap, IntoHeaderPair}, + header::{HeaderMap, TryIntoHeaderPair}, payload::Payload, Request, }; @@ -92,10 +92,7 @@ impl TestRequest { } /// Insert a header, replacing any that were set with an equivalent field name. - pub fn insert_header(&mut self, header: H) -> &mut Self - where - H: IntoHeaderPair, - { + pub fn insert_header(&mut self, header: impl TryIntoHeaderPair) -> &mut Self { match header.try_into_header_pair() { Ok((key, value)) => { parts(&mut self.0).headers.insert(key, value); @@ -109,10 +106,7 @@ impl TestRequest { } /// Append a header, keeping any that were set with an equivalent field name. - pub fn append_header(&mut self, header: H) -> &mut Self - where - H: IntoHeaderPair, - { + pub fn append_header(&mut self, header: impl TryIntoHeaderPair) -> &mut Self { match header.try_into_header_pair() { Ok((key, value)) => { parts(&mut self.0).headers.append(key, value); diff --git a/awc/CHANGES.md b/awc/CHANGES.md index 798b2ce6b..a4948c827 100644 --- a/awc/CHANGES.md +++ b/awc/CHANGES.md @@ -60,7 +60,7 @@ * `ConnectorService` type is renamed to `BoxConnectorService`. [#2081] * Fix http/https encoding when enabling `compress` feature. [#2116] * Rename `TestResponse::header` to `append_header`, `set` to `insert_header`. `TestResponse` header - methods now take `IntoHeaderPair` tuples. [#2094] + methods now take `TryIntoHeaderPair` tuples. [#2094] [#2081]: https://github.com/actix/actix-web/pull/2081 [#2094]: https://github.com/actix/actix-web/pull/2094 diff --git a/awc/src/request.rs b/awc/src/request.rs index 3e1f83a82..bfb72f1db 100644 --- a/awc/src/request.rs +++ b/awc/src/request.rs @@ -6,7 +6,7 @@ use serde::Serialize; use actix_http::{ error::HttpError, - header::{self, HeaderMap, HeaderValue, IntoHeaderPair}, + header::{self, HeaderMap, HeaderValue, TryIntoHeaderPair}, ConnectionType, Method, RequestHead, Uri, Version, }; @@ -147,10 +147,7 @@ impl ClientRequest { } /// Insert a header, replacing any that were set with an equivalent field name. - pub fn insert_header(mut self, header: H) -> Self - where - H: IntoHeaderPair, - { + pub fn insert_header(mut self, header: impl TryIntoHeaderPair) -> Self { match header.try_into_header_pair() { Ok((key, value)) => { self.head.headers.insert(key, value); @@ -162,10 +159,7 @@ impl ClientRequest { } /// Insert a header only if it is not yet set. - pub fn insert_header_if_none(mut self, header: H) -> Self - where - H: IntoHeaderPair, - { + pub fn insert_header_if_none(mut self, header: impl TryIntoHeaderPair) -> Self { match header.try_into_header_pair() { Ok((key, value)) => { if !self.head.headers.contains_key(&key) { @@ -192,10 +186,7 @@ impl ClientRequest { /// .insert_header((CONTENT_TYPE, mime::APPLICATION_JSON)); /// # } /// ``` - pub fn append_header(mut self, header: H) -> Self - where - H: IntoHeaderPair, - { + pub fn append_header(mut self, header: impl TryIntoHeaderPair) -> Self { match header.try_into_header_pair() { Ok((key, value)) => self.head.headers.append(key, value), Err(e) => self.err = Some(e.into()), diff --git a/awc/src/test.rs b/awc/src/test.rs index 4a5c8e7ea..a3b5fa0eb 100644 --- a/awc/src/test.rs +++ b/awc/src/test.rs @@ -1,6 +1,6 @@ //! Test helpers for actix http client to use during testing. -use actix_http::{h1, header::IntoHeaderPair, Payload, ResponseHead, StatusCode, Version}; +use actix_http::{h1, header::TryIntoHeaderPair, Payload, ResponseHead, StatusCode, Version}; use bytes::Bytes; #[cfg(feature = "cookies")] @@ -28,10 +28,7 @@ impl Default for TestResponse { impl TestResponse { /// Create TestResponse and set header - pub fn with_header(header: H) -> Self - where - H: IntoHeaderPair, - { + pub fn with_header(header: impl TryIntoHeaderPair) -> Self { Self::default().insert_header(header) } @@ -42,10 +39,7 @@ impl TestResponse { } /// Insert a header - pub fn insert_header(mut self, header: H) -> Self - where - H: IntoHeaderPair, - { + pub fn insert_header(mut self, header: impl TryIntoHeaderPair) -> Self { if let Ok((key, value)) = header.try_into_header_pair() { self.head.headers.insert(key, value); return self; @@ -54,10 +48,7 @@ impl TestResponse { } /// Append a header - pub fn append_header(mut self, header: H) -> Self - where - H: IntoHeaderPair, - { + pub fn append_header(mut self, header: impl TryIntoHeaderPair) -> Self { if let Ok((key, value)) = header.try_into_header_pair() { self.head.headers.append(key, value); return self; diff --git a/examples/basic.rs b/examples/basic.rs index d29546129..0dfc1490f 100644 --- a/examples/basic.rs +++ b/examples/basic.rs @@ -22,14 +22,14 @@ async fn main() -> std::io::Result<()> { HttpServer::new(|| { App::new() - .wrap(middleware::DefaultHeaders::new().header("X-Version", "0.2")) + .wrap(middleware::DefaultHeaders::new().insert(("X-Version", "0.2"))) .wrap(middleware::Compress::default()) .wrap(middleware::Logger::default()) .service(index) .service(no_params) .service( web::resource("/resource2/index.html") - .wrap(middleware::DefaultHeaders::new().header("X-Version-R2", "0.3")) + .wrap(middleware::DefaultHeaders::new().insert(("X-Version-R2", "0.3"))) .default_service(web::route().to(HttpResponse::MethodNotAllowed)) .route(web::get().to(index_async)), ) diff --git a/examples/uds.rs b/examples/uds.rs index 1db252fef..e99ee3822 100644 --- a/examples/uds.rs +++ b/examples/uds.rs @@ -26,14 +26,14 @@ async fn main() -> std::io::Result<()> { HttpServer::new(|| { App::new() - .wrap(middleware::DefaultHeaders::new().header("X-Version", "0.2")) + .wrap(middleware::DefaultHeaders::new().insert(("X-Version", "0.2"))) .wrap(middleware::Compress::default()) .wrap(middleware::Logger::default()) .service(index) .service(no_params) .service( web::resource("/resource2/index.html") - .wrap(middleware::DefaultHeaders::new().header("X-Version-R2", "0.3")) + .wrap(middleware::DefaultHeaders::new().insert(("X-Version-R2", "0.3"))) .default_service(web::route().to(HttpResponse::MethodNotAllowed)) .route(web::get().to(index_async)), ) diff --git a/src/app.rs b/src/app.rs index 5323cb33a..64b23f1d4 100644 --- a/src/app.rs +++ b/src/app.rs @@ -602,7 +602,7 @@ mod tests { App::new() .wrap( DefaultHeaders::new() - .header(header::CONTENT_TYPE, HeaderValue::from_static("0001")), + .insert((header::CONTENT_TYPE, HeaderValue::from_static("0001"))), ) .route("/test", web::get().to(HttpResponse::Ok)), ) @@ -623,7 +623,7 @@ mod tests { .route("/test", web::get().to(HttpResponse::Ok)) .wrap( DefaultHeaders::new() - .header(header::CONTENT_TYPE, HeaderValue::from_static("0001")), + .insert((header::CONTENT_TYPE, HeaderValue::from_static("0001"))), ), ) .await; diff --git a/src/lib.rs b/src/lib.rs index a44c9b3fb..171a2d101 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -86,7 +86,6 @@ pub mod middleware; mod request; mod request_data; mod resource; -mod responder; mod response; mod rmap; mod route; @@ -109,12 +108,10 @@ pub use crate::error::{Error, ResponseError, Result}; pub use crate::extract::FromRequest; pub use crate::request::HttpRequest; pub use crate::resource::Resource; -pub use crate::responder::Responder; -pub use crate::response::{HttpResponse, HttpResponseBuilder}; +pub use crate::response::{CustomizeResponder, HttpResponse, HttpResponseBuilder, Responder}; pub use crate::route::Route; pub use crate::scope::Scope; pub use crate::server::HttpServer; -// TODO: is exposing the error directly really needed -pub use crate::types::{Either, EitherExtractError}; +pub use crate::types::Either; pub(crate) type BoxError = Box; diff --git a/src/middleware/default_headers.rs b/src/middleware/default_headers.rs index dceca44c2..bef4ea0c0 100644 --- a/src/middleware/default_headers.rs +++ b/src/middleware/default_headers.rs @@ -16,7 +16,7 @@ use pin_project_lite::pin_project; use crate::{ dev::{Service, Transform}, - http::header::{HeaderMap, HeaderName, HeaderValue, CONTENT_TYPE}, + http::header::{HeaderMap, HeaderName, HeaderValue, TryIntoHeaderPair, CONTENT_TYPE}, service::{ServiceRequest, ServiceResponse}, Error, }; @@ -29,79 +29,82 @@ use crate::{ /// ``` /// use actix_web::{web, http, middleware, App, HttpResponse}; /// -/// fn main() { -/// let app = App::new() -/// .wrap(middleware::DefaultHeaders::new().header("X-Version", "0.2")) -/// .service( -/// web::resource("/test") -/// .route(web::get().to(|| HttpResponse::Ok())) -/// .route(web::method(http::Method::HEAD).to(|| HttpResponse::MethodNotAllowed())) -/// ); -/// } +/// let app = App::new() +/// .wrap(middleware::DefaultHeaders::new().insert("X-Version", "0.2")) +/// .service( +/// web::resource("/test") +/// .route(web::get().to(|| HttpResponse::Ok())) +/// .route(web::method(http::Method::HEAD).to(|| HttpResponse::MethodNotAllowed())) +/// ); /// ``` -#[derive(Clone)] +#[derive(Debug, Clone, Default)] pub struct DefaultHeaders { inner: Rc, } +#[derive(Debug, Default)] struct Inner { headers: HeaderMap, } -impl Default for DefaultHeaders { - fn default() -> Self { - DefaultHeaders { - inner: Rc::new(Inner { - headers: HeaderMap::new(), - }), - } - } -} - impl DefaultHeaders { /// Constructs an empty `DefaultHeaders` middleware. + #[inline] pub fn new() -> DefaultHeaders { DefaultHeaders::default() } /// Adds a header to the default set. - #[inline] - pub fn header(mut self, key: K, value: V) -> Self + /// + /// # Panics + /// Panics when resolved header name or value is invalid. + pub fn insert(mut self, header: impl TryIntoHeaderPair) -> Self { + match header.try_into_header_pair() { + Ok((key, value)) => Rc::get_mut(&mut self.inner) + .expect( + "DefaultHeaders has been cloned. Add all default headers before cloning.", + ) + .headers + .append(key, value), + Err(err) => panic!("Invalid header: {}", err.into()), + } + + self + } + + #[doc(hidden)] + #[deprecated(since = "4.0.0", note = "Prefer `insert`.")] + pub fn header(self, key: K, value: V) -> Self where HeaderName: TryFrom, >::Error: Into, HeaderValue: TryFrom, >::Error: Into, { - #[allow(clippy::match_wild_err_arm)] - match HeaderName::try_from(key) { - Ok(key) => match HeaderValue::try_from(value) { - Ok(value) => { - Rc::get_mut(&mut self.inner) - .expect("Multiple copies exist") - .headers - .append(key, value); - } - Err(_) => panic!("Can not create header value"), - }, - Err(_) => panic!("Can not create header name"), - } - self + self.insert(( + HeaderName::try_from(key) + .map_err(Into::into) + .expect("Invalid header name"), + HeaderValue::try_from(value) + .map_err(Into::into) + .expect("Invalid header value"), + )) } /// Adds a default *Content-Type* header if response does not contain one. /// /// Default is `application/octet-stream`. - pub fn add_content_type(mut self) -> Self { - Rc::get_mut(&mut self.inner) - .expect("Multiple `Inner` copies exist.") - .headers - .insert( - CONTENT_TYPE, - HeaderValue::from_static("application/octet-stream"), - ); + pub fn insert_content_type(self) -> Self { + self.insert(( + CONTENT_TYPE, + HeaderValue::from_static("application/octet-stream"), + )) + } - self + #[doc(hidden)] + #[deprecated(since = "4.0.0", note = "Renamed to `insert_content_type`.")] + pub fn add_content_type(self) -> Self { + self.insert_content_type() } } @@ -119,7 +122,7 @@ where fn new_transform(&self, service: S) -> Self::Future { ready(Ok(DefaultHeadersMiddleware { service, - inner: self.inner.clone(), + inner: Rc::clone(&self.inner), })) } } @@ -197,17 +200,22 @@ mod tests { }; #[actix_rt::test] - async fn test_default_headers() { + async fn adding_default_headers() { let mw = DefaultHeaders::new() - .header(CONTENT_TYPE, "0001") + .insert(("X-TEST", "0001")) + .insert(("X-TEST-TWO", HeaderValue::from_static("123"))) .new_transform(ok_service()) .await .unwrap(); let req = TestRequest::default().to_srv_request(); - let resp = mw.call(req).await.unwrap(); - assert_eq!(resp.headers().get(CONTENT_TYPE).unwrap(), "0001"); + let res = mw.call(req).await.unwrap(); + assert_eq!(res.headers().get("x-test").unwrap(), "0001"); + assert_eq!(res.headers().get("x-test-two").unwrap(), "123"); + } + #[actix_rt::test] + async fn no_override_existing() { let req = TestRequest::default().to_srv_request(); let srv = |req: ServiceRequest| { ok(req.into_response( @@ -217,7 +225,7 @@ mod tests { )) }; let mw = DefaultHeaders::new() - .header(CONTENT_TYPE, "0001") + .insert((CONTENT_TYPE, "0001")) .new_transform(srv.into_service()) .await .unwrap(); @@ -226,10 +234,10 @@ mod tests { } #[actix_rt::test] - async fn test_content_type() { + async fn adding_content_type() { let srv = |req: ServiceRequest| ok(req.into_response(HttpResponse::Ok().finish())); let mw = DefaultHeaders::new() - .add_content_type() + .insert_content_type() .new_transform(srv.into_service()) .await .unwrap(); @@ -241,4 +249,16 @@ mod tests { "application/octet-stream" ); } + + #[test] + #[should_panic] + fn invalid_header_name() { + DefaultHeaders::new().insert((":", "hello")); + } + + #[test] + #[should_panic] + fn invalid_header_value() { + DefaultHeaders::new().insert(("x-test", "\n")); + } } diff --git a/src/middleware/mod.rs b/src/middleware/mod.rs index d19cb64e9..6984f04b8 100644 --- a/src/middleware/mod.rs +++ b/src/middleware/mod.rs @@ -33,7 +33,7 @@ mod tests { let _ = App::new() .wrap(Compat::new(Logger::default())) .wrap(Condition::new(true, DefaultHeaders::new())) - .wrap(DefaultHeaders::new().header("X-Test2", "X-Value2")) + .wrap(DefaultHeaders::new().insert(("X-Test2", "X-Value2"))) .wrap(ErrorHandlers::new().handler(StatusCode::FORBIDDEN, |res| { Ok(ErrorHandlerResponse::Response(res)) })) @@ -46,7 +46,7 @@ mod tests { .wrap(ErrorHandlers::new().handler(StatusCode::FORBIDDEN, |res| { Ok(ErrorHandlerResponse::Response(res)) })) - .wrap(DefaultHeaders::new().header("X-Test2", "X-Value2")) + .wrap(DefaultHeaders::new().insert(("X-Test2", "X-Value2"))) .wrap(Condition::new(true, DefaultHeaders::new())) .wrap(Compat::new(Logger::default())); diff --git a/src/resource.rs b/src/resource.rs index 420374a86..d4682fb92 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -15,13 +15,12 @@ use crate::{ dev::{ensure_leading_slash, AppService, ResourceDef}, guard::Guard, handler::Handler, - responder::Responder, route::{Route, RouteService}, service::{ BoxedHttpService, BoxedHttpServiceFactory, HttpServiceFactory, ServiceRequest, ServiceResponse, }, - BoxError, Error, FromRequest, HttpResponse, + BoxError, Error, FromRequest, HttpResponse, Responder, }; /// *Resource* is an entry in resources table which corresponds to requested URL. @@ -526,7 +525,7 @@ mod tests { .name("test") .wrap( DefaultHeaders::new() - .header(header::CONTENT_TYPE, HeaderValue::from_static("0001")), + .insert((header::CONTENT_TYPE, HeaderValue::from_static("0001"))), ) .route(web::get().to(HttpResponse::Ok)), ), diff --git a/src/response/builder.rs b/src/response/builder.rs index 18a1c8a7f..e1d6f7e72 100644 --- a/src/response/builder.rs +++ b/src/response/builder.rs @@ -9,7 +9,7 @@ use std::{ use actix_http::{ body::{BodyStream, BoxBody, MessageBody}, error::HttpError, - header::{self, HeaderName, IntoHeaderPair, IntoHeaderValue}, + header::{self, HeaderName, IntoHeaderValue, TryIntoHeaderPair}, ConnectionType, Extensions, Response, ResponseHead, StatusCode, }; use bytes::Bytes; @@ -67,10 +67,7 @@ impl HttpResponseBuilder { /// .insert_header(("X-TEST", "value")) /// .finish(); /// ``` - pub fn insert_header(&mut self, header: H) -> &mut Self - where - H: IntoHeaderPair, - { + pub fn insert_header(&mut self, header: impl TryIntoHeaderPair) -> &mut Self { if let Some(parts) = self.inner() { match header.try_into_header_pair() { Ok((key, value)) => { @@ -94,10 +91,7 @@ impl HttpResponseBuilder { /// .append_header(("X-TEST", "value2")) /// .finish(); /// ``` - pub fn append_header(&mut self, header: H) -> &mut Self - where - H: IntoHeaderPair, - { + pub fn append_header(&mut self, header: impl TryIntoHeaderPair) -> &mut Self { if let Some(parts) = self.inner() { match header.try_into_header_pair() { Ok((key, value)) => parts.headers.append(key, value), diff --git a/src/response/customize_responder.rs b/src/response/customize_responder.rs new file mode 100644 index 000000000..9b587a047 --- /dev/null +++ b/src/response/customize_responder.rs @@ -0,0 +1,219 @@ +use actix_http::{ + body::{EitherBody, MessageBody}, + error::HttpError, + header::HeaderMap, + header::TryIntoHeaderPair, + StatusCode, +}; + +use crate::{BoxError, HttpRequest, HttpResponse, Responder}; + +/// Allows overriding status code and headers for a responder. +pub struct CustomizeResponder { + inner: CustomizeResponderInner, + error: Option, +} + +struct CustomizeResponderInner { + responder: R, + status: Option, + override_headers: HeaderMap, + append_headers: HeaderMap, +} + +impl CustomizeResponder { + pub(crate) fn new(responder: R) -> Self { + CustomizeResponder { + inner: CustomizeResponderInner { + responder, + status: None, + override_headers: HeaderMap::new(), + append_headers: HeaderMap::new(), + }, + error: None, + } + } + + /// Override a status code for the Responder's response. + /// + /// ``` + /// use actix_web::{HttpRequest, Responder, http::StatusCode}; + /// + /// fn index(req: HttpRequest) -> impl Responder { + /// "Welcome!".with_status(StatusCode::OK) + /// } + /// ``` + pub fn with_status(mut self, status: StatusCode) -> Self { + if let Some(inner) = self.inner() { + inner.status = Some(status); + } + + self + } + + /// Insert header to the final response. + /// + /// Overrides other headers with the same name. + /// + /// ``` + /// use actix_web::{web, HttpRequest, Responder}; + /// use serde::Serialize; + /// + /// #[derive(Serialize)] + /// struct MyObj { + /// name: String, + /// } + /// + /// fn index(req: HttpRequest) -> impl Responder { + /// web::Json(MyObj { name: "Name".to_string() }) + /// .with_header(("x-version", "1.2.3")) + /// .with_header(("x-version", "1.2.3")) + /// } + /// ``` + pub fn insert_header(mut self, header: impl TryIntoHeaderPair) -> Self { + if let Some(inner) = self.inner() { + match header.try_into_header_pair() { + Ok((key, value)) => { + inner.override_headers.insert(key, value); + } + Err(err) => self.error = Some(err.into()), + }; + } + + self + } + + pub fn append_header(mut self, header: impl TryIntoHeaderPair) -> Self { + if let Some(inner) = self.inner() { + match header.try_into_header_pair() { + Ok((key, value)) => { + inner.append_headers.append(key, value); + } + Err(err) => self.error = Some(err.into()), + }; + } + + self + } + + #[doc(hidden)] + #[deprecated(since = "4.0.0", note = "Renamed to `insert_header`.")] + pub fn with_header(self, header: impl TryIntoHeaderPair) -> Self + where + Self: Sized, + { + self.insert_header(header) + } + + fn inner(&mut self) -> Option<&mut CustomizeResponderInner> { + if self.error.is_some() { + None + } else { + Some(&mut self.inner) + } + } +} + +impl Responder for CustomizeResponder +where + T: Responder, + ::Error: Into, +{ + type Body = EitherBody; + + fn respond_to(self, req: &HttpRequest) -> HttpResponse { + if let Some(err) = self.error { + return HttpResponse::from_error(err).map_into_right_body(); + } + + let mut res = self.inner.responder.respond_to(req); + + if let Some(status) = self.inner.status { + *res.status_mut() = status; + } + + for (k, v) in self.inner.override_headers { + res.headers_mut().insert(k, v); + } + + for (k, v) in self.inner.append_headers { + res.headers_mut().append(k, v); + } + + res.map_into_left_body() + } +} + +#[cfg(test)] +mod tests { + use bytes::Bytes; + + use actix_http::body::to_bytes; + + use super::*; + use crate::{ + http::{ + header::{HeaderValue, CONTENT_TYPE}, + StatusCode, + }, + test::TestRequest, + }; + + #[actix_rt::test] + async fn customize_responder() { + let req = TestRequest::default().to_http_request(); + let res = "test" + .to_string() + .customize() + .with_status(StatusCode::BAD_REQUEST) + .respond_to(&req); + + assert_eq!(res.status(), StatusCode::BAD_REQUEST); + assert_eq!( + to_bytes(res.into_body()).await.unwrap(), + Bytes::from_static(b"test"), + ); + + let res = "test" + .to_string() + .customize() + .insert_header(("content-type", "json")) + .respond_to(&req); + + assert_eq!(res.status(), StatusCode::OK); + assert_eq!( + res.headers().get(CONTENT_TYPE).unwrap(), + HeaderValue::from_static("json") + ); + assert_eq!( + to_bytes(res.into_body()).await.unwrap(), + Bytes::from_static(b"test"), + ); + } + + #[actix_rt::test] + async fn tuple_responder_with_status_code() { + let req = TestRequest::default().to_http_request(); + let res = ("test".to_string(), StatusCode::BAD_REQUEST).respond_to(&req); + assert_eq!(res.status(), StatusCode::BAD_REQUEST); + assert_eq!( + to_bytes(res.into_body()).await.unwrap(), + Bytes::from_static(b"test"), + ); + + let req = TestRequest::default().to_http_request(); + let res = ("test".to_string(), StatusCode::OK) + .customize() + .insert_header((CONTENT_TYPE, mime::APPLICATION_JSON)) + .respond_to(&req); + assert_eq!(res.status(), StatusCode::OK); + assert_eq!( + res.headers().get(CONTENT_TYPE).unwrap(), + HeaderValue::from_static("application/json") + ); + assert_eq!( + to_bytes(res.into_body()).await.unwrap(), + Bytes::from_static(b"test"), + ); + } +} diff --git a/src/response/mod.rs b/src/response/mod.rs index 8401db9d2..977147104 100644 --- a/src/response/mod.rs +++ b/src/response/mod.rs @@ -1,9 +1,13 @@ mod builder; +mod customize_responder; mod http_codes; +mod responder; #[allow(clippy::module_inception)] mod response; pub use self::builder::HttpResponseBuilder; +pub use self::customize_responder::CustomizeResponder; +pub use self::responder::Responder; pub use self::response::HttpResponse; #[cfg(feature = "cookies")] diff --git a/src/responder.rs b/src/response/responder.rs similarity index 63% rename from src/responder.rs rename to src/response/responder.rs index e72739a71..fac3cbeaa 100644 --- a/src/responder.rs +++ b/src/response/responder.rs @@ -2,64 +2,59 @@ use std::borrow::Cow; use actix_http::{ body::{BoxBody, EitherBody, MessageBody}, - error::HttpError, - header::HeaderMap, - header::IntoHeaderPair, + header::TryIntoHeaderPair, StatusCode, }; use bytes::{Bytes, BytesMut}; use crate::{BoxError, Error, HttpRequest, HttpResponse, HttpResponseBuilder}; +use super::CustomizeResponder; + /// 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. +// # TODO: more about implementation notes and foreign impls pub trait Responder { type Body: MessageBody + 'static; /// Convert self to `HttpResponse`. fn respond_to(self, req: &HttpRequest) -> HttpResponse; - /// Override a status code for a Responder. + /// Wraps responder in [`CustomizeResponder`] that allows modification of response details. /// - /// ``` - /// use actix_web::{http::StatusCode, HttpRequest, Responder}; + /// See [`CustomizeResponder`] docs for more. /// - /// fn index(req: HttpRequest) -> impl Responder { - /// "Welcome!".with_status(StatusCode::OK) - /// } + /// # Examples /// ``` - fn with_status(self, status: StatusCode) -> CustomResponder + /// use actix_web::{Responder, test}; + /// + /// let request = test::TestRequest::default().to_http_request(); + /// + /// let responder = "Hello world!" + /// .customize() + /// .with_status(400) + /// .insert_header(("x-hello", "world")) + /// + /// let response = res.respond_to(&req); + /// assert_eq!(res.status(), 400); + /// assert_eq!(res.headers().get("x-hello").unwrap(), "world"); + /// ``` + #[inline] + fn customize(self) -> CustomizeResponder where Self: Sized, { - CustomResponder::new(self).with_status(status) + CustomizeResponder::new(self) } - /// Insert header to the final response. - /// - /// Overrides other headers with the same name. - /// - /// ``` - /// use actix_web::{web, HttpRequest, Responder}; - /// use serde::Serialize; - /// - /// #[derive(Serialize)] - /// struct MyObj { - /// name: String, - /// } - /// - /// fn index(req: HttpRequest) -> impl Responder { - /// web::Json(MyObj { name: "Name".to_owned() }) - /// .with_header(("x-version", "1.2.3")) - /// } - /// ``` - fn with_header(self, header: H) -> CustomResponder + #[doc(hidden)] + #[deprecated(since = "4.0.0", note = "Prefer `.customize().insert_header()`.")] + fn with_header(self, header: impl TryIntoHeaderPair) -> CustomizeResponder where Self: Sized, - H: IntoHeaderPair, { - CustomResponder::new(self).with_header(header) + self.customize().insert_header(header) } } @@ -181,98 +176,6 @@ macro_rules! impl_into_string_responder { impl_into_string_responder!(&'_ String); impl_into_string_responder!(Cow<'_, str>); -/// Allows overriding status code and headers for a responder. -pub struct CustomResponder { - responder: T, - status: Option, - headers: Result, -} - -impl CustomResponder { - fn new(responder: T) -> Self { - CustomResponder { - responder, - status: None, - headers: Ok(HeaderMap::new()), - } - } - - /// Override a status code for the Responder's response. - /// - /// ``` - /// use actix_web::{HttpRequest, Responder, http::StatusCode}; - /// - /// fn index(req: HttpRequest) -> impl Responder { - /// "Welcome!".with_status(StatusCode::OK) - /// } - /// ``` - pub fn with_status(mut self, status: StatusCode) -> Self { - self.status = Some(status); - self - } - - /// Insert header to the final response. - /// - /// Overrides other headers with the same name. - /// - /// ``` - /// use actix_web::{web, HttpRequest, Responder}; - /// use serde::Serialize; - /// - /// #[derive(Serialize)] - /// struct MyObj { - /// name: String, - /// } - /// - /// fn index(req: HttpRequest) -> impl Responder { - /// web::Json(MyObj { name: "Name".to_string() }) - /// .with_header(("x-version", "1.2.3")) - /// .with_header(("x-version", "1.2.3")) - /// } - /// ``` - pub fn with_header(mut self, header: H) -> Self - where - H: IntoHeaderPair, - { - if let Ok(ref mut headers) = self.headers { - match header.try_into_header_pair() { - Ok((key, value)) => headers.append(key, value), - Err(e) => self.headers = Err(e.into()), - }; - } - - self - } -} - -impl Responder for CustomResponder -where - T: Responder, - ::Error: Into, -{ - type Body = EitherBody; - - fn respond_to(self, req: &HttpRequest) -> HttpResponse { - let headers = match self.headers { - Ok(headers) => headers, - Err(err) => return HttpResponse::from_error(err).map_into_right_body(), - }; - - let mut res = self.responder.respond_to(req); - - if let Some(status) = self.status { - *res.status_mut() = status; - } - - for (k, v) in headers { - // TODO: before v4, decide if this should be append instead - res.headers_mut().insert(k, v); - } - - res.map_into_left_body() - } -} - #[cfg(test)] pub(crate) mod tests { use actix_service::Service; @@ -440,59 +343,4 @@ pub(crate) mod tests { assert_eq!(res.status(), StatusCode::BAD_REQUEST); } - - #[actix_rt::test] - async fn test_custom_responder() { - let req = TestRequest::default().to_http_request(); - let res = "test" - .to_string() - .with_status(StatusCode::BAD_REQUEST) - .respond_to(&req); - - assert_eq!(res.status(), StatusCode::BAD_REQUEST); - assert_eq!( - to_bytes(res.into_body()).await.unwrap(), - Bytes::from_static(b"test"), - ); - - let res = "test" - .to_string() - .with_header(("content-type", "json")) - .respond_to(&req); - - assert_eq!(res.status(), StatusCode::OK); - assert_eq!( - res.headers().get(CONTENT_TYPE).unwrap(), - HeaderValue::from_static("json") - ); - assert_eq!( - to_bytes(res.into_body()).await.unwrap(), - Bytes::from_static(b"test"), - ); - } - - #[actix_rt::test] - async fn test_tuple_responder_with_status_code() { - let req = TestRequest::default().to_http_request(); - let res = ("test".to_string(), StatusCode::BAD_REQUEST).respond_to(&req); - assert_eq!(res.status(), StatusCode::BAD_REQUEST); - assert_eq!( - to_bytes(res.into_body()).await.unwrap(), - Bytes::from_static(b"test"), - ); - - let req = TestRequest::default().to_http_request(); - let res = ("test".to_string(), StatusCode::OK) - .with_header((CONTENT_TYPE, mime::APPLICATION_JSON)) - .respond_to(&req); - assert_eq!(res.status(), StatusCode::OK); - assert_eq!( - res.headers().get(CONTENT_TYPE).unwrap(), - HeaderValue::from_static("application/json") - ); - assert_eq!( - to_bytes(res.into_body()).await.unwrap(), - Bytes::from_static(b"test"), - ); - } } diff --git a/src/scope.rs b/src/scope.rs index 74523cd94..eaffdc1b4 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -935,7 +935,7 @@ mod tests { web::scope("app") .wrap( DefaultHeaders::new() - .header(header::CONTENT_TYPE, HeaderValue::from_static("0001")), + .insert((header::CONTENT_TYPE, HeaderValue::from_static("0001"))), ) .service(web::resource("/test").route(web::get().to(HttpResponse::Ok))), ), diff --git a/src/test.rs b/src/test.rs index cfb3ef8f2..5ef2343a8 100644 --- a/src/test.rs +++ b/src/test.rs @@ -4,8 +4,8 @@ use std::{borrow::Cow, net::SocketAddr, rc::Rc}; pub use actix_http::test::TestBuffer; use actix_http::{ - header::IntoHeaderPair, test::TestRequest as HttpTestRequest, Extensions, Method, Request, - StatusCode, Uri, Version, + header::TryIntoHeaderPair, test::TestRequest as HttpTestRequest, Extensions, Method, + Request, StatusCode, Uri, Version, }; use actix_router::{Path, ResourceDef, Url}; use actix_service::{IntoService, IntoServiceFactory, Service, ServiceFactory}; @@ -445,19 +445,13 @@ impl TestRequest { } /// Insert a header, replacing any that were set with an equivalent field name. - pub fn insert_header(mut self, header: H) -> Self - where - H: IntoHeaderPair, - { + pub fn insert_header(mut self, header: impl TryIntoHeaderPair) -> Self { self.req.insert_header(header); self } /// Append a header, keeping any that were set with an equivalent field name. - pub fn append_header(mut self, header: H) -> Self - where - H: IntoHeaderPair, - { + pub fn append_header(mut self, header: impl TryIntoHeaderPair) -> Self { self.req.append_header(header); self } diff --git a/src/web.rs b/src/web.rs index 16dbace60..042b8a008 100644 --- a/src/web.rs +++ b/src/web.rs @@ -8,7 +8,7 @@ pub use bytes::{Buf, BufMut, Bytes, BytesMut}; use crate::{ body::MessageBody, error::BlockingError, extract::FromRequest, handler::Handler, - resource::Resource, responder::Responder, route::Route, scope::Scope, service::WebService, + resource::Resource, route::Route, scope::Scope, service::WebService, Responder, }; pub use crate::config::ServiceConfig;