From 07036b56407c4abdd3034ee5594a43a0ea57cd0e Mon Sep 17 00:00:00 2001 From: Ibraheem Ahmed Date: Thu, 22 Apr 2021 08:54:29 -0400 Subject: [PATCH 1/9] static form extract future (#2181) --- src/types/form.rs | 53 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 14 deletions(-) diff --git a/src/types/form.rs b/src/types/form.rs index 9d2311a45..d1deac937 100644 --- a/src/types/form.rs +++ b/src/types/form.rs @@ -12,7 +12,7 @@ use std::{ use actix_http::Payload; use bytes::BytesMut; use encoding_rs::{Encoding, UTF_8}; -use futures_core::future::LocalBoxFuture; +use futures_core::{future::LocalBoxFuture, ready}; use futures_util::{FutureExt as _, StreamExt as _}; use serde::{de::DeserializeOwned, Serialize}; @@ -123,11 +123,10 @@ where { type Config = FormConfig; type Error = Error; - type Future = LocalBoxFuture<'static, Result>; + type Future = FormExtractFut; #[inline] fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future { - let req2 = req.clone(); let (limit, err_handler) = req .app_data::() .or_else(|| { @@ -137,16 +136,42 @@ where .map(|c| (c.limit, c.err_handler.clone())) .unwrap_or((16384, None)); - UrlEncoded::new(req, payload) - .limit(limit) - .map(move |res| match res { - Err(err) => match err_handler { - Some(err_handler) => Err((err_handler)(err, &req2)), - None => Err(err.into()), - }, - Ok(item) => Ok(Form(item)), - }) - .boxed_local() + FormExtractFut { + fut: UrlEncoded::new(req, payload).limit(limit), + req: req.clone(), + err_handler, + } + } +} + +type FormErrHandler = Option Error>>; + +pub struct FormExtractFut { + fut: UrlEncoded, + err_handler: FormErrHandler, + req: HttpRequest, +} + +impl Future for FormExtractFut +where + T: DeserializeOwned + 'static, +{ + type Output = Result, Error>; + + fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + let this = self.get_mut(); + + let res = ready!(Pin::new(&mut this.fut).poll(cx)); + + let res = match res { + Err(err) => match &this.err_handler { + Some(err_handler) => Err((err_handler)(err, &this.req)), + None => Err(err.into()), + }, + Ok(item) => Ok(Form(item)), + }; + + Poll::Ready(res) } } @@ -193,7 +218,7 @@ impl Responder for Form { #[derive(Clone)] pub struct FormConfig { limit: usize, - err_handler: Option Error>>, + err_handler: FormErrHandler, } impl FormConfig { From f44a0bc159680f5c65c14bbf7947768272818276 Mon Sep 17 00:00:00 2001 From: tglman Date: Thu, 22 Apr 2021 18:13:13 +0100 Subject: [PATCH 2/9] add support of filtering guards in Files of actix-files (#2046) Co-authored-by: Rob Ede --- actix-files/CHANGES.md | 3 ++ actix-files/src/files.rs | 49 ++++++++++++++++--- .../tests/fixtures/guards/first/index.txt | 1 + .../tests/fixtures/guards/second/index.txt | 1 + actix-files/tests/guard.rs | 36 ++++++++++++++ src/guard.rs | 8 +++ 6 files changed, 91 insertions(+), 7 deletions(-) create mode 100644 actix-files/tests/fixtures/guards/first/index.txt create mode 100644 actix-files/tests/fixtures/guards/second/index.txt create mode 100644 actix-files/tests/guard.rs diff --git a/actix-files/CHANGES.md b/actix-files/CHANGES.md index 6c28e42d0..0586a2fd3 100644 --- a/actix-files/CHANGES.md +++ b/actix-files/CHANGES.md @@ -11,6 +11,9 @@ ## 0.6.0-beta.4 - 2021-04-02 * No notable changes. +* Add support for `.guard` in `Files` to selectively filter `Files` services. [#2046] + +[#2046]: https://github.com/actix/actix-web/pull/2046 ## 0.6.0-beta.3 - 2021-03-09 * No notable changes. diff --git a/actix-files/src/files.rs b/actix-files/src/files.rs index 9b24c4b21..00e4dd5d1 100644 --- a/actix-files/src/files.rs +++ b/actix-files/src/files.rs @@ -37,7 +37,8 @@ pub struct Files { renderer: Rc, mime_override: Option>, file_flags: named::Flags, - guards: Option>, + use_guards: Option>, + guards: Vec>>, hidden_files: bool, } @@ -59,12 +60,12 @@ impl Clone for Files { file_flags: self.file_flags, path: self.path.clone(), mime_override: self.mime_override.clone(), + use_guards: self.use_guards.clone(), guards: self.guards.clone(), hidden_files: self.hidden_files, } } } - impl Files { /// Create new `Files` instance for a specified base directory. /// @@ -104,7 +105,8 @@ impl Files { renderer: Rc::new(directory_listing), mime_override: None, file_flags: named::Flags::default(), - guards: None, + use_guards: None, + guards: Vec::new(), hidden_files: false, } } @@ -185,7 +187,28 @@ impl Files { /// Default behaviour allows GET and HEAD. #[inline] pub fn use_guards(mut self, guards: G) -> Self { - self.guards = Some(Rc::new(guards)); + self.use_guards = Some(Rc::new(guards)); + self + } + + /// Add match guard to use on directory listings and files. + /// + /// ```rust + /// use actix_web::{guard, App}; + /// use actix_files::Files; + /// + /// + /// fn main() { + /// let app = App::new() + /// .service( + /// Files::new("/","/my/site/files") + /// .guard(guard::Header("Host", "example.com")) + /// ); + /// } + /// ``` + #[inline] + pub fn guard(mut self, guard: G) -> Self { + self.guards.push(Box::new(Rc::new(guard))); self } @@ -238,7 +261,19 @@ impl Files { } impl HttpServiceFactory for Files { - fn register(self, config: &mut AppService) { + fn register(mut self, config: &mut AppService) { + let guards = if self.guards.is_empty() { + None + } else { + let guards = std::mem::take(&mut self.guards); + Some( + guards + .into_iter() + .map(|guard| -> Box { guard }) + .collect::>(), + ) + }; + if self.default.borrow().is_none() { *self.default.borrow_mut() = Some(config.default_service()); } @@ -249,7 +284,7 @@ impl HttpServiceFactory for Files { ResourceDef::prefix(&self.path) }; - config.register_service(rdef, None, self, None) + config.register_service(rdef, guards, self, None) } } @@ -271,7 +306,7 @@ impl ServiceFactory for Files { renderer: self.renderer.clone(), mime_override: self.mime_override.clone(), file_flags: self.file_flags, - guards: self.guards.clone(), + guards: self.use_guards.clone(), hidden_files: self.hidden_files, }; diff --git a/actix-files/tests/fixtures/guards/first/index.txt b/actix-files/tests/fixtures/guards/first/index.txt new file mode 100644 index 000000000..fe4f02ad0 --- /dev/null +++ b/actix-files/tests/fixtures/guards/first/index.txt @@ -0,0 +1 @@ +first \ No newline at end of file diff --git a/actix-files/tests/fixtures/guards/second/index.txt b/actix-files/tests/fixtures/guards/second/index.txt new file mode 100644 index 000000000..2147e4188 --- /dev/null +++ b/actix-files/tests/fixtures/guards/second/index.txt @@ -0,0 +1 @@ +second \ No newline at end of file diff --git a/actix-files/tests/guard.rs b/actix-files/tests/guard.rs new file mode 100644 index 000000000..8b1785e7f --- /dev/null +++ b/actix-files/tests/guard.rs @@ -0,0 +1,36 @@ +use actix_files::Files; +use actix_web::{ + guard::Host, + http::StatusCode, + test::{self, TestRequest}, + App, +}; +use bytes::Bytes; + +#[actix_rt::test] +async fn test_guard_filter() { + let srv = test::init_service( + App::new() + .service(Files::new("/", "./tests/fixtures/guards/first").guard(Host("first.com"))) + .service( + Files::new("/", "./tests/fixtures/guards/second").guard(Host("second.com")), + ), + ) + .await; + + let req = TestRequest::with_uri("/index.txt") + .append_header(("Host", "first.com")) + .to_request(); + let res = test::call_service(&srv, req).await; + + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(test::read_body(res).await, Bytes::from("first")); + + let req = TestRequest::with_uri("/index.txt") + .append_header(("Host", "second.com")) + .to_request(); + let res = test::call_service(&srv, req).await; + + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(test::read_body(res).await, Bytes::from("second")); +} diff --git a/src/guard.rs b/src/guard.rs index 3a1f5bb14..c71d64a29 100644 --- a/src/guard.rs +++ b/src/guard.rs @@ -26,6 +26,8 @@ //! ``` #![allow(non_snake_case)] use std::convert::TryFrom; +use std::ops::Deref; +use std::rc::Rc; use actix_http::http::{self, header, uri::Uri}; use actix_http::RequestHead; @@ -40,6 +42,12 @@ pub trait Guard { fn check(&self, request: &RequestHead) -> bool; } +impl Guard for Rc { + fn check(&self, request: &RequestHead) -> bool { + self.deref().check(request) + } +} + /// Create guard object for supplied function. /// /// ``` From 75867bd073e4bc16b416bc10e13a185c3cfcf14a Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Thu, 22 Apr 2021 18:31:21 +0100 Subject: [PATCH 3/9] clean up files service docs and rename method follow on from #2046 --- actix-files/src/files.rs | 61 ++++++++++++++++++++++------------------ actix-files/src/lib.rs | 2 +- 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/actix-files/src/files.rs b/actix-files/src/files.rs index 00e4dd5d1..aeb75e116 100644 --- a/actix-files/src/files.rs +++ b/actix-files/src/files.rs @@ -158,7 +158,6 @@ impl Files { /// Specifies whether to use ETag or not. /// /// Default is true. - #[inline] pub fn use_etag(mut self, value: bool) -> Self { self.file_flags.set(named::Flags::ETAG, value); self @@ -167,7 +166,6 @@ impl Files { /// Specifies whether to use Last-Modified or not. /// /// Default is true. - #[inline] pub fn use_last_modified(mut self, value: bool) -> Self { self.file_flags.set(named::Flags::LAST_MD, value); self @@ -176,46 +174,55 @@ impl Files { /// Specifies whether text responses should signal a UTF-8 encoding. /// /// Default is false (but will default to true in a future version). - #[inline] pub fn prefer_utf8(mut self, value: bool) -> Self { self.file_flags.set(named::Flags::PREFER_UTF8, value); self } - /// Specifies custom guards to use for directory listings and files. + /// Adds a routing guard. /// - /// Default behaviour allows GET and HEAD. - #[inline] - pub fn use_guards(mut self, guards: G) -> Self { - self.use_guards = Some(Rc::new(guards)); - self - } - - /// Add match guard to use on directory listings and files. + /// Use this to allow multiple chained file services that respond to strictly different + /// properties of a request. Due to the way routing works, if a guard check returns true and the + /// request starts being handled by the file service, it will not be able to back-out and try + /// the next service, you will simply get a 404 (or 405) error response. /// - /// ```rust - /// use actix_web::{guard, App}; + /// To allow `POST` requests to retrieve files, see [`Files::use_guards`]. + /// + /// # Examples + /// ``` + /// use actix_web::{guard::Header, App}; /// use actix_files::Files; /// - /// - /// fn main() { - /// let app = App::new() - /// .service( - /// Files::new("/","/my/site/files") - /// .guard(guard::Header("Host", "example.com")) - /// ); - /// } + /// App::new().service( + /// Files::new("/","/my/site/files") + /// .guard(Header("Host", "example.com")) + /// ); /// ``` - #[inline] pub fn guard(mut self, guard: G) -> Self { self.guards.push(Box::new(Rc::new(guard))); self } + /// Specifies guard to check before fetching directory listings or files. + /// + /// Note that this guard has no effect on routing; it's main use is to guard on the request's + /// method just before serving the file, only allowing `GET` and `HEAD` requests by default. + /// See [`Files::guard`] for routing guards. + pub fn method_guard(mut self, guard: G) -> Self { + self.use_guards = Some(Rc::new(guard)); + self + } + + #[doc(hidden)] + #[deprecated(since = "0.6.0", note = "Renamed to `method_guard`.")] + /// See [`Files::method_guard`]. + pub fn use_guards(self, guard: G) -> Self { + self.method_guard(guard) + } + /// Disable `Content-Disposition` header. /// /// By default Content-Disposition` header is enabled. - #[inline] pub fn disable_content_disposition(mut self) -> Self { self.file_flags.remove(named::Flags::CONTENT_DISPOSITION); self @@ -223,8 +230,9 @@ impl Files { /// Sets default handler which is used when no matched file could be found. /// - /// For example, you could set a fall back static file handler: - /// ```rust + /// # Examples + /// Setting a fallback static file handler: + /// ``` /// use actix_files::{Files, NamedFile}; /// /// # fn run() -> Result<(), actix_web::Error> { @@ -253,7 +261,6 @@ impl Files { } /// Enables serving hidden files and directories, allowing a leading dots in url fragments. - #[inline] pub fn use_hidden_files(mut self) -> Self { self.hidden_files = true; self diff --git a/actix-files/src/lib.rs b/actix-files/src/lib.rs index 34b02581e..0384ff2b0 100644 --- a/actix-files/src/lib.rs +++ b/actix-files/src/lib.rs @@ -532,7 +532,7 @@ mod tests { #[actix_rt::test] async fn test_files_guards() { let srv = test::init_service( - App::new().service(Files::new("/", ".").use_guards(guard::Post())), + App::new().service(Files::new("/", ".").method_guard(guard::Post())), ) .await; From 6a29a50f25fd103c61e93d97dbee5745640bb911 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Thu, 22 Apr 2021 18:37:45 +0100 Subject: [PATCH 4/9] files doc wording --- actix-files/src/files.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/actix-files/src/files.rs b/actix-files/src/files.rs index aeb75e116..8e28cb45e 100644 --- a/actix-files/src/files.rs +++ b/actix-files/src/files.rs @@ -81,10 +81,9 @@ impl Files { /// If the mount path is set as the root path `/`, services registered after this one will /// be inaccessible. Register more specific handlers and services first. /// - /// `Files` uses a threadpool for blocking filesystem operations. By default, the pool uses a - /// max number of threads equal to `512 * HttpServer::worker`. Real time thread count are - /// adjusted with work load. More threads would spawn when need and threads goes idle for a - /// period of time would be de-spawned. + /// `Files` utilizes the existing Tokio thread-pool for blocking filesystem operations. + /// The number of running threads is adjusted over time as needed, up to a maximum of 512 times + /// the number of server [workers](HttpServer::workers), by default. pub fn new>(mount_path: &str, serve_from: T) -> Files { let orig_dir = serve_from.into(); let dir = match orig_dir.canonicalize() { From 1fcf92e11f1b18053ecc1df13be2d79e0efc1ca9 Mon Sep 17 00:00:00 2001 From: Voldracarno Draconor Date: Wed, 28 Apr 2021 02:23:12 +0200 Subject: [PATCH 5/9] Update dependency "language-tags" (#2188) --- CHANGES.md | 2 ++ Cargo.toml | 2 +- actix-http/CHANGES.md | 1 + actix-http/Cargo.toml | 2 +- src/http/header/accept_language.rs | 14 +++++--------- src/http/header/content_language.rs | 12 +++++------- 6 files changed, 15 insertions(+), 18 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index bea9ab935..6fe0174ad 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,8 @@ # Changes ## Unreleased - 2021-xx-xx +### Changed +* Update `language-tags` to `0.3`. ## 4.0.0-beta.6 - 2021-04-17 diff --git a/Cargo.toml b/Cargo.toml index 714da13a0..75b5e3a8e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -78,7 +78,7 @@ encoding_rs = "0.8" futures-core = { version = "0.3.7", default-features = false } futures-util = { version = "0.3.7", default-features = false } itoa = "0.4" -language-tags = "0.2" +language-tags = "0.3" once_cell = "1.5" log = "0.4" mime = "0.3" diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index e89208748..f1d24c3c3 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -9,6 +9,7 @@ ### Changed * `header` mod is now public. [#2171] * `uri` mod is now public. [#2171] +* Update `language-tags` to `0.3`. ### Removed * Stop re-exporting `http` crate's `HeaderMap` types in addition to ours. [#2171] diff --git a/actix-http/Cargo.toml b/actix-http/Cargo.toml index 55dc6d05e..638557807 100644 --- a/actix-http/Cargo.toml +++ b/actix-http/Cargo.toml @@ -57,7 +57,7 @@ h2 = "0.3.1" http = "0.2.2" httparse = "1.3" itoa = "0.4" -language-tags = "0.2" +language-tags = "0.3" local-channel = "0.1" once_cell = "1.5" log = "0.4" diff --git a/src/http/header/accept_language.rs b/src/http/header/accept_language.rs index 5fab4f79c..034946d4d 100644 --- a/src/http/header/accept_language.rs +++ b/src/http/header/accept_language.rs @@ -24,14 +24,11 @@ crate::__define_common_header! { /// # Examples /// /// ``` - /// use language_tags::langtag; /// use actix_web::HttpResponse; /// use actix_web::http::header::{AcceptLanguage, LanguageTag, qitem}; /// /// let mut builder = HttpResponse::Ok(); - /// let mut langtag: LanguageTag = Default::default(); - /// langtag.language = Some("en".to_owned()); - /// langtag.region = Some("US".to_owned()); + /// let langtag = LanguageTag::parse("en-US").unwrap(); /// builder.insert_header( /// AcceptLanguage(vec![ /// qitem(langtag), @@ -40,16 +37,15 @@ crate::__define_common_header! { /// ``` /// /// ``` - /// use language_tags::langtag; /// use actix_web::HttpResponse; - /// use actix_web::http::header::{AcceptLanguage, QualityItem, q, qitem}; + /// use actix_web::http::header::{AcceptLanguage, LanguageTag, QualityItem, q, qitem}; /// /// let mut builder = HttpResponse::Ok(); /// builder.insert_header( /// AcceptLanguage(vec![ - /// qitem(langtag!(da)), - /// QualityItem::new(langtag!(en;;;GB), q(800)), - /// QualityItem::new(langtag!(en), q(700)), + /// qitem(LanguageTag::parse("da").unwrap()), + /// QualityItem::new(LanguageTag::parse("en-GB").unwrap(), q(800)), + /// QualityItem::new(LanguageTag::parse("en").unwrap(), q(700)), /// ]) /// ); /// ``` diff --git a/src/http/header/content_language.rs b/src/http/header/content_language.rs index 41e6d9eff..c2469edd1 100644 --- a/src/http/header/content_language.rs +++ b/src/http/header/content_language.rs @@ -24,28 +24,26 @@ crate::__define_common_header! { /// # Examples /// /// ``` - /// use language_tags::langtag; /// use actix_web::HttpResponse; - /// use actix_web::http::header::{ContentLanguage, qitem}; + /// use actix_web::http::header::{ContentLanguage, LanguageTag, qitem}; /// /// let mut builder = HttpResponse::Ok(); /// builder.insert_header( /// ContentLanguage(vec![ - /// qitem(langtag!(en)), + /// qitem(LanguageTag::parse("en").unwrap()), /// ]) /// ); /// ``` /// /// ``` - /// use language_tags::langtag; /// use actix_web::HttpResponse; - /// use actix_web::http::header::{ContentLanguage, qitem}; + /// use actix_web::http::header::{ContentLanguage, LanguageTag, qitem}; /// /// let mut builder = HttpResponse::Ok(); /// builder.insert_header( /// ContentLanguage(vec![ - /// qitem(langtag!(da)), - /// qitem(langtag!(en;;;GB)), + /// qitem(LanguageTag::parse("da").unwrap()), + /// qitem(LanguageTag::parse("en-GB").unwrap()), /// ]) /// ); /// ``` From 3a0fb3f89e82fa3287d9741533e4d8dfd38c1548 Mon Sep 17 00:00:00 2001 From: Ibraheem Ahmed Date: Fri, 30 Apr 2021 22:02:56 -0400 Subject: [PATCH 6/9] Static either extract future (#2184) --- src/types/either.rs | 130 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 103 insertions(+), 27 deletions(-) diff --git a/src/types/either.rs b/src/types/either.rs index 210495e47..d3b003587 100644 --- a/src/types/either.rs +++ b/src/types/either.rs @@ -1,8 +1,14 @@ //! For either helper, see [`Either`]. +use std::{ + future::Future, + mem, + pin::Pin, + task::{Context, Poll}, +}; + use bytes::Bytes; -use futures_core::future::LocalBoxFuture; -use futures_util::{FutureExt as _, TryFutureExt as _}; +use futures_core::ready; use crate::{ dev, @@ -180,41 +186,111 @@ where R: FromRequest + 'static, { type Error = EitherExtractError; - type Future = LocalBoxFuture<'static, Result>; + type Future = EitherExtractFut; type Config = (); fn from_request(req: &HttpRequest, payload: &mut dev::Payload) -> Self::Future { - let req2 = req.clone(); - - Bytes::from_request(req, payload) - .map_err(EitherExtractError::Bytes) - .and_then(|bytes| bytes_to_l_or_r(req2, bytes)) - .boxed_local() + EitherExtractFut { + req: req.clone(), + state: EitherExtractState::Bytes { + bytes: Bytes::from_request(req, payload), + }, + } } } -async fn bytes_to_l_or_r( - req: HttpRequest, - bytes: Bytes, -) -> Result, EitherExtractError> +#[pin_project::pin_project] +pub struct EitherExtractFut where - L: FromRequest + 'static, - R: FromRequest + 'static, + R: FromRequest, + L: FromRequest, { - let fallback = bytes.clone(); - let a_err; + req: HttpRequest, + #[pin] + state: EitherExtractState, +} - let mut pl = payload_from_bytes(bytes); - match L::from_request(&req, &mut pl).await { - Ok(a_data) => return Ok(Either::Left(a_data)), - // store A's error for returning if B also fails - Err(err) => a_err = err, - }; +#[pin_project::pin_project(project = EitherExtractProj)] +pub enum EitherExtractState +where + L: FromRequest, + R: FromRequest, +{ + Bytes { + #[pin] + bytes: ::Future, + }, + Left { + #[pin] + left: L::Future, + fallback: Bytes, + }, + Right { + #[pin] + right: R::Future, + left_err: Option, + }, +} - let mut pl = payload_from_bytes(fallback); - match R::from_request(&req, &mut pl).await { - Ok(b_data) => return Ok(Either::Right(b_data)), - Err(b_err) => Err(EitherExtractError::Extract(a_err, b_err)), +impl Future for EitherExtractFut +where + L: FromRequest, + R: FromRequest, + LF: Future> + 'static, + RF: Future> + 'static, + LE: Into, + RE: Into, +{ + type Output = Result, EitherExtractError>; + + fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + let mut this = self.project(); + let ready = loop { + let next = match this.state.as_mut().project() { + EitherExtractProj::Bytes { bytes } => { + let res = ready!(bytes.poll(cx)); + match res { + Ok(bytes) => { + let fallback = bytes.clone(); + let left = + L::from_request(&this.req, &mut payload_from_bytes(bytes)); + EitherExtractState::Left { left, fallback } + } + Err(err) => break Err(EitherExtractError::Bytes(err)), + } + } + EitherExtractProj::Left { left, fallback } => { + let res = ready!(left.poll(cx)); + match res { + Ok(extracted) => break Ok(Either::Left(extracted)), + Err(left_err) => { + let right = R::from_request( + &this.req, + &mut payload_from_bytes(mem::take(fallback)), + ); + EitherExtractState::Right { + left_err: Some(left_err), + right, + } + } + } + } + EitherExtractProj::Right { right, left_err } => { + let res = ready!(right.poll(cx)); + match res { + Ok(data) => break Ok(Either::Right(data)), + Err(err) => { + break Err(EitherExtractError::Extract( + left_err.take().unwrap(), + err, + )); + } + } + } + }; + this.state.set(next); + }; + Poll::Ready(ready) } } From c17662fe39523ce4230234004c572618932be2c7 Mon Sep 17 00:00:00 2001 From: Luca Palmieri Date: Mon, 3 May 2021 00:58:14 +0100 Subject: [PATCH 7/9] Reduce the level of the emitted log line from `error` to `debug`. (#2196) Co-authored-by: Rob Ede --- actix-http/CHANGES.md | 2 ++ actix-http/src/response.rs | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index f1d24c3c3..cc6330288 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -10,11 +10,13 @@ * `header` mod is now public. [#2171] * `uri` mod is now public. [#2171] * Update `language-tags` to `0.3`. +* Reduce the level from `error` to `debug` for the log line that is emitted when a `500 Internal Server Error` is built using `HttpResponse::from_error`. [#2196] ### Removed * Stop re-exporting `http` crate's `HeaderMap` types in addition to ours. [#2171] [#2171]: https://github.com/actix/actix-web/pull/2171 +[#2196]: https://github.com/actix/actix-web/pull/2196 ## 3.0.0-beta.6 - 2021-04-17 diff --git a/actix-http/src/response.rs b/actix-http/src/response.rs index a3ab1175c..e11ceb18f 100644 --- a/actix-http/src/response.rs +++ b/actix-http/src/response.rs @@ -78,7 +78,7 @@ impl Response { pub fn from_error(error: Error) -> Response { let mut resp = error.as_response_error().error_response(); if resp.head.status == StatusCode::INTERNAL_SERVER_ERROR { - error!("Internal Server Error: {:?}", error); + debug!("Internal Server Error: {:?}", error); } resp.error = Some(error); resp From dd1a3e7675df26748d070e94912ebe1587cb9241 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 5 May 2021 06:16:12 -0400 Subject: [PATCH 8/9] Fix loophole in soundness of `__private_get_type_id__` (#2199) --- actix-http/src/macros.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/actix-http/src/macros.rs b/actix-http/src/macros.rs index 7cf0e288b..be8e63d6e 100644 --- a/actix-http/src/macros.rs +++ b/actix-http/src/macros.rs @@ -15,8 +15,15 @@ macro_rules! downcast_get_type_id { /// making it impossible for safe code to construct outside of /// this module. This ensures that safe code cannot violate /// type-safety by implementing this method. + /// + /// We also take `PrivateHelper` as a parameter, to ensure that + /// safe code cannot obtain a `PrivateHelper` instance by + /// delegating to an existing implementation of `__private_get_type_id__` #[doc(hidden)] - fn __private_get_type_id__(&self) -> (std::any::TypeId, PrivateHelper) + fn __private_get_type_id__( + &self, + _: PrivateHelper, + ) -> (std::any::TypeId, PrivateHelper) where Self: 'static, { @@ -39,7 +46,9 @@ macro_rules! downcast { impl dyn $name + 'static { /// Downcasts generic body to a specific type. pub fn downcast_ref(&self) -> Option<&T> { - if self.__private_get_type_id__().0 == std::any::TypeId::of::() { + if self.__private_get_type_id__(PrivateHelper(())).0 + == std::any::TypeId::of::() + { // SAFETY: external crates cannot override the default // implementation of `__private_get_type_id__`, since // it requires returning a private type. We can therefore @@ -53,7 +62,9 @@ macro_rules! downcast { /// Downcasts a generic body to a mutable specific type. pub fn downcast_mut(&mut self) -> Option<&mut T> { - if self.__private_get_type_id__().0 == std::any::TypeId::of::() { + if self.__private_get_type_id__(PrivateHelper(())).0 + == std::any::TypeId::of::() + { // SAFETY: external crates cannot override the default // implementation of `__private_get_type_id__`, since // it requires returning a private type. We can therefore From ddaf8c3e4379d915ff9ba05bcf2d55227c21ab8e Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Wed, 5 May 2021 18:36:02 +0100 Subject: [PATCH 9/9] add associated error type to MessageBody (#2183) --- actix-http/CHANGES.md | 4 + actix-http/src/body/body.rs | 75 +++++++++++++++++-- actix-http/src/body/body_stream.rs | 4 +- actix-http/src/body/message_body.rs | 105 +++++++++++++++++++++++---- actix-http/src/body/mod.rs | 10 ++- actix-http/src/body/response_body.rs | 25 +++++-- actix-http/src/body/sized_stream.rs | 4 +- actix-http/src/builder.rs | 8 +- actix-http/src/client/connection.rs | 5 +- actix-http/src/client/h1proto.rs | 9 ++- actix-http/src/client/h2proto.rs | 30 +++++--- actix-http/src/encoding/encoder.rs | 85 +++++++++++++++++++--- actix-http/src/error.rs | 6 +- actix-http/src/h1/dispatcher.rs | 32 ++++++++ actix-http/src/h1/service.rs | 22 ++++++ actix-http/src/h1/utils.rs | 2 + actix-http/src/h2/dispatcher.rs | 7 ++ actix-http/src/h2/service.rs | 13 ++++ actix-http/src/response.rs | 6 +- actix-http/src/service.rs | 31 +++++++- actix-test/src/lib.rs | 2 + src/middleware/compat.rs | 6 +- src/middleware/logger.rs | 13 +++- src/response/response.rs | 6 +- src/server.rs | 1 + src/service.rs | 6 +- src/test.rs | 4 + 27 files changed, 447 insertions(+), 74 deletions(-) diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index cc6330288..f398b1c92 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -2,11 +2,13 @@ ## Unreleased - 2021-xx-xx ### Added +* `BoxAnyBody`: a boxed message body with boxed errors. [#2183] * Re-export `http` crate's `Error` type as `error::HttpError`. [#2171] * Re-export `StatusCode`, `Method`, `Version` and `Uri` at the crate root. [#2171] * Re-export `ContentEncoding` and `ConnectionType` at the crate root. [#2171] ### Changed +* The `MessageBody` trait now has an associated `Error` type. [#2183] * `header` mod is now public. [#2171] * `uri` mod is now public. [#2171] * Update `language-tags` to `0.3`. @@ -14,8 +16,10 @@ ### Removed * Stop re-exporting `http` crate's `HeaderMap` types in addition to ours. [#2171] +* Down-casting for `MessageBody` types. [#2183] [#2171]: https://github.com/actix/actix-web/pull/2171 +[#2183]: https://github.com/actix/actix-web/pull/2183 [#2196]: https://github.com/actix/actix-web/pull/2196 diff --git a/actix-http/src/body/body.rs b/actix-http/src/body/body.rs index 4fe18338a..4c95bd31a 100644 --- a/actix-http/src/body/body.rs +++ b/actix-http/src/body/body.rs @@ -1,16 +1,17 @@ use std::{ borrow::Cow, + error::Error as StdError, fmt, mem, pin::Pin, task::{Context, Poll}, }; use bytes::{Bytes, BytesMut}; -use futures_core::Stream; +use futures_core::{ready, Stream}; use crate::error::Error; -use super::{BodySize, BodyStream, MessageBody, SizedStream}; +use super::{BodySize, BodyStream, MessageBody, MessageBodyMapErr, SizedStream}; /// Represents various types of HTTP message body. // #[deprecated(since = "4.0.0", note = "Use body types directly.")] @@ -25,7 +26,7 @@ pub enum Body { Bytes(Bytes), /// Generic message body. - Message(Pin>), + Message(BoxAnyBody), } impl Body { @@ -35,12 +36,18 @@ impl Body { } /// Create body from generic message body. - pub fn from_message(body: B) -> Body { - Body::Message(Box::pin(body)) + pub fn from_message(body: B) -> Body + where + B: MessageBody + 'static, + B::Error: Into>, + { + Self::Message(BoxAnyBody::from_body(body)) } } impl MessageBody for Body { + type Error = Error; + fn size(&self) -> BodySize { match self { Body::None => BodySize::None, @@ -53,7 +60,7 @@ impl MessageBody for Body { fn poll_next( self: Pin<&mut Self>, cx: &mut Context<'_>, - ) -> Poll>> { + ) -> Poll>> { match self.get_mut() { Body::None => Poll::Ready(None), Body::Empty => Poll::Ready(None), @@ -65,7 +72,13 @@ impl MessageBody for Body { Poll::Ready(Some(Ok(mem::take(bin)))) } } - Body::Message(body) => body.as_mut().poll_next(cx), + + // TODO: MSRV 1.51: poll_map_err + Body::Message(body) => match ready!(body.as_pin_mut().poll_next(cx)) { + Some(Err(err)) => Poll::Ready(Some(Err(err.into()))), + Some(Ok(val)) => Poll::Ready(Some(Ok(val))), + None => Poll::Ready(None), + }, } } } @@ -166,3 +179,51 @@ where Body::from_message(s) } } + +/// A boxed message body with boxed errors. +pub struct BoxAnyBody(Pin>>>); + +impl BoxAnyBody { + /// Boxes a `MessageBody` and any errors it generates. + pub fn from_body(body: B) -> Self + where + B: MessageBody + 'static, + B::Error: Into>, + { + let body = MessageBodyMapErr::new(body, Into::into); + Self(Box::pin(body)) + } + + /// Returns a mutable pinned reference to the inner message body type. + pub fn as_pin_mut( + &mut self, + ) -> Pin<&mut (dyn MessageBody>)> { + self.0.as_mut() + } +} + +impl fmt::Debug for BoxAnyBody { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("BoxAnyBody(dyn MessageBody)") + } +} + +impl MessageBody for BoxAnyBody { + type Error = Error; + + fn size(&self) -> BodySize { + self.0.size() + } + + fn poll_next( + mut self: Pin<&mut Self>, + cx: &mut Context<'_>, + ) -> Poll>> { + // TODO: MSRV 1.51: poll_map_err + match ready!(self.0.as_mut().poll_next(cx)) { + Some(Err(err)) => Poll::Ready(Some(Err(err.into()))), + Some(Ok(val)) => Poll::Ready(Some(Ok(val))), + None => Poll::Ready(None), + } + } +} diff --git a/actix-http/src/body/body_stream.rs b/actix-http/src/body/body_stream.rs index b81aeb4c1..ebe872022 100644 --- a/actix-http/src/body/body_stream.rs +++ b/actix-http/src/body/body_stream.rs @@ -36,6 +36,8 @@ where S: Stream>, E: Into, { + type Error = Error; + fn size(&self) -> BodySize { BodySize::Stream } @@ -48,7 +50,7 @@ where fn poll_next( mut self: Pin<&mut Self>, cx: &mut Context<'_>, - ) -> Poll>> { + ) -> Poll>> { loop { let stream = self.as_mut().project().stream; diff --git a/actix-http/src/body/message_body.rs b/actix-http/src/body/message_body.rs index 894a5fa98..2d2642ba7 100644 --- a/actix-http/src/body/message_body.rs +++ b/actix-http/src/body/message_body.rs @@ -1,12 +1,15 @@ //! [`MessageBody`] trait and foreign implementations. use std::{ + convert::Infallible, mem, pin::Pin, task::{Context, Poll}, }; use bytes::{Bytes, BytesMut}; +use futures_core::ready; +use pin_project_lite::pin_project; use crate::error::Error; @@ -14,6 +17,8 @@ use super::BodySize; /// An interface for response bodies. pub trait MessageBody { + type Error; + /// Body size hint. fn size(&self) -> BodySize; @@ -21,14 +26,12 @@ pub trait MessageBody { fn poll_next( self: Pin<&mut Self>, cx: &mut Context<'_>, - ) -> Poll>>; - - downcast_get_type_id!(); + ) -> Poll>>; } -downcast!(MessageBody); - impl MessageBody for () { + type Error = Infallible; + fn size(&self) -> BodySize { BodySize::Empty } @@ -36,12 +39,18 @@ impl MessageBody for () { fn poll_next( self: Pin<&mut Self>, _: &mut Context<'_>, - ) -> Poll>> { + ) -> Poll>> { Poll::Ready(None) } } -impl MessageBody for Box { +impl MessageBody for Box +where + B: MessageBody + Unpin, + B::Error: Into, +{ + type Error = B::Error; + fn size(&self) -> BodySize { self.as_ref().size() } @@ -49,12 +58,18 @@ impl MessageBody for Box { fn poll_next( self: Pin<&mut Self>, cx: &mut Context<'_>, - ) -> Poll>> { + ) -> Poll>> { Pin::new(self.get_mut().as_mut()).poll_next(cx) } } -impl MessageBody for Pin> { +impl MessageBody for Pin> +where + B: MessageBody, + B::Error: Into, +{ + type Error = B::Error; + fn size(&self) -> BodySize { self.as_ref().size() } @@ -62,12 +77,14 @@ impl MessageBody for Pin> { fn poll_next( mut self: Pin<&mut Self>, cx: &mut Context<'_>, - ) -> Poll>> { + ) -> Poll>> { self.as_mut().poll_next(cx) } } impl MessageBody for Bytes { + type Error = Infallible; + fn size(&self) -> BodySize { BodySize::Sized(self.len() as u64) } @@ -75,7 +92,7 @@ impl MessageBody for Bytes { fn poll_next( self: Pin<&mut Self>, _: &mut Context<'_>, - ) -> Poll>> { + ) -> Poll>> { if self.is_empty() { Poll::Ready(None) } else { @@ -85,6 +102,8 @@ impl MessageBody for Bytes { } impl MessageBody for BytesMut { + type Error = Infallible; + fn size(&self) -> BodySize { BodySize::Sized(self.len() as u64) } @@ -92,7 +111,7 @@ impl MessageBody for BytesMut { fn poll_next( self: Pin<&mut Self>, _: &mut Context<'_>, - ) -> Poll>> { + ) -> Poll>> { if self.is_empty() { Poll::Ready(None) } else { @@ -102,6 +121,8 @@ impl MessageBody for BytesMut { } impl MessageBody for &'static str { + type Error = Infallible; + fn size(&self) -> BodySize { BodySize::Sized(self.len() as u64) } @@ -109,7 +130,7 @@ impl MessageBody for &'static str { fn poll_next( self: Pin<&mut Self>, _: &mut Context<'_>, - ) -> Poll>> { + ) -> Poll>> { if self.is_empty() { Poll::Ready(None) } else { @@ -121,6 +142,8 @@ impl MessageBody for &'static str { } impl MessageBody for Vec { + type Error = Infallible; + fn size(&self) -> BodySize { BodySize::Sized(self.len() as u64) } @@ -128,7 +151,7 @@ impl MessageBody for Vec { fn poll_next( self: Pin<&mut Self>, _: &mut Context<'_>, - ) -> Poll>> { + ) -> Poll>> { if self.is_empty() { Poll::Ready(None) } else { @@ -138,6 +161,8 @@ impl MessageBody for Vec { } impl MessageBody for String { + type Error = Infallible; + fn size(&self) -> BodySize { BodySize::Sized(self.len() as u64) } @@ -145,7 +170,7 @@ impl MessageBody for String { fn poll_next( self: Pin<&mut Self>, _: &mut Context<'_>, - ) -> Poll>> { + ) -> Poll>> { if self.is_empty() { Poll::Ready(None) } else { @@ -155,3 +180,53 @@ impl MessageBody for String { } } } + +pin_project! { + pub(crate) struct MessageBodyMapErr { + #[pin] + body: B, + mapper: Option, + } +} + +impl MessageBodyMapErr +where + B: MessageBody, + F: FnOnce(B::Error) -> E, +{ + pub(crate) fn new(body: B, mapper: F) -> Self { + Self { + body, + mapper: Some(mapper), + } + } +} + +impl MessageBody for MessageBodyMapErr +where + B: MessageBody, + F: FnOnce(B::Error) -> E, +{ + type Error = E; + + fn size(&self) -> BodySize { + self.body.size() + } + + fn poll_next( + mut self: Pin<&mut Self>, + cx: &mut Context<'_>, + ) -> Poll>> { + let this = self.as_mut().project(); + + match ready!(this.body.poll_next(cx)) { + Some(Err(err)) => { + let f = self.as_mut().project().mapper.take().unwrap(); + let mapped_err = (f)(err); + Poll::Ready(Some(Err(mapped_err))) + } + Some(Ok(val)) => Poll::Ready(Some(Ok(val))), + None => Poll::Ready(None), + } + } +} diff --git a/actix-http/src/body/mod.rs b/actix-http/src/body/mod.rs index f26d6a8cf..d21cca60b 100644 --- a/actix-http/src/body/mod.rs +++ b/actix-http/src/body/mod.rs @@ -15,9 +15,10 @@ mod response_body; mod size; mod sized_stream; -pub use self::body::Body; +pub use self::body::{Body, BoxAnyBody}; pub use self::body_stream::BodyStream; pub use self::message_body::MessageBody; +pub(crate) use self::message_body::MessageBodyMapErr; pub use self::response_body::ResponseBody; pub use self::size::BodySize; pub use self::sized_stream::SizedStream; @@ -41,7 +42,7 @@ pub use self::sized_stream::SizedStream; /// assert_eq!(bytes, b"123"[..]); /// # } /// ``` -pub async fn to_bytes(body: impl MessageBody) -> Result { +pub async fn to_bytes(body: B) -> Result { let cap = match body.size() { BodySize::None | BodySize::Empty | BodySize::Sized(0) => return Ok(Bytes::new()), BodySize::Sized(size) => size as usize, @@ -237,10 +238,13 @@ mod tests { ); } + // down-casting used to be done with a method on MessageBody trait + // test is kept to demonstrate equivalence of Any trait #[actix_rt::test] async fn test_body_casting() { let mut body = String::from("hello cast"); - let resp_body: &mut dyn MessageBody = &mut body; + // let mut resp_body: &mut dyn MessageBody = &mut body; + let resp_body: &mut dyn std::any::Any = &mut body; let body = resp_body.downcast_ref::().unwrap(); assert_eq!(body, "hello cast"); let body = &mut resp_body.downcast_mut::().unwrap(); diff --git a/actix-http/src/body/response_body.rs b/actix-http/src/body/response_body.rs index b27112475..855c742f2 100644 --- a/actix-http/src/body/response_body.rs +++ b/actix-http/src/body/response_body.rs @@ -5,7 +5,7 @@ use std::{ }; use bytes::Bytes; -use futures_core::Stream; +use futures_core::{ready, Stream}; use pin_project::pin_project; use crate::error::Error; @@ -43,7 +43,13 @@ impl ResponseBody { } } -impl MessageBody for ResponseBody { +impl MessageBody for ResponseBody +where + B: MessageBody, + B::Error: Into, +{ + type Error = Error; + fn size(&self) -> BodySize { match self { ResponseBody::Body(ref body) => body.size(), @@ -54,12 +60,16 @@ impl MessageBody for ResponseBody { fn poll_next( self: Pin<&mut Self>, cx: &mut Context<'_>, - ) -> Poll>> { + ) -> Poll>> { Stream::poll_next(self, cx) } } -impl Stream for ResponseBody { +impl Stream for ResponseBody +where + B: MessageBody, + B::Error: Into, +{ type Item = Result; fn poll_next( @@ -67,7 +77,12 @@ impl Stream for ResponseBody { cx: &mut Context<'_>, ) -> Poll> { match self.project() { - ResponseBodyProj::Body(body) => body.poll_next(cx), + // TODO: MSRV 1.51: poll_map_err + ResponseBodyProj::Body(body) => match ready!(body.poll_next(cx)) { + Some(Err(err)) => Poll::Ready(Some(Err(err.into()))), + Some(Ok(val)) => Poll::Ready(Some(Ok(val))), + None => Poll::Ready(None), + }, ResponseBodyProj::Other(body) => Pin::new(body).poll_next(cx), } } diff --git a/actix-http/src/body/sized_stream.rs b/actix-http/src/body/sized_stream.rs index f0332fc8f..4af132389 100644 --- a/actix-http/src/body/sized_stream.rs +++ b/actix-http/src/body/sized_stream.rs @@ -36,6 +36,8 @@ impl MessageBody for SizedStream where S: Stream>, { + type Error = Error; + fn size(&self) -> BodySize { BodySize::Sized(self.size as u64) } @@ -48,7 +50,7 @@ where fn poll_next( mut self: Pin<&mut Self>, cx: &mut Context<'_>, - ) -> Poll>> { + ) -> Poll>> { loop { let stream = self.as_mut().project().stream; diff --git a/actix-http/src/builder.rs b/actix-http/src/builder.rs index 623bfdda2..660cd9817 100644 --- a/actix-http/src/builder.rs +++ b/actix-http/src/builder.rs @@ -202,11 +202,13 @@ where /// Finish service configuration and create a HTTP service for HTTP/2 protocol. pub fn h2(self, service: F) -> H2Service where - B: MessageBody + 'static, F: IntoServiceFactory, S::Error: Into + 'static, S::InitError: fmt::Debug, S::Response: Into> + 'static, + + B: MessageBody + 'static, + B::Error: Into, { let cfg = ServiceConfig::new( self.keep_alive, @@ -223,11 +225,13 @@ where /// Finish service configuration and create `HttpService` instance. pub fn finish(self, service: F) -> HttpService where - B: MessageBody + 'static, F: IntoServiceFactory, S::Error: Into + 'static, S::InitError: fmt::Debug, S::Response: Into> + 'static, + + B: MessageBody + 'static, + B::Error: Into, { let cfg = ServiceConfig::new( self.keep_alive, diff --git a/actix-http/src/client/connection.rs b/actix-http/src/client/connection.rs index 0e3e97f3f..a30f651ca 100644 --- a/actix-http/src/client/connection.rs +++ b/actix-http/src/client/connection.rs @@ -12,10 +12,10 @@ use bytes::Bytes; use futures_core::future::LocalBoxFuture; use h2::client::SendRequest; -use crate::body::MessageBody; use crate::h1::ClientCodec; use crate::message::{RequestHeadType, ResponseHead}; use crate::payload::Payload; +use crate::{body::MessageBody, Error}; use super::error::SendRequestError; use super::pool::Acquired; @@ -256,8 +256,9 @@ where body: RB, ) -> LocalBoxFuture<'static, Result<(ResponseHead, Payload), SendRequestError>> where - RB: MessageBody + 'static, H: Into + 'static, + RB: MessageBody + 'static, + RB::Error: Into, { Box::pin(async move { match self { diff --git a/actix-http/src/client/h1proto.rs b/actix-http/src/client/h1proto.rs index fa4469d35..65a30748c 100644 --- a/actix-http/src/client/h1proto.rs +++ b/actix-http/src/client/h1proto.rs @@ -11,7 +11,6 @@ use bytes::{Bytes, BytesMut}; use futures_core::{ready, Stream}; use futures_util::SinkExt as _; -use crate::error::PayloadError; use crate::h1; use crate::http::{ header::{HeaderMap, IntoHeaderValue, EXPECT, HOST}, @@ -19,6 +18,7 @@ use crate::http::{ }; use crate::message::{RequestHeadType, ResponseHead}; use crate::payload::Payload; +use crate::{error::PayloadError, Error}; use super::connection::{ConnectionIo, H1Connection}; use super::error::{ConnectError, SendRequestError}; @@ -32,6 +32,7 @@ pub(crate) async fn send_request( where Io: ConnectionIo, B: MessageBody, + B::Error: Into, { // set request host header if !head.as_ref().headers.contains_key(HOST) @@ -154,6 +155,7 @@ pub(crate) async fn send_body( where Io: ConnectionIo, B: MessageBody, + B::Error: Into, { actix_rt::pin!(body); @@ -161,9 +163,10 @@ where while !eof { while !eof && !framed.as_ref().is_write_buf_full() { match poll_fn(|cx| body.as_mut().poll_next(cx)).await { - Some(result) => { - framed.as_mut().write(h1::Message::Chunk(Some(result?)))?; + Some(Ok(chunk)) => { + framed.as_mut().write(h1::Message::Chunk(Some(chunk)))?; } + Some(Err(err)) => return Err(err.into().into()), None => { eof = true; framed.as_mut().write(h1::Message::Chunk(None))?; diff --git a/actix-http/src/client/h2proto.rs b/actix-http/src/client/h2proto.rs index 8cb2e2522..cf423ef12 100644 --- a/actix-http/src/client/h2proto.rs +++ b/actix-http/src/client/h2proto.rs @@ -9,14 +9,19 @@ use h2::{ use http::header::{HeaderValue, CONNECTION, CONTENT_LENGTH, TRANSFER_ENCODING}; use http::{request::Request, Method, Version}; -use crate::body::{BodySize, MessageBody}; -use crate::header::HeaderMap; -use crate::message::{RequestHeadType, ResponseHead}; -use crate::payload::Payload; +use crate::{ + body::{BodySize, MessageBody}, + header::HeaderMap, + message::{RequestHeadType, ResponseHead}, + payload::Payload, + Error, +}; -use super::config::ConnectorConfig; -use super::connection::{ConnectionIo, H2Connection}; -use super::error::SendRequestError; +use super::{ + config::ConnectorConfig, + connection::{ConnectionIo, H2Connection}, + error::SendRequestError, +}; pub(crate) async fn send_request( mut io: H2Connection, @@ -26,6 +31,7 @@ pub(crate) async fn send_request( where Io: ConnectionIo, B: MessageBody, + B::Error: Into, { trace!("Sending client request: {:?} {:?}", head, body.size()); @@ -125,10 +131,14 @@ where Ok((head, payload)) } -async fn send_body( +async fn send_body( body: B, mut send: SendStream, -) -> Result<(), SendRequestError> { +) -> Result<(), SendRequestError> +where + B: MessageBody, + B::Error: Into, +{ let mut buf = None; actix_rt::pin!(body); loop { @@ -138,7 +148,7 @@ async fn send_body( send.reserve_capacity(b.len()); buf = Some(b); } - Some(Err(e)) => return Err(e.into()), + Some(Err(e)) => return Err(e.into().into()), None => { if let Err(e) = send.send_data(Bytes::new(), true) { return Err(e.into()); diff --git a/actix-http/src/encoding/encoder.rs b/actix-http/src/encoding/encoder.rs index add6ee980..b8bc8b68d 100644 --- a/actix-http/src/encoding/encoder.rs +++ b/actix-http/src/encoding/encoder.rs @@ -1,6 +1,7 @@ //! Stream encoders. use std::{ + error::Error as StdError, future::Future, io::{self, Write as _}, pin::Pin, @@ -10,12 +11,13 @@ use std::{ use actix_rt::task::{spawn_blocking, JoinHandle}; use brotli2::write::BrotliEncoder; use bytes::Bytes; +use derive_more::Display; use flate2::write::{GzEncoder, ZlibEncoder}; use futures_core::ready; use pin_project::pin_project; use crate::{ - body::{Body, BodySize, MessageBody, ResponseBody}, + body::{Body, BodySize, BoxAnyBody, MessageBody, ResponseBody}, http::{ header::{ContentEncoding, CONTENT_ENCODING}, HeaderValue, StatusCode, @@ -92,10 +94,16 @@ impl Encoder { enum EncoderBody { Bytes(Bytes), Stream(#[pin] B), - BoxedStream(Pin>), + BoxedStream(BoxAnyBody), } -impl MessageBody for EncoderBody { +impl MessageBody for EncoderBody +where + B: MessageBody, + B::Error: Into, +{ + type Error = EncoderError; + fn size(&self) -> BodySize { match self { EncoderBody::Bytes(ref b) => b.size(), @@ -107,7 +115,7 @@ impl MessageBody for EncoderBody { fn poll_next( self: Pin<&mut Self>, cx: &mut Context<'_>, - ) -> Poll>> { + ) -> Poll>> { match self.project() { EncoderBodyProj::Bytes(b) => { if b.is_empty() { @@ -116,13 +124,32 @@ impl MessageBody for EncoderBody { Poll::Ready(Some(Ok(std::mem::take(b)))) } } - EncoderBodyProj::Stream(b) => b.poll_next(cx), - EncoderBodyProj::BoxedStream(ref mut b) => b.as_mut().poll_next(cx), + // TODO: MSRV 1.51: poll_map_err + EncoderBodyProj::Stream(b) => match ready!(b.poll_next(cx)) { + Some(Err(err)) => Poll::Ready(Some(Err(EncoderError::Body(err)))), + Some(Ok(val)) => Poll::Ready(Some(Ok(val))), + None => Poll::Ready(None), + }, + EncoderBodyProj::BoxedStream(ref mut b) => { + match ready!(b.as_pin_mut().poll_next(cx)) { + Some(Err(err)) => { + Poll::Ready(Some(Err(EncoderError::Boxed(err.into())))) + } + Some(Ok(val)) => Poll::Ready(Some(Ok(val))), + None => Poll::Ready(None), + } + } } } } -impl MessageBody for Encoder { +impl MessageBody for Encoder +where + B: MessageBody, + B::Error: Into, +{ + type Error = EncoderError; + fn size(&self) -> BodySize { if self.encoder.is_none() { self.body.size() @@ -134,7 +161,7 @@ impl MessageBody for Encoder { fn poll_next( self: Pin<&mut Self>, cx: &mut Context<'_>, - ) -> Poll>> { + ) -> Poll>> { let mut this = self.project(); loop { if *this.eof { @@ -142,8 +169,9 @@ impl MessageBody for Encoder { } if let Some(ref mut fut) = this.fut { - let mut encoder = - ready!(Pin::new(fut).poll(cx)).map_err(|_| BlockingError)??; + let mut encoder = ready!(Pin::new(fut).poll(cx)) + .map_err(|_| EncoderError::Blocking(BlockingError))? + .map_err(EncoderError::Io)?; let chunk = encoder.take(); *this.encoder = Some(encoder); @@ -162,7 +190,7 @@ impl MessageBody for Encoder { Some(Ok(chunk)) => { if let Some(mut encoder) = this.encoder.take() { if chunk.len() < MAX_CHUNK_SIZE_ENCODE_IN_PLACE { - encoder.write(&chunk)?; + encoder.write(&chunk).map_err(EncoderError::Io)?; let chunk = encoder.take(); *this.encoder = Some(encoder); @@ -182,7 +210,7 @@ impl MessageBody for Encoder { None => { if let Some(encoder) = this.encoder.take() { - let chunk = encoder.finish()?; + let chunk = encoder.finish().map_err(EncoderError::Io)?; if chunk.is_empty() { return Poll::Ready(None); } else { @@ -281,3 +309,36 @@ impl ContentEncoder { } } } + +#[derive(Debug, Display)] +#[non_exhaustive] +pub enum EncoderError { + #[display(fmt = "body")] + Body(E), + + #[display(fmt = "boxed")] + Boxed(Error), + + #[display(fmt = "blocking")] + Blocking(BlockingError), + + #[display(fmt = "io")] + Io(io::Error), +} + +impl StdError for EncoderError { + fn source(&self) -> Option<&(dyn StdError + 'static)> { + None + } +} + +impl> From> for Error { + fn from(err: EncoderError) -> Self { + match err { + EncoderError::Body(err) => err.into(), + EncoderError::Boxed(err) => err, + EncoderError::Blocking(err) => err.into(), + EncoderError::Io(err) => err.into(), + } + } +} diff --git a/actix-http/src/error.rs b/actix-http/src/error.rs index 39ffa29e7..c92f9076d 100644 --- a/actix-http/src/error.rs +++ b/actix-http/src/error.rs @@ -2,6 +2,7 @@ use std::{ cell::RefCell, + error::Error as StdError, fmt, io::{self, Write as _}, str::Utf8Error, @@ -105,8 +106,7 @@ impl From<()> for Error { impl From for Error { fn from(_: std::convert::Infallible) -> Self { - // `std::convert::Infallible` indicates an error - // that will never happen + // hint that an error that will never happen unreachable!() } } @@ -145,6 +145,8 @@ impl From for Error { #[display(fmt = "Unknown Error")] struct UnitError; +impl ResponseError for Box {} + /// Returns [`StatusCode::INTERNAL_SERVER_ERROR`] for [`UnitError`]. impl ResponseError for UnitError {} diff --git a/actix-http/src/h1/dispatcher.rs b/actix-http/src/h1/dispatcher.rs index 3b272f0fb..7ab89ba87 100644 --- a/actix-http/src/h1/dispatcher.rs +++ b/actix-http/src/h1/dispatcher.rs @@ -51,9 +51,13 @@ pub struct Dispatcher where S: Service, S::Error: Into, + B: MessageBody, + B::Error: Into, + X: Service, X::Error: Into, + U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display, { @@ -69,9 +73,13 @@ enum DispatcherState where S: Service, S::Error: Into, + B: MessageBody, + B::Error: Into, + X: Service, X::Error: Into, + U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display, { @@ -84,9 +92,13 @@ struct InnerDispatcher where S: Service, S::Error: Into, + B: MessageBody, + B::Error: Into, + X: Service, X::Error: Into, + U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display, { @@ -122,7 +134,9 @@ enum State where S: Service, X: Service, + B: MessageBody, + B::Error: Into, { None, ExpectCall(#[pin] X::Future), @@ -133,8 +147,11 @@ where impl State where S: Service, + X: Service, + B: MessageBody, + B::Error: Into, { fn is_empty(&self) -> bool { matches!(self, State::None) @@ -150,12 +167,17 @@ enum PollResponse { impl Dispatcher where T: AsyncRead + AsyncWrite + Unpin, + S: Service, S::Error: Into, S::Response: Into>, + B: MessageBody, + B::Error: Into, + X: Service, X::Error: Into, + U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display, { @@ -206,12 +228,17 @@ where impl InnerDispatcher where T: AsyncRead + AsyncWrite + Unpin, + S: Service, S::Error: Into, S::Response: Into>, + B: MessageBody, + B::Error: Into, + X: Service, X::Error: Into, + U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display, { @@ -817,12 +844,17 @@ where impl Future for Dispatcher where T: AsyncRead + AsyncWrite + Unpin, + S: Service, S::Error: Into, S::Response: Into>, + B: MessageBody, + B::Error: Into, + X: Service, X::Error: Into, + U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display, { diff --git a/actix-http/src/h1/service.rs b/actix-http/src/h1/service.rs index 916643a18..1ab85cbf3 100644 --- a/actix-http/src/h1/service.rs +++ b/actix-http/src/h1/service.rs @@ -64,11 +64,15 @@ where S::Error: Into, S::InitError: fmt::Debug, S::Response: Into>, + B: MessageBody, + B::Error: Into, + X: ServiceFactory, X::Future: 'static, X::Error: Into, X::InitError: fmt::Debug, + U: ServiceFactory<(Request, Framed), Config = (), Response = ()>, U::Future: 'static, U::Error: fmt::Display + Into, @@ -109,11 +113,15 @@ mod openssl { S::Error: Into, S::InitError: fmt::Debug, S::Response: Into>, + B: MessageBody, + B::Error: Into, + X: ServiceFactory, X::Future: 'static, X::Error: Into, X::InitError: fmt::Debug, + U: ServiceFactory< (Request, Framed, Codec>), Config = (), @@ -165,11 +173,15 @@ mod rustls { S::Error: Into, S::InitError: fmt::Debug, S::Response: Into>, + B: MessageBody, + B::Error: Into, + X: ServiceFactory, X::Future: 'static, X::Error: Into, X::InitError: fmt::Debug, + U: ServiceFactory< (Request, Framed, Codec>), Config = (), @@ -253,16 +265,21 @@ impl ServiceFactory<(T, Option)> for H1Service where T: AsyncRead + AsyncWrite + Unpin + 'static, + S: ServiceFactory, S::Future: 'static, S::Error: Into, S::Response: Into>, S::InitError: fmt::Debug, + B: MessageBody, + B::Error: Into, + X: ServiceFactory, X::Future: 'static, X::Error: Into, X::InitError: fmt::Debug, + U: ServiceFactory<(Request, Framed), Config = (), Response = ()>, U::Future: 'static, U::Error: fmt::Display + Into, @@ -319,12 +336,17 @@ impl Service<(T, Option)> for HttpServiceHandler where T: AsyncRead + AsyncWrite + Unpin, + S: Service, S::Error: Into, S::Response: Into>, + B: MessageBody, + B::Error: Into, + X: Service, X::Error: Into, + U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display + Into, { diff --git a/actix-http/src/h1/utils.rs b/actix-http/src/h1/utils.rs index 9e9c57137..73f01e913 100644 --- a/actix-http/src/h1/utils.rs +++ b/actix-http/src/h1/utils.rs @@ -22,6 +22,7 @@ pub struct SendResponse { impl SendResponse where B: MessageBody, + B::Error: Into, { pub fn new(framed: Framed, response: Response) -> Self { let (res, body) = response.into_parts(); @@ -38,6 +39,7 @@ impl Future for SendResponse where T: AsyncRead + AsyncWrite + Unpin, B: MessageBody + Unpin, + B::Error: Into, { type Output = Result, Error>; diff --git a/actix-http/src/h2/dispatcher.rs b/actix-http/src/h2/dispatcher.rs index 87dd66fe7..07636470b 100644 --- a/actix-http/src/h2/dispatcher.rs +++ b/actix-http/src/h2/dispatcher.rs @@ -69,11 +69,14 @@ where impl Future for Dispatcher where T: AsyncRead + AsyncWrite + Unpin, + S: Service, S::Error: Into + 'static, S::Future: 'static, S::Response: Into> + 'static, + B: MessageBody + 'static, + B::Error: Into, { type Output = Result<(), DispatchError>; @@ -140,7 +143,9 @@ where F: Future>, E: Into, I: Into>, + B: MessageBody, + B::Error: Into, { fn prepare_response( &self, @@ -216,7 +221,9 @@ where F: Future>, E: Into, I: Into>, + B: MessageBody, + B::Error: Into, { type Output = (); diff --git a/actix-http/src/h2/service.rs b/actix-http/src/h2/service.rs index 1a0b8c7f5..a75abef7d 100644 --- a/actix-http/src/h2/service.rs +++ b/actix-http/src/h2/service.rs @@ -40,7 +40,9 @@ where S::Error: Into + 'static, S::Response: Into> + 'static, >::Future: 'static, + B: MessageBody + 'static, + B::Error: Into, { /// Create new `H2Service` instance with config. pub(crate) fn with_config>( @@ -69,7 +71,9 @@ where S::Error: Into + 'static, S::Response: Into> + 'static, >::Future: 'static, + B: MessageBody + 'static, + B::Error: Into, { /// Create plain TCP based service pub fn tcp( @@ -106,7 +110,9 @@ mod openssl { S::Error: Into + 'static, S::Response: Into> + 'static, >::Future: 'static, + B: MessageBody + 'static, + B::Error: Into, { /// Create OpenSSL based service pub fn openssl( @@ -150,7 +156,9 @@ mod rustls { S::Error: Into + 'static, S::Response: Into> + 'static, >::Future: 'static, + B: MessageBody + 'static, + B::Error: Into, { /// Create Rustls based service pub fn rustls( @@ -185,12 +193,15 @@ mod rustls { impl ServiceFactory<(T, Option)> for H2Service where T: AsyncRead + AsyncWrite + Unpin + 'static, + S: ServiceFactory, S::Future: 'static, S::Error: Into + 'static, S::Response: Into> + 'static, >::Future: 'static, + B: MessageBody + 'static, + B::Error: Into, { type Response = (); type Error = DispatchError; @@ -252,6 +263,7 @@ where S::Future: 'static, S::Response: Into> + 'static, B: MessageBody + 'static, + B::Error: Into, { type Response = (); type Error = DispatchError; @@ -316,6 +328,7 @@ where S::Future: 'static, S::Response: Into> + 'static, B: MessageBody, + B::Error: Into, { type Output = Result<(), DispatchError>; diff --git a/actix-http/src/response.rs b/actix-http/src/response.rs index e11ceb18f..da5c7e000 100644 --- a/actix-http/src/response.rs +++ b/actix-http/src/response.rs @@ -242,7 +242,11 @@ impl Response { } } -impl fmt::Debug for Response { +impl fmt::Debug for Response +where + B: MessageBody, + B::Error: Into, +{ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let res = writeln!( f, diff --git a/actix-http/src/service.rs b/actix-http/src/service.rs index ff4b49f1d..d25a67a19 100644 --- a/actix-http/src/service.rs +++ b/actix-http/src/service.rs @@ -59,6 +59,7 @@ where S::Response: Into> + 'static, >::Future: 'static, B: MessageBody + 'static, + B::Error: Into, { /// Create new `HttpService` instance. pub fn new>(service: F) -> Self { @@ -157,6 +158,7 @@ where >::Future: 'static, B: MessageBody + 'static, + B::Error: Into, X: ServiceFactory, X::Future: 'static, @@ -208,6 +210,7 @@ mod openssl { >::Future: 'static, B: MessageBody + 'static, + B::Error: Into, X: ServiceFactory, X::Future: 'static, @@ -275,6 +278,7 @@ mod rustls { >::Future: 'static, B: MessageBody + 'static, + B::Error: Into, X: ServiceFactory, X::Future: 'static, @@ -339,6 +343,7 @@ where >::Future: 'static, B: MessageBody + 'static, + B::Error: Into, X: ServiceFactory, X::Future: 'static, @@ -465,13 +470,18 @@ impl Service<(T, Protocol, Option)> for HttpServiceHandler where T: AsyncRead + AsyncWrite + Unpin, + S: Service, S::Error: Into + 'static, S::Future: 'static, S::Response: Into> + 'static, + B: MessageBody + 'static, + B::Error: Into, + X: Service, X::Error: Into, + U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display + Into, { @@ -522,13 +532,18 @@ where #[pin_project(project = StateProj)] enum State where + T: AsyncRead + AsyncWrite + Unpin, + S: Service, S::Future: 'static, S::Error: Into, - T: AsyncRead + AsyncWrite + Unpin, + B: MessageBody, + B::Error: Into, + X: Service, X::Error: Into, + U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display, { @@ -549,13 +564,18 @@ where pub struct HttpServiceHandlerResponse where T: AsyncRead + AsyncWrite + Unpin, + S: Service, S::Error: Into + 'static, S::Future: 'static, S::Response: Into> + 'static, - B: MessageBody + 'static, + + B: MessageBody, + B::Error: Into, + X: Service, X::Error: Into, + U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display, { @@ -566,13 +586,18 @@ where impl Future for HttpServiceHandlerResponse where T: AsyncRead + AsyncWrite + Unpin, + S: Service, S::Error: Into + 'static, S::Future: 'static, S::Response: Into> + 'static, - B: MessageBody, + + B: MessageBody + 'static, + B::Error: Into, + X: Service, X::Error: Into, + U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display, { diff --git a/actix-test/src/lib.rs b/actix-test/src/lib.rs index 8fab33289..5d85c2687 100644 --- a/actix-test/src/lib.rs +++ b/actix-test/src/lib.rs @@ -86,6 +86,7 @@ where S::Response: Into> + 'static, >::Future: 'static, B: MessageBody + 'static, + B::Error: Into, { start_with(TestServerConfig::default(), factory) } @@ -125,6 +126,7 @@ where S::Response: Into> + 'static, >::Future: 'static, B: MessageBody + 'static, + B::Error: Into, { let (tx, rx) = mpsc::channel(); diff --git a/src/middleware/compat.rs b/src/middleware/compat.rs index 0e3a4f2b7..3a85591da 100644 --- a/src/middleware/compat.rs +++ b/src/middleware/compat.rs @@ -113,7 +113,11 @@ pub trait MapServiceResponseBody { fn map_body(self) -> ServiceResponse; } -impl MapServiceResponseBody for ServiceResponse { +impl MapServiceResponseBody for ServiceResponse +where + B: MessageBody + Unpin + 'static, + B::Error: Into, +{ fn map_body(self) -> ServiceResponse { self.map_body(|_, body| ResponseBody::Other(Body::from_message(body))) } diff --git a/src/middleware/logger.rs b/src/middleware/logger.rs index 40ed9258f..8a60d6c70 100644 --- a/src/middleware/logger.rs +++ b/src/middleware/logger.rs @@ -22,10 +22,9 @@ use time::OffsetDateTime; use crate::{ dev::{BodySize, MessageBody, ResponseBody}, - error::{Error, Result}, http::{HeaderName, StatusCode}, service::{ServiceRequest, ServiceResponse}, - HttpResponse, + Error, HttpResponse, Result, }; /// Middleware for logging request and response summaries to the terminal. @@ -327,7 +326,13 @@ impl PinnedDrop for StreamLog { } } -impl MessageBody for StreamLog { +impl MessageBody for StreamLog +where + B: MessageBody, + B::Error: Into, +{ + type Error = Error; + fn size(&self) -> BodySize { self.body.size() } @@ -335,7 +340,7 @@ impl MessageBody for StreamLog { fn poll_next( self: Pin<&mut Self>, cx: &mut Context<'_>, - ) -> Poll>> { + ) -> Poll>> { let this = self.project(); match this.body.poll_next(cx) { Poll::Ready(Some(Ok(chunk))) => { diff --git a/src/response/response.rs b/src/response/response.rs index 31868fe0b..6e09a0136 100644 --- a/src/response/response.rs +++ b/src/response/response.rs @@ -243,7 +243,11 @@ impl HttpResponse { } } -impl fmt::Debug for HttpResponse { +impl fmt::Debug for HttpResponse +where + B: MessageBody, + B::Error: Into, +{ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("HttpResponse") .field("error", &self.error) diff --git a/src/server.rs b/src/server.rs index 6577f4d1f..6e11c642f 100644 --- a/src/server.rs +++ b/src/server.rs @@ -81,6 +81,7 @@ where S::Service: 'static, // S::Service: 'static, B: MessageBody + 'static, + B::Error: Into, { /// Create new HTTP server with application factory pub fn new(factory: F) -> Self { diff --git a/src/service.rs b/src/service.rs index f6d1f9ebf..0c03f84ad 100644 --- a/src/service.rs +++ b/src/service.rs @@ -443,7 +443,11 @@ impl From> for Response { } } -impl fmt::Debug for ServiceResponse { +impl fmt::Debug for ServiceResponse +where + B: MessageBody, + B::Error: Into, +{ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let res = writeln!( f, diff --git a/src/test.rs b/src/test.rs index c2e456e58..9fe5e9b5d 100644 --- a/src/test.rs +++ b/src/test.rs @@ -151,6 +151,7 @@ pub async fn read_response(app: &S, req: Request) -> Bytes where S: Service, Error = Error>, B: MessageBody + Unpin, + B::Error: Into, { let mut resp = app .call(req) @@ -196,6 +197,7 @@ where pub async fn read_body(mut res: ServiceResponse) -> Bytes where B: MessageBody + Unpin, + B::Error: Into, { let mut body = res.take_body(); let mut bytes = BytesMut::new(); @@ -245,6 +247,7 @@ where pub async fn read_body_json(res: ServiceResponse) -> T where B: MessageBody + Unpin, + B::Error: Into, T: DeserializeOwned, { let body = read_body(res).await; @@ -306,6 +309,7 @@ pub async fn read_response_json(app: &S, req: Request) -> T where S: Service, Error = Error>, B: MessageBody + Unpin, + B::Error: Into, T: DeserializeOwned, { let body = read_response(app, req).await;