From 2ffc21dd4f8925d22206d23b7c21d62b83a68ee4 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Wed, 19 Jan 2022 02:09:25 +0000 Subject: [PATCH] move response extensions out of head (#2585) --- CHANGES.md | 4 ++ actix-http/CHANGES.md | 6 +++ actix-http/examples/echo.rs | 12 +++--- actix-http/examples/echo2.rs | 28 +++++++------- actix-http/src/error.rs | 23 +++--------- actix-http/src/http_message.rs | 4 +- actix-http/src/message.rs | 8 ++-- actix-http/src/requests/head.rs | 2 +- actix-http/src/requests/request.rs | 23 ++++++------ actix-http/src/responses/builder.rs | 26 ++++--------- actix-http/src/responses/head.rs | 45 +++++++--------------- actix-http/src/responses/response.rs | 33 +++++++++++----- awc/src/responses/response.rs | 15 ++++++-- scripts/bump | 2 +- src/app_service.rs | 32 ++++++++-------- src/guard.rs | 10 ++--- src/request.rs | 38 +++++++++---------- src/request_data.rs | 13 ++++--- src/response/builder.rs | 28 ++++++-------- src/response/response.rs | 23 +++++++----- src/service.rs | 56 +++++++++++----------------- 21 files changed, 205 insertions(+), 226 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index fa4feab6e..31b17cba8 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,10 @@ # Changes ## Unreleased - 2021-xx-xx +### Removed +- `HttpRequest::req_data[_mut]()`; request-local data is still available through `.extensions()`. [#2585] + +[#2585]: https://github.com/actix/actix-web/pull/2585 ## 4.0.0-beta.20 - 2022-01-14 diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index d03a45969..7fd635e3d 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -3,11 +3,17 @@ ## Unreleased - 2021-xx-xx ### Added - Response headers can be sent as camel case using `res.head_mut().set_camel_case_headers(true)`. [#2587] +- `ResponseHead` now implements `Clone`. [#2585] ### Changed - Brotli (de)compression support is now provided by the `brotli` crate. [#2538] +### Removed +- `ResponseHead::extensions[_mut]()`. [#2585] +- `ResponseBuilder::extensions[_mut]()`. [#2585] + [#2538]: https://github.com/actix/actix-web/pull/2538 +[#2585]: https://github.com/actix/actix-web/pull/2585 [#2587]: https://github.com/actix/actix-web/pull/2587 diff --git a/actix-http/examples/echo.rs b/actix-http/examples/echo.rs index 22f553f38..f9188ed9f 100644 --- a/actix-http/examples/echo.rs +++ b/actix-http/examples/echo.rs @@ -15,6 +15,7 @@ async fn main() -> io::Result<()> { HttpService::build() .client_timeout(1000) .client_disconnect(1000) + // handles HTTP/1.1 and HTTP/2 .finish(|mut req: Request| async move { let mut body = BytesMut::new(); while let Some(item) = req.payload().next().await { @@ -23,12 +24,13 @@ async fn main() -> io::Result<()> { log::info!("request body: {:?}", body); - Ok::<_, Error>( - Response::build(StatusCode::OK) - .insert_header(("x-head", HeaderValue::from_static("dummy value!"))) - .body(body), - ) + let res = Response::build(StatusCode::OK) + .insert_header(("x-head", HeaderValue::from_static("dummy value!"))) + .body(body); + + Ok::<_, Error>(res) }) + // No TLS .tcp() })? .run() diff --git a/actix-http/examples/echo2.rs b/actix-http/examples/echo2.rs index e3b915e05..605572d8b 100644 --- a/actix-http/examples/echo2.rs +++ b/actix-http/examples/echo2.rs @@ -1,32 +1,34 @@ use std::io; use actix_http::{ - body::MessageBody, header::HeaderValue, Error, HttpService, Request, Response, StatusCode, + body::{BodyStream, MessageBody}, + header, Error, HttpMessage, HttpService, Request, Response, StatusCode, }; -use actix_server::Server; -use bytes::BytesMut; -use futures_util::StreamExt as _; async fn handle_request(mut req: Request) -> Result, Error> { - let mut body = BytesMut::new(); - while let Some(item) = req.payload().next().await { - body.extend_from_slice(&item?) + let mut res = Response::build(StatusCode::OK); + + if let Some(ct) = req.headers().get(header::CONTENT_TYPE) { + res.insert_header((header::CONTENT_TYPE, ct)); } - log::info!("request body: {:?}", body); + // echo request payload stream as (chunked) response body + let res = res.message_body(BodyStream::new(req.payload().take()))?; - Ok(Response::build(StatusCode::OK) - .insert_header(("x-head", HeaderValue::from_static("dummy value!"))) - .body(body)) + Ok(res) } #[actix_rt::main] async fn main() -> io::Result<()> { env_logger::init_from_env(env_logger::Env::new().default_filter_or("info")); - Server::build() + actix_server::Server::build() .bind("echo", ("127.0.0.1", 8080), || { - HttpService::build().finish(handle_request).tcp() + HttpService::build() + // handles HTTP/1.1 only + .h1(handle_request) + // No TLS + .tcp() })? .run() .await diff --git a/actix-http/src/error.rs b/actix-http/src/error.rs index cdf495c45..df6d3813a 100644 --- a/actix-http/src/error.rs +++ b/actix-http/src/error.rs @@ -387,6 +387,7 @@ impl StdError for DispatchError { /// A set of error that can occur during parsing content type. #[derive(Debug, Display, Error)] +#[cfg_attr(test, derive(PartialEq))] #[non_exhaustive] pub enum ContentTypeError { /// Can not parse content type @@ -398,28 +399,14 @@ pub enum ContentTypeError { UnknownEncoding, } -#[cfg(test)] -mod content_type_test_impls { - use super::*; - - impl std::cmp::PartialEq for ContentTypeError { - fn eq(&self, other: &Self) -> bool { - match self { - Self::ParseError => matches!(other, ContentTypeError::ParseError), - Self::UnknownEncoding => { - matches!(other, ContentTypeError::UnknownEncoding) - } - } - } - } -} - #[cfg(test)] mod tests { - use super::*; - use http::{Error as HttpError, StatusCode}; use std::io; + use http::{Error as HttpError, StatusCode}; + + use super::*; + #[test] fn test_into_response() { let resp: Response = ParseError::Incomplete.into(); diff --git a/actix-http/src/http_message.rs b/actix-http/src/http_message.rs index ccaa320fa..068e23b96 100644 --- a/actix-http/src/http_message.rs +++ b/actix-http/src/http_message.rs @@ -25,10 +25,10 @@ pub trait HttpMessage: Sized { /// Message payload stream fn take_payload(&mut self) -> Payload; - /// Request's extensions container + /// Returns a reference to the request-local data/extensions container. fn extensions(&self) -> Ref<'_, Extensions>; - /// Mutable reference to a the request's extensions container + /// Returns a mutable reference to the request-local data/extensions container. fn extensions_mut(&self) -> RefMut<'_, Extensions>; /// Get a header. diff --git a/actix-http/src/message.rs b/actix-http/src/message.rs index ecd08fbb3..5616a4762 100644 --- a/actix-http/src/message.rs +++ b/actix-http/src/message.rs @@ -5,13 +5,13 @@ use bitflags::bitflags; /// Represents various types of connection #[derive(Copy, Clone, PartialEq, Debug)] pub enum ConnectionType { - /// Close connection after response + /// Close connection after response. Close, - /// Keep connection alive after response + /// Keep connection alive after response. KeepAlive, - /// Connection is upgraded to different type + /// Connection is upgraded to different type. Upgrade, } @@ -69,8 +69,8 @@ impl Drop for Message { } } +/// Generic `Head` object pool. #[doc(hidden)] -/// Request's objects pool pub struct MessagePool(RefCell>>); impl MessagePool { diff --git a/actix-http/src/requests/head.rs b/actix-http/src/requests/head.rs index 524075b61..06fd0429e 100644 --- a/actix-http/src/requests/head.rs +++ b/actix-http/src/requests/head.rs @@ -142,8 +142,8 @@ impl RequestHead { } } -#[derive(Debug)] #[allow(clippy::large_enum_variant)] +#[derive(Debug)] pub enum RequestHeadType { Owned(RequestHead), Rc(Rc, Option), diff --git a/actix-http/src/requests/request.rs b/actix-http/src/requests/request.rs index 4eaaba8e1..0f8e78d46 100644 --- a/actix-http/src/requests/request.rs +++ b/actix-http/src/requests/request.rs @@ -19,7 +19,7 @@ pub struct Request

{ pub(crate) payload: Payload

, pub(crate) head: Message, pub(crate) conn_data: Option>, - pub(crate) req_data: RefCell, + pub(crate) extensions: RefCell, } impl

HttpMessage for Request

{ @@ -34,16 +34,14 @@ impl

HttpMessage for Request

{ mem::replace(&mut self.payload, Payload::None) } - /// Request extensions #[inline] fn extensions(&self) -> Ref<'_, Extensions> { - self.req_data.borrow() + self.extensions.borrow() } - /// Mutable reference to a the request's extensions #[inline] fn extensions_mut(&self) -> RefMut<'_, Extensions> { - self.req_data.borrow_mut() + self.extensions.borrow_mut() } } @@ -52,7 +50,7 @@ impl From> for Request { Request { head, payload: Payload::None, - req_data: RefCell::new(Extensions::default()), + extensions: RefCell::new(Extensions::default()), conn_data: None, } } @@ -65,7 +63,7 @@ impl Request { Request { head: Message::new(), payload: Payload::None, - req_data: RefCell::new(Extensions::default()), + extensions: RefCell::new(Extensions::default()), conn_data: None, } } @@ -77,7 +75,7 @@ impl

Request

{ Request { payload, head: Message::new(), - req_data: RefCell::new(Extensions::default()), + extensions: RefCell::new(Extensions::default()), conn_data: None, } } @@ -90,7 +88,7 @@ impl

Request

{ Request { payload, head: self.head, - req_data: self.req_data, + extensions: self.extensions, conn_data: self.conn_data, }, pl, @@ -195,16 +193,17 @@ impl

Request

{ .and_then(|container| container.get::()) } - /// Returns the connection data container if an [on-connect] callback was registered. + /// Returns the connection-level data/extensions container if an [on-connect] callback was + /// registered, leaving an empty one in its place. /// /// [on-connect]: crate::HttpServiceBuilder::on_connect_ext pub fn take_conn_data(&mut self) -> Option> { self.conn_data.take() } - /// Returns the request data container, leaving an empty one in it's place. + /// Returns the request-local data/extensions container, leaving an empty one in its place. pub fn take_req_data(&mut self) -> Extensions { - mem::take(self.req_data.get_mut()) + mem::take(self.extensions.get_mut()) } } diff --git a/actix-http/src/responses/builder.rs b/actix-http/src/responses/builder.rs index 5854863de..4a67423b1 100644 --- a/actix-http/src/responses/builder.rs +++ b/actix-http/src/responses/builder.rs @@ -1,9 +1,6 @@ //! HTTP response builder. -use std::{ - cell::{Ref, RefMut}, - fmt, str, -}; +use std::{cell::RefCell, fmt, str}; use crate::{ body::{EitherBody, MessageBody}, @@ -202,20 +199,6 @@ impl ResponseBuilder { self } - /// Responses extensions - #[inline] - pub fn extensions(&self) -> Ref<'_, Extensions> { - let head = self.head.as_ref().expect("cannot reuse response builder"); - head.extensions.borrow() - } - - /// Mutable reference to a the response's extensions - #[inline] - pub fn extensions_mut(&mut self) -> RefMut<'_, Extensions> { - let head = self.head.as_ref().expect("cannot reuse response builder"); - head.extensions.borrow_mut() - } - /// Generate response with a wrapped body. /// /// This `ResponseBuilder` will be left in a useless state. @@ -238,7 +221,12 @@ impl ResponseBuilder { } let head = self.head.take().expect("cannot reuse response builder"); - Ok(Response { head, body }) + + Ok(Response { + head, + body, + extensions: RefCell::new(Extensions::new()), + }) } /// Generate response with an empty body. diff --git a/actix-http/src/responses/head.rs b/actix-http/src/responses/head.rs index d11ba8fde..91e96a928 100644 --- a/actix-http/src/responses/head.rs +++ b/actix-http/src/responses/head.rs @@ -1,25 +1,19 @@ //! Response head type and caching pool. -use std::{ - cell::{Ref, RefCell, RefMut}, - ops, -}; +use std::{cell::RefCell, ops}; -use crate::{ - header::HeaderMap, message::Flags, ConnectionType, Extensions, StatusCode, Version, -}; +use crate::{header::HeaderMap, message::Flags, ConnectionType, StatusCode, Version}; thread_local! { static RESPONSE_POOL: BoxedResponsePool = BoxedResponsePool::create(); } -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct ResponseHead { pub version: Version, pub status: StatusCode, pub headers: HeaderMap, pub reason: Option<&'static str>, - pub(crate) extensions: RefCell, pub(crate) flags: Flags, } @@ -33,18 +27,17 @@ impl ResponseHead { headers: HeaderMap::with_capacity(12), reason: None, flags: Flags::empty(), - extensions: RefCell::new(Extensions::new()), } } - #[inline] /// Read the message headers. + #[inline] pub fn headers(&self) -> &HeaderMap { &self.headers } - #[inline] /// Mutable reference to the message headers. + #[inline] pub fn headers_mut(&mut self) -> &mut HeaderMap { &mut self.headers } @@ -61,20 +54,8 @@ impl ResponseHead { } } - /// Message extensions - #[inline] - pub fn extensions(&self) -> Ref<'_, Extensions> { - self.extensions.borrow() - } - - /// Mutable reference to a the message's extensions - #[inline] - pub fn extensions_mut(&self) -> RefMut<'_, Extensions> { - self.extensions.borrow_mut() - } - - #[inline] /// Set connection type of the message + #[inline] pub fn set_connection_type(&mut self, ctype: ConnectionType) { match ctype { ConnectionType::Close => self.flags.insert(Flags::CLOSE), @@ -133,14 +114,14 @@ impl ResponseHead { } } - #[inline] /// Get response body chunking state + #[inline] pub fn chunked(&self) -> bool { !self.flags.contains(Flags::NO_CHUNKING) } - #[inline] /// Set no chunking for payload + #[inline] pub fn no_chunking(&mut self, val: bool) { if val { self.flags.insert(Flags::NO_CHUNKING); @@ -183,7 +164,7 @@ impl Drop for BoxedResponseHead { } } -/// Request's objects pool +/// Response head object pool. #[doc(hidden)] pub struct BoxedResponsePool(#[allow(clippy::vec_box)] RefCell>>); @@ -192,7 +173,7 @@ impl BoxedResponsePool { BoxedResponsePool(RefCell::new(Vec::with_capacity(128))) } - /// Get message from the pool + /// Get message from the pool. #[inline] fn get_message(&self, status: StatusCode) -> BoxedResponseHead { if let Some(mut head) = self.0.borrow_mut().pop() { @@ -208,12 +189,12 @@ impl BoxedResponsePool { } } - /// Release request instance + /// Release request instance. #[inline] - fn release(&self, mut msg: Box) { + fn release(&self, msg: Box) { let pool = &mut self.0.borrow_mut(); + if pool.len() < 128 { - msg.extensions.get_mut().clear(); pool.push(msg); } } diff --git a/actix-http/src/responses/response.rs b/actix-http/src/responses/response.rs index ec9157afb..6efc3c5f1 100644 --- a/actix-http/src/responses/response.rs +++ b/actix-http/src/responses/response.rs @@ -1,7 +1,7 @@ //! HTTP response. use std::{ - cell::{Ref, RefMut}, + cell::{Ref, RefCell, RefMut}, fmt, str, }; @@ -9,7 +9,7 @@ use bytes::{Bytes, BytesMut}; use bytestring::ByteString; use crate::{ - body::{BoxBody, MessageBody}, + body::{BoxBody, EitherBody, MessageBody}, header::{self, HeaderMap, TryIntoHeaderValue}, responses::BoxedResponseHead, Error, Extensions, ResponseBuilder, ResponseHead, StatusCode, @@ -19,6 +19,7 @@ use crate::{ pub struct Response { pub(crate) head: BoxedResponseHead, pub(crate) body: B, + pub(crate) extensions: RefCell, } impl Response { @@ -28,6 +29,7 @@ impl Response { Response { head: BoxedResponseHead::new(status), body: BoxBody::new(()), + extensions: RefCell::new(Extensions::new()), } } @@ -74,6 +76,7 @@ impl Response { Response { head: BoxedResponseHead::new(status), body, + extensions: RefCell::new(Extensions::new()), } } @@ -120,20 +123,21 @@ impl Response { } /// Returns true if keep-alive is enabled. + #[inline] pub fn keep_alive(&self) -> bool { self.head.keep_alive() } - /// Returns a reference to the extensions of this response. + /// Returns a reference to the request-local data/extensions container. #[inline] pub fn extensions(&self) -> Ref<'_, Extensions> { - self.head.extensions.borrow() + self.extensions.borrow() } - /// Returns a mutable reference to the extensions of this response. + /// Returns a mutable reference to the request-local data/extensions container. #[inline] pub fn extensions_mut(&mut self) -> RefMut<'_, Extensions> { - self.head.extensions.borrow_mut() + self.extensions.borrow_mut() } /// Returns a reference to the body of this response. @@ -143,24 +147,29 @@ impl Response { } /// Sets new body. + #[inline] pub fn set_body(self, body: B2) -> Response { Response { head: self.head, body, + extensions: self.extensions, } } /// Drops body and returns new response. + #[inline] pub fn drop_body(self) -> Response<()> { self.set_body(()) } /// Sets new body, returning new response and previous body value. + #[inline] pub(crate) fn replace_body(self, body: B2) -> (Response, B) { ( Response { head: self.head, body, + extensions: self.extensions, }, self.body, ) @@ -171,11 +180,15 @@ impl Response { /// # Implementation Notes /// Due to internal performance optimizations, the first element of the returned tuple is a /// `Response` as well but only contains the head of the response this was called on. + #[inline] pub fn into_parts(self) -> (Response<()>, B) { self.replace_body(()) } - /// Returns new response with mapped body. + /// Map the current body type to another using a closure. Returns a new response. + /// + /// Closure receives the response head and the current body type. + #[inline] pub fn map_body(mut self, f: F) -> Response where F: FnOnce(&mut ResponseHead, B) -> B2, @@ -185,6 +198,7 @@ impl Response { Response { head: self.head, body, + extensions: self.extensions, } } @@ -197,6 +211,7 @@ impl Response { } /// Returns body, consuming this response. + #[inline] pub fn into_body(self) -> B { self.body } @@ -239,9 +254,9 @@ impl>, E: Into> From> for Response } } -impl From for Response { +impl From for Response> { fn from(mut builder: ResponseBuilder) -> Self { - builder.finish().map_into_boxed_body() + builder.finish() } } diff --git a/awc/src/responses/response.rs b/awc/src/responses/response.rs index 0197265f1..02ffdbab2 100644 --- a/awc/src/responses/response.rs +++ b/awc/src/responses/response.rs @@ -1,5 +1,5 @@ use std::{ - cell::{Ref, RefMut}, + cell::{Ref, RefCell, RefMut}, fmt, mem, pin::Pin, task::{Context, Poll}, @@ -28,6 +28,8 @@ pin_project! { #[pin] pub(crate) payload: Payload, pub(crate) timeout: ResponseTimeout, + pub(crate) extensions: RefCell, + } } @@ -38,6 +40,7 @@ impl ClientResponse { head, payload, timeout: ResponseTimeout::default(), + extensions: RefCell::new(Extensions::new()), } } @@ -64,7 +67,9 @@ impl ClientResponse { &self.head().headers } - /// Set a body and return previous body value + /// Map the current body type to another using a closure. Returns a new response. + /// + /// Closure receives the response head and the current body type. pub fn map_body(mut self, f: F) -> ClientResponse where F: FnOnce(&mut ResponseHead, Payload) -> Payload, @@ -75,6 +80,7 @@ impl ClientResponse { payload, head: self.head, timeout: self.timeout, + extensions: self.extensions, } } @@ -101,6 +107,7 @@ impl ClientResponse { payload: self.payload, head: self.head, timeout, + extensions: self.extensions, } } @@ -224,11 +231,11 @@ impl HttpMessage for ClientResponse { } fn extensions(&self) -> Ref<'_, Extensions> { - self.head.extensions() + self.extensions.borrow() } fn extensions_mut(&self) -> RefMut<'_, Extensions> { - self.head.extensions_mut() + self.extensions.borrow_mut() } } diff --git a/scripts/bump b/scripts/bump index c43b92dc8..209e2281d 100755 --- a/scripts/bump +++ b/scripts/bump @@ -31,7 +31,7 @@ fi # get current version PACKAGE_NAME="$(sed -nE 's/^name ?= ?"([^"]+)"$/\1/ p' "$CARGO_MANIFEST" | head -n 1)" -CURRENT_VERSION="$(sed -nE 's/^version ?= ?"([^"]+)"$/\1/ p' "$CARGO_MANIFEST")" +CURRENT_VERSION="$(sed -nE 's/^version ?= ?"([^"]+)"$/\1/ p' "$CARGO_MANIFEST" | head -n 1)" CHANGE_CHUNK_FILE="$(mktemp)" echo saving changelog to $CHANGE_CHUNK_FILE diff --git a/src/app_service.rs b/src/app_service.rs index 56b24f0d8..b7c016e81 100644 --- a/src/app_service.rs +++ b/src/app_service.rs @@ -201,27 +201,29 @@ where actix_service::forward_ready!(service); fn call(&self, mut req: Request) -> Self::Future { - let req_data = Rc::new(RefCell::new(req.take_req_data())); + let extensions = Rc::new(RefCell::new(req.take_req_data())); let conn_data = req.take_conn_data(); let (head, payload) = req.into_parts(); - let req = if let Some(mut req) = self.app_state.pool().pop() { - let inner = Rc::get_mut(&mut req.inner).unwrap(); - inner.path.get_mut().update(&head.uri); - inner.path.reset(); - inner.head = head; - inner.conn_data = conn_data; - inner.req_data = req_data; - req - } else { - HttpRequest::new( + let req = match self.app_state.pool().pop() { + Some(mut req) => { + let inner = Rc::get_mut(&mut req.inner).unwrap(); + inner.path.get_mut().update(&head.uri); + inner.path.reset(); + inner.head = head; + inner.conn_data = conn_data; + inner.extensions = extensions; + req + } + + None => HttpRequest::new( Path::new(Url::new(head.uri.clone())), head, - self.app_state.clone(), - self.app_data.clone(), + Rc::clone(&self.app_state), + Rc::clone(&self.app_data), conn_data, - req_data, - ) + extensions, + ), }; self.service.call(ServiceRequest::new(req, payload)) diff --git a/src/guard.rs b/src/guard.rs index f4200a382..596b9f9fe 100644 --- a/src/guard.rs +++ b/src/guard.rs @@ -54,7 +54,7 @@ use std::{ use actix_http::{header, uri::Uri, Extensions, Method as HttpMethod, RequestHead}; -use crate::{http::header::Header, service::ServiceRequest}; +use crate::{http::header::Header, service::ServiceRequest, HttpMessage as _}; /// Provides access to request parts that are useful during routing. #[derive(Debug)] @@ -69,16 +69,16 @@ impl<'a> GuardContext<'a> { self.req.head() } - /// Returns reference to the request-local data container. + /// Returns reference to the request-local data/extensions container. #[inline] pub fn req_data(&self) -> Ref<'a, Extensions> { - self.req.req_data() + self.req.extensions() } - /// Returns mutable reference to the request-local data container. + /// Returns mutable reference to the request-local data/extensions container. #[inline] pub fn req_data_mut(&self) -> RefMut<'a, Extensions> { - self.req.req_data_mut() + self.req.extensions_mut() } /// Extracts a typed header from the request. diff --git a/src/request.rs b/src/request.rs index e876c3b4d..bcab79205 100644 --- a/src/request.rs +++ b/src/request.rs @@ -5,10 +5,7 @@ use std::{ str, }; -use actix_http::{ - header::HeaderMap, Extensions, HttpMessage, Message, Method, Payload, RequestHead, Uri, - Version, -}; +use actix_http::{Message, RequestHead}; use actix_router::{Path, Url}; use actix_utils::future::{ok, Ready}; #[cfg(feature = "cookies")] @@ -16,8 +13,14 @@ use cookie::{Cookie, ParseError as CookieParseError}; use smallvec::SmallVec; use crate::{ - app_service::AppInitServiceState, config::AppConfig, error::UrlGenerationError, - info::ConnectionInfo, rmap::ResourceMap, Error, FromRequest, + app_service::AppInitServiceState, + config::AppConfig, + dev::{Extensions, Payload}, + error::UrlGenerationError, + http::{header::HeaderMap, Method, Uri, Version}, + info::ConnectionInfo, + rmap::ResourceMap, + Error, FromRequest, HttpMessage, }; #[cfg(feature = "cookies")] @@ -38,7 +41,7 @@ pub(crate) struct HttpRequestInner { pub(crate) path: Path, pub(crate) app_data: SmallVec<[Rc; 4]>, pub(crate) conn_data: Option>, - pub(crate) req_data: Rc>, + pub(crate) extensions: Rc>, app_state: Rc, } @@ -50,7 +53,7 @@ impl HttpRequest { app_state: Rc, app_data: Rc, conn_data: Option>, - req_data: Rc>, + extensions: Rc>, ) -> HttpRequest { let mut data = SmallVec::<[Rc; 4]>::new(); data.push(app_data); @@ -62,7 +65,7 @@ impl HttpRequest { app_state, app_data: data, conn_data, - req_data, + extensions, }), } } @@ -159,14 +162,6 @@ impl HttpRequest { self.resource_map().match_name(self.path()) } - pub fn req_data(&self) -> Ref<'_, Extensions> { - self.inner.req_data.borrow() - } - - pub fn req_data_mut(&self) -> RefMut<'_, Extensions> { - self.inner.req_data.borrow_mut() - } - /// Returns a reference a piece of connection data set in an [on-connect] callback. /// /// ```ignore @@ -356,12 +351,12 @@ impl HttpMessage for HttpRequest { #[inline] fn extensions(&self) -> Ref<'_, Extensions> { - self.req_data() + self.inner.extensions.borrow() } #[inline] fn extensions_mut(&self) -> RefMut<'_, Extensions> { - self.req_data_mut() + self.inner.extensions.borrow_mut() } #[inline] @@ -382,7 +377,10 @@ impl Drop for HttpRequest { // Inner is borrowed mut here and; get req data mutably to reduce borrow check. Also // we know the req_data Rc will not have any cloned at this point to unwrap is okay. - Rc::get_mut(&mut inner.req_data).unwrap().get_mut().clear(); + Rc::get_mut(&mut inner.extensions) + .unwrap() + .get_mut() + .clear(); // a re-borrow of pool is necessary here. let req = Rc::clone(&self.inner); diff --git a/src/request_data.rs b/src/request_data.rs index b685fd0d6..68103a7e9 100644 --- a/src/request_data.rs +++ b/src/request_data.rs @@ -2,7 +2,10 @@ use std::{any::type_name, ops::Deref}; use actix_utils::future::{err, ok, Ready}; -use crate::{dev::Payload, error::ErrorInternalServerError, Error, FromRequest, HttpRequest}; +use crate::{ + dev::Payload, error::ErrorInternalServerError, Error, FromRequest, HttpMessage as _, + HttpRequest, +}; /// Request-local data extractor. /// @@ -17,13 +20,13 @@ 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::req_data_mut`] or +/// mutated in-place. To mutate request data, continue to use [`HttpRequest::extensions_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. /// /// # Example /// ```no_run -/// # use actix_web::{web, HttpResponse, HttpRequest, Responder}; +/// # use actix_web::{web, HttpResponse, HttpRequest, Responder, HttpMessage as _}; /// /// #[derive(Debug, Clone, PartialEq)] /// struct FlagFromMiddleware(String); @@ -35,7 +38,7 @@ use crate::{dev::Payload, error::ErrorInternalServerError, Error, FromRequest, H /// ) -> impl Responder { /// // use an option extractor if middleware is not guaranteed to add this type of req data /// if let Some(flag) = opt_flag { -/// assert_eq!(&flag.into_inner(), req.req_data().get::().unwrap()); +/// assert_eq!(&flag.into_inner(), req.extensions().get::().unwrap()); /// } /// /// HttpResponse::Ok() @@ -67,7 +70,7 @@ impl FromRequest for ReqData { type Future = Ready>; fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future { - if let Some(st) = req.req_data().get::() { + if let Some(st) = req.extensions().get::() { ok(ReqData(st.clone())) } else { log::debug!( diff --git a/src/response/builder.rs b/src/response/builder.rs index bdb0aaa12..c8e44729a 100644 --- a/src/response/builder.rs +++ b/src/response/builder.rs @@ -7,7 +7,6 @@ use std::{ }; use actix_http::{ - body::{BodyStream, BoxBody, MessageBody}, error::HttpError, header::{self, HeaderName, TryIntoHeaderPair, TryIntoHeaderValue}, ConnectionType, Extensions, Response, ResponseHead, StatusCode, @@ -16,12 +15,8 @@ use bytes::Bytes; use futures_core::Stream; use serde::Serialize; -#[cfg(feature = "cookies")] -use actix_http::header::HeaderValue; -#[cfg(feature = "cookies")] -use cookie::{Cookie, CookieJar}; - use crate::{ + body::{BodyStream, BoxBody, MessageBody}, error::{Error, JsonPayloadError}, BoxError, HttpRequest, HttpResponse, Responder, }; @@ -33,7 +28,7 @@ pub struct HttpResponseBuilder { res: Option>, err: Option, #[cfg(feature = "cookies")] - cookies: Option, + cookies: Option, } impl HttpResponseBuilder { @@ -242,9 +237,9 @@ impl HttpResponseBuilder { /// .finish(); /// ``` #[cfg(feature = "cookies")] - pub fn cookie<'c>(&mut self, cookie: Cookie<'c>) -> &mut Self { + pub fn cookie<'c>(&mut self, cookie: cookie::Cookie<'c>) -> &mut Self { if self.cookies.is_none() { - let mut jar = CookieJar::new(); + let mut jar = cookie::CookieJar::new(); jar.add(cookie.into_owned()); self.cookies = Some(jar) } else { @@ -271,9 +266,9 @@ impl HttpResponseBuilder { /// } /// ``` #[cfg(feature = "cookies")] - pub fn del_cookie(&mut self, cookie: &Cookie<'_>) -> &mut Self { + pub fn del_cookie(&mut self, cookie: &cookie::Cookie<'_>) -> &mut Self { if self.cookies.is_none() { - self.cookies = Some(CookieJar::new()) + self.cookies = Some(cookie::CookieJar::new()) } let jar = self.cookies.as_mut().unwrap(); let cookie = cookie.clone().into_owned(); @@ -282,7 +277,7 @@ impl HttpResponseBuilder { self } - /// Responses extensions + /// Returns a reference to the response-local data/extensions container. #[inline] pub fn extensions(&self) -> Ref<'_, Extensions> { self.res @@ -291,7 +286,8 @@ impl HttpResponseBuilder { .extensions() } - /// Mutable reference to a the response's extensions + /// Returns a mutable reference to the response-local data/extensions container. + #[inline] pub fn extensions_mut(&mut self) -> RefMut<'_, Extensions> { self.res .as_mut() @@ -332,7 +328,7 @@ impl HttpResponseBuilder { #[cfg(feature = "cookies")] if let Some(ref jar) = self.cookies { for cookie in jar.delta() { - match HeaderValue::from_str(&cookie.to_string()) { + match actix_http::header::HeaderValue::from_str(&cookie.to_string()) { Ok(val) => res.headers_mut().append(header::SET_COOKIE, val), Err(err) => return Err(err.into()), }; @@ -394,7 +390,6 @@ impl HttpResponseBuilder { } } - #[inline] fn inner(&mut self) -> Option<&mut ResponseHead> { if self.err.is_some() { return None; @@ -435,10 +430,9 @@ impl Responder for HttpResponseBuilder { #[cfg(test)] mod tests { - use actix_http::body; - use super::*; use crate::{ + body, http::{ header::{self, HeaderValue, CONTENT_TYPE}, StatusCode, diff --git a/src/response/response.rs b/src/response/response.rs index f24a75b19..4aba4b623 100644 --- a/src/response/response.rs +++ b/src/response/response.rs @@ -168,34 +168,37 @@ impl HttpResponse { self.res.keep_alive() } - /// Responses extensions + /// Returns reference to the response-local data/extensions container. #[inline] pub fn extensions(&self) -> Ref<'_, Extensions> { self.res.extensions() } - /// Mutable reference to a the response's extensions + /// Returns reference to the response-local data/extensions container. #[inline] pub fn extensions_mut(&mut self) -> RefMut<'_, Extensions> { self.res.extensions_mut() } - /// Get body of this response + /// Returns a reference to this response's body. #[inline] pub fn body(&self) -> &B { self.res.body() } - /// Set a body + /// Sets new body. pub fn set_body(self, body: B2) -> HttpResponse { HttpResponse { res: self.res.set_body(body), - error: None, - // error: self.error, ?? + error: self.error, } } - /// Split response and body + /// Returns split head and body. + /// + /// # Implementation Notes + /// Due to internal performance optimizations, the first element of the returned tuple is an + /// `HttpResponse` as well but only contains the head of the response this was called on. pub fn into_parts(self) -> (HttpResponse<()>, B) { let (head, body) = self.res.into_parts(); @@ -208,7 +211,7 @@ impl HttpResponse { ) } - /// Drop request's body + /// Drops body and returns new response. pub fn drop_body(self) -> HttpResponse<()> { HttpResponse { res: self.res.drop_body(), @@ -216,7 +219,9 @@ impl HttpResponse { } } - /// Set a body and return previous body value + /// Map the current body type to another using a closure. Returns a new response. + /// + /// Closure receives the response head and the current body type. pub fn map_body(self, f: F) -> HttpResponse where F: FnOnce(&mut ResponseHead, B) -> B2, diff --git a/src/service.rs b/src/service.rs index f15cbfc9f..03ea0b97b 100644 --- a/src/service.rs +++ b/src/service.rs @@ -103,6 +103,7 @@ impl ServiceRequest { /// Construct request from request. /// /// The returned `ServiceRequest` would have no payload. + #[inline] pub fn from_request(req: HttpRequest) -> Self { ServiceRequest { req, @@ -256,18 +257,6 @@ impl ServiceRequest { self.req.conn_data() } - /// Counterpart to [`HttpRequest::req_data`]. - #[inline] - pub fn req_data(&self) -> Ref<'_, Extensions> { - self.req.req_data() - } - - /// Counterpart to [`HttpRequest::req_data_mut`]. - #[inline] - pub fn req_data_mut(&self) -> RefMut<'_, Extensions> { - self.req.req_data_mut() - } - #[cfg(feature = "cookies")] #[inline] pub fn cookies(&self) -> Result>>, CookieParseError> { @@ -320,18 +309,15 @@ impl HttpMessage for ServiceRequest { type Stream = BoxedPayloadStream; #[inline] - /// Returns Request's headers. fn headers(&self) -> &HeaderMap { &self.head().headers } - /// Request extensions #[inline] fn extensions(&self) -> Ref<'_, Extensions> { self.req.extensions() } - /// Mutable reference to a the request's extensions #[inline] fn extensions_mut(&self) -> RefMut<'_, Extensions> { self.req.extensions_mut() @@ -398,32 +384,32 @@ impl ServiceResponse { ServiceResponse::new(self.request, response) } - /// Get reference to original request + /// Returns reference to original request. #[inline] pub fn request(&self) -> &HttpRequest { &self.request } - /// Get reference to response + /// Returns reference to response. #[inline] pub fn response(&self) -> &HttpResponse { &self.response } - /// Get mutable reference to response + /// Returns mutable reference to response. #[inline] pub fn response_mut(&mut self) -> &mut HttpResponse { &mut self.response } - /// Get the response status code + /// Returns response status code. #[inline] pub fn status(&self) -> StatusCode { self.response.status() } - #[inline] /// Returns response's headers. + #[inline] pub fn headers(&self) -> &HeaderMap { self.response.headers() } @@ -440,13 +426,9 @@ impl ServiceResponse { (self.request, self.response) } - /// Extract response body - #[inline] - pub fn into_body(self) -> B { - self.response.into_body() - } - - /// Set a new body + /// Map the current body type to another using a closure. Returns a new response. + /// + /// Closure receives the response head and the current body type. #[inline] pub fn map_body(self, f: F) -> ServiceResponse where @@ -477,6 +459,12 @@ impl ServiceResponse { { self.map_body(|_, body| body.boxed()) } + + /// Consumes the response and returns its body. + #[inline] + pub fn into_body(self) -> B { + self.response.into_body() + } } impl From> for HttpResponse { @@ -546,14 +534,12 @@ impl WebService { /// Ok(req.into_response(HttpResponse::Ok().finish())) /// } /// - /// fn main() { - /// let app = App::new() - /// .service( - /// web::service("/app") - /// .guard(guard::Header("content-type", "text/plain")) - /// .finish(index) - /// ); - /// } + /// let app = App::new() + /// .service( + /// web::service("/app") + /// .guard(guard::Header("content-type", "text/plain")) + /// .finish(index) + /// ); /// ``` pub fn guard(mut self, guard: G) -> Self { self.guards.push(Box::new(guard));