From 11ee8ec3ab22da345b448c4cf2d8e51781815873 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Mon, 13 Dec 2021 16:08:08 +0000 Subject: [PATCH] align remaining header map terminology (#2510) --- CHANGES.md | 11 + actix-files/src/named.rs | 62 ++--- actix-http/CHANGES.md | 12 +- actix-http/src/h2/service.rs | 15 +- actix-http/src/header/as_name.rs | 5 + actix-http/src/header/into_pair.rs | 56 ++-- actix-http/src/header/into_value.rs | 30 +-- actix-http/src/header/map.rs | 4 +- actix-http/src/header/mod.rs | 6 +- .../src/header/shared/content_encoding.rs | 4 +- actix-http/src/header/shared/http_date.rs | 5 +- actix-http/src/response.rs | 2 +- actix-http/src/response_builder.rs | 20 +- actix-http/src/test.rs | 16 +- awc/CHANGES.md | 5 +- awc/src/builder.rs | 83 +++--- awc/src/client/h1proto.rs | 2 +- awc/src/frozen.rs | 6 +- awc/src/lib.rs | 10 +- awc/src/middleware/redirect.rs | 8 +- awc/src/request.rs | 27 +- awc/src/sender.rs | 4 +- awc/src/test.rs | 21 +- awc/src/ws.rs | 10 +- examples/basic.rs | 4 +- examples/uds.rs | 4 +- src/app.rs | 4 +- src/error/internal.rs | 2 +- src/error/response_error.rs | 2 +- src/http/header/content_disposition.rs | 4 +- src/http/header/content_range.rs | 4 +- src/http/header/entity.rs | 4 +- src/http/header/if_range.rs | 6 +- src/http/header/macros.rs | 8 +- src/http/header/range.rs | 4 +- src/lib.rs | 7 +- src/middleware/default_headers.rs | 127 +++++---- src/middleware/mod.rs | 4 +- src/request_data.rs | 2 +- src/resource.rs | 5 +- src/response/builder.rs | 24 +- src/response/customize_responder.rs | 245 ++++++++++++++++++ src/response/mod.rs | 4 + src/{ => response}/responder.rs | 205 ++------------- src/scope.rs | 2 +- src/test.rs | 14 +- src/web.rs | 2 +- 47 files changed, 608 insertions(+), 503 deletions(-) create mode 100644 src/response/customize_responder.rs rename src/{ => response}/responder.rs (63%) diff --git a/CHANGES.md b/CHANGES.md index 3e0b12d9e..2df820027 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,17 @@ # Changes ## Unreleased - 2021-xx-xx +### Added +* Method on `Responder` trait (`customize`) for customizing responders and `CustomizeResponder` struct. [#2510] +* Implement `Debug` for `DefaultHeaders`. [#2510] + +### Changed +* Align `DefaultHeader` method terminology, deprecating previous methods. [#2510] + +### Removed +* Top-level `EitherExtractError` export. [#2510] + +[#2510]: https://github.com/actix/actix-web/pull/2510 ## 4.0.0-beta.14 - 2021-12-11 diff --git a/actix-files/src/named.rs b/actix-files/src/named.rs index 0848543a8..810988f0c 100644 --- a/actix-files/src/named.rs +++ b/actix-files/src/named.rs @@ -2,14 +2,10 @@ use std::{ fmt, fs::Metadata, io, - ops::{Deref, DerefMut}, path::{Path, PathBuf}, time::{SystemTime, UNIX_EPOCH}, }; -#[cfg(unix)] -use std::os::unix::fs::MetadataExt; - use actix_service::{Service, ServiceFactory}; use actix_web::{ body::{self, BoxBody, SizedStream}, @@ -27,6 +23,7 @@ use actix_web::{ Error, HttpMessage, HttpRequest, HttpResponse, Responder, }; use bitflags::bitflags; +use derive_more::{Deref, DerefMut}; use futures_core::future::LocalBoxFuture; use mime_guess::from_path; @@ -71,8 +68,11 @@ impl Default for Flags { /// NamedFile::open_async("./static/index.html").await /// } /// ``` +#[derive(Deref, DerefMut)] pub struct NamedFile { path: PathBuf, + #[deref] + #[deref_mut] file: File, modified: Option, pub(crate) md: Metadata, @@ -364,14 +364,18 @@ impl NamedFile { self } + /// Creates a etag in a format is similar to Apache's. pub(crate) fn etag(&self) -> Option { - // This etag format is similar to Apache's. self.modified.as_ref().map(|mtime| { let ino = { #[cfg(unix)] { + #[cfg(unix)] + use std::os::unix::fs::MetadataExt as _; + self.md.ino() } + #[cfg(not(unix))] { 0 @@ -472,17 +476,17 @@ impl NamedFile { false }; - let mut resp = HttpResponse::build(self.status_code); + let mut res = HttpResponse::build(self.status_code); if self.flags.contains(Flags::PREFER_UTF8) { let ct = equiv_utf8_text(self.content_type.clone()); - resp.insert_header((header::CONTENT_TYPE, ct.to_string())); + res.insert_header((header::CONTENT_TYPE, ct.to_string())); } else { - resp.insert_header((header::CONTENT_TYPE, self.content_type.to_string())); + res.insert_header((header::CONTENT_TYPE, self.content_type.to_string())); } if self.flags.contains(Flags::CONTENT_DISPOSITION) { - resp.insert_header(( + res.insert_header(( header::CONTENT_DISPOSITION, self.content_disposition.to_string(), )); @@ -490,18 +494,18 @@ impl NamedFile { // default compressing if let Some(current_encoding) = self.encoding { - resp.encoding(current_encoding); + res.encoding(current_encoding); } if let Some(lm) = last_modified { - resp.insert_header((header::LAST_MODIFIED, lm.to_string())); + res.insert_header((header::LAST_MODIFIED, lm.to_string())); } if let Some(etag) = etag { - resp.insert_header((header::ETAG, etag.to_string())); + res.insert_header((header::ETAG, etag.to_string())); } - resp.insert_header((header::ACCEPT_RANGES, "bytes")); + res.insert_header((header::ACCEPT_RANGES, "bytes")); let mut length = self.md.len(); let mut offset = 0; @@ -513,24 +517,24 @@ impl NamedFile { length = ranges[0].length; offset = ranges[0].start; - resp.encoding(ContentEncoding::Identity); - resp.insert_header(( + res.encoding(ContentEncoding::Identity); + res.insert_header(( header::CONTENT_RANGE, format!("bytes {}-{}/{}", offset, offset + length - 1, self.md.len()), )); } else { - resp.insert_header((header::CONTENT_RANGE, format!("bytes */{}", length))); - return resp.status(StatusCode::RANGE_NOT_SATISFIABLE).finish(); + res.insert_header((header::CONTENT_RANGE, format!("bytes */{}", length))); + return res.status(StatusCode::RANGE_NOT_SATISFIABLE).finish(); }; } else { - return resp.status(StatusCode::BAD_REQUEST).finish(); + return res.status(StatusCode::BAD_REQUEST).finish(); }; }; if precondition_failed { - return resp.status(StatusCode::PRECONDITION_FAILED).finish(); + return res.status(StatusCode::PRECONDITION_FAILED).finish(); } else if not_modified { - return resp + return res .status(StatusCode::NOT_MODIFIED) .body(body::None::new()) .map_into_boxed_body(); @@ -539,10 +543,10 @@ impl NamedFile { let reader = chunked::new_chunked_read(length, offset, self.file); if offset != 0 || length != self.md.len() { - resp.status(StatusCode::PARTIAL_CONTENT); + res.status(StatusCode::PARTIAL_CONTENT); } - resp.body(SizedStream::new(length, reader)) + res.body(SizedStream::new(length, reader)) } } @@ -586,20 +590,6 @@ fn none_match(etag: Option<&header::EntityTag>, req: &HttpRequest) -> bool { } } -impl Deref for NamedFile { - type Target = File; - - fn deref(&self) -> &Self::Target { - &self.file - } -} - -impl DerefMut for NamedFile { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.file - } -} - impl Responder for NamedFile { type Body = BoxBody; diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index fe47902f7..011e2c608 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -1,6 +1,12 @@ # Changes ## Unreleased - 2021-xx-xx +### Changed +* Rename trait `IntoHeaderPair => TryIntoHeaderPair`. [#2510] +* Rename `TryIntoHeaderPair::{try_into_header_pair => try_into_pair}`. [#2510] +* Rename trait `IntoHeaderValue => TryIntoHeaderValue`. [#2510] + +[#2510]: https://github.com/actix/actix-web/pull/2510 ## 3.0.0-beta.15 - 2021-12-11 @@ -260,7 +266,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] @@ -271,9 +277,9 @@ * `trust-dns` optional feature to enable `trust-dns-resolver` as client dns resolver. [#1969] ### Changed -* `ResponseBuilder::content_type` now takes an `impl IntoHeaderValue` to support using typed +* `ResponseBuilder::content_type` now takes an `impl TryIntoHeaderValue` to support using typed `mime` types. [#1894] -* Renamed `IntoHeaderValue::{try_into => try_into_value}` to avoid ambiguity with std +* Renamed `TryIntoHeaderValue::{try_into => try_into_value}` to avoid ambiguity with std `TryInto` trait. [#1894] * `Extensions::insert` returns Option of replaced item. [#1904] * Remove `HttpResponseBuilder::json2()`. [#1903] 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/as_name.rs b/actix-http/src/header/as_name.rs index 17d007f2f..a895010b1 100644 --- a/actix-http/src/header/as_name.rs +++ b/actix-http/src/header/as_name.rs @@ -16,6 +16,7 @@ pub trait Sealed { } impl Sealed for HeaderName { + #[inline] fn try_as_name(&self, _: Seal) -> Result, InvalidHeaderName> { Ok(Cow::Borrowed(self)) } @@ -23,6 +24,7 @@ impl Sealed for HeaderName { impl AsHeaderName for HeaderName {} impl Sealed for &HeaderName { + #[inline] fn try_as_name(&self, _: Seal) -> Result, InvalidHeaderName> { Ok(Cow::Borrowed(*self)) } @@ -30,6 +32,7 @@ impl Sealed for &HeaderName { impl AsHeaderName for &HeaderName {} impl Sealed for &str { + #[inline] fn try_as_name(&self, _: Seal) -> Result, InvalidHeaderName> { HeaderName::from_str(self).map(Cow::Owned) } @@ -37,6 +40,7 @@ impl Sealed for &str { impl AsHeaderName for &str {} impl Sealed for String { + #[inline] fn try_as_name(&self, _: Seal) -> Result, InvalidHeaderName> { HeaderName::from_str(self).map(Cow::Owned) } @@ -44,6 +48,7 @@ impl Sealed for String { impl AsHeaderName for String {} impl Sealed for &String { + #[inline] fn try_as_name(&self, _: Seal) -> Result, InvalidHeaderName> { HeaderName::from_str(self).map(Cow::Owned) } diff --git a/actix-http/src/header/into_pair.rs b/actix-http/src/header/into_pair.rs index b4250e06e..91c3e6640 100644 --- a/actix-http/src/header/into_pair.rs +++ b/actix-http/src/header/into_pair.rs @@ -1,22 +1,20 @@ -//! [`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, InvalidHeaderName, InvalidHeaderValue, TryIntoHeaderValue, }; +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>; + fn try_into_pair(self) -> Result<(HeaderName, HeaderValue), Self::Error>; } #[derive(Debug)] @@ -34,14 +32,14 @@ impl From for HttpError { } } -impl IntoHeaderPair for (HeaderName, V) +impl TryIntoHeaderPair for (HeaderName, V) where - V: IntoHeaderValue, + V: TryIntoHeaderValue, V::Error: Into, { type Error = InvalidHeaderPart; - fn try_into_header_pair(self) -> Result<(HeaderName, HeaderValue), Self::Error> { + fn try_into_pair(self) -> Result<(HeaderName, HeaderValue), Self::Error> { let (name, value) = self; let value = value .try_into_value() @@ -50,14 +48,14 @@ where } } -impl IntoHeaderPair for (&HeaderName, V) +impl TryIntoHeaderPair for (&HeaderName, V) where - V: IntoHeaderValue, + V: TryIntoHeaderValue, V::Error: Into, { type Error = InvalidHeaderPart; - fn try_into_header_pair(self) -> Result<(HeaderName, HeaderValue), Self::Error> { + fn try_into_pair(self) -> Result<(HeaderName, HeaderValue), Self::Error> { let (name, value) = self; let value = value .try_into_value() @@ -66,14 +64,14 @@ where } } -impl IntoHeaderPair for (&[u8], V) +impl TryIntoHeaderPair for (&[u8], V) where - V: IntoHeaderValue, + V: TryIntoHeaderValue, V::Error: Into, { type Error = InvalidHeaderPart; - fn try_into_header_pair(self) -> Result<(HeaderName, HeaderValue), Self::Error> { + fn try_into_pair(self) -> Result<(HeaderName, HeaderValue), Self::Error> { let (name, value) = self; let name = HeaderName::try_from(name).map_err(InvalidHeaderPart::Name)?; let value = value @@ -83,14 +81,14 @@ where } } -impl IntoHeaderPair for (&str, V) +impl TryIntoHeaderPair for (&str, V) where - V: IntoHeaderValue, + V: TryIntoHeaderValue, V::Error: Into, { type Error = InvalidHeaderPart; - fn try_into_header_pair(self) -> Result<(HeaderName, HeaderValue), Self::Error> { + fn try_into_pair(self) -> Result<(HeaderName, HeaderValue), Self::Error> { let (name, value) = self; let name = HeaderName::try_from(name).map_err(InvalidHeaderPart::Name)?; let value = value @@ -100,23 +98,25 @@ where } } -impl IntoHeaderPair for (String, V) +impl TryIntoHeaderPair for (String, V) where - V: IntoHeaderValue, + V: TryIntoHeaderValue, V::Error: Into, { type Error = InvalidHeaderPart; - fn try_into_header_pair(self) -> Result<(HeaderName, HeaderValue), Self::Error> { + #[inline] + fn try_into_pair(self) -> Result<(HeaderName, HeaderValue), Self::Error> { let (name, value) = self; - (name.as_str(), value).try_into_header_pair() + (name.as_str(), value).try_into_pair() } } -impl IntoHeaderPair for T { - type Error = ::Error; +impl TryIntoHeaderPair for T { + type Error = ::Error; - fn try_into_header_pair(self) -> Result<(HeaderName, HeaderValue), Self::Error> { + #[inline] + fn try_into_pair(self) -> Result<(HeaderName, HeaderValue), Self::Error> { Ok((T::name(), self.try_into_value()?)) } } diff --git a/actix-http/src/header/into_value.rs b/actix-http/src/header/into_value.rs index bad05db64..6d369ee65 100644 --- a/actix-http/src/header/into_value.rs +++ b/actix-http/src/header/into_value.rs @@ -1,4 +1,4 @@ -//! [`IntoHeaderValue`] trait and implementations. +//! [`TryIntoHeaderValue`] trait and implementations. use std::convert::TryFrom as _; @@ -7,7 +7,7 @@ use http::{header::InvalidHeaderValue, Error as HttpError, HeaderValue}; use mime::Mime; /// An interface for types that can be converted into a [`HeaderValue`]. -pub trait IntoHeaderValue: Sized { +pub trait TryIntoHeaderValue: Sized { /// The type returned in the event of a conversion error. type Error: Into; @@ -15,7 +15,7 @@ pub trait IntoHeaderValue: Sized { fn try_into_value(self) -> Result; } -impl IntoHeaderValue for HeaderValue { +impl TryIntoHeaderValue for HeaderValue { type Error = InvalidHeaderValue; #[inline] @@ -24,7 +24,7 @@ impl IntoHeaderValue for HeaderValue { } } -impl IntoHeaderValue for &HeaderValue { +impl TryIntoHeaderValue for &HeaderValue { type Error = InvalidHeaderValue; #[inline] @@ -33,7 +33,7 @@ impl IntoHeaderValue for &HeaderValue { } } -impl IntoHeaderValue for &str { +impl TryIntoHeaderValue for &str { type Error = InvalidHeaderValue; #[inline] @@ -42,7 +42,7 @@ impl IntoHeaderValue for &str { } } -impl IntoHeaderValue for &[u8] { +impl TryIntoHeaderValue for &[u8] { type Error = InvalidHeaderValue; #[inline] @@ -51,7 +51,7 @@ impl IntoHeaderValue for &[u8] { } } -impl IntoHeaderValue for Bytes { +impl TryIntoHeaderValue for Bytes { type Error = InvalidHeaderValue; #[inline] @@ -60,7 +60,7 @@ impl IntoHeaderValue for Bytes { } } -impl IntoHeaderValue for Vec { +impl TryIntoHeaderValue for Vec { type Error = InvalidHeaderValue; #[inline] @@ -69,7 +69,7 @@ impl IntoHeaderValue for Vec { } } -impl IntoHeaderValue for String { +impl TryIntoHeaderValue for String { type Error = InvalidHeaderValue; #[inline] @@ -78,7 +78,7 @@ impl IntoHeaderValue for String { } } -impl IntoHeaderValue for usize { +impl TryIntoHeaderValue for usize { type Error = InvalidHeaderValue; #[inline] @@ -87,7 +87,7 @@ impl IntoHeaderValue for usize { } } -impl IntoHeaderValue for i64 { +impl TryIntoHeaderValue for i64 { type Error = InvalidHeaderValue; #[inline] @@ -96,7 +96,7 @@ impl IntoHeaderValue for i64 { } } -impl IntoHeaderValue for u64 { +impl TryIntoHeaderValue for u64 { type Error = InvalidHeaderValue; #[inline] @@ -105,7 +105,7 @@ impl IntoHeaderValue for u64 { } } -impl IntoHeaderValue for i32 { +impl TryIntoHeaderValue for i32 { type Error = InvalidHeaderValue; #[inline] @@ -114,7 +114,7 @@ impl IntoHeaderValue for i32 { } } -impl IntoHeaderValue for u32 { +impl TryIntoHeaderValue for u32 { type Error = InvalidHeaderValue; #[inline] @@ -123,7 +123,7 @@ impl IntoHeaderValue for u32 { } } -impl IntoHeaderValue for Mime { +impl TryIntoHeaderValue for Mime { type Error = InvalidHeaderValue; #[inline] diff --git a/actix-http/src/header/map.rs b/actix-http/src/header/map.rs index 12c8f9462..748410375 100644 --- a/actix-http/src/header/map.rs +++ b/actix-http/src/header/map.rs @@ -333,7 +333,7 @@ impl HeaderMap { } } - /// Inserts a name-value pair into the map. + /// Inserts (overrides) a name-value pair in the map. /// /// If the map already contained this key, the new value is associated with the key and all /// previous values are removed and returned as a `Removed` iterator. The key is not updated; @@ -372,7 +372,7 @@ impl HeaderMap { Removed::new(value) } - /// Inserts a name-value pair into the map. + /// Appends a name-value pair to the map. /// /// If the map already contained this key, the new value is added to the list of values /// currently associated with the key. The key is not updated; this matters for types that can diff --git a/actix-http/src/header/mod.rs b/actix-http/src/header/mod.rs index 5fe76381b..dd4f06106 100644 --- a/actix-http/src/header/mod.rs +++ b/actix-http/src/header/mod.rs @@ -37,8 +37,8 @@ mod shared; mod utils; pub use self::as_name::AsHeaderName; -pub use self::into_pair::IntoHeaderPair; -pub use self::into_value::IntoHeaderValue; +pub use self::into_pair::TryIntoHeaderPair; +pub use self::into_value::TryIntoHeaderValue; pub use self::map::HeaderMap; pub use self::shared::{ parse_extended_value, q, Charset, ContentEncoding, ExtendedValue, HttpDate, LanguageTag, @@ -49,7 +49,7 @@ pub use self::utils::{ }; /// An interface for types that already represent a valid header. -pub trait Header: IntoHeaderValue { +pub trait Header: TryIntoHeaderValue { /// Returns the name of the header field fn name() -> HeaderName; diff --git a/actix-http/src/header/shared/content_encoding.rs b/actix-http/src/header/shared/content_encoding.rs index 073d90dce..a6e52138d 100644 --- a/actix-http/src/header/shared/content_encoding.rs +++ b/actix-http/src/header/shared/content_encoding.rs @@ -5,7 +5,7 @@ use http::header::InvalidHeaderValue; use crate::{ error::ParseError, - header::{self, from_one_raw_str, Header, HeaderName, HeaderValue, IntoHeaderValue}, + header::{self, from_one_raw_str, Header, HeaderName, HeaderValue, TryIntoHeaderValue}, HttpMessage, }; @@ -96,7 +96,7 @@ impl TryFrom<&str> for ContentEncoding { } } -impl IntoHeaderValue for ContentEncoding { +impl TryIntoHeaderValue for ContentEncoding { type Error = InvalidHeaderValue; fn try_into_value(self) -> Result { diff --git a/actix-http/src/header/shared/http_date.rs b/actix-http/src/header/shared/http_date.rs index 228f6f00e..473d6cad0 100644 --- a/actix-http/src/header/shared/http_date.rs +++ b/actix-http/src/header/shared/http_date.rs @@ -4,7 +4,8 @@ use bytes::BytesMut; use http::header::{HeaderValue, InvalidHeaderValue}; use crate::{ - config::DATE_VALUE_LENGTH, error::ParseError, header::IntoHeaderValue, helpers::MutWriter, + config::DATE_VALUE_LENGTH, error::ParseError, header::TryIntoHeaderValue, + helpers::MutWriter, }; /// A timestamp with HTTP-style formatting and parsing. @@ -29,7 +30,7 @@ impl fmt::Display for HttpDate { } } -impl IntoHeaderValue for HttpDate { +impl TryIntoHeaderValue for HttpDate { type Error = InvalidHeaderValue; fn try_into_value(self) -> Result { diff --git a/actix-http/src/response.rs b/actix-http/src/response.rs index 861cab2cb..9f799f669 100644 --- a/actix-http/src/response.rs +++ b/actix-http/src/response.rs @@ -11,7 +11,7 @@ use bytestring::ByteString; use crate::{ body::{BoxBody, MessageBody}, extensions::Extensions, - header::{self, HeaderMap, IntoHeaderValue}, + header::{self, HeaderMap, TryIntoHeaderValue}, message::{BoxedResponseHead, ResponseHead}, Error, ResponseBuilder, StatusCode, }; diff --git a/actix-http/src/response_builder.rs b/actix-http/src/response_builder.rs index dfc2612fb..adbe86fca 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, TryIntoHeaderPair, TryIntoHeaderValue}, message::{BoxedResponseHead, ConnectionType, ResponseHead}, Extensions, Response, StatusCode, }; @@ -90,12 +90,9 @@ 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() { + match header.try_into_pair() { Ok((key, value)) => { parts.headers.insert(key, value); } @@ -121,12 +118,9 @@ 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() { + match header.try_into_pair() { Ok((key, value)) => parts.headers.append(key, value), Err(e) => self.err = Some(e.into()), }; @@ -157,7 +151,7 @@ impl ResponseBuilder { #[inline] pub fn upgrade(&mut self, value: V) -> &mut Self where - V: IntoHeaderValue, + V: TryIntoHeaderValue, { if let Some(parts) = self.inner() { parts.set_connection_type(ConnectionType::Upgrade); @@ -195,7 +189,7 @@ impl ResponseBuilder { #[inline] pub fn content_type(&mut self, value: V) -> &mut Self where - V: IntoHeaderValue, + V: TryIntoHeaderValue, { if let Some(parts) = self.inner() { match value.try_into_value() { diff --git a/actix-http/src/test.rs b/actix-http/src/test.rs index ec781743d..7e26ee865 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,11 +92,8 @@ 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, - { - match header.try_into_header_pair() { + pub fn insert_header(&mut self, header: impl TryIntoHeaderPair) -> &mut Self { + match header.try_into_pair() { Ok((key, value)) => { parts(&mut self.0).headers.insert(key, value); } @@ -109,11 +106,8 @@ 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, - { - match header.try_into_header_pair() { + pub fn append_header(&mut self, header: impl TryIntoHeaderPair) -> &mut Self { + match header.try_into_pair() { Ok((key, value)) => { parts(&mut self.0).headers.append(key, value); } diff --git a/awc/CHANGES.md b/awc/CHANGES.md index 798b2ce6b..8a3fea46a 100644 --- a/awc/CHANGES.md +++ b/awc/CHANGES.md @@ -1,6 +1,9 @@ # Changes ## Unreleased - 2021-xx-xx +* Add `ClientBuilder::add_default_header` and deprecate `ClientBuilder::header`. [#2510] + +[#2510]: https://github.com/actix/actix-web/pull/2510 ## 3.0.0-beta.13 - 2021-12-11 @@ -60,7 +63,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/builder.rs b/awc/src/builder.rs index 43e5c0def..30f203bb8 100644 --- a/awc/src/builder.rs +++ b/awc/src/builder.rs @@ -2,7 +2,7 @@ use std::{convert::TryFrom, fmt, net::IpAddr, rc::Rc, time::Duration}; use actix_http::{ error::HttpError, - header::{self, HeaderMap, HeaderName}, + header::{self, HeaderMap, HeaderName, TryIntoHeaderPair}, Uri, }; use actix_rt::net::{ActixStream, TcpStream}; @@ -21,11 +21,11 @@ use crate::{ /// This type can be used to construct an instance of `Client` through a /// builder-like pattern. pub struct ClientBuilder { - default_headers: bool, max_http_version: Option, stream_window_size: Option, conn_window_size: Option, - headers: HeaderMap, + fundamental_headers: bool, + default_headers: HeaderMap, timeout: Option, connector: Connector, middleware: M, @@ -44,15 +44,15 @@ impl ClientBuilder { (), > { ClientBuilder { - middleware: (), - default_headers: true, - headers: HeaderMap::new(), - timeout: Some(Duration::from_secs(5)), - local_address: None, - connector: Connector::new(), max_http_version: None, stream_window_size: None, conn_window_size: None, + fundamental_headers: true, + default_headers: HeaderMap::new(), + timeout: Some(Duration::from_secs(5)), + connector: Connector::new(), + middleware: (), + local_address: None, max_redirects: 10, } } @@ -78,8 +78,8 @@ where { ClientBuilder { middleware: self.middleware, + fundamental_headers: self.fundamental_headers, default_headers: self.default_headers, - headers: self.headers, timeout: self.timeout, local_address: self.local_address, connector, @@ -153,30 +153,46 @@ where self } - /// Do not add default request headers. + /// Do not add fundamental default request headers. + /// /// By default `Date` and `User-Agent` headers are set. pub fn no_default_headers(mut self) -> Self { - self.default_headers = false; + self.fundamental_headers = false; self } - /// Add default header. Headers added by this method - /// get added to every request. + /// Add default header. + /// + /// Headers added by this method get added to every request unless overriden by . + /// + /// # Panics + /// Panics if header name or value is invalid. + pub fn add_default_header(mut self, header: impl TryIntoHeaderPair) -> Self { + match header.try_into_pair() { + Ok((key, value)) => self.default_headers.append(key, value), + Err(err) => panic!("Header error: {:?}", err.into()), + } + + self + } + + #[doc(hidden)] + #[deprecated(since = "3.0.0", note = "Prefer `add_default_header((key, value))`.")] pub fn header(mut self, key: K, value: V) -> Self where HeaderName: TryFrom, >::Error: fmt::Debug + Into, - V: header::IntoHeaderValue, + V: header::TryIntoHeaderValue, V::Error: fmt::Debug, { match HeaderName::try_from(key) { Ok(key) => match value.try_into_value() { Ok(value) => { - self.headers.append(key, value); + self.default_headers.append(key, value); } - Err(e) => log::error!("Header value error: {:?}", e), + Err(err) => log::error!("Header value error: {:?}", err), }, - Err(e) => log::error!("Header name error: {:?}", e), + Err(err) => log::error!("Header name error: {:?}", err), } self } @@ -190,10 +206,10 @@ where Some(password) => format!("{}:{}", username, password), None => format!("{}:", username), }; - self.header( + self.add_default_header(( header::AUTHORIZATION, format!("Basic {}", base64::encode(&auth)), - ) + )) } /// Set client wide HTTP bearer authentication header @@ -201,13 +217,12 @@ where where T: fmt::Display, { - self.header(header::AUTHORIZATION, format!("Bearer {}", token)) + self.add_default_header((header::AUTHORIZATION, format!("Bearer {}", token))) } - /// Registers middleware, in the form of a middleware component (type), - /// that runs during inbound and/or outbound processing in the request - /// life-cycle (request -> response), modifying request/response as - /// necessary, across all requests managed by the Client. + /// Registers middleware, in the form of a middleware component (type), that runs during inbound + /// and/or outbound processing in the request life-cycle (request -> response), + /// modifying request/response as necessary, across all requests managed by the `Client`. pub fn wrap( self, mw: M1, @@ -218,11 +233,11 @@ where { ClientBuilder { middleware: NestTransform::new(self.middleware, mw), - default_headers: self.default_headers, + fundamental_headers: self.fundamental_headers, max_http_version: self.max_http_version, stream_window_size: self.stream_window_size, conn_window_size: self.conn_window_size, - headers: self.headers, + default_headers: self.default_headers, timeout: self.timeout, connector: self.connector, local_address: self.local_address, @@ -237,10 +252,10 @@ where M::Transform: Service, { - let redirect_time = self.max_redirects; + let max_redirects = self.max_redirects; - if redirect_time > 0 { - self.wrap(Redirect::new().max_redirect_times(redirect_time)) + if max_redirects > 0 { + self.wrap(Redirect::new().max_redirect_times(max_redirects)) ._finish() } else { self._finish() @@ -272,7 +287,7 @@ where let connector = boxed::rc_service(self.middleware.new_transform(connector)); Client(ClientConfig { - headers: Rc::new(self.headers), + default_headers: Rc::new(self.default_headers), timeout: self.timeout, connector, }) @@ -288,7 +303,7 @@ mod tests { let client = ClientBuilder::new().basic_auth("username", Some("password")); assert_eq!( client - .headers + .default_headers .get(header::AUTHORIZATION) .unwrap() .to_str() @@ -299,7 +314,7 @@ mod tests { let client = ClientBuilder::new().basic_auth("username", None); assert_eq!( client - .headers + .default_headers .get(header::AUTHORIZATION) .unwrap() .to_str() @@ -313,7 +328,7 @@ mod tests { let client = ClientBuilder::new().bearer_auth("someS3cr3tAutht0k3n"); assert_eq!( client - .headers + .default_headers .get(header::AUTHORIZATION) .unwrap() .to_str() diff --git a/awc/src/client/h1proto.rs b/awc/src/client/h1proto.rs index c8b9a3fae..1028a2178 100644 --- a/awc/src/client/h1proto.rs +++ b/awc/src/client/h1proto.rs @@ -9,7 +9,7 @@ use actix_http::{ body::{BodySize, MessageBody}, error::PayloadError, h1, - header::{HeaderMap, IntoHeaderValue, EXPECT, HOST}, + header::{HeaderMap, TryIntoHeaderValue, EXPECT, HOST}, Payload, RequestHeadType, ResponseHead, StatusCode, }; use actix_utils::future::poll_fn; diff --git a/awc/src/frozen.rs b/awc/src/frozen.rs index 7497f85c8..cd93a1d60 100644 --- a/awc/src/frozen.rs +++ b/awc/src/frozen.rs @@ -6,7 +6,7 @@ use serde::Serialize; use actix_http::{ error::HttpError, - header::{HeaderMap, HeaderName, IntoHeaderValue}, + header::{HeaderMap, HeaderName, TryIntoHeaderValue}, Method, RequestHead, Uri, }; @@ -114,7 +114,7 @@ impl FrozenClientRequest { where HeaderName: TryFrom, >::Error: Into, - V: IntoHeaderValue, + V: TryIntoHeaderValue, { self.extra_headers(HeaderMap::new()) .extra_header(key, value) @@ -142,7 +142,7 @@ impl FrozenSendBuilder { where HeaderName: TryFrom, >::Error: Into, - V: IntoHeaderValue, + V: TryIntoHeaderValue, { match HeaderName::try_from(key) { Ok(key) => match value.try_into_value() { diff --git a/awc/src/lib.rs b/awc/src/lib.rs index 06fd33fac..00c559406 100644 --- a/awc/src/lib.rs +++ b/awc/src/lib.rs @@ -168,7 +168,7 @@ pub struct Client(ClientConfig); #[derive(Clone)] pub(crate) struct ClientConfig { pub(crate) connector: BoxConnectorService, - pub(crate) headers: Rc, + pub(crate) default_headers: Rc, pub(crate) timeout: Option, } @@ -204,7 +204,9 @@ impl Client { { let mut req = ClientRequest::new(method, url, self.0.clone()); - for header in self.0.headers.iter() { + for header in self.0.default_headers.iter() { + // header map is empty + // TODO: probably append instead req = req.insert_header_if_none(header); } req @@ -297,7 +299,7 @@ impl Client { >::Error: Into, { let mut req = ws::WebsocketsRequest::new(url, self.0.clone()); - for (key, value) in self.0.headers.iter() { + for (key, value) in self.0.default_headers.iter() { req.head.headers.insert(key.clone(), value.clone()); } req @@ -308,6 +310,6 @@ impl Client { /// Returns Some(&mut HeaderMap) when Client object is unique /// (No other clone of client exists at the same time). pub fn headers(&mut self) -> Option<&mut HeaderMap> { - Rc::get_mut(&mut self.0.headers) + Rc::get_mut(&mut self.0.default_headers) } } diff --git a/awc/src/middleware/redirect.rs b/awc/src/middleware/redirect.rs index 0fde48074..704d2d79d 100644 --- a/awc/src/middleware/redirect.rs +++ b/awc/src/middleware/redirect.rs @@ -442,13 +442,15 @@ mod tests { }); let client = ClientBuilder::new() - .header("custom", "value") + .add_default_header(("custom", "value")) .disable_redirects() .finish(); let res = client.get(srv.url("/")).send().await.unwrap(); assert_eq!(res.status().as_u16(), 302); - let client = ClientBuilder::new().header("custom", "value").finish(); + let client = ClientBuilder::new() + .add_default_header(("custom", "value")) + .finish(); let res = client.get(srv.url("/")).send().await.unwrap(); assert_eq!(res.status().as_u16(), 200); @@ -520,7 +522,7 @@ mod tests { // send a request to different origins, http://srv1/ then http://srv2/. So it should remove the header let client = ClientBuilder::new() - .header(header::AUTHORIZATION, "auth_key_value") + .add_default_header((header::AUTHORIZATION, "auth_key_value")) .finish(); let res = client.get(srv1.url("/")).send().await.unwrap(); assert_eq!(res.status().as_u16(), 200); diff --git a/awc/src/request.rs b/awc/src/request.rs index 3e1f83a82..9e37b2755 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,11 +147,8 @@ 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, - { - match header.try_into_header_pair() { + pub fn insert_header(mut self, header: impl TryIntoHeaderPair) -> Self { + match header.try_into_pair() { Ok((key, value)) => { self.head.headers.insert(key, value); } @@ -162,11 +159,8 @@ 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, - { - match header.try_into_header_pair() { + pub fn insert_header_if_none(mut self, header: impl TryIntoHeaderPair) -> Self { + match header.try_into_pair() { Ok((key, value)) => { if !self.head.headers.contains_key(&key) { self.head.headers.insert(key, value); @@ -192,11 +186,8 @@ impl ClientRequest { /// .insert_header((CONTENT_TYPE, mime::APPLICATION_JSON)); /// # } /// ``` - pub fn append_header(mut self, header: H) -> Self - where - H: IntoHeaderPair, - { - match header.try_into_header_pair() { + pub fn append_header(mut self, header: impl TryIntoHeaderPair) -> Self { + match header.try_into_pair() { Ok((key, value)) => self.head.headers.append(key, value), Err(e) => self.err = Some(e.into()), }; @@ -588,7 +579,7 @@ mod tests { #[actix_rt::test] async fn test_client_header() { let req = Client::builder() - .header(header::CONTENT_TYPE, "111") + .add_default_header((header::CONTENT_TYPE, "111")) .finish() .get("/"); @@ -606,7 +597,7 @@ mod tests { #[actix_rt::test] async fn test_client_header_override() { let req = Client::builder() - .header(header::CONTENT_TYPE, "111") + .add_default_header((header::CONTENT_TYPE, "111")) .finish() .get("/") .insert_header((header::CONTENT_TYPE, "222")); diff --git a/awc/src/sender.rs b/awc/src/sender.rs index 1faf6140a..f83a70a9b 100644 --- a/awc/src/sender.rs +++ b/awc/src/sender.rs @@ -10,7 +10,7 @@ use std::{ use actix_http::{ body::BodyStream, error::HttpError, - header::{self, HeaderMap, HeaderName, IntoHeaderValue}, + header::{self, HeaderMap, HeaderName, TryIntoHeaderValue}, RequestHead, RequestHeadType, }; use actix_rt::time::{sleep, Sleep}; @@ -298,7 +298,7 @@ impl RequestSender { fn set_header_if_none(&mut self, key: HeaderName, value: V) -> Result<(), HttpError> where - V: IntoHeaderValue, + V: TryIntoHeaderValue, { match self { RequestSender::Owned(head) => { diff --git a/awc/src/test.rs b/awc/src/test.rs index 4a5c8e7ea..1b41efc93 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,11 +39,8 @@ impl TestResponse { } /// Insert a header - pub fn insert_header(mut self, header: H) -> Self - where - H: IntoHeaderPair, - { - if let Ok((key, value)) = header.try_into_header_pair() { + pub fn insert_header(mut self, header: impl TryIntoHeaderPair) -> Self { + if let Ok((key, value)) = header.try_into_pair() { self.head.headers.insert(key, value); return self; } @@ -54,11 +48,8 @@ impl TestResponse { } /// Append a header - pub fn append_header(mut self, header: H) -> Self - where - H: IntoHeaderPair, - { - if let Ok((key, value)) = header.try_into_header_pair() { + pub fn append_header(mut self, header: impl TryIntoHeaderPair) -> Self { + if let Ok((key, value)) = header.try_into_pair() { self.head.headers.append(key, value); return self; } diff --git a/awc/src/ws.rs b/awc/src/ws.rs index f0d421dbc..06d54aadb 100644 --- a/awc/src/ws.rs +++ b/awc/src/ws.rs @@ -39,7 +39,7 @@ use crate::{ connect::{BoxedSocket, ConnectRequest}, error::{HttpError, InvalidUrl, SendRequestError, WsClientError}, http::{ - header::{self, HeaderName, HeaderValue, IntoHeaderValue, AUTHORIZATION}, + header::{self, HeaderName, HeaderValue, TryIntoHeaderValue, AUTHORIZATION}, ConnectionType, Method, StatusCode, Uri, Version, }, response::ClientResponse, @@ -171,7 +171,7 @@ impl WebsocketsRequest { where HeaderName: TryFrom, >::Error: Into, - V: IntoHeaderValue, + V: TryIntoHeaderValue, { match HeaderName::try_from(key) { Ok(key) => match value.try_into_value() { @@ -190,7 +190,7 @@ impl WebsocketsRequest { where HeaderName: TryFrom, >::Error: Into, - V: IntoHeaderValue, + V: TryIntoHeaderValue, { match HeaderName::try_from(key) { Ok(key) => match value.try_into_value() { @@ -209,7 +209,7 @@ impl WebsocketsRequest { where HeaderName: TryFrom, >::Error: Into, - V: IntoHeaderValue, + V: TryIntoHeaderValue, { match HeaderName::try_from(key) { Ok(key) => { @@ -445,7 +445,7 @@ mod tests { #[actix_rt::test] async fn test_header_override() { let req = Client::builder() - .header(header::CONTENT_TYPE, "111") + .add_default_header((header::CONTENT_TYPE, "111")) .finish() .ws("/") .set_header(header::CONTENT_TYPE, "222"); diff --git a/examples/basic.rs b/examples/basic.rs index d29546129..598d13a40 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().add(("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().add(("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..cf0ffebde 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().add(("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().add(("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..feb35d7ae 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")), + .add((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")), + .add((header::CONTENT_TYPE, HeaderValue::from_static("0001"))), ), ) .await; diff --git a/src/error/internal.rs b/src/error/internal.rs index b8e169018..37195dc2e 100644 --- a/src/error/internal.rs +++ b/src/error/internal.rs @@ -2,7 +2,7 @@ use std::{cell::RefCell, fmt, io::Write as _}; use actix_http::{ body::BoxBody, - header::{self, IntoHeaderValue as _}, + header::{self, TryIntoHeaderValue as _}, StatusCode, }; use bytes::{BufMut as _, BytesMut}; diff --git a/src/error/response_error.rs b/src/error/response_error.rs index 7260efa1a..e0b4af44c 100644 --- a/src/error/response_error.rs +++ b/src/error/response_error.rs @@ -8,7 +8,7 @@ use std::{ use actix_http::{ body::BoxBody, - header::{self, IntoHeaderValue}, + header::{self, TryIntoHeaderValue}, Response, StatusCode, }; use bytes::BytesMut; diff --git a/src/http/header/content_disposition.rs b/src/http/header/content_disposition.rs index 945a58f7f..26a9d8e76 100644 --- a/src/http/header/content_disposition.rs +++ b/src/http/header/content_disposition.rs @@ -14,7 +14,7 @@ use once_cell::sync::Lazy; use regex::Regex; use std::fmt::{self, Write}; -use super::{ExtendedValue, Header, IntoHeaderValue, Writer}; +use super::{ExtendedValue, Header, TryIntoHeaderValue, Writer}; use crate::http::header; /// Split at the index of the first `needle` if it exists or at the end. @@ -454,7 +454,7 @@ impl ContentDisposition { } } -impl IntoHeaderValue for ContentDisposition { +impl TryIntoHeaderValue for ContentDisposition { type Error = header::InvalidHeaderValue; fn try_into_value(self) -> Result { diff --git a/src/http/header/content_range.rs b/src/http/header/content_range.rs index 90b3f7fe2..bcbe77e66 100644 --- a/src/http/header/content_range.rs +++ b/src/http/header/content_range.rs @@ -3,7 +3,7 @@ use std::{ str::FromStr, }; -use super::{HeaderValue, IntoHeaderValue, InvalidHeaderValue, Writer, CONTENT_RANGE}; +use super::{HeaderValue, InvalidHeaderValue, TryIntoHeaderValue, Writer, CONTENT_RANGE}; use crate::error::ParseError; crate::http::header::common_header! { @@ -196,7 +196,7 @@ impl Display for ContentRangeSpec { } } -impl IntoHeaderValue for ContentRangeSpec { +impl TryIntoHeaderValue for ContentRangeSpec { type Error = InvalidHeaderValue; fn try_into_value(self) -> Result { diff --git a/src/http/header/entity.rs b/src/http/header/entity.rs index 50b40b7b2..76fe39f23 100644 --- a/src/http/header/entity.rs +++ b/src/http/header/entity.rs @@ -3,7 +3,7 @@ use std::{ str::FromStr, }; -use super::{HeaderValue, IntoHeaderValue, InvalidHeaderValue, Writer}; +use super::{HeaderValue, InvalidHeaderValue, TryIntoHeaderValue, Writer}; /// check that each char in the slice is either: /// 1. `%x21`, or @@ -159,7 +159,7 @@ impl FromStr for EntityTag { } } -impl IntoHeaderValue for EntityTag { +impl TryIntoHeaderValue for EntityTag { type Error = InvalidHeaderValue; fn try_into_value(self) -> Result { diff --git a/src/http/header/if_range.rs b/src/http/header/if_range.rs index 5af9255f6..b845fb3bf 100644 --- a/src/http/header/if_range.rs +++ b/src/http/header/if_range.rs @@ -1,8 +1,8 @@ use std::fmt::{self, Display, Write}; use super::{ - from_one_raw_str, EntityTag, Header, HeaderName, HeaderValue, HttpDate, IntoHeaderValue, - InvalidHeaderValue, Writer, + from_one_raw_str, EntityTag, Header, HeaderName, HeaderValue, HttpDate, InvalidHeaderValue, + TryIntoHeaderValue, Writer, }; use crate::error::ParseError; use crate::http::header; @@ -96,7 +96,7 @@ impl Display for IfRange { } } -impl IntoHeaderValue for IfRange { +impl TryIntoHeaderValue for IfRange { type Error = InvalidHeaderValue; fn try_into_value(self) -> Result { diff --git a/src/http/header/macros.rs b/src/http/header/macros.rs index ca3792a37..25f40a52b 100644 --- a/src/http/header/macros.rs +++ b/src/http/header/macros.rs @@ -125,7 +125,7 @@ macro_rules! common_header { } } - impl $crate::http::header::IntoHeaderValue for $id { + impl $crate::http::header::TryIntoHeaderValue for $id { type Error = $crate::http::header::InvalidHeaderValue; #[inline] @@ -172,7 +172,7 @@ macro_rules! common_header { } } - impl $crate::http::header::IntoHeaderValue for $id { + impl $crate::http::header::TryIntoHeaderValue for $id { type Error = $crate::http::header::InvalidHeaderValue; #[inline] @@ -211,7 +211,7 @@ macro_rules! common_header { } } - impl $crate::http::header::IntoHeaderValue for $id { + impl $crate::http::header::TryIntoHeaderValue for $id { type Error = $crate::http::header::InvalidHeaderValue; #[inline] @@ -266,7 +266,7 @@ macro_rules! common_header { } } - impl $crate::http::header::IntoHeaderValue for $id { + impl $crate::http::header::TryIntoHeaderValue for $id { type Error = $crate::http::header::InvalidHeaderValue; #[inline] diff --git a/src/http/header/range.rs b/src/http/header/range.rs index c1d60f1ee..68028f53a 100644 --- a/src/http/header/range.rs +++ b/src/http/header/range.rs @@ -6,7 +6,7 @@ use std::{ use actix_http::{error::ParseError, header, HttpMessage}; -use super::{Header, HeaderName, HeaderValue, IntoHeaderValue, InvalidHeaderValue, Writer}; +use super::{Header, HeaderName, HeaderValue, InvalidHeaderValue, TryIntoHeaderValue, Writer}; /// `Range` header, defined /// in [RFC 7233 ยง3.1](https://datatracker.ietf.org/doc/html/rfc7233#section-3.1) @@ -274,7 +274,7 @@ impl Header for Range { } } -impl IntoHeaderValue for Range { +impl TryIntoHeaderValue for Range { type Error = InvalidHeaderValue; fn try_into_value(self) -> Result { 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..257467710 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,81 @@ 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().add(("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. + #[allow(clippy::should_implement_trait)] + pub fn add(mut self, header: impl TryIntoHeaderPair) -> Self { + // standard header terminology `insert` or `append` for this method would make the behavior + // of this middleware less obvious since it only adds the headers if they are not present + + match header.try_into_pair() { + Ok((key, value)) => Rc::get_mut(&mut self.inner) + .expect("All default headers must be added before cloning.") + .headers + .append(key, value), + Err(err) => panic!("Invalid header: {}", err.into()), + } + + self + } + + #[doc(hidden)] + #[deprecated( + since = "4.0.0", + note = "Prefer `.add((key, value))`. Will be removed in v5." + )] + 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.add(( + 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"), - ); - - self + pub fn add_content_type(self) -> Self { + self.add(( + CONTENT_TYPE, + HeaderValue::from_static("application/octet-stream"), + )) } } @@ -119,7 +121,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 +199,22 @@ mod tests { }; #[actix_rt::test] - async fn test_default_headers() { + async fn adding_default_headers() { let mw = DefaultHeaders::new() - .header(CONTENT_TYPE, "0001") + .add(("X-TEST", "0001")) + .add(("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 +224,7 @@ mod tests { )) }; let mw = DefaultHeaders::new() - .header(CONTENT_TYPE, "0001") + .add((CONTENT_TYPE, "0001")) .new_transform(srv.into_service()) .await .unwrap(); @@ -226,7 +233,7 @@ 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() @@ -241,4 +248,16 @@ mod tests { "application/octet-stream" ); } + + #[test] + #[should_panic] + fn invalid_header_name() { + DefaultHeaders::new().add((":", "hello")); + } + + #[test] + #[should_panic] + fn invalid_header_value() { + DefaultHeaders::new().add(("x-test", "\n")); + } } diff --git a/src/middleware/mod.rs b/src/middleware/mod.rs index d19cb64e9..42d285580 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().add(("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().add(("X-Test2", "X-Value2"))) .wrap(Condition::new(true, DefaultHeaders::new())) .wrap(Compat::new(Logger::default())); diff --git a/src/request_data.rs b/src/request_data.rs index 680f3e566..b685fd0d6 100644 --- a/src/request_data.rs +++ b/src/request_data.rs @@ -17,7 +17,7 @@ use crate::{dev::Payload, error::ErrorInternalServerError, Error, FromRequest, H /// # Mutating Request Data /// Note that since extractors must output owned data, only types that `impl Clone` can use this /// extractor. A clone is taken of the required request data and can, therefore, not be directly -/// mutated in-place. To mutate request data, continue to use [`HttpRequest::extensions_mut`] or +/// mutated in-place. To mutate request data, continue to use [`HttpRequest::req_data_mut`] or /// re-insert the cloned data back into the extensions map. A `DerefMut` impl is intentionally not /// provided to make this potential foot-gun more obvious. /// diff --git a/src/resource.rs b/src/resource.rs index 420374a86..53104930a 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")), + .add((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..b500ab331 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, TryIntoHeaderPair, TryIntoHeaderValue}, ConnectionType, Extensions, Response, ResponseHead, StatusCode, }; use bytes::Bytes; @@ -67,12 +67,9 @@ 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() { + match header.try_into_pair() { Ok((key, value)) => { parts.headers.insert(key, value); } @@ -94,12 +91,9 @@ 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() { + match header.try_into_pair() { Ok((key, value)) => parts.headers.append(key, value), Err(e) => self.err = Some(e.into()), }; @@ -118,7 +112,7 @@ impl HttpResponseBuilder { where K: TryInto, K::Error: Into, - V: IntoHeaderValue, + V: TryIntoHeaderValue, { if self.err.is_some() { return self; @@ -143,7 +137,7 @@ impl HttpResponseBuilder { where K: TryInto, K::Error: Into, - V: IntoHeaderValue, + V: TryIntoHeaderValue, { if self.err.is_some() { return self; @@ -180,7 +174,7 @@ impl HttpResponseBuilder { #[inline] pub fn upgrade(&mut self, value: V) -> &mut Self where - V: IntoHeaderValue, + V: TryIntoHeaderValue, { if let Some(parts) = self.inner() { parts.set_connection_type(ConnectionType::Upgrade); @@ -218,7 +212,7 @@ impl HttpResponseBuilder { #[inline] pub fn content_type(&mut self, value: V) -> &mut Self where - V: IntoHeaderValue, + V: TryIntoHeaderValue, { if let Some(parts) = self.inner() { match value.try_into_value() { diff --git a/src/response/customize_responder.rs b/src/response/customize_responder.rs new file mode 100644 index 000000000..11f6b2916 --- /dev/null +++ b/src/response/customize_responder.rs @@ -0,0 +1,245 @@ +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`]. +/// +/// Created by the [`Responder::customize`] method. +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. + /// + /// # Examples + /// ``` + /// use actix_web::{Responder, http::StatusCode, test::TestRequest}; + /// + /// let responder = "Welcome!".customize().with_status(StatusCode::ACCEPTED); + /// + /// let request = TestRequest::default().to_http_request(); + /// let response = responder.respond_to(&request); + /// assert_eq!(response.status(), StatusCode::ACCEPTED); + /// ``` + pub fn with_status(mut self, status: StatusCode) -> Self { + if let Some(inner) = self.inner() { + inner.status = Some(status); + } + + self + } + + /// Insert (override) header in the final response. + /// + /// Overrides other headers with the same name. + /// See [`HeaderMap::insert`](crate::http::header::HeaderMap::insert). + /// + /// Headers added with this method will be inserted before those added + /// with [`append_header`](Self::append_header). As such, header(s) can be overridden with more + /// than one new header by first calling `insert_header` followed by `append_header`. + /// + /// # Examples + /// ``` + /// use actix_web::{Responder, test::TestRequest}; + /// + /// let responder = "Hello world!" + /// .customize() + /// .insert_header(("x-version", "1.2.3")); + /// + /// let request = TestRequest::default().to_http_request(); + /// let response = responder.respond_to(&request); + /// assert_eq!(response.headers().get("x-version").unwrap(), "1.2.3"); + /// ``` + pub fn insert_header(mut self, header: impl TryIntoHeaderPair) -> Self { + if let Some(inner) = self.inner() { + match header.try_into_pair() { + Ok((key, value)) => { + inner.override_headers.insert(key, value); + } + Err(err) => self.error = Some(err.into()), + }; + } + + self + } + + /// Append header to the final response. + /// + /// Unlike [`insert_header`](Self::insert_header), this will not override existing headers. + /// See [`HeaderMap::append`](crate::http::header::HeaderMap::append). + /// + /// Headers added here are appended _after_ additions/overrides from `insert_header`. + /// + /// # Examples + /// ``` + /// use actix_web::{Responder, test::TestRequest}; + /// + /// let responder = "Hello world!" + /// .customize() + /// .append_header(("x-version", "1.2.3")); + /// + /// let request = TestRequest::default().to_http_request(); + /// let response = responder.respond_to(&request); + /// assert_eq!(response.headers().get("x-version").unwrap(), "1.2.3"); + /// ``` + pub fn append_header(mut self, header: impl TryIntoHeaderPair) -> Self { + if let Some(inner) = self.inner() { + match header.try_into_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..319b824f1 100644 --- a/src/responder.rs +++ b/src/response/responder.rs @@ -2,64 +2,58 @@ 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 to allow alteration of its response. /// - /// ``` - /// use actix_web::{http::StatusCode, HttpRequest, Responder}; + /// See [`CustomizeResponder`] docs for its capabilities. /// - /// fn index(req: HttpRequest) -> impl Responder { - /// "Welcome!".with_status(StatusCode::OK) - /// } + /// # Examples /// ``` - fn with_status(self, status: StatusCode) -> CustomResponder + /// use actix_web::{Responder, http::StatusCode, test::TestRequest}; + /// + /// let responder = "Hello world!" + /// .customize() + /// .with_status(StatusCode::BAD_REQUEST) + /// .insert_header(("x-hello", "world")); + /// + /// let request = TestRequest::default().to_http_request(); + /// let response = responder.respond_to(&request); + /// assert_eq!(response.status(), StatusCode::BAD_REQUEST); + /// assert_eq!(response.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(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 +175,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 +342,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..c35584770 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")), + .add((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;