From d45a1aa6b6e805e07d1a92c230b368342c9a2ec8 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sat, 24 Oct 2020 18:49:50 +0100 Subject: [PATCH 1/5] Add `web::ReqData` extractor (#1748) Co-authored-by: Jonas Platte --- CHANGES.md | 10 ++- src/data.rs | 33 +++------ src/lib.rs | 1 + src/request_data.rs | 175 ++++++++++++++++++++++++++++++++++++++++++++ src/web.rs | 1 + 5 files changed, 197 insertions(+), 23 deletions(-) create mode 100644 src/request_data.rs diff --git a/CHANGES.md b/CHANGES.md index b3d3c180c..9ea3a2094 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,11 +1,17 @@ # Changes ## Unreleased - 2020-xx-xx -* Implement Logger middleware regex exclude pattern [#1723] -* Print unconfigured `Data` type when attempting extraction. [#1743] +### Added +* Implement `exclude_regex` for Logger middleware. [#1723] +* Add request-local data extractor `web::ReqData`. [#1748] + +### Changed +* Print non-configured `Data` type when attempting extraction. [#1743] [#1723]: https://github.com/actix/actix-web/pull/1723 [#1743]: https://github.com/actix/actix-web/pull/1743 +[#1748]: https://github.com/actix/actix-web/pull/1748 + ## 3.1.0 - 2020-09-29 ### Changed diff --git a/src/data.rs b/src/data.rs index 01d36569b..19c258ff0 100644 --- a/src/data.rs +++ b/src/data.rs @@ -20,25 +20,20 @@ pub(crate) type FnDataFactory = /// Application data. /// -/// Application data is an arbitrary data attached to the app. -/// Application data is available to all routes and could be added -/// during application configuration process -/// with `App::data()` method. +/// Application level data is a piece of arbitrary data attached to the app, scope, or resource. +/// Application data is available to all routes and can be added during the application +/// configuration process via `App::data()`. /// -/// Application data could be accessed by using `Data` -/// extractor where `T` is data type. +/// Application data can be accessed by using `Data` extractor where `T` is data type. /// -/// **Note**: http server accepts an application factory rather than -/// an application instance. Http server constructs an application -/// instance for each thread, thus application data must be constructed -/// multiple times. If you want to share data between different -/// threads, a shareable object should be used, e.g. `Send + Sync`. Application -/// data does not need to be `Send` or `Sync`. Internally `Data` type -/// uses `Arc`. if your data implements `Send` + `Sync` traits you can -/// use `web::Data::new()` and avoid double `Arc`. +/// **Note**: http server accepts an application factory rather than an application instance. HTTP +/// server constructs an application instance for each thread, thus application data must be +/// constructed multiple times. If you want to share data between different threads, a shareable +/// object should be used, e.g. `Send + Sync`. Application data does not need to be `Send` +/// or `Sync`. Internally `Data` uses `Arc`. /// -/// If route data is not set for a handler, using `Data` extractor would -/// cause *Internal Server Error* response. +/// If route data is not set for a handler, using `Data` extractor would cause *Internal +/// Server Error* response. /// /// ```rust /// use std::sync::Mutex; @@ -48,7 +43,7 @@ pub(crate) type FnDataFactory = /// counter: usize, /// } /// -/// /// Use `Data` extractor to access data in handler. +/// /// Use the `Data` extractor to access data in a handler. /// async fn index(data: web::Data>) -> impl Responder { /// let mut data = data.lock().unwrap(); /// data.counter += 1; @@ -71,10 +66,6 @@ pub struct Data(Arc); impl Data { /// Create new `Data` instance. - /// - /// Internally `Data` type uses `Arc`. if your data implements - /// `Send` + `Sync` traits you can use `web::Data::new()` and - /// avoid double `Arc`. pub fn new(state: T) -> Data { Data(Arc::new(state)) } diff --git a/src/lib.rs b/src/lib.rs index 327cba954..088444e05 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -81,6 +81,7 @@ mod handler; mod info; pub mod middleware; mod request; +mod request_data; mod resource; mod responder; mod rmap; diff --git a/src/request_data.rs b/src/request_data.rs new file mode 100644 index 000000000..c01930418 --- /dev/null +++ b/src/request_data.rs @@ -0,0 +1,175 @@ +use std::{any::type_name, ops::Deref}; + +use actix_http::error::{Error, ErrorInternalServerError}; +use futures_util::future; + +use crate::{dev::Payload, FromRequest, HttpRequest}; + +/// Request-local data extractor. +/// +/// Request-local data is arbitrary data attached to an individual request, usually +/// by middleware. It can be set via `extensions_mut` on [`HttpRequest`][htr_ext_mut] +/// or [`ServiceRequest`][srv_ext_mut]. +/// +/// Unlike app data, request data is dropped when the request has finished processing. This makes it +/// useful as a kind of messaging system between middleware and request handlers. It uses the same +/// types-as-keys storage system as app data. +/// +/// # Mutating Request Data +/// Note that since extractors must output owned data, only types that `impl Clone` can use this +/// extractor. A clone is taken of the required request data and can, therefore, not be directly +/// mutated in-place. To mutate request data, continue to use [`HttpRequest::extensions_mut`] or +/// 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 +/// ```rust,no_run +/// # use actix_web::{web, HttpResponse, HttpRequest, Responder}; +/// +/// #[derive(Debug, Clone, PartialEq)] +/// struct FlagFromMiddleware(String); +/// +/// /// Use the `ReqData` extractor to access request data in a handler. +/// async fn handler( +/// req: HttpRequest, +/// opt_flag: Option>, +/// ) -> impl Responder { +/// // use an optional extractor if the middleware is +/// // not guaranteed to add this type of requests data +/// if let Some(flag) = opt_flag { +/// assert_eq!(&flag.into_inner(), req.extensions().get::().unwrap()); +/// } +/// +/// HttpResponse::Ok() +/// } +/// ``` +/// +/// [htr_ext_mut]: crate::HttpRequest::extensions_mut +/// [srv_ext_mut]: crate::dev::ServiceRequest::extensions_mut +#[derive(Debug, Clone)] +pub struct ReqData(T); + +impl ReqData { + /// Consumes the `ReqData`, returning it's wrapped data. + pub fn into_inner(self) -> T { + self.0 + } +} + +impl Deref for ReqData { + type Target = T; + + fn deref(&self) -> &T { + &self.0 + } +} + +impl FromRequest for ReqData { + type Config = (); + type Error = Error; + type Future = future::Ready>; + + fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future { + if let Some(st) = req.extensions().get::() { + future::ok(ReqData(st.clone())) + } else { + log::debug!( + "Failed to construct App-level ReqData extractor. \ + Request path: {:?} (type: {})", + req.path(), + type_name::(), + ); + future::err(ErrorInternalServerError( + "Missing expected request extension data", + )) + } + } +} + +#[cfg(test)] +mod tests { + use std::{cell::RefCell, rc::Rc}; + + use futures_util::TryFutureExt as _; + + use super::*; + use crate::{ + dev::Service, + http::{Method, StatusCode}, + test::{init_service, TestRequest}, + web, App, HttpMessage, HttpResponse, + }; + + #[actix_rt::test] + async fn req_data_extractor() { + let mut srv = init_service( + App::new() + .wrap_fn(|req, srv| { + if req.method() == Method::POST { + req.extensions_mut().insert(42u32); + } + + srv.call(req) + }) + .service(web::resource("/test").to( + |req: HttpRequest, data: Option>| { + if req.method() != Method::POST { + assert!(data.is_none()); + } + + if let Some(data) = data { + assert_eq!(*data, 42); + assert_eq!( + Some(data.into_inner()), + req.extensions().get::().copied() + ); + } + + HttpResponse::Ok() + }, + )), + ) + .await; + + let req = TestRequest::get().uri("/test").to_request(); + let resp = srv.call(req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + + let req = TestRequest::post().uri("/test").to_request(); + let resp = srv.call(req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + } + + #[actix_rt::test] + async fn req_data_internal_mutability() { + let mut srv = init_service( + App::new() + .wrap_fn(|req, srv| { + let data_before = Rc::new(RefCell::new(42u32)); + req.extensions_mut().insert(data_before); + + srv.call(req).map_ok(|res| { + { + let ext = res.request().extensions(); + let data_after = ext.get::>>().unwrap(); + assert_eq!(*data_after.borrow(), 53u32); + } + + res + }) + }) + .default_service(web::to(|data: ReqData>>| { + assert_eq!(*data.borrow(), 42); + *data.borrow_mut() += 11; + assert_eq!(*data.borrow(), 53); + + HttpResponse::Ok() + })), + ) + .await; + + let req = TestRequest::get().uri("/test").to_request(); + let resp = srv.call(req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + } +} diff --git a/src/web.rs b/src/web.rs index 1d1174f41..ee895f8e7 100644 --- a/src/web.rs +++ b/src/web.rs @@ -19,6 +19,7 @@ use crate::service::WebService; pub use crate::config::ServiceConfig; pub use crate::data::Data; pub use crate::request::HttpRequest; +pub use crate::request_data::ReqData; pub use crate::types::*; /// Create resource for a specific path. From 41e7cec72f6f92dd795121adce42a594db3bc597 Mon Sep 17 00:00:00 2001 From: Daniel Egger Date: Sat, 24 Oct 2020 21:31:23 +0200 Subject: [PATCH 2/5] Re-export bytes::Buf and bytes::BufMut as well (#1750) Co-authored-by: Daniel Egger Co-authored-by: Rob Ede --- CHANGES.md | 2 ++ src/web.rs | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 9ea3a2094..a74d7490f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -7,10 +7,12 @@ ### Changed * Print non-configured `Data` type when attempting extraction. [#1743] +* Re-export bytes::Buf{Mut} in web module. [#1750] [#1723]: https://github.com/actix/actix-web/pull/1723 [#1743]: https://github.com/actix/actix-web/pull/1743 [#1748]: https://github.com/actix/actix-web/pull/1748 +[#1750]: https://github.com/actix/actix-web/pull/1750 ## 3.1.0 - 2020-09-29 diff --git a/src/web.rs b/src/web.rs index ee895f8e7..bf2158917 100644 --- a/src/web.rs +++ b/src/web.rs @@ -4,7 +4,7 @@ use actix_router::IntoPattern; use std::future::Future; pub use actix_http::Response as HttpResponse; -pub use bytes::{Bytes, BytesMut}; +pub use bytes::{Buf, BufMut, Bytes, BytesMut}; pub use futures_channel::oneshot::Canceled; use crate::error::BlockingError; From 06e5042b94c93aaa8c18b8702eaf6611c895b42f Mon Sep 17 00:00:00 2001 From: Jonas Date: Sat, 24 Oct 2020 22:15:01 +0200 Subject: [PATCH 3/5] use idenity encoding on client if no compression features are enabled (#1737) Co-authored-by: Yuki Okushi Co-authored-by: Rob Ede --- awc/CHANGES.md | 7 +++++++ awc/Cargo.toml | 1 + awc/src/request.rs | 13 +++++++++---- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/awc/CHANGES.md b/awc/CHANGES.md index 8babba113..0b02b3cfa 100644 --- a/awc/CHANGES.md +++ b/awc/CHANGES.md @@ -1,8 +1,15 @@ # Changes ## Unreleased - 2020-xx-xx +### Changed * Upgrade `base64` to `0.13`. +### Fixed +* Use `Accept-Encoding: identity` instead of `Accept-Encoding: br` when no compression feature is enabled [#1737] + +[#1737]: https://github.com/actix/actix-web/pull/1737 + + ## 2.0.0 - 2020-09-11 ### Changed * `Client::build` was renamed to `Client::builder`. diff --git a/awc/Cargo.toml b/awc/Cargo.toml index b8cf53e06..b7d8b0a22 100644 --- a/awc/Cargo.toml +++ b/awc/Cargo.toml @@ -44,6 +44,7 @@ actix-rt = "1.0.0" base64 = "0.13" bytes = "0.5.3" +cfg-if = "1.0" derive_more = "0.99.2" futures-core = { version = "0.3.5", default-features = false } log =" 0.4" diff --git a/awc/src/request.rs b/awc/src/request.rs index dcada2c6d..11e1da6a3 100644 --- a/awc/src/request.rs +++ b/awc/src/request.rs @@ -21,10 +21,15 @@ use crate::frozen::FrozenClientRequest; use crate::sender::{PrepForSendingError, RequestSender, SendClientRequest}; use crate::ClientConfig; -#[cfg(any(feature = "flate2-zlib", feature = "flate2-rust"))] -const HTTPS_ENCODING: &str = "br, gzip, deflate"; -#[cfg(not(any(feature = "flate2-zlib", feature = "flate2-rust")))] -const HTTPS_ENCODING: &str = "br"; +cfg_if::cfg_if! { + if #[cfg(any(feature = "flate2-zlib", feature = "flate2-rust"))] { + const HTTPS_ENCODING: &str = "br, gzip, deflate"; + } else if #[cfg(feature = "compress")] { + const HTTPS_ENCODING: &str = "br"; + } else { + const HTTPS_ENCODING: &str = "identity"; + } +} /// An HTTP Client request builder /// From 20078fe60394091f45d795b4552581421d0f3953 Mon Sep 17 00:00:00 2001 From: ghizzo01 <36126125+ghizzo01@users.noreply.github.com> Date: Sun, 25 Oct 2020 11:41:44 +0100 Subject: [PATCH 4/5] Bump pin-project to 1.0 (#1733) --- CHANGES.md | 1 + Cargo.toml | 2 +- actix-http/CHANGES.md | 1 + actix-http/Cargo.toml | 2 +- actix-web-actors/CHANGES.md | 2 +- actix-web-actors/Cargo.toml | 2 +- 6 files changed, 6 insertions(+), 4 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index a74d7490f..baa462a5d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,6 +8,7 @@ ### Changed * Print non-configured `Data` type when attempting extraction. [#1743] * Re-export bytes::Buf{Mut} in web module. [#1750] +* Upgrade `pin-project` to `1.0`. [#1723]: https://github.com/actix/actix-web/pull/1723 [#1743]: https://github.com/actix/actix-web/pull/1743 diff --git a/Cargo.toml b/Cargo.toml index 3960b4d36..5d64cfd91 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -90,7 +90,7 @@ fxhash = "0.2.1" log = "0.4" mime = "0.3" socket2 = "0.3" -pin-project = "0.4.17" +pin-project = "1.0.0" regex = "1.4" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index b72a7801a..990c9c071 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -2,6 +2,7 @@ ## Unreleased - 2020-xx-xx * Upgrade `base64` to `0.13`. +* Upgrade `pin-project` to `1.0`. ## 2.0.0 - 2020-09-11 * No significant changes from `2.0.0-beta.4`. diff --git a/actix-http/Cargo.toml b/actix-http/Cargo.toml index a4c100b92..9d2f7464f 100644 --- a/actix-http/Cargo.toml +++ b/actix-http/Cargo.toml @@ -71,7 +71,7 @@ language-tags = "0.2" log = "0.4" mime = "0.3" percent-encoding = "2.1" -pin-project = "0.4.17" +pin-project = "1.0.0" rand = "0.7" regex = "1.3" serde = "1.0" diff --git a/actix-web-actors/CHANGES.md b/actix-web-actors/CHANGES.md index 4b9381a33..9df0df159 100644 --- a/actix-web-actors/CHANGES.md +++ b/actix-web-actors/CHANGES.md @@ -1,7 +1,7 @@ # Changes ## Unreleased - 2020-xx-xx - +* Upgrade `pin-project` to `1.0`. ## 3.0.0 - 2020-09-11 * No significant changes from `3.0.0-beta.2`. diff --git a/actix-web-actors/Cargo.toml b/actix-web-actors/Cargo.toml index 2f3c63022..920940c40 100644 --- a/actix-web-actors/Cargo.toml +++ b/actix-web-actors/Cargo.toml @@ -23,7 +23,7 @@ actix-codec = "0.3.0" bytes = "0.5.2" futures-channel = { version = "0.3.5", default-features = false } futures-core = { version = "0.3.5", default-features = false } -pin-project = "0.4.17" +pin-project = "1.0.0" [dev-dependencies] actix-rt = "1.1.1" From 7030bf5fe82dc3c23e61a6f0e7a6b89b53a03e6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Augusto=20C=C3=A9sar=20Dias?= Date: Mon, 26 Oct 2020 18:02:45 +0100 Subject: [PATCH 5/5] Adding app_data to ServiceConfig (#1758) Co-authored-by: Rob Ede Co-authored-by: Augusto --- CHANGES.md | 1 + actix-http/src/extensions.rs | 35 +++++++++++++++++++++++++++++++++++ src/app.rs | 1 + src/config.rs | 23 ++++++++++++++++++----- src/scope.rs | 3 +++ 5 files changed, 58 insertions(+), 5 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index baa462a5d..af34c3b49 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,6 +4,7 @@ ### Added * Implement `exclude_regex` for Logger middleware. [#1723] * Add request-local data extractor `web::ReqData`. [#1748] +* Add `app_data` to `ServiceConfig`. [#1757] ### Changed * Print non-configured `Data` type when attempting extraction. [#1743] diff --git a/actix-http/src/extensions.rs b/actix-http/src/extensions.rs index 96e01767b..09f1b711f 100644 --- a/actix-http/src/extensions.rs +++ b/actix-http/src/extensions.rs @@ -61,6 +61,11 @@ impl Extensions { pub fn clear(&mut self) { self.map.clear(); } + + /// Extends self with the items from another `Extensions`. + pub fn extend(&mut self, other: Extensions) { + self.map.extend(other.map); + } } impl fmt::Debug for Extensions { @@ -178,4 +183,34 @@ mod tests { assert_eq!(extensions.get::(), None); assert_eq!(extensions.get(), Some(&MyType(10))); } + + #[test] + fn test_extend() { + #[derive(Debug, PartialEq)] + struct MyType(i32); + + let mut extensions = Extensions::new(); + + extensions.insert(5i32); + extensions.insert(MyType(10)); + + let mut other = Extensions::new(); + + other.insert(15i32); + other.insert(20u8); + + extensions.extend(other); + + assert_eq!(extensions.get(), Some(&15i32)); + assert_eq!(extensions.get_mut(), Some(&mut 15i32)); + + assert_eq!(extensions.remove::(), Some(15i32)); + assert!(extensions.get::().is_none()); + + assert_eq!(extensions.get::(), None); + assert_eq!(extensions.get(), Some(&MyType(10))); + + assert_eq!(extensions.get(), Some(&20u8)); + assert_eq!(extensions.get_mut(), Some(&mut 20u8)); + } } diff --git a/src/app.rs b/src/app.rs index 6a4b97b69..8dd86f7ec 100644 --- a/src/app.rs +++ b/src/app.rs @@ -183,6 +183,7 @@ where self.data.extend(cfg.data); self.services.extend(cfg.services); self.external.extend(cfg.external); + self.extensions.extend(cfg.extensions); self } diff --git a/src/config.rs b/src/config.rs index f7bebb4c5..03ba82732 100644 --- a/src/config.rs +++ b/src/config.rs @@ -178,6 +178,7 @@ pub struct ServiceConfig { pub(crate) services: Vec>, pub(crate) data: Vec>, pub(crate) external: Vec, + pub(crate) extensions: Extensions, } impl ServiceConfig { @@ -186,6 +187,7 @@ impl ServiceConfig { services: Vec::new(), data: Vec::new(), external: Vec::new(), + extensions: Extensions::new(), } } @@ -198,6 +200,14 @@ impl ServiceConfig { self } + /// Set arbitrary data item. + /// + /// This is same as `App::data()` method. + pub fn app_data(&mut self, ext: U) -> &mut Self { + self.extensions.insert(ext); + self + } + /// Configure route for a specific path. /// /// This is same as `App::route()` method. @@ -254,13 +264,16 @@ mod tests { async fn test_data() { let cfg = |cfg: &mut ServiceConfig| { cfg.data(10usize); + cfg.app_data(15u8); }; - let mut srv = - init_service(App::new().configure(cfg).service( - web::resource("/").to(|_: web::Data| HttpResponse::Ok()), - )) - .await; + let mut srv = init_service(App::new().configure(cfg).service( + web::resource("/").to(|_: web::Data, req: HttpRequest| { + assert_eq!(*req.app_data::().unwrap(), 15u8); + HttpResponse::Ok() + }), + )) + .await; let req = TestRequest::default().to_request(); let resp = srv.call(req).await.unwrap(); assert_eq!(resp.status(), StatusCode::OK); diff --git a/src/scope.rs b/src/scope.rs index 1c5d8700b..681d142be 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -209,6 +209,9 @@ where self.data = Some(data); } + self.data + .get_or_insert_with(Extensions::new) + .extend(cfg.extensions); self }