From 1281a748d03aa63fb7662c638d725d91b29a44c7 Mon Sep 17 00:00:00 2001 From: fakeshadow <24548779@qq.com> Date: Mon, 29 Mar 2021 19:06:16 -0700 Subject: [PATCH 1/2] merge H1ServiceHandler requests into HttpServiceHandler (#2126) --- actix-http/src/h1/service.rs | 68 +-------- actix-http/src/service.rs | 260 ++++++++++++----------------------- 2 files changed, 95 insertions(+), 233 deletions(-) diff --git a/actix-http/src/h1/service.rs b/actix-http/src/h1/service.rs index 4fe79736b..f915bfa47 100644 --- a/actix-http/src/h1/service.rs +++ b/actix-http/src/h1/service.rs @@ -6,7 +6,7 @@ use std::{fmt, net}; use actix_codec::{AsyncRead, AsyncWrite, Framed}; use actix_rt::net::TcpStream; use actix_service::{pipeline_factory, IntoServiceFactory, Service, ServiceFactory}; -use futures_core::{future::LocalBoxFuture, ready}; +use futures_core::future::LocalBoxFuture; use futures_util::future::ready; use crate::body::MessageBody; @@ -14,7 +14,7 @@ use crate::config::ServiceConfig; use crate::error::{DispatchError, Error}; use crate::request::Request; use crate::response::Response; -use crate::service::HttpFlow; +use crate::service::HttpServiceHandler; use crate::{ConnectCallback, OnConnectData}; use super::codec::Codec; @@ -315,47 +315,10 @@ where } /// `Service` implementation for HTTP/1 transport -pub struct H1ServiceHandler -where - S: Service, - X: Service, - U: Service<(Request, Framed)>, -{ - flow: Rc>, - on_connect_ext: Option>>, - cfg: ServiceConfig, - _phantom: PhantomData, -} - -impl H1ServiceHandler -where - S: Service, - S::Error: Into, - S::Response: Into>, - B: MessageBody, - X: Service, - X::Error: Into, - U: Service<(Request, Framed), Response = ()>, - U::Error: fmt::Display, -{ - fn new( - cfg: ServiceConfig, - service: S, - expect: X, - upgrade: Option, - on_connect_ext: Option>>, - ) -> H1ServiceHandler { - H1ServiceHandler { - flow: HttpFlow::new(service, expect, upgrade), - cfg, - on_connect_ext, - _phantom: PhantomData, - } - } -} +pub type H1ServiceHandler = HttpServiceHandler; impl Service<(T, Option)> - for H1ServiceHandler + for HttpServiceHandler where T: AsyncRead + AsyncWrite + Unpin, S: Service, @@ -372,27 +335,10 @@ where type Future = Dispatcher; fn poll_ready(&self, cx: &mut Context<'_>) -> Poll> { - ready!(self.flow.expect.poll_ready(cx)).map_err(|e| { - let e = e.into(); - log::error!("Http expect service readiness error: {:?}", e); + self._poll_ready(cx).map_err(|e| { + log::error!("HTTP/1 service readiness error: {:?}", e); DispatchError::Service(e) - })?; - - if let Some(ref upg) = self.flow.upgrade { - ready!(upg.poll_ready(cx)).map_err(|e| { - let e = e.into(); - log::error!("Http upgrade service readiness error: {:?}", e); - DispatchError::Service(e) - })?; - }; - - ready!(self.flow.service.poll_ready(cx)).map_err(|e| { - let e = e.into(); - log::error!("Http service readiness error: {:?}", e); - DispatchError::Service(e) - })?; - - Poll::Ready(Ok(())) + }) } fn call(&self, (io, addr): (T, Option)) -> Self::Future { diff --git a/actix-http/src/service.rs b/actix-http/src/service.rs index 1a06cec3d..fd97fb5ef 100644 --- a/actix-http/src/service.rs +++ b/actix-http/src/service.rs @@ -12,7 +12,7 @@ use actix_codec::{AsyncRead, AsyncWrite, Framed}; use actix_rt::net::TcpStream; use actix_service::{pipeline_factory, IntoServiceFactory, Service, ServiceFactory}; use bytes::Bytes; -use futures_core::ready; +use futures_core::{future::LocalBoxFuture, ready}; use h2::server::{handshake, Handshake}; use pin_project::pin_project; @@ -107,7 +107,6 @@ where X1: ServiceFactory, X1::Error: Into, X1::InitError: fmt::Debug, - >::Future: 'static, { HttpService { expect, @@ -128,7 +127,6 @@ where U1: ServiceFactory<(Request, Framed), Config = (), Response = ()>, U1::Error: fmt::Display, U1::InitError: fmt::Debug, - )>>::Future: 'static, { HttpService { upgrade, @@ -150,23 +148,24 @@ where impl HttpService where S: ServiceFactory, + S::Future: 'static, S::Error: Into + 'static, S::InitError: fmt::Debug, S::Response: Into> + 'static, >::Future: 'static, B: MessageBody + 'static, X: ServiceFactory, + X::Future: 'static, X::Error: Into, X::InitError: fmt::Debug, - >::Future: 'static, U: ServiceFactory< (Request, Framed), Config = (), Response = (), >, + U::Future: 'static, U::Error: fmt::Display + Into, U::InitError: fmt::Debug, - )>>::Future: 'static, { /// Create simple tcp stream service pub fn tcp( @@ -196,23 +195,24 @@ mod openssl { impl HttpService, S, B, X, U> where S: ServiceFactory, + S::Future: 'static, S::Error: Into + 'static, S::InitError: fmt::Debug, S::Response: Into> + 'static, >::Future: 'static, B: MessageBody + 'static, X: ServiceFactory, + X::Future: 'static, X::Error: Into, X::InitError: fmt::Debug, - >::Future: 'static, U: ServiceFactory< (Request, Framed, h1::Codec>), Config = (), Response = (), >, + U::Future: 'static, U::Error: fmt::Display + Into, U::InitError: fmt::Debug, - , h1::Codec>)>>::Future: 'static, { /// Create openssl based service pub fn openssl( @@ -261,23 +261,24 @@ mod rustls { impl HttpService, S, B, X, U> where S: ServiceFactory, + S::Future: 'static, S::Error: Into + 'static, S::InitError: fmt::Debug, S::Response: Into> + 'static, >::Future: 'static, B: MessageBody + 'static, X: ServiceFactory, + X::Future: 'static, X::Error: Into, X::InitError: fmt::Debug, - >::Future: 'static, U: ServiceFactory< (Request, Framed, h1::Codec>), Config = (), Response = (), >, + U::Future: 'static, U::Error: fmt::Display + Into, U::InitError: fmt::Debug, - , h1::Codec>)>>::Future: 'static, { /// Create openssl based service pub fn rustls( @@ -319,137 +320,117 @@ mod rustls { impl ServiceFactory<(T, Protocol, Option)> for HttpService where - T: AsyncRead + AsyncWrite + Unpin, + T: AsyncRead + AsyncWrite + Unpin + 'static, S: ServiceFactory, + S::Future: 'static, S::Error: Into + 'static, S::InitError: fmt::Debug, S::Response: Into> + 'static, >::Future: 'static, B: MessageBody + 'static, X: ServiceFactory, + X::Future: 'static, X::Error: Into, X::InitError: fmt::Debug, - >::Future: 'static, U: ServiceFactory<(Request, Framed), Config = (), Response = ()>, + U::Future: 'static, U::Error: fmt::Display + Into, U::InitError: fmt::Debug, - )>>::Future: 'static, { type Response = (); type Error = DispatchError; type Config = (); type Service = HttpServiceHandler; type InitError = (); - type Future = HttpServiceResponse; + type Future = LocalBoxFuture<'static, Result>; fn new_service(&self, _: ()) -> Self::Future { - HttpServiceResponse { - fut: self.srv.new_service(()), - fut_ex: Some(self.expect.new_service(())), - fut_upg: self.upgrade.as_ref().map(|f| f.new_service(())), - expect: None, - upgrade: None, - on_connect_ext: self.on_connect_ext.clone(), - cfg: self.cfg.clone(), - _phantom: PhantomData, - } - } -} + let service = self.srv.new_service(()); + let expect = self.expect.new_service(()); + let upgrade = self.upgrade.as_ref().map(|s| s.new_service(())); + let on_connect_ext = self.on_connect_ext.clone(); + let cfg = self.cfg.clone(); -#[doc(hidden)] -#[pin_project] -pub struct HttpServiceResponse -where - S: ServiceFactory, - X: ServiceFactory, - U: ServiceFactory<(Request, Framed)>, -{ - #[pin] - fut: S::Future, - #[pin] - fut_ex: Option, - #[pin] - fut_upg: Option, - expect: Option, - upgrade: Option, - on_connect_ext: Option>>, - cfg: ServiceConfig, - _phantom: PhantomData, -} + Box::pin(async move { + let expect = expect + .await + .map_err(|e| log::error!("Init http expect service error: {:?}", e))?; -impl Future for HttpServiceResponse -where - T: AsyncRead + AsyncWrite + Unpin, - S: ServiceFactory, - S::Error: Into + 'static, - S::InitError: fmt::Debug, - S::Response: Into> + 'static, - >::Future: 'static, - B: MessageBody + 'static, - X: ServiceFactory, - X::Error: Into, - X::InitError: fmt::Debug, - >::Future: 'static, - U: ServiceFactory<(Request, Framed), Response = ()>, - U::Error: fmt::Display, - U::InitError: fmt::Debug, - )>>::Future: 'static, -{ - type Output = - Result, ()>; + let upgrade = match upgrade { + Some(upgrade) => { + let upgrade = upgrade.await.map_err(|e| { + log::error!("Init http upgrade service error: {:?}", e) + })?; + Some(upgrade) + } + None => None, + }; - fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - let mut this = self.as_mut().project(); + let service = service + .await + .map_err(|e| log::error!("Init http service error: {:?}", e))?; - if let Some(fut) = this.fut_ex.as_pin_mut() { - let expect = ready!(fut - .poll(cx) - .map_err(|e| log::error!("Init http service error: {:?}", e)))?; - this = self.as_mut().project(); - *this.expect = Some(expect); - this.fut_ex.set(None); - } - - if let Some(fut) = this.fut_upg.as_pin_mut() { - let upgrade = ready!(fut - .poll(cx) - .map_err(|e| log::error!("Init http service error: {:?}", e)))?; - this = self.as_mut().project(); - *this.upgrade = Some(upgrade); - this.fut_upg.set(None); - } - - let result = ready!(this - .fut - .poll(cx) - .map_err(|e| log::error!("Init http service error: {:?}", e))); - - Poll::Ready(result.map(|service| { - let this = self.as_mut().project(); - HttpServiceHandler::new( - this.cfg.clone(), + Ok(HttpServiceHandler::new( + cfg, service, - this.expect.take().unwrap(), - this.upgrade.take(), - this.on_connect_ext.clone(), - ) - })) + expect, + upgrade, + on_connect_ext, + )) + }) } } -/// `Service` implementation for HTTP transport +/// `Service` implementation for HTTP/1 and HTTP/2 transport pub struct HttpServiceHandler where S: Service, X: Service, U: Service<(Request, Framed)>, { - flow: Rc>, - cfg: ServiceConfig, - on_connect_ext: Option>>, + pub(super) flow: Rc>, + pub(super) cfg: ServiceConfig, + pub(super) on_connect_ext: Option>>, _phantom: PhantomData, } +impl HttpServiceHandler +where + S: Service, + S::Error: Into, + X: Service, + X::Error: Into, + U: Service<(Request, Framed)>, + U::Error: Into, +{ + pub(super) fn new( + cfg: ServiceConfig, + service: S, + expect: X, + upgrade: Option, + on_connect_ext: Option>>, + ) -> HttpServiceHandler { + HttpServiceHandler { + cfg, + on_connect_ext, + flow: HttpFlow::new(service, expect, upgrade), + _phantom: PhantomData, + } + } + + pub(super) fn _poll_ready(&self, cx: &mut Context<'_>) -> Poll> { + ready!(self.flow.expect.poll_ready(cx).map_err(Into::into))?; + + ready!(self.flow.service.poll_ready(cx).map_err(Into::into))?; + + if let Some(ref upg) = self.flow.upgrade { + ready!(upg.poll_ready(cx).map_err(Into::into))?; + }; + + Poll::Ready(Ok(())) + } +} + /// A collection of services that describe an HTTP request flow. pub(super) struct HttpFlow { pub(super) service: S, @@ -467,34 +448,6 @@ impl HttpFlow { } } -impl HttpServiceHandler -where - S: Service, - S::Error: Into + 'static, - S::Future: 'static, - S::Response: Into> + 'static, - B: MessageBody + 'static, - X: Service, - X::Error: Into, - U: Service<(Request, Framed), Response = ()>, - U::Error: fmt::Display, -{ - fn new( - cfg: ServiceConfig, - service: S, - expect: X, - upgrade: Option, - on_connect_ext: Option>>, - ) -> HttpServiceHandler { - HttpServiceHandler { - cfg, - on_connect_ext, - flow: HttpFlow::new(service, expect, upgrade), - _phantom: PhantomData, - } - } -} - impl Service<(T, Protocol, Option)> for HttpServiceHandler where @@ -514,47 +467,10 @@ where type Future = HttpServiceHandlerResponse; fn poll_ready(&self, cx: &mut Context<'_>) -> Poll> { - let ready = self - .flow - .expect - .poll_ready(cx) - .map_err(|e| { - let e = e.into(); - log::error!("Http service readiness error: {:?}", e); - DispatchError::Service(e) - })? - .is_ready(); - - let ready = self - .flow - .service - .poll_ready(cx) - .map_err(|e| { - let e = e.into(); - log::error!("Http service readiness error: {:?}", e); - DispatchError::Service(e) - })? - .is_ready() - && ready; - - let ready = if let Some(ref upg) = self.flow.upgrade { - upg.poll_ready(cx) - .map_err(|e| { - let e = e.into(); - log::error!("Http service readiness error: {:?}", e); - DispatchError::Service(e) - })? - .is_ready() - && ready - } else { - ready - }; - - if ready { - Poll::Ready(Ok(())) - } else { - Poll::Pending - } + self._poll_ready(cx).map_err(|e| { + log::error!("HTTP service readiness error: {:?}", e); + DispatchError::Service(e) + }) } fn call( From f66774e30b239546991fb6c0d54c8885171d1b8f Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Tue, 30 Mar 2021 03:32:22 +0100 Subject: [PATCH 2/2] remove `From` impl from HttpDate fully removes time crate from public api of -http --- .cargo/config.toml | 3 +++ actix-http/src/header/shared/httpdate.rs | 24 ++++++++++-------------- docs/graphs/net-only.dot | 2 +- 3 files changed, 14 insertions(+), 15 deletions(-) create mode 100644 .cargo/config.toml diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 000000000..9d0d9da8c --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,3 @@ +[alias] +chk = "hack check --workspace --tests --examples" +lint = "hack --clean-per-run clippy --workspace --tests --examples" diff --git a/actix-http/src/header/shared/httpdate.rs b/actix-http/src/header/shared/httpdate.rs index 72a225589..18278a6d8 100644 --- a/actix-http/src/header/shared/httpdate.rs +++ b/actix-http/src/header/shared/httpdate.rs @@ -1,18 +1,20 @@ -use std::fmt::{self, Display}; -use std::io::Write; -use std::str::FromStr; -use std::time::{SystemTime, UNIX_EPOCH}; +use std::{ + fmt, + io::Write, + str::FromStr, + time::{SystemTime, UNIX_EPOCH}, +}; use bytes::buf::BufMut; use bytes::BytesMut; use http::header::{HeaderValue, InvalidHeaderValue}; -use time::{offset, OffsetDateTime, PrimitiveDateTime}; +use time::{OffsetDateTime, PrimitiveDateTime, UtcOffset}; use crate::error::ParseError; use crate::header::IntoHeaderValue; use crate::time_parser; -/// A timestamp with HTTP formatting and parsing +/// A timestamp with HTTP formatting and parsing. #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct HttpDate(OffsetDateTime); @@ -27,18 +29,12 @@ impl FromStr for HttpDate { } } -impl Display for HttpDate { +impl fmt::Display for HttpDate { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Display::fmt(&self.0.format("%a, %d %b %Y %H:%M:%S GMT"), f) } } -impl From for HttpDate { - fn from(dt: OffsetDateTime) -> HttpDate { - HttpDate(dt) - } -} - impl From for HttpDate { fn from(sys: SystemTime) -> HttpDate { HttpDate(PrimitiveDateTime::from(sys).assume_utc()) @@ -54,7 +50,7 @@ impl IntoHeaderValue for HttpDate { wrt, "{}", self.0 - .to_offset(offset!(UTC)) + .to_offset(UtcOffset::UTC) .format("%a, %d %b %Y %H:%M:%S GMT") ) .unwrap(); diff --git a/docs/graphs/net-only.dot b/docs/graphs/net-only.dot index babd612a6..84227cdb0 100644 --- a/docs/graphs/net-only.dot +++ b/docs/graphs/net-only.dot @@ -7,7 +7,7 @@ digraph { } "actix-codec" -> { "actix-rt" "actix-service" "local-channel" "tokio" } - "actix-utils" -> { "actix-rt" "actix-service" "local-waker" } + "actix-utils" -> { "local-waker" } "actix-tracing" -> { "actix-service" } "actix-tls" -> { "actix-service" "actix-codec" "actix-utils" "actix-rt" } "actix-server" -> { "actix-service" "actix-rt" "actix-codec" "actix-utils" "tokio" }