diff --git a/.cargo/config.toml b/.cargo/config.toml index 0cf09f710..72f445d8a 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -3,6 +3,7 @@ chk = "check --workspace --all-features --tests --examples --bins" lint = "clippy --workspace --tests --examples" ci-min = "hack check --workspace --no-default-features" ci-min-test = "hack check --workspace --no-default-features --tests --examples" -ci-default = "hack check --workspace" -ci-full = "check --workspace --bins --examples --tests" -ci-test = "test --workspace --all-features --no-fail-fast" +ci-default = "check --workspace --bins --tests --examples" +ci-full = "check --workspace --all-features --bins --tests --examples" +ci-test = "test --workspace --all-features --lib --tests --no-fail-fast -- --nocapture" +ci-doctest = "hack test --workspace --all-features --doc --no-fail-fast -- --nocapture" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c57db463a..be595e35c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -66,43 +66,33 @@ jobs: - name: check minimal uses: actions-rs/cargo@v1 - with: - command: hack - args: check --workspace --no-default-features + with: { command: ci-min } - name: check minimal + tests uses: actions-rs/cargo@v1 - with: - command: hack - args: check --workspace --no-default-features --tests --examples + with: { command: ci-min-test } + - name: check default + uses: actions-rs/cargo@v1 + with: { command: ci-default } + - name: check full uses: actions-rs/cargo@v1 - with: - command: check - args: --workspace --bins --examples --tests + with: { command: ci-full } - name: tests - uses: actions-rs/cargo@v1 - with: - command: test - args: --workspace --all-features --no-fail-fast -- --nocapture - --skip=test_h2_content_length - --skip=test_reading_deflate_encoding_large_random_rustls - - - name: tests (actix-http) uses: actions-rs/cargo@v1 timeout-minutes: 40 with: - command: test - args: --package=actix-http --no-default-features --features=rustls -- --nocapture + command: ci-test + args: --skip=test_reading_deflate_encoding_large_random_rustls - - name: tests (awc) + - name: doc tests + # due to unknown issue with running doc tests on macOS + if: matrix.target.os == 'ubuntu-latest' uses: actions-rs/cargo@v1 timeout-minutes: 40 - with: - command: test - args: --package=awc --no-default-features --features=rustls -- --nocapture + with: { command: ci-doctest } - name: Generate coverage file if: > diff --git a/.github/workflows/clippy-fmt.yml b/.github/workflows/clippy-fmt.yml index e966fa4ab..957256d32 100644 --- a/.github/workflows/clippy-fmt.yml +++ b/.github/workflows/clippy-fmt.yml @@ -36,4 +36,4 @@ jobs: uses: actions-rs/clippy-check@v1 with: token: ${{ secrets.GITHUB_TOKEN }} - args: --workspace --tests --all-features + args: --workspace --all-features --tests diff --git a/CHANGES.md b/CHANGES.md index 17ae711d6..876e1c03d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,10 +1,21 @@ # Changes ## Unreleased - 2021-xx-xx +### Added +* Add `ServiceRequest::parts_mut`. [#2177] +* Add extractors for `Uri` and `Method`. [#2263] +* Add extractors for `ConnectionInfo` and `PeerAddr`. [#2263] +* Add `Route::service` for using hand-written services as handlers. [#2262] + ### Changed * Change compression algorithm features flags. [#2250] +* Deprecate `App::data` and `App::data_factory`. [#2271] +[#2177]: https://github.com/actix/actix-web/pull/2177 [#2250]: https://github.com/actix/actix-web/pull/2250 +[#2271]: https://github.com/actix/actix-web/pull/2271 +[#2262]: https://github.com/actix/actix-web/pull/2262 +[#2263]: https://github.com/actix/actix-web/pull/2263 ## 4.0.0-beta.7 - 2021-06-17 diff --git a/Cargo.toml b/Cargo.toml index 320751c66..779b52255 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -113,7 +113,6 @@ flate2 = "1.0.13" zstd = "0.7" rand = "0.8" rcgen = "0.8" -serde_derive = "1.0" tls-openssl = { package = "openssl", version = "0.10.9" } tls-rustls = { package = "rustls", version = "0.19.0" } diff --git a/awc/src/request.rs b/awc/src/request.rs index 46dae7fa3..3f312f6e7 100644 --- a/awc/src/request.rs +++ b/awc/src/request.rs @@ -480,6 +480,7 @@ impl ClientRequest { // supported, so we cannot guess Accept-Encoding HTTP header. if slf.response_decompress { // Set Accept-Encoding with compression algorithm awc is built with. + #[allow(clippy::vec_init_then_push)] #[cfg(feature = "__compress")] let accept_encoding = { let mut encoding = vec![]; @@ -496,7 +497,11 @@ impl ClientRequest { #[cfg(feature = "compress-zstd")] encoding.push("zstd"); - assert!(!encoding.is_empty(), "encoding cannot be empty unless __compress feature has been explictily enabled."); + assert!( + !encoding.is_empty(), + "encoding can not be empty unless __compress feature has been explicitly enabled" + ); + encoding.join(", ") }; diff --git a/src/app.rs b/src/app.rs index 357d45eeb..677f73805 100644 --- a/src/app.rs +++ b/src/app.rs @@ -68,43 +68,83 @@ where InitError = (), >, { - /// Set application data. Application data could be accessed - /// by using `Data` extractor where `T` is data type. + /// Set application (root level) data. /// - /// **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 shared object should be used, e.g. `Arc`. Internally `Data` type - /// uses `Arc` so data could be created outside of app factory and clones could - /// be stored via `App::app_data()` method. + /// Application data stored with `App::app_data()` method is available through the + /// [`HttpRequest::app_data`](crate::HttpRequest::app_data) method at runtime. + /// + /// # [`Data`] + /// Any [`Data`] type added here can utilize it's extractor implementation in handlers. + /// Types not wrapped in `Data` cannot use this extractor. See [its docs](Data) for more + /// about its usage and patterns. /// /// ``` /// use std::cell::Cell; - /// use actix_web::{web, App, HttpResponse, Responder}; + /// use actix_web::{web, App, HttpRequest, HttpResponse, Responder}; /// /// struct MyData { - /// counter: Cell, + /// count: std::cell::Cell, /// } /// - /// async fn index(data: web::Data) -> impl Responder { - /// data.counter.set(data.counter.get() + 1); - /// HttpResponse::Ok() + /// async fn handler(req: HttpRequest, counter: web::Data) -> impl Responder { + /// // note this cannot use the Data extractor because it was not added with it + /// let incr = *req.app_data::().unwrap(); + /// assert_eq!(incr, 3); + /// + /// // update counter using other value from app data + /// counter.count.set(counter.count.get() + incr); + /// + /// HttpResponse::Ok().body(counter.count.get().to_string()) /// } /// - /// let app = App::new() - /// .data(MyData{ counter: Cell::new(0) }) - /// .service( - /// web::resource("/index.html").route( - /// web::get().to(index))); + /// let app = App::new().service( + /// web::resource("/") + /// .app_data(3usize) + /// .app_data(web::Data::new(MyData { count: Default::default() })) + /// .route(web::get().to(handler)) + /// ); /// ``` + /// + /// # Shared Mutable State + /// [`HttpServer::new`](crate::HttpServer::new) accepts an application factory rather than an + /// application instance; the factory closure is called on each worker thread independently. + /// Therefore, if you want to share a data object between different workers, a shareable object + /// needs to be created first, outside the `HttpServer::new` closure and cloned into it. + /// [`Data`] is an example of such a sharable object. + /// + /// ```ignore + /// let counter = web::Data::new(AppStateWithCounter { + /// counter: Mutex::new(0), + /// }); + /// + /// HttpServer::new(move || { + /// // move counter object into the closure and clone for each worker + /// + /// App::new() + /// .app_data(counter.clone()) + /// .route("/", web::get().to(handler)) + /// }) + /// ``` + pub fn app_data(mut self, ext: U) -> Self { + self.extensions.insert(ext); + self + } + + /// Add application (root) data after wrapping in `Data`. + /// + /// Deprecated in favor of [`app_data`](Self::app_data). + #[deprecated(since = "4.0.0", note = "Use `.app_data(Data::new(val))` instead.")] pub fn data(self, data: U) -> Self { self.app_data(Data::new(data)) } - /// Set application data factory. This function is - /// similar to `.data()` but it accepts data factory. Data object get - /// constructed asynchronously during application initialization. + /// Add application data factory. This function is similar to `.data()` but it accepts a + /// "data factory". Data values are constructed asynchronously during application + /// initialization, before the server starts accepting requests. + #[deprecated( + since = "4.0.0", + note = "Construct data value before starting server and use `.app_data(Data::new(val))` instead." + )] pub fn data_factory(mut self, data: F) -> Self where F: Fn() -> Out + 'static, @@ -133,18 +173,6 @@ where self } - /// Set application level arbitrary data item. - /// - /// Application data stored with `App::app_data()` method is available - /// via `HttpRequest::app_data()` method at runtime. - /// - /// This method could be used for storing `Data` as well, in that case - /// data could be accessed by using `Data` extractor. - pub fn app_data(mut self, ext: U) -> Self { - self.extensions.insert(ext); - self - } - /// Run external configuration as part of the application building /// process /// @@ -518,6 +546,8 @@ mod tests { assert_eq!(resp.status(), StatusCode::CREATED); } + // allow deprecated App::data + #[allow(deprecated)] #[actix_rt::test] async fn test_data_factory() { let srv = init_service( @@ -541,6 +571,8 @@ mod tests { assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); } + // allow deprecated App::data + #[allow(deprecated)] #[actix_rt::test] async fn test_data_factory_errors() { let srv = try_init_service( diff --git a/src/app_service.rs b/src/app_service.rs index ca6f36202..0e590e2b7 100644 --- a/src/app_service.rs +++ b/src/app_service.rs @@ -144,7 +144,9 @@ where } } -/// Service that takes a [`Request`] and delegates to a service that take a [`ServiceRequest`]. +/// The [`Service`] that is passed to `actix-http`'s server builder. +/// +/// Wraps a service receiving a [`ServiceRequest`] into one receiving a [`Request`]. pub struct AppInitService where T: Service, Error = Error>, @@ -275,6 +277,7 @@ impl ServiceFactory for AppRoutingFactory { } } +/// The Actix Web router default entry point. pub struct AppRouting { router: Router, default: HttpService, @@ -349,6 +352,8 @@ mod tests { } } + // allow deprecated App::data + #[allow(deprecated)] #[actix_rt::test] async fn test_drop_data() { let data = Arc::new(AtomicBool::new(false)); diff --git a/src/config.rs b/src/config.rs index d22bc856e..966141193 100644 --- a/src/config.rs +++ b/src/config.rs @@ -62,6 +62,8 @@ impl AppService { (self.config, self.services) } + /// Clones inner config and default service, returning new `AppService` with empty service list + /// marked as non-root. pub(crate) fn clone_config(&self) -> Self { AppService { config: self.config.clone(), @@ -71,12 +73,12 @@ impl AppService { } } - /// Service configuration + /// Returns reference to configuration. pub fn config(&self) -> &AppConfig { &self.config } - /// Default resource + /// Returns default handler factory. pub fn default_service(&self) -> Rc { self.default.clone() } @@ -116,7 +118,7 @@ impl AppConfig { AppConfig { secure, host, addr } } - /// Needed in actix-test crate. + /// Needed in actix-test crate. Semver exempt. #[doc(hidden)] pub fn __priv_test_new(secure: bool, host: String, addr: SocketAddr) -> Self { AppConfig::new(secure, host, addr) @@ -192,6 +194,7 @@ impl ServiceConfig { /// Add shared app data item. /// /// Counterpart to [`App::data()`](crate::App::data). + #[deprecated(since = "4.0.0", note = "Use `.app_data(Data::new(val))` instead.")] pub fn data(&mut self, data: U) -> &mut Self { self.app_data(Data::new(data)); self @@ -257,6 +260,8 @@ mod tests { use crate::test::{call_service, init_service, read_body, TestRequest}; use crate::{web, App, HttpRequest, HttpResponse}; + // allow deprecated `ServiceConfig::data` + #[allow(deprecated)] #[actix_rt::test] async fn test_data() { let cfg = |cfg: &mut ServiceConfig| { diff --git a/src/data.rs b/src/data.rs index f09a88891..174faba37 100644 --- a/src/data.rs +++ b/src/data.rs @@ -36,6 +36,11 @@ pub(crate) type FnDataFactory = /// If route data is not set for a handler, using `Data` extractor would cause *Internal /// Server Error* response. /// +// TODO: document `dyn T` functionality through converting an Arc +// TODO: note equivalence of req.app_data> and Data extractor +// TODO: note that data must be inserted using Data in order to extract it +/// +/// # Examples /// ``` /// use std::sync::Mutex; /// use actix_web::{web, App, HttpResponse, Responder}; @@ -154,6 +159,8 @@ mod tests { web, App, HttpResponse, }; + // allow deprecated App::data + #[allow(deprecated)] #[actix_rt::test] async fn test_data_extractor() { let srv = init_service(App::new().data("TEST".to_string()).service( @@ -221,6 +228,8 @@ mod tests { assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); } + // allow deprecated App::data + #[allow(deprecated)] #[actix_rt::test] async fn test_route_data_extractor() { let srv = init_service( @@ -250,6 +259,8 @@ mod tests { assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); } + // allow deprecated App::data + #[allow(deprecated)] #[actix_rt::test] async fn test_override_data() { let srv = diff --git a/src/extract.rs b/src/extract.rs index 45cb330a3..592f7ab83 100644 --- a/src/extract.rs +++ b/src/extract.rs @@ -1,12 +1,14 @@ //! Request extractors use std::{ + convert::Infallible, future::Future, pin::Pin, task::{Context, Poll}, }; -use actix_utils::future::{ready, Ready}; +use actix_http::http::{Method, Uri}; +use actix_utils::future::{ok, Ready}; use futures_core::ready; use crate::{dev::Payload, Error, HttpRequest}; @@ -52,7 +54,7 @@ pub trait FromRequest: Sized { /// use actix_web::{web, dev, App, Error, HttpRequest, FromRequest}; /// use actix_web::error::ErrorBadRequest; /// use futures_util::future::{ok, err, Ready}; -/// use serde_derive::Deserialize; +/// use serde::Deserialize; /// use rand; /// /// #[derive(Debug, Deserialize)] @@ -143,7 +145,7 @@ where /// use actix_web::{web, dev, App, Result, Error, HttpRequest, FromRequest}; /// use actix_web::error::ErrorBadRequest; /// use futures_util::future::{ok, err, Ready}; -/// use serde_derive::Deserialize; +/// use serde::Deserialize; /// use rand; /// /// #[derive(Debug, Deserialize)] @@ -216,14 +218,58 @@ where } } +/// Extract the request's URI. +/// +/// # Examples +/// ``` +/// use actix_web::{http::Uri, web, App, Responder}; +/// +/// async fn handler(uri: Uri) -> impl Responder { +/// format!("Requested path: {}", uri.path()) +/// } +/// +/// let app = App::new().default_service(web::to(handler)); +/// ``` +impl FromRequest for Uri { + type Error = Infallible; + type Future = Ready>; + type Config = (); + + fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future { + ok(req.uri().clone()) + } +} + +/// Extract the request's method. +/// +/// # Examples +/// ``` +/// use actix_web::{http::Method, web, App, Responder}; +/// +/// async fn handler(method: Method) -> impl Responder { +/// format!("Request method: {}", method) +/// } +/// +/// let app = App::new().default_service(web::to(handler)); +/// ``` +impl FromRequest for Method { + type Error = Infallible; + type Future = Ready>; + type Config = (); + + fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future { + ok(req.method().clone()) + } +} + #[doc(hidden)] impl FromRequest for () { - type Error = Error; - type Future = Ready>; + type Error = Infallible; + type Future = Ready>; type Config = (); fn from_request(_: &HttpRequest, _: &mut Payload) -> Self::Future { - ready(Ok(())) + ok(()) } } @@ -330,7 +376,7 @@ mod m { mod tests { use actix_http::http::header; use bytes::Bytes; - use serde_derive::Deserialize; + use serde::Deserialize; use super::*; use crate::test::TestRequest; @@ -411,4 +457,18 @@ mod tests { .unwrap(); assert!(r.is_err()); } + + #[actix_rt::test] + async fn test_uri() { + let req = TestRequest::default().uri("/foo/bar").to_http_request(); + let uri = Uri::extract(&req).await.unwrap(); + assert_eq!(uri.path(), "/foo/bar"); + } + + #[actix_rt::test] + async fn test_method() { + let req = TestRequest::default().method(Method::GET).to_http_request(); + let method = Method::extract(&req).await.unwrap(); + assert_eq!(method, Method::GET); + } } diff --git a/src/info.rs b/src/info.rs index c9ddf6ec4..c6ff54efe 100644 --- a/src/info.rs +++ b/src/info.rs @@ -1,13 +1,36 @@ -use std::cell::Ref; +use std::{cell::Ref, convert::Infallible, net::SocketAddr}; -use crate::dev::{AppConfig, RequestHead}; -use crate::http::header::{self, HeaderName}; +use actix_utils::future::{err, ok, Ready}; +use derive_more::{Display, Error}; + +use crate::{ + dev::{AppConfig, Payload, RequestHead}, + http::header::{self, HeaderName}, + FromRequest, HttpRequest, ResponseError, +}; const X_FORWARDED_FOR: &[u8] = b"x-forwarded-for"; const X_FORWARDED_HOST: &[u8] = b"x-forwarded-host"; const X_FORWARDED_PROTO: &[u8] = b"x-forwarded-proto"; -/// `HttpRequest` connection information +/// HTTP connection information. +/// +/// `ConnectionInfo` implements `FromRequest` and can be extracted in handlers. +/// +/// # Examples +/// ``` +/// # use actix_web::{HttpResponse, Responder}; +/// use actix_web::dev::ConnectionInfo; +/// +/// async fn handler(conn: ConnectionInfo) -> impl Responder { +/// match conn.host() { +/// "actix.rs" => HttpResponse::Ok().body("Welcome!"), +/// "admin.actix.rs" => HttpResponse::Ok().body("Admin portal."), +/// _ => HttpResponse::NotFound().finish() +/// } +/// } +/// # let _svc = actix_web::web::to(handler); +/// ``` #[derive(Debug, Clone, Default)] pub struct ConnectionInfo { scheme: String, @@ -187,6 +210,65 @@ impl ConnectionInfo { } } +impl FromRequest for ConnectionInfo { + type Error = Infallible; + type Future = Ready>; + type Config = (); + + fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future { + ok(req.connection_info().clone()) + } +} + +/// Extractor for peer's socket address. +/// +/// Also see [`HttpRequest::peer_addr`]. +/// +/// # Examples +/// ``` +/// # use actix_web::Responder; +/// use actix_web::dev::PeerAddr; +/// +/// async fn handler(peer_addr: PeerAddr) -> impl Responder { +/// let socket_addr = peer_addr.0; +/// socket_addr.to_string() +/// } +/// # let _svc = actix_web::web::to(handler); +/// ``` +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord, Display)] +#[display(fmt = "{}", _0)] +pub struct PeerAddr(pub SocketAddr); + +impl PeerAddr { + /// Unwrap into inner `SocketAddr` value. + pub fn into_inner(self) -> SocketAddr { + self.0 + } +} + +#[derive(Debug, Display, Error)] +#[non_exhaustive] +#[display(fmt = "Missing peer address")] +pub struct MissingPeerAddr; + +impl ResponseError for MissingPeerAddr {} + +impl FromRequest for PeerAddr { + type Error = MissingPeerAddr; + type Future = Ready>; + type Config = (); + + fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future { + match req.peer_addr() { + Some(addr) => ok(PeerAddr(addr)), + None => { + log::error!("Missing peer address."); + err(MissingPeerAddr) + } + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -239,4 +321,25 @@ mod tests { let info = req.connection_info(); assert_eq!(info.scheme(), "https"); } + + #[actix_rt::test] + async fn test_conn_info() { + let req = TestRequest::default() + .uri("http://actix.rs/") + .to_http_request(); + let conn_info = ConnectionInfo::extract(&req).await.unwrap(); + assert_eq!(conn_info.scheme(), "http"); + } + + #[actix_rt::test] + async fn test_peer_addr() { + let addr = "127.0.0.1:8080".parse().unwrap(); + let req = TestRequest::default().peer_addr(addr).to_http_request(); + let peer_addr = PeerAddr::extract(&req).await.unwrap(); + assert_eq!(peer_addr, PeerAddr(addr)); + + let req = TestRequest::default().to_http_request(); + let res = PeerAddr::extract(&req).await; + assert!(res.is_err()); + } } diff --git a/src/lib.rs b/src/lib.rs index 4bcef3988..920abccb6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -131,7 +131,7 @@ pub mod dev { pub use crate::config::{AppConfig, AppService}; #[doc(hidden)] pub use crate::handler::Handler; - pub use crate::info::ConnectionInfo; + pub use crate::info::{ConnectionInfo, PeerAddr}; pub use crate::rmap::ResourceMap; pub use crate::service::{HttpServiceFactory, ServiceRequest, ServiceResponse, WebService}; @@ -149,7 +149,9 @@ pub mod dev { pub use actix_http::{Extensions, Payload, PayloadStream, RequestHead, ResponseHead}; pub use actix_router::{Path, ResourceDef, ResourcePath, Url}; pub use actix_server::Server; - pub use actix_service::{always_ready, forward_ready, Service, Transform}; + pub use actix_service::{ + always_ready, fn_factory, fn_service, forward_ready, Service, Transform, + }; pub(crate) fn insert_slash(mut patterns: Vec) -> Vec { for path in &mut patterns { diff --git a/src/request.rs b/src/request.rs index a364f8b1f..5c5c43d26 100644 --- a/src/request.rs +++ b/src/request.rs @@ -347,7 +347,7 @@ impl Drop for HttpRequest { /// # Examples /// ``` /// use actix_web::{web, App, HttpRequest}; -/// use serde_derive::Deserialize; +/// use serde::Deserialize; /// /// /// extract `Thing` from request /// async fn index(req: HttpRequest) -> String { @@ -711,6 +711,8 @@ mod tests { assert_eq!(body, Bytes::from_static(b"1")); } + // allow deprecated App::data + #[allow(deprecated)] #[actix_rt::test] async fn test_extensions_dropped() { struct Tracker { diff --git a/src/resource.rs b/src/resource.rs index 8c2b83b60..7a4c1248b 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -169,40 +169,38 @@ where self } - /// Provide resource specific data. This method allows to add extractor - /// configuration or specific state available via `Data` extractor. - /// Provided data is available for all routes registered for the current resource. - /// Resource data overrides data registered by `App::data()` method. - /// - /// ``` - /// use actix_web::{web, App, FromRequest}; - /// - /// /// extract text data from request - /// async fn index(body: String) -> String { - /// format!("Body {}!", body) - /// } - /// - /// fn main() { - /// let app = App::new().service( - /// web::resource("/index.html") - /// // limit size of the payload - /// .data(String::configure(|cfg| { - /// cfg.limit(4096) - /// })) - /// .route( - /// web::get() - /// // register handler - /// .to(index) - /// )); - /// } - /// ``` - pub fn data(self, data: U) -> Self { - self.app_data(Data::new(data)) - } - /// Add resource data. /// - /// Data of different types from parent contexts will still be accessible. + /// Data of different types from parent contexts will still be accessible. Any `Data` types + /// set here can be extracted in handlers using the `Data` extractor. + /// + /// # Examples + /// ``` + /// use std::cell::Cell; + /// use actix_web::{web, App, HttpRequest, HttpResponse, Responder}; + /// + /// struct MyData { + /// count: std::cell::Cell, + /// } + /// + /// async fn handler(req: HttpRequest, counter: web::Data) -> impl Responder { + /// // note this cannot use the Data extractor because it was not added with it + /// let incr = *req.app_data::().unwrap(); + /// assert_eq!(incr, 3); + /// + /// // update counter using other value from app data + /// counter.count.set(counter.count.get() + incr); + /// + /// HttpResponse::Ok().body(counter.count.get().to_string()) + /// } + /// + /// let app = App::new().service( + /// web::resource("/") + /// .app_data(3usize) + /// .app_data(web::Data::new(MyData { count: Default::default() })) + /// .route(web::get().to(handler)) + /// ); + /// ``` pub fn app_data(mut self, data: U) -> Self { self.app_data .get_or_insert_with(Extensions::new) @@ -211,6 +209,14 @@ where self } + /// Add resource data after wrapping in `Data`. + /// + /// Deprecated in favor of [`app_data`](Self::app_data). + #[deprecated(since = "4.0.0", note = "Use `.app_data(Data::new(val))` instead.")] + pub fn data(self, data: U) -> Self { + self.app_data(Data::new(data)) + } + /// Register a new route and add handler. This route matches all requests. /// /// ``` @@ -226,7 +232,6 @@ where /// This is shortcut for: /// /// ``` - /// # extern crate actix_web; /// # use actix_web::*; /// # fn index(req: HttpRequest) -> HttpResponse { unimplemented!() } /// App::new().service(web::resource("/").route(web::route().to(index))); @@ -694,6 +699,8 @@ mod tests { assert_eq!(resp.status(), StatusCode::NO_CONTENT); } + // allow deprecated `{App, Resource}::data` + #[allow(deprecated)] #[actix_rt::test] async fn test_data() { let srv = init_service( @@ -726,6 +733,8 @@ mod tests { assert_eq!(resp.status(), StatusCode::OK); } + // allow deprecated `{App, Resource}::data` + #[allow(deprecated)] #[actix_rt::test] async fn test_data_default_service() { let srv = init_service( diff --git a/src/response/response.rs b/src/response/response.rs index 9dd804be0..9a3bb2874 100644 --- a/src/response/response.rs +++ b/src/response/response.rs @@ -49,7 +49,10 @@ impl HttpResponse { /// Create an error response. #[inline] pub fn from_error(error: impl Into) -> Self { - error.into().as_response_error().error_response() + let error = error.into(); + let mut response = error.as_response_error().error_response(); + response.error = Some(error); + response } } diff --git a/src/route.rs b/src/route.rs index 44f7e30b8..d85b940bd 100644 --- a/src/route.rs +++ b/src/route.rs @@ -5,7 +5,7 @@ use std::{future::Future, rc::Rc}; use actix_http::http::Method; use actix_service::{ boxed::{self, BoxService, BoxServiceFactory}, - Service, ServiceFactory, + Service, ServiceFactory, ServiceFactoryExt, }; use futures_core::future::LocalBoxFuture; @@ -128,9 +128,10 @@ impl Route { /// Set handler function, use request extractors for parameters. /// + /// # Examples /// ``` /// use actix_web::{web, http, App}; - /// use serde_derive::Deserialize; + /// use serde::Deserialize; /// /// #[derive(Deserialize)] /// struct Info { @@ -154,7 +155,7 @@ impl Route { /// /// ``` /// # use std::collections::HashMap; - /// # use serde_derive::Deserialize; + /// # use serde::Deserialize; /// use actix_web::{web, App}; /// /// #[derive(Deserialize)] @@ -184,6 +185,53 @@ impl Route { self.service = boxed::factory(HandlerService::new(handler)); self } + + /// Set raw service to be constructed and called as the request handler. + /// + /// # Examples + /// ``` + /// # use std::convert::Infallible; + /// # use futures_util::future::LocalBoxFuture; + /// # use actix_web::{*, dev::*, http::header}; + /// struct HelloWorld; + /// + /// impl Service for HelloWorld { + /// type Response = ServiceResponse; + /// type Error = Infallible; + /// type Future = LocalBoxFuture<'static, Result>; + /// + /// always_ready!(); + /// + /// fn call(&self, req: ServiceRequest) -> Self::Future { + /// let (req, _) = req.into_parts(); + /// + /// let res = HttpResponse::Ok() + /// .insert_header(header::ContentType::plaintext()) + /// .body("Hello world!"); + /// + /// Box::pin(async move { Ok(ServiceResponse::new(req, res)) }) + /// } + /// } + /// + /// App::new().route( + /// "/", + /// web::get().service(fn_factory(|| async { Ok(HelloWorld) })), + /// ); + /// ``` + pub fn service(mut self, service_factory: S) -> Self + where + S: ServiceFactory< + ServiceRequest, + Response = ServiceResponse, + Error = E, + InitError = (), + Config = (), + > + 'static, + E: Into + 'static, + { + self.service = boxed::factory(service_factory.map_err(Into::into)); + self + } } #[cfg(test)] @@ -192,9 +240,12 @@ mod tests { use actix_rt::time::sleep; use bytes::Bytes; - use serde_derive::Serialize; + use futures_core::future::LocalBoxFuture; + use serde::Serialize; - use crate::http::{Method, StatusCode}; + use crate::dev::{always_ready, fn_factory, fn_service, Service}; + use crate::http::{header, Method, StatusCode}; + use crate::service::{ServiceRequest, ServiceResponse}; use crate::test::{call_service, init_service, read_body, TestRequest}; use crate::{error, web, App, HttpResponse}; @@ -268,4 +319,65 @@ mod tests { let body = read_body(resp).await; assert_eq!(body, Bytes::from_static(b"{\"name\":\"test\"}")); } + + #[actix_rt::test] + async fn test_service_handler() { + struct HelloWorld; + + impl Service for HelloWorld { + type Response = ServiceResponse; + type Error = crate::Error; + type Future = LocalBoxFuture<'static, Result>; + + always_ready!(); + + fn call(&self, req: ServiceRequest) -> Self::Future { + let (req, _) = req.into_parts(); + + let res = HttpResponse::Ok() + .insert_header(header::ContentType::plaintext()) + .body("Hello world!"); + + Box::pin(async move { Ok(ServiceResponse::new(req, res)) }) + } + } + + let srv = init_service( + App::new() + .route( + "/hello", + web::get().service(fn_factory(|| async { Ok(HelloWorld) })), + ) + .route( + "/bye", + web::get().service(fn_factory(|| async { + Ok::<_, ()>(fn_service(|req: ServiceRequest| async { + let (req, _) = req.into_parts(); + + let res = HttpResponse::Ok() + .insert_header(header::ContentType::plaintext()) + .body("Goodbye, and thanks for all the fish!"); + + Ok::<_, Infallible>(ServiceResponse::new(req, res)) + })) + })), + ), + ) + .await; + + let req = TestRequest::get().uri("/hello").to_request(); + let resp = call_service(&srv, req).await; + assert_eq!(resp.status(), StatusCode::OK); + let body = read_body(resp).await; + assert_eq!(body, Bytes::from_static(b"Hello world!")); + + let req = TestRequest::get().uri("/bye").to_request(); + let resp = call_service(&srv, req).await; + assert_eq!(resp.status(), StatusCode::OK); + let body = read_body(resp).await; + assert_eq!( + body, + Bytes::from_static(b"Goodbye, and thanks for all the fish!") + ); + } } diff --git a/src/scope.rs b/src/scope.rs index 412c01d95..0caf06ee3 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -1,28 +1,23 @@ -use std::cell::RefCell; -use std::fmt; -use std::future::Future; -use std::rc::Rc; +use std::{cell::RefCell, fmt, future::Future, mem, rc::Rc}; use actix_http::Extensions; use actix_router::{ResourceDef, Router}; -use actix_service::boxed::{self, BoxService, BoxServiceFactory}; use actix_service::{ - apply, apply_fn_factory, IntoServiceFactory, Service, ServiceFactory, ServiceFactoryExt, - Transform, + apply, apply_fn_factory, + boxed::{self, BoxService, BoxServiceFactory}, + IntoServiceFactory, Service, ServiceFactory, ServiceFactoryExt, Transform, }; use futures_core::future::LocalBoxFuture; use futures_util::future::join_all; -use crate::config::ServiceConfig; -use crate::data::Data; -use crate::dev::{AppService, HttpServiceFactory}; -use crate::error::Error; -use crate::guard::Guard; -use crate::resource::Resource; -use crate::rmap::ResourceMap; -use crate::route::Route; -use crate::service::{ - AppServiceFactory, ServiceFactoryWrapper, ServiceRequest, ServiceResponse, +use crate::{ + config::ServiceConfig, + data::Data, + dev::{AppService, HttpServiceFactory}, + guard::Guard, + rmap::ResourceMap, + service::{AppServiceFactory, ServiceFactoryWrapper, ServiceRequest, ServiceResponse}, + Error, Resource, Route, }; type Guards = Vec>; @@ -71,16 +66,17 @@ pub struct Scope { impl Scope { /// Create a new scope pub fn new(path: &str) -> Scope { - let fref = Rc::new(RefCell::new(None)); + let factory_ref = Rc::new(RefCell::new(None)); + Scope { - endpoint: ScopeEndpoint::new(fref.clone()), + endpoint: ScopeEndpoint::new(Rc::clone(&factory_ref)), rdef: path.to_string(), app_data: None, guards: Vec::new(), services: Vec::new(), default: None, external: Vec::new(), - factory_ref: fref, + factory_ref, } } } @@ -120,39 +116,38 @@ where self } - /// Set or override application data. Application data could be accessed - /// by using `Data` extractor where `T` is data type. - /// - /// ``` - /// use std::cell::Cell; - /// use actix_web::{web, App, HttpResponse, Responder}; - /// - /// struct MyData { - /// counter: Cell, - /// } - /// - /// async fn index(data: web::Data) -> impl Responder { - /// data.counter.set(data.counter.get() + 1); - /// HttpResponse::Ok() - /// } - /// - /// fn main() { - /// let app = App::new().service( - /// web::scope("/app") - /// .data(MyData{ counter: Cell::new(0) }) - /// .service( - /// web::resource("/index.html").route( - /// web::get().to(index))) - /// ); - /// } - /// ``` - pub fn data(self, data: U) -> Self { - self.app_data(Data::new(data)) - } - /// Add scope data. /// - /// Data of different types from parent contexts will still be accessible. + /// Data of different types from parent contexts will still be accessible. Any `Data` types + /// set here can be extracted in handlers using the `Data` extractor. + /// + /// # Examples + /// ``` + /// use std::cell::Cell; + /// use actix_web::{web, App, HttpRequest, HttpResponse, Responder}; + /// + /// struct MyData { + /// count: std::cell::Cell, + /// } + /// + /// async fn handler(req: HttpRequest, counter: web::Data) -> impl Responder { + /// // note this cannot use the Data extractor because it was not added with it + /// let incr = *req.app_data::().unwrap(); + /// assert_eq!(incr, 3); + /// + /// // update counter using other value from app data + /// counter.count.set(counter.count.get() + incr); + /// + /// HttpResponse::Ok().body(counter.count.get().to_string()) + /// } + /// + /// let app = App::new().service( + /// web::scope("/app") + /// .app_data(3usize) + /// .app_data(web::Data::new(MyData { count: Default::default() })) + /// .route("/", web::get().to(handler)) + /// ); + /// ``` pub fn app_data(mut self, data: U) -> Self { self.app_data .get_or_insert_with(Extensions::new) @@ -161,15 +156,20 @@ where self } - /// Run external configuration as part of the scope building - /// process + /// Add scope data after wrapping in `Data`. /// - /// This function is useful for moving parts of configuration to a - /// different module or even library. For example, - /// some of the resource's configuration could be moved to different module. + /// Deprecated in favor of [`app_data`](Self::app_data). + #[deprecated(since = "4.0.0", note = "Use `.app_data(Data::new(val))` instead.")] + pub fn data(self, data: U) -> Self { + self.app_data(Data::new(data)) + } + + /// Run external configuration as part of the scope building process. + /// + /// This function is useful for moving parts of configuration to a different module or library. + /// For example, some of the resource's configuration could be moved to different module. /// /// ``` - /// # extern crate actix_web; /// use actix_web::{web, middleware, App, HttpResponse}; /// /// // this function could be located in different module @@ -190,18 +190,21 @@ where /// .route("/index.html", web::get().to(|| HttpResponse::Ok())); /// } /// ``` - pub fn configure(mut self, f: F) -> Self + pub fn configure(mut self, cfg_fn: F) -> Self where F: FnOnce(&mut ServiceConfig), { let mut cfg = ServiceConfig::new(); - f(&mut cfg); + cfg_fn(&mut cfg); + self.services.extend(cfg.services); self.external.extend(cfg.external); + // TODO: add Extensions::is_empty check and conditionally insert data self.app_data .get_or_insert_with(Extensions::new) .extend(cfg.app_data); + self } @@ -418,7 +421,7 @@ where let mut rmap = ResourceMap::new(ResourceDef::root_prefix(&self.rdef)); // external resources - for mut rdef in std::mem::take(&mut self.external) { + for mut rdef in mem::take(&mut self.external) { rmap.add(&mut rdef, None); } @@ -990,6 +993,8 @@ mod tests { ); } + // allow deprecated {App, Scope}::data + #[allow(deprecated)] #[actix_rt::test] async fn test_override_data() { let srv = init_service(App::new().data(1usize).service( @@ -1008,6 +1013,8 @@ mod tests { assert_eq!(resp.status(), StatusCode::OK); } + // allow deprecated `{App, Scope}::data` + #[allow(deprecated)] #[actix_rt::test] async fn test_override_data_default_service() { let srv = init_service(App::new().data(1usize).service( diff --git a/src/service.rs b/src/service.rs index e3e975cef..592577467 100644 --- a/src/service.rs +++ b/src/service.rs @@ -17,9 +17,8 @@ use crate::{ dev::insert_slash, guard::Guard, info::ConnectionInfo, - request::HttpRequest, rmap::ResourceMap, - Error, HttpResponse, + Error, HttpRequest, HttpResponse, }; pub trait HttpServiceFactory { @@ -80,6 +79,12 @@ impl ServiceRequest { (self.req, self.payload) } + /// Get mutable access to inner `HttpRequest` and `Payload` + #[inline] + pub fn parts_mut(&mut self) -> (&mut HttpRequest, &mut Payload) { + (&mut self.req, &mut self.payload) + } + /// Construct request from parts. pub fn from_parts(req: HttpRequest, payload: Payload) -> Self { Self { req, payload } @@ -649,6 +654,8 @@ mod tests { assert_eq!(resp.status(), http::StatusCode::NOT_FOUND); } + // allow deprecated App::data + #[allow(deprecated)] #[actix_rt::test] async fn test_service_data() { let srv = diff --git a/src/test.rs b/src/test.rs index de97dc8aa..05a4ba7f2 100644 --- a/src/test.rs +++ b/src/test.rs @@ -839,6 +839,8 @@ mod tests { assert!(res.status().is_success()); } + // allow deprecated App::data + #[allow(deprecated)] #[actix_rt::test] async fn test_server_data() { async fn handler(data: web::Data) -> impl Responder { diff --git a/src/types/form.rs b/src/types/form.rs index 44d1b952e..4ce075d99 100644 --- a/src/types/form.rs +++ b/src/types/form.rs @@ -80,6 +80,10 @@ use crate::{ /// }) /// } /// ``` +/// +/// # Panics +/// URL encoded forms consist of unordered `key=value` pairs, therefore they cannot be decoded into +/// any type which depends upon data ordering (eg. tuples). Trying to do so will result in a panic. #[derive(PartialEq, Eq, PartialOrd, Ord, Debug)] pub struct Form(pub T); diff --git a/src/types/payload.rs b/src/types/payload.rs index 3b0d1d6c6..87378701b 100644 --- a/src/types/payload.rs +++ b/src/types/payload.rs @@ -398,6 +398,8 @@ mod tests { assert!(cfg.check_mimetype(&req).is_ok()); } + // allow deprecated App::data + #[allow(deprecated)] #[actix_rt::test] async fn test_config_recall_locations() { async fn bytes_handler(_: Bytes) -> impl Responder { diff --git a/src/web.rs b/src/web.rs index 8662848a4..40ac46275 100644 --- a/src/web.rs +++ b/src/web.rs @@ -43,7 +43,6 @@ pub use crate::types::*; /// the exposed `Params` object: /// /// ``` -/// # extern crate actix_web; /// use actix_web::{web, App, HttpResponse}; /// /// let app = App::new().service( diff --git a/tests/test_error_propagation.rs b/tests/test_error_propagation.rs new file mode 100644 index 000000000..3e7320920 --- /dev/null +++ b/tests/test_error_propagation.rs @@ -0,0 +1,99 @@ +use std::sync::Arc; + +use actix_utils::future::{ok, Ready}; +use actix_web::{ + dev::{forward_ready, Service, ServiceRequest, ServiceResponse, Transform}, + get, + test::{call_service, init_service, TestRequest}, + ResponseError, +}; +use futures_core::future::LocalBoxFuture; +use futures_util::lock::Mutex; + +#[derive(Debug, Clone)] +pub struct MyError; + +impl ResponseError for MyError {} + +impl std::fmt::Display for MyError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "A custom error") + } +} + +#[get("/test")] +async fn test() -> Result { + return Err(MyError.into()); +} + +#[derive(Clone)] +pub struct SpyMiddleware(Arc>>); + +impl Transform for SpyMiddleware +where + S: Service, Error = actix_web::Error>, + S::Future: 'static, + B: 'static, +{ + type Response = ServiceResponse; + type Error = actix_web::Error; + type Transform = Middleware; + type InitError = (); + type Future = Ready>; + + fn new_transform(&self, service: S) -> Self::Future { + ok(Middleware { + was_error: self.0.clone(), + service, + }) + } +} + +#[doc(hidden)] +pub struct Middleware { + was_error: Arc>>, + service: S, +} + +impl Service for Middleware +where + S: Service, Error = actix_web::Error>, + S::Future: 'static, + B: 'static, +{ + type Response = ServiceResponse; + type Error = actix_web::Error; + type Future = LocalBoxFuture<'static, Result>; + + forward_ready!(service); + + fn call(&self, req: ServiceRequest) -> Self::Future { + let lock = self.was_error.clone(); + let response_future = self.service.call(req); + Box::pin(async move { + let response = response_future.await; + if let Ok(success) = &response { + *lock.lock().await = Some(success.response().error().is_some()); + } + response + }) + } +} + +#[actix_rt::test] +async fn error_cause_should_be_propagated_to_middlewares() { + let lock = Arc::new(Mutex::new(None)); + let spy_middleware = SpyMiddleware(lock.clone()); + + let app = init_service( + actix_web::App::new() + .wrap(spy_middleware.clone()) + .service(test), + ) + .await; + + call_service(&app, TestRequest::with_uri("/test").to_request()).await; + + let was_error_captured = lock.lock().await.unwrap(); + assert!(was_error_captured); +} diff --git a/tests/test_server.rs b/tests/test_server.rs index 520eb5ce2..afea39dd9 100644 --- a/tests/test_server.rs +++ b/tests/test_server.rs @@ -879,7 +879,7 @@ async fn test_brotli_encoding_large_openssl() { assert_eq!(bytes, Bytes::from(data)); } -#[cfg(all(feature = "rustls", feature = "openssl"))] +#[cfg(feature = "rustls")] mod plus_rustls { use std::io::BufReader; @@ -1028,6 +1028,8 @@ async fn test_normalize() { assert!(response.status().is_success()); } +// allow deprecated App::data +#[allow(deprecated)] #[actix_rt::test] async fn test_data_drop() { use std::sync::{