diff --git a/CHANGES.md b/CHANGES.md index 25fd10952..70f7705c8 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,7 +1,21 @@ # Changes ## Unreleased - 2021-xx-xx +### Added +* The method `Either, web::Form>::into_inner()` which returns the inner type for + whichever variant was created. Also works for `Either, web::Json>`. [#1894] +### Changed +* Rework `Responder` trait to be sync and returns `Response`/`HttpResponse` directly. + Making it more simple and performant. [#1891] +* Our `Either` type now uses `Left`/`Right` variants (instead of `A`/`B`) [#1894] + +### Removed +* Public field of `web::Path` has been made private. [#1894] +* Public field of `web::Query` has been made private. [#1894] + +[#1891]: https://github.com/actix/actix-web/pull/1891 +[#1894]: https://github.com/actix/actix-web/pull/1894 ## 4.0.0-beta.1 - 2021-01-07 ### Added @@ -24,13 +38,15 @@ ### Removed * Public modules `middleware::{normalize, err_handlers}`. All necessary middleware structs are now exposed directly by the `middleware` module. +* Remove `actix-threadpool` as dependency. `actix_threadpool::BlockingError` error type can be imported + from `actix_web::error` module. [#1878] [#1812]: https://github.com/actix/actix-web/pull/1812 [#1813]: https://github.com/actix/actix-web/pull/1813 [#1852]: https://github.com/actix/actix-web/pull/1852 [#1865]: https://github.com/actix/actix-web/pull/1865 [#1875]: https://github.com/actix/actix-web/pull/1875 - +[#1878]: https://github.com/actix/actix-web/pull/1878 ## 3.3.2 - 2020-12-01 ### Fixed diff --git a/Cargo.toml b/Cargo.toml index 87183c327..bae6cb6cb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -76,11 +76,10 @@ required-features = ["rustls"] actix-codec = "0.4.0-beta.1" actix-macros = "0.1.0" actix-router = "0.2.4" -actix-rt = "2.0.0-beta.1" +actix-rt = "2.0.0-beta.2" actix-server = "2.0.0-beta.2" -actix-service = "2.0.0-beta.2" +actix-service = "2.0.0-beta.3" actix-utils = "3.0.0-beta.1" -actix-threadpool = "0.3.1" actix-tls = { version = "3.0.0-beta.2", default-features = false, optional = true } actix-web-codegen = "0.4.0" @@ -90,6 +89,7 @@ awc = { version = "3.0.0-beta.1", default-features = false } ahash = "0.6" bytes = "1" derive_more = "0.99.5" +either = "1.5.3" encoding_rs = "0.8" futures-core = { version = "0.3.7", default-features = false } futures-util = { version = "0.3.7", default-features = false } @@ -139,3 +139,7 @@ harness = false [[bench]] name = "service" harness = false + +[[bench]] +name = "responder" +harness = false diff --git a/actix-files/Cargo.toml b/actix-files/Cargo.toml index f93450ff8..bde2cb717 100644 --- a/actix-files/Cargo.toml +++ b/actix-files/Cargo.toml @@ -18,7 +18,7 @@ path = "src/lib.rs" [dependencies] actix-web = { version = "4.0.0-beta.1", default-features = false } -actix-service = "2.0.0-beta.2" +actix-service = "2.0.0-beta.3" bitflags = "1" bytes = "1" futures-core = { version = "0.3.7", default-features = false } @@ -31,5 +31,5 @@ percent-encoding = "2.1" v_htmlescape = "0.12" [dev-dependencies] -actix-rt = "2.0.0-beta.1" +actix-rt = "2.0.0-beta.2" actix-web = "4.0.0-beta.1" diff --git a/actix-files/src/chunked.rs b/actix-files/src/chunked.rs index 580b06787..5b7b17dc4 100644 --- a/actix-files/src/chunked.rs +++ b/actix-files/src/chunked.rs @@ -8,17 +8,11 @@ use std::{ }; use actix_web::{ - error::{BlockingError, Error}, - web, + error::{Error, ErrorInternalServerError}, + rt::task::{spawn_blocking, JoinHandle}, }; use bytes::Bytes; use futures_core::{ready, Stream}; -use futures_util::future::{FutureExt, LocalBoxFuture}; - -use crate::handle_error; - -type ChunkedBoxFuture = - LocalBoxFuture<'static, Result<(File, Bytes), BlockingError>>; #[doc(hidden)] /// A helper created from a `std::fs::File` which reads the file @@ -27,7 +21,7 @@ pub struct ChunkedReadFile { pub(crate) size: u64, pub(crate) offset: u64, pub(crate) file: Option, - pub(crate) fut: Option, + pub(crate) fut: Option>>, pub(crate) counter: u64, } @@ -45,18 +39,20 @@ impl Stream for ChunkedReadFile { cx: &mut Context<'_>, ) -> Poll> { if let Some(ref mut fut) = self.fut { - return match ready!(Pin::new(fut).poll(cx)) { - Ok((file, bytes)) => { + let res = match ready!(Pin::new(fut).poll(cx)) { + Ok(Ok((file, bytes))) => { self.fut.take(); self.file = Some(file); self.offset += bytes.len() as u64; self.counter += bytes.len() as u64; - Poll::Ready(Some(Ok(bytes))) + Ok(bytes) } - Err(e) => Poll::Ready(Some(Err(handle_error(e)))), + Ok(Err(e)) => Err(e.into()), + Err(_) => Err(ErrorInternalServerError("Unexpected error")), }; + return Poll::Ready(Some(res)); } let size = self.size; @@ -68,25 +64,21 @@ impl Stream for ChunkedReadFile { } else { let mut file = self.file.take().expect("Use after completion"); - self.fut = Some( - web::block(move || { - let max_bytes = - cmp::min(size.saturating_sub(counter), 65_536) as usize; + self.fut = Some(spawn_blocking(move || { + let max_bytes = cmp::min(size.saturating_sub(counter), 65_536) as usize; - let mut buf = Vec::with_capacity(max_bytes); - file.seek(io::SeekFrom::Start(offset))?; + let mut buf = Vec::with_capacity(max_bytes); + file.seek(io::SeekFrom::Start(offset))?; - let n_bytes = - file.by_ref().take(max_bytes as u64).read_to_end(&mut buf)?; + let n_bytes = + file.by_ref().take(max_bytes as u64).read_to_end(&mut buf)?; - if n_bytes == 0 { - return Err(io::ErrorKind::UnexpectedEof.into()); - } + if n_bytes == 0 { + return Err(io::ErrorKind::UnexpectedEof.into()); + } - Ok((file, Bytes::from(buf))) - }) - .boxed_local(), - ); + Ok((file, Bytes::from(buf))) + })); self.poll_next(cx) } diff --git a/actix-files/src/lib.rs b/actix-files/src/lib.rs index 662fba0a3..b7225fbc0 100644 --- a/actix-files/src/lib.rs +++ b/actix-files/src/lib.rs @@ -14,12 +14,10 @@ #![deny(rust_2018_idioms)] #![warn(missing_docs, missing_debug_implementations)] -use std::io; - use actix_service::boxed::{BoxService, BoxServiceFactory}; use actix_web::{ dev::{ServiceRequest, ServiceResponse}, - error::{BlockingError, Error, ErrorInternalServerError}, + error::Error, http::header::DispositionType, }; use mime_guess::from_ext; @@ -56,13 +54,6 @@ pub fn file_extension_to_mime(ext: &str) -> mime::Mime { from_ext(ext).first_or_octet_stream() } -pub(crate) fn handle_error(err: BlockingError) -> Error { - match err { - BlockingError::Error(err) => err.into(), - BlockingError::Canceled => ErrorInternalServerError("Unexpected error"), - } -} - type MimeOverride = dyn Fn(&mime::Name<'_>) -> DispositionType; #[cfg(test)] diff --git a/actix-files/src/named.rs b/actix-files/src/named.rs index a9b95bad1..b3c247b1f 100644 --- a/actix-files/src/named.rs +++ b/actix-files/src/named.rs @@ -16,10 +16,9 @@ use actix_web::{ }, ContentEncoding, StatusCode, }, - Error, HttpMessage, HttpRequest, HttpResponse, Responder, + HttpMessage, HttpRequest, HttpResponse, Responder, }; use bitflags::bitflags; -use futures_util::future::{ready, Ready}; use mime_guess::from_path; use crate::ChunkedReadFile; @@ -277,7 +276,7 @@ impl NamedFile { } /// Creates an `HttpResponse` with file as a streaming body. - pub fn into_response(self, req: &HttpRequest) -> Result { + pub fn into_response(self, req: &HttpRequest) -> HttpResponse { if self.status_code != StatusCode::OK { let mut res = HttpResponse::build(self.status_code); @@ -307,7 +306,7 @@ impl NamedFile { counter: 0, }; - return Ok(res.streaming(reader)); + return res.streaming(reader); } let etag = if self.flags.contains(Flags::ETAG) { @@ -411,17 +410,17 @@ impl NamedFile { ); } else { resp.header(header::CONTENT_RANGE, format!("bytes */{}", length)); - return Ok(resp.status(StatusCode::RANGE_NOT_SATISFIABLE).finish()); + return resp.status(StatusCode::RANGE_NOT_SATISFIABLE).finish(); }; } else { - return Ok(resp.status(StatusCode::BAD_REQUEST).finish()); + return resp.status(StatusCode::BAD_REQUEST).finish(); }; }; if precondition_failed { - return Ok(resp.status(StatusCode::PRECONDITION_FAILED).finish()); + return resp.status(StatusCode::PRECONDITION_FAILED).finish(); } else if not_modified { - return Ok(resp.status(StatusCode::NOT_MODIFIED).finish()); + return resp.status(StatusCode::NOT_MODIFIED).finish(); } let reader = ChunkedReadFile { @@ -436,7 +435,7 @@ impl NamedFile { resp.status(StatusCode::PARTIAL_CONTENT); } - Ok(resp.body(SizedStream::new(length, reader))) + resp.body(SizedStream::new(length, reader)) } } @@ -495,10 +494,7 @@ fn none_match(etag: Option<&header::EntityTag>, req: &HttpRequest) -> bool { } impl Responder for NamedFile { - type Error = Error; - type Future = Ready>; - - fn respond_to(self, req: &HttpRequest) -> Self::Future { - ready(self.into_response(req)) + fn respond_to(self, req: &HttpRequest) -> HttpResponse { + self.into_response(req) } } diff --git a/actix-files/src/service.rs b/actix-files/src/service.rs index 1e3d64a0d..05431db38 100644 --- a/actix-files/src/service.rs +++ b/actix-files/src/service.rs @@ -120,10 +120,8 @@ impl Service for FilesService { named_file.flags = self.file_flags; let (req, _) = req.into_parts(); - Either::Left(ok(match named_file.into_response(&req) { - Ok(item) => ServiceResponse::new(req, item), - Err(e) => ServiceResponse::from_err(e, req), - })) + let res = named_file.into_response(&req); + Either::Left(ok(ServiceResponse::new(req, res))) } Err(e) => self.handle_err(e, req), } @@ -154,12 +152,8 @@ impl Service for FilesService { named_file.flags = self.file_flags; let (req, _) = req.into_parts(); - match named_file.into_response(&req) { - Ok(item) => { - Either::Left(ok(ServiceResponse::new(req.clone(), item))) - } - Err(e) => Either::Left(ok(ServiceResponse::from_err(e, req))), - } + let res = named_file.into_response(&req); + Either::Left(ok(ServiceResponse::new(req, res))) } Err(e) => self.handle_err(e, req), } diff --git a/actix-http-test/Cargo.toml b/actix-http-test/Cargo.toml index a056b833e..772b60f76 100644 --- a/actix-http-test/Cargo.toml +++ b/actix-http-test/Cargo.toml @@ -29,11 +29,11 @@ default = [] openssl = ["open-ssl", "awc/openssl"] [dependencies] -actix-service = "2.0.0-beta.2" +actix-service = "2.0.0-beta.3" actix-codec = "0.4.0-beta.1" actix-tls = "3.0.0-beta.2" actix-utils = "3.0.0-beta.1" -actix-rt = "2.0.0-beta.1" +actix-rt = "2.0.0-beta.2" actix-server = "2.0.0-beta.2" awc = "3.0.0-beta.1" diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index 6abd0ba76..e9a94300b 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -1,6 +1,9 @@ # Changes ## Unreleased - 2021-xx-xx +* `Response::content_type` now takes an `impl IntoHeaderValue` to support `mime` types. [#1894] + +[#1894]: https://github.com/actix/actix-web/pull/1894 ## 3.0.0-beta.1 - 2021-01-07 @@ -22,11 +25,14 @@ * Remove `ConnectError::SslHandshakeError` and re-export of `HandshakeError`. due to the removal of this type from `tokio-openssl` crate. openssl handshake error would return as `ConnectError::SslError`. [#1813] +* Remove `actix-threadpool` dependency. Use `actix_rt::task::spawn_blocking`. + Due to this change `actix_threadpool::BlockingError` type is moved into + `actix_http::error` module. [#1878] [#1813]: https://github.com/actix/actix-web/pull/1813 [#1857]: https://github.com/actix/actix-web/pull/1857 [#1864]: https://github.com/actix/actix-web/pull/1864 - +[#1878]: https://github.com/actix/actix-web/pull/1878 ## 2.2.0 - 2020-11-25 ### Added diff --git a/actix-http/Cargo.toml b/actix-http/Cargo.toml index b64c71a8a..0cc8e5cf9 100644 --- a/actix-http/Cargo.toml +++ b/actix-http/Cargo.toml @@ -40,11 +40,10 @@ secure-cookies = ["cookie/secure"] actors = ["actix"] [dependencies] -actix-service = "2.0.0-beta.2" +actix-service = "2.0.0-beta.3" actix-codec = "0.4.0-beta.1" actix-utils = "3.0.0-beta.1" -actix-rt = "2.0.0-beta.1" -actix-threadpool = "0.3.1" +actix-rt = "2.0.0-beta.2" actix-tls = "3.0.0-beta.2" actix = { version = "0.11.0-beta.1", optional = true } diff --git a/actix-http/src/encoding/decoder.rs b/actix-http/src/encoding/decoder.rs index b60435859..b26609911 100644 --- a/actix-http/src/encoding/decoder.rs +++ b/actix-http/src/encoding/decoder.rs @@ -3,14 +3,14 @@ use std::io::{self, Write}; use std::pin::Pin; use std::task::{Context, Poll}; -use actix_threadpool::{run, CpuFuture}; +use actix_rt::task::{spawn_blocking, JoinHandle}; use brotli2::write::BrotliDecoder; use bytes::Bytes; use flate2::write::{GzDecoder, ZlibDecoder}; use futures_core::{ready, Stream}; use super::Writer; -use crate::error::PayloadError; +use crate::error::{BlockingError, PayloadError}; use crate::http::header::{ContentEncoding, HeaderMap, CONTENT_ENCODING}; const INPLACE: usize = 2049; @@ -19,7 +19,7 @@ pub struct Decoder { decoder: Option, stream: S, eof: bool, - fut: Option, ContentDecoder), io::Error>>, + fut: Option, ContentDecoder), io::Error>>>, } impl Decoder @@ -80,8 +80,13 @@ where loop { if let Some(ref mut fut) = self.fut { let (chunk, decoder) = match ready!(Pin::new(fut).poll(cx)) { - Ok(item) => item, - Err(e) => return Poll::Ready(Some(Err(e.into()))), + Ok(Ok(item)) => item, + Ok(Err(e)) => { + return Poll::Ready(Some(Err(BlockingError::Error(e).into()))) + } + Err(_) => { + return Poll::Ready(Some(Err(BlockingError::Canceled.into()))) + } }; self.decoder = Some(decoder); self.fut.take(); @@ -105,7 +110,7 @@ where return Poll::Ready(Some(Ok(chunk))); } } else { - self.fut = Some(run(move || { + self.fut = Some(spawn_blocking(move || { let chunk = decoder.feed_data(chunk)?; Ok((chunk, decoder)) })); diff --git a/actix-http/src/encoding/encoder.rs b/actix-http/src/encoding/encoder.rs index eb1821285..28c757076 100644 --- a/actix-http/src/encoding/encoder.rs +++ b/actix-http/src/encoding/encoder.rs @@ -4,7 +4,7 @@ use std::io::{self, Write}; use std::pin::Pin; use std::task::{Context, Poll}; -use actix_threadpool::{run, CpuFuture}; +use actix_rt::task::{spawn_blocking, JoinHandle}; use brotli2::write::BrotliEncoder; use bytes::Bytes; use flate2::write::{GzEncoder, ZlibEncoder}; @@ -17,6 +17,7 @@ use crate::http::{HeaderValue, StatusCode}; use crate::{Error, ResponseHead}; use super::Writer; +use crate::error::BlockingError; const INPLACE: usize = 1024; @@ -26,7 +27,7 @@ pub struct Encoder { #[pin] body: EncoderBody, encoder: Option, - fut: Option>, + fut: Option>>, } impl Encoder { @@ -136,8 +137,15 @@ impl MessageBody for Encoder { if let Some(ref mut fut) = this.fut { let mut encoder = match ready!(Pin::new(fut).poll(cx)) { - Ok(item) => item, - Err(e) => return Poll::Ready(Some(Err(e.into()))), + Ok(Ok(item)) => item, + Ok(Err(e)) => { + return Poll::Ready(Some(Err(BlockingError::Error(e).into()))) + } + Err(_) => { + return Poll::Ready(Some(Err( + BlockingError::::Canceled.into(), + ))) + } }; let chunk = encoder.take(); *this.encoder = Some(encoder); @@ -160,7 +168,7 @@ impl MessageBody for Encoder { return Poll::Ready(Some(Ok(chunk))); } } else { - *this.fut = Some(run(move || { + *this.fut = Some(spawn_blocking(move || { encoder.write(&chunk)?; Ok(encoder) })); diff --git a/actix-http/src/error.rs b/actix-http/src/error.rs index 03e5467c5..a585962be 100644 --- a/actix-http/src/error.rs +++ b/actix-http/src/error.rs @@ -7,7 +7,6 @@ use std::string::FromUtf8Error; use std::{fmt, io, result}; use actix_codec::{Decoder, Encoder}; -pub use actix_threadpool::BlockingError; use actix_utils::dispatcher::DispatcherError as FramedDispatcherError; use actix_utils::timeout::TimeoutError; use bytes::BytesMut; @@ -100,10 +99,6 @@ impl fmt::Debug for Error { } impl std::error::Error for Error { - fn cause(&self) -> Option<&dyn std::error::Error> { - None - } - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } @@ -190,9 +185,6 @@ impl ResponseError for DeError { /// `InternalServerError` for `Canceled` impl ResponseError for Canceled {} -/// `InternalServerError` for `BlockingError` -impl ResponseError for BlockingError {} - /// Return `BAD_REQUEST` for `Utf8Error` impl ResponseError for Utf8Error { fn status_code(&self) -> StatusCode { @@ -304,33 +296,64 @@ impl From for ParseError { } } +/// A set of errors that can occur running blocking tasks in thread pool. +#[derive(Debug, Display)] +pub enum BlockingError { + #[display(fmt = "{:?}", _0)] + Error(E), + #[display(fmt = "Thread pool is gone")] + Canceled, +} + +impl std::error::Error for BlockingError {} + +/// `InternalServerError` for `BlockingError` +impl ResponseError for BlockingError {} + #[derive(Display, Debug)] /// A set of errors that can occur during payload parsing pub enum PayloadError { /// A payload reached EOF, but is not complete. #[display( - fmt = "A payload reached EOF, but is not complete. With error: {:?}", + fmt = "A payload reached EOF, but is not complete. Inner error: {:?}", _0 )] Incomplete(Option), - /// Content encoding stream corruption + + /// Content encoding stream corruption. #[display(fmt = "Can not decode content-encoding.")] EncodingCorrupted, - /// A payload reached size limit. - #[display(fmt = "A payload reached size limit.")] + + /// Payload reached size limit. + #[display(fmt = "Payload reached size limit.")] Overflow, - /// A payload length is unknown. - #[display(fmt = "A payload length is unknown.")] + + /// Payload length is unknown. + #[display(fmt = "Payload length is unknown.")] UnknownLength, - /// Http2 payload error + + /// HTTP/2 payload error. #[display(fmt = "{}", _0)] Http2Payload(h2::Error), - /// Io error + + /// Generic I/O error. #[display(fmt = "{}", _0)] Io(io::Error), } -impl std::error::Error for PayloadError {} +impl std::error::Error for PayloadError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + PayloadError::Incomplete(None) => None, + PayloadError::Incomplete(Some(err)) => Some(err as &dyn std::error::Error), + PayloadError::EncodingCorrupted => None, + PayloadError::Overflow => None, + PayloadError::UnknownLength => None, + PayloadError::Http2Payload(err) => Some(err as &dyn std::error::Error), + PayloadError::Io(err) => Some(err as &dyn std::error::Error), + } + } +} impl From for PayloadError { fn from(err: h2::Error) -> Self { @@ -1009,22 +1032,22 @@ mod tests { fn test_payload_error() { let err: PayloadError = io::Error::new(io::ErrorKind::Other, "ParseError").into(); - assert!(format!("{}", err).contains("ParseError")); + assert!(err.to_string().contains("ParseError")); let err = PayloadError::Incomplete(None); assert_eq!( - format!("{}", err), - "A payload reached EOF, but is not complete. With error: None" + err.to_string(), + "A payload reached EOF, but is not complete. Inner error: None" ); } macro_rules! from { ($from:expr => $error:pat) => { match ParseError::from($from) { - e @ $error => { - assert!(format!("{}", e).len() >= 5); + err @ $error => { + assert!(err.to_string().len() >= 5); } - e => unreachable!("{:?}", e), + err => unreachable!("{:?}", err), } }; } @@ -1067,7 +1090,7 @@ mod tests { let err = PayloadError::Overflow; let resp_err: &dyn ResponseError = &err; let err = resp_err.downcast_ref::().unwrap(); - assert_eq!(err.to_string(), "A payload reached size limit."); + assert_eq!(err.to_string(), "Payload reached size limit."); let not_err = resp_err.downcast_ref::(); assert!(not_err.is_none()); } diff --git a/actix-http/src/message.rs b/actix-http/src/message.rs index 1a5500c31..bccb4d53e 100644 --- a/actix-http/src/message.rs +++ b/actix-http/src/message.rs @@ -34,7 +34,9 @@ bitflags! { pub trait Head: Default + 'static { fn clear(&mut self); - fn pool() -> &'static MessagePool; + fn with_pool(f: F) -> R + where + F: FnOnce(&MessagePool) -> R; } #[derive(Debug)] @@ -69,8 +71,11 @@ impl Head for RequestHead { self.extensions.get_mut().clear(); } - fn pool() -> &'static MessagePool { - REQUEST_POOL.with(|p| *p) + fn with_pool(f: F) -> R + where + F: FnOnce(&MessagePool) -> R, + { + REQUEST_POOL.with(|p| f(p)) } } @@ -344,7 +349,7 @@ pub struct Message { impl Message { /// Get new message from the pool of objects pub fn new() -> Self { - T::pool().get_message() + T::with_pool(|p| p.get_message()) } } @@ -373,7 +378,7 @@ impl std::ops::DerefMut for Message { impl Drop for Message { fn drop(&mut self) { if Rc::strong_count(&self.head) == 1 { - T::pool().release(self.head.clone()); + T::with_pool(|p| p.release(self.head.clone())) } } } @@ -426,18 +431,17 @@ pub struct MessagePool(RefCell>>); /// Request's objects pool pub struct BoxedResponsePool(RefCell>>); -thread_local!(static REQUEST_POOL: &'static MessagePool = MessagePool::::create()); -thread_local!(static RESPONSE_POOL: &'static BoxedResponsePool = BoxedResponsePool::create()); +thread_local!(static REQUEST_POOL: MessagePool = MessagePool::::create()); +thread_local!(static RESPONSE_POOL: BoxedResponsePool = BoxedResponsePool::create()); impl MessagePool { - fn create() -> &'static MessagePool { - let pool = MessagePool(RefCell::new(Vec::with_capacity(128))); - Box::leak(Box::new(pool)) + fn create() -> MessagePool { + MessagePool(RefCell::new(Vec::with_capacity(128))) } /// Get message from the pool #[inline] - fn get_message(&'static self) -> Message { + fn get_message(&self) -> Message { if let Some(mut msg) = self.0.borrow_mut().pop() { // Message is put in pool only when it's the last copy. // which means it's guaranteed to be unique when popped out. @@ -463,14 +467,13 @@ impl MessagePool { } impl BoxedResponsePool { - fn create() -> &'static BoxedResponsePool { - let pool = BoxedResponsePool(RefCell::new(Vec::with_capacity(128))); - Box::leak(Box::new(pool)) + fn create() -> BoxedResponsePool { + BoxedResponsePool(RefCell::new(Vec::with_capacity(128))) } /// Get message from the pool #[inline] - fn get_message(&'static self, status: StatusCode) -> BoxedResponseHead { + fn get_message(&self, status: StatusCode) -> BoxedResponseHead { if let Some(mut head) = self.0.borrow_mut().pop() { head.reason = None; head.status = status; diff --git a/actix-http/src/response.rs b/actix-http/src/response.rs index df2f5be50..0a1f2cfd2 100644 --- a/actix-http/src/response.rs +++ b/actix-http/src/response.rs @@ -481,15 +481,14 @@ impl ResponseBuilder { self } - /// Set response content type + /// Set response content type. #[inline] pub fn content_type(&mut self, value: V) -> &mut Self where - HeaderValue: TryFrom, - >::Error: Into, + V: IntoHeaderValue, { if let Some(parts) = parts(&mut self.head, &self.err) { - match HeaderValue::try_from(value) { + match value.try_into() { Ok(value) => { parts.headers.insert(header::CONTENT_TYPE, value); } @@ -802,7 +801,7 @@ impl From for Response { impl From<&'static str> for Response { fn from(val: &'static str) -> Self { Response::Ok() - .content_type("text/plain; charset=utf-8") + .content_type(mime::TEXT_PLAIN_UTF_8) .body(val) } } @@ -810,7 +809,7 @@ impl From<&'static str> for Response { impl From<&'static [u8]> for Response { fn from(val: &'static [u8]) -> Self { Response::Ok() - .content_type("application/octet-stream") + .content_type(mime::APPLICATION_OCTET_STREAM) .body(val) } } @@ -818,7 +817,7 @@ impl From<&'static [u8]> for Response { impl From for Response { fn from(val: String) -> Self { Response::Ok() - .content_type("text/plain; charset=utf-8") + .content_type(mime::TEXT_PLAIN_UTF_8) .body(val) } } @@ -826,7 +825,7 @@ impl From for Response { impl<'a> From<&'a String> for Response { fn from(val: &'a String) -> Self { Response::Ok() - .content_type("text/plain; charset=utf-8") + .content_type(mime::TEXT_PLAIN_UTF_8) .body(val) } } @@ -834,7 +833,7 @@ impl<'a> From<&'a String> for Response { impl From for Response { fn from(val: Bytes) -> Self { Response::Ok() - .content_type("application/octet-stream") + .content_type(mime::APPLICATION_OCTET_STREAM) .body(val) } } @@ -842,7 +841,7 @@ impl From for Response { impl From for Response { fn from(val: BytesMut) -> Self { Response::Ok() - .content_type("application/octet-stream") + .content_type(mime::APPLICATION_OCTET_STREAM) .body(val) } } diff --git a/actix-http/src/ws/codec.rs b/actix-http/src/ws/codec.rs index 84f5b3c73..d01e8dab9 100644 --- a/actix-http/src/ws/codec.rs +++ b/actix-http/src/ws/codec.rs @@ -89,7 +89,7 @@ impl Codec { /// Set max frame size. /// - /// By default max size is set to 64kb. + /// By default max size is set to 64kB. pub fn max_size(mut self, size: usize) -> Self { self.max_size = size; self diff --git a/actix-multipart/Cargo.toml b/actix-multipart/Cargo.toml index d22cf7ef0..44a7e8d16 100644 --- a/actix-multipart/Cargo.toml +++ b/actix-multipart/Cargo.toml @@ -28,5 +28,5 @@ mime = "0.3" twoway = "0.2" [dev-dependencies] -actix-rt = "2.0.0-beta.1" +actix-rt = "2.0.0-beta.2" actix-http = "3.0.0-beta.1" diff --git a/actix-web-actors/Cargo.toml b/actix-web-actors/Cargo.toml index 331363543..0f90edb07 100644 --- a/actix-web-actors/Cargo.toml +++ b/actix-web-actors/Cargo.toml @@ -28,6 +28,6 @@ pin-project = "1.0.0" tokio = { version = "1", features = ["sync"] } [dev-dependencies] -actix-rt = "2.0.0-beta.1" +actix-rt = "2.0.0-beta.2" env_logger = "0.7" futures-util = { version = "0.3.7", default-features = false } diff --git a/actix-web-codegen/Cargo.toml b/actix-web-codegen/Cargo.toml index 25e88d9e1..00875cf1b 100644 --- a/actix-web-codegen/Cargo.toml +++ b/actix-web-codegen/Cargo.toml @@ -19,7 +19,7 @@ syn = { version = "1", features = ["full", "parsing"] } proc-macro2 = "1" [dev-dependencies] -actix-rt = "2.0.0-beta.1" +actix-rt = "2.0.0-beta.2" actix-web = "4.0.0-beta.1" futures-util = { version = "0.3.7", default-features = false } trybuild = "1" diff --git a/awc/Cargo.toml b/awc/Cargo.toml index b92df8247..0dbf80d33 100644 --- a/awc/Cargo.toml +++ b/awc/Cargo.toml @@ -38,9 +38,9 @@ compress = ["actix-http/compress"] [dependencies] actix-codec = "0.4.0-beta.1" -actix-service = "2.0.0-beta.2" +actix-service = "2.0.0-beta.3" actix-http = "3.0.0-beta.1" -actix-rt = "2.0.0-beta.1" +actix-rt = "2.0.0-beta.2" base64 = "0.13" bytes = "1" diff --git a/awc/src/response.rs b/awc/src/response.rs index a32412b23..c3e7d71ce 100644 --- a/awc/src/response.rs +++ b/awc/src/response.rs @@ -184,7 +184,7 @@ where } } - /// Change max size of payload. By default max size is 256Kb + /// Change max size of payload. By default max size is 256kB pub fn limit(mut self, limit: usize) -> Self { if let Some(ref mut fut) = self.fut { fut.limit = limit; @@ -276,7 +276,7 @@ where } } - /// Change max size of payload. By default max size is 64Kb + /// Change max size of payload. By default max size is 64kB pub fn limit(mut self, limit: usize) -> Self { if let Some(ref mut fut) = self.fut { fut.limit = limit; diff --git a/awc/src/ws.rs b/awc/src/ws.rs index fda2aefca..f747f701f 100644 --- a/awc/src/ws.rs +++ b/awc/src/ws.rs @@ -147,7 +147,7 @@ impl WebsocketsRequest { /// Set max frame size /// - /// By default max size is set to 64kb + /// By default max size is set to 64kB pub fn max_frame_size(mut self, size: usize) -> Self { self.max_size = size; self diff --git a/benches/responder.rs b/benches/responder.rs new file mode 100644 index 000000000..61180d575 --- /dev/null +++ b/benches/responder.rs @@ -0,0 +1,113 @@ +use std::future::Future; +use std::time::Instant; + +use actix_http::Response; +use actix_web::http::StatusCode; +use actix_web::test::TestRequest; +use actix_web::{error, Error, HttpRequest, HttpResponse, Responder}; +use criterion::{criterion_group, criterion_main, Criterion}; +use futures_util::future::{ready, Either, Ready}; + +// responder simulate the old responder trait. +trait FutureResponder { + type Error; + type Future: Future>; + + fn future_respond_to(self, req: &HttpRequest) -> Self::Future; +} + +// a simple option responder type. +struct OptionResponder(Option); + +// a simple wrapper type around string +struct StringResponder(String); + +impl FutureResponder for StringResponder { + type Error = Error; + type Future = Ready>; + + fn future_respond_to(self, _: &HttpRequest) -> Self::Future { + // this is default builder for string response in both new and old responder trait. + ready(Ok(Response::build(StatusCode::OK) + .content_type("text/plain; charset=utf-8") + .body(self.0))) + } +} + +impl FutureResponder for OptionResponder +where + T: FutureResponder, + T::Future: Future>, +{ + type Error = Error; + type Future = Either>>; + + fn future_respond_to(self, req: &HttpRequest) -> Self::Future { + match self.0 { + Some(t) => Either::Left(t.future_respond_to(req)), + None => Either::Right(ready(Err(error::ErrorInternalServerError("err")))), + } + } +} + +impl Responder for StringResponder { + fn respond_to(self, _: &HttpRequest) -> HttpResponse { + Response::build(StatusCode::OK) + .content_type("text/plain; charset=utf-8") + .body(self.0) + } +} + +impl Responder for OptionResponder { + fn respond_to(self, req: &HttpRequest) -> HttpResponse { + match self.0 { + Some(t) => t.respond_to(req), + None => Response::from_error(error::ErrorInternalServerError("err")), + } + } +} + +fn future_responder(c: &mut Criterion) { + let rt = actix_rt::System::new("test"); + let req = TestRequest::default().to_http_request(); + + c.bench_function("future_responder", move |b| { + b.iter_custom(|_| { + let futs = (0..100_000).map(|_| async { + StringResponder(String::from("Hello World!!")) + .future_respond_to(&req) + .await + }); + + let futs = futures_util::future::join_all(futs); + + let start = Instant::now(); + + let _res = rt.block_on(async { futs.await }); + + start.elapsed() + }) + }); +} + +fn responder(c: &mut Criterion) { + let rt = actix_rt::System::new("test"); + let req = TestRequest::default().to_http_request(); + c.bench_function("responder", move |b| { + b.iter_custom(|_| { + let responders = + (0..100_000).map(|_| StringResponder(String::from("Hello World!!"))); + + let start = Instant::now(); + let _res = rt.block_on(async { + // don't need runtime block on but to be fair. + responders.map(|r| r.respond_to(&req)).collect::>() + }); + + start.elapsed() + }) + }); +} + +criterion_group!(responder_bench, future_responder, responder); +criterion_main!(responder_bench); diff --git a/src/app_service.rs b/src/app_service.rs index a8d7054f3..c4ac0b094 100644 --- a/src/app_service.rs +++ b/src/app_service.rs @@ -13,7 +13,7 @@ use crate::config::{AppConfig, AppService}; use crate::data::{DataFactory, FnDataFactory}; use crate::error::Error; use crate::guard::Guard; -use crate::request::HttpRequest; +use crate::request::{HttpRequest, HttpRequestPool}; use crate::rmap::ResourceMap; use crate::service::{AppServiceFactory, ServiceRequest, ServiceResponse}; @@ -62,7 +62,8 @@ where type Future = LocalBoxFuture<'static, Result>; fn new_service(&self, config: AppConfig) -> Self::Future { - // update resource default service + // set AppService's default service to 404 NotFound + // if no user defined default service exists. let default = self.default.clone().unwrap_or_else(|| { Rc::new(boxed::factory(fn_service(|req: ServiceRequest| async { Ok(req.into_response(Response::NotFound().finish())) @@ -141,9 +142,8 @@ where Ok(AppInitService { service, - rmap, - config, app_data: Rc::new(app_data), + app_state: AppInitServiceState::new(rmap, config), }) }) } @@ -155,9 +155,42 @@ where T: Service, Error = Error>, { service: T, + app_data: Rc, + app_state: Rc, +} + +// a collection of AppInitService state that shared between HttpRequests. +pub(crate) struct AppInitServiceState { rmap: Rc, config: AppConfig, - app_data: Rc, + pool: HttpRequestPool, +} + +impl AppInitServiceState { + pub(crate) fn new(rmap: Rc, config: AppConfig) -> Rc { + Rc::new(AppInitServiceState { + rmap, + config, + // TODO: AppConfig can be used to pass user defined HttpRequestPool + // capacity. + pool: HttpRequestPool::default(), + }) + } + + #[inline] + pub(crate) fn rmap(&self) -> &ResourceMap { + &*self.rmap + } + + #[inline] + pub(crate) fn config(&self) -> &AppConfig { + &self.config + } + + #[inline] + pub(crate) fn pool(&self) -> &HttpRequestPool { + &self.pool + } } impl Service for AppInitService @@ -175,7 +208,7 @@ where fn call(&mut self, req: Request) -> Self::Future { let (head, payload) = req.into_parts(); - let req = if let Some(mut req) = self.config.pool().get() { + let req = if let Some(mut req) = self.app_state.pool().pop() { let inner = Rc::get_mut(&mut req.inner).unwrap(); inner.path.get_mut().update(&head.uri); inner.path.reset(); @@ -187,8 +220,7 @@ where Path::new(Url::new(head.uri.clone())), head, payload, - self.rmap.clone(), - self.config.clone(), + self.app_state.clone(), self.app_data.clone(), ) }; @@ -196,13 +228,12 @@ where } } -// TODO: remove the drop impl as pool is not leaked anymore. impl Drop for AppInitService where T: Service, Error = Error>, { fn drop(&mut self) { - self.config.pool().clear(); + self.app_state.pool().clear(); } } diff --git a/src/config.rs b/src/config.rs index e62c36b91..2b93ae892 100644 --- a/src/config.rs +++ b/src/config.rs @@ -8,7 +8,6 @@ use actix_service::{boxed, IntoServiceFactory, ServiceFactory}; use crate::data::{Data, DataFactory}; use crate::error::Error; use crate::guard::Guard; -use crate::request::HttpRequestPool; use crate::resource::Resource; use crate::rmap::ResourceMap; use crate::route::Route; @@ -126,23 +125,15 @@ impl AppService { /// Application connection config #[derive(Clone)] -pub struct AppConfig(Rc); - -struct AppConfigInner { +pub struct AppConfig { secure: bool, host: String, addr: SocketAddr, - pool: HttpRequestPool, } impl AppConfig { pub(crate) fn new(secure: bool, addr: SocketAddr, host: String) -> Self { - AppConfig(Rc::new(AppConfigInner { - secure, - addr, - host, - pool: HttpRequestPool::default(), - })) + AppConfig { secure, addr, host } } /// Server host name. @@ -153,22 +144,17 @@ impl AppConfig { /// /// By default host name is set to a "localhost" value. pub fn host(&self) -> &str { - &self.0.host + &self.host } /// Returns true if connection is secure(https) pub fn secure(&self) -> bool { - self.0.secure + self.secure } /// Returns the socket address of the local half of this TCP connection pub fn local_addr(&self) -> SocketAddr { - self.0.addr - } - - #[inline] - pub(crate) fn pool(&self) -> &HttpRequestPool { - &self.0.pool + self.addr } } diff --git a/src/error.rs b/src/error.rs index 60af8fa11..c0d6f8af9 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,12 +1,11 @@ //! Error and Result module pub use actix_http::error::*; -use derive_more::{Display, From}; +use derive_more::{Display, Error, From}; use serde_json::error::Error as JsonError; use url::ParseError as UrlParseError; -use crate::http::StatusCode; -use crate::HttpResponse; +use crate::{http::StatusCode, HttpResponse}; /// Errors which can occur when attempting to generate resource uri. #[derive(Debug, PartialEq, Display, From)] @@ -28,34 +27,37 @@ impl std::error::Error for UrlGenerationError {} impl ResponseError for UrlGenerationError {} /// A set of errors that can occur during parsing urlencoded payloads -#[derive(Debug, Display, From)] +#[derive(Debug, Display, Error, From)] pub enum UrlencodedError { - /// Can not decode chunked transfer encoding - #[display(fmt = "Can not decode chunked transfer encoding")] + /// Can not decode chunked transfer encoding. + #[display(fmt = "Can not decode chunked transfer encoding.")] Chunked, - /// Payload size is bigger than allowed. (default: 256kB) + + /// Payload size is larger than allowed. (default limit: 256kB). #[display( - fmt = "Urlencoded payload size is bigger ({} bytes) than allowed (default: {} bytes)", + fmt = "URL encoded payload is larger ({} bytes) than allowed (limit: {} bytes).", size, limit )] Overflow { size: usize, limit: usize }, - /// Payload size is now known - #[display(fmt = "Payload size is now known")] + + /// Payload size is now known. + #[display(fmt = "Payload size is now known.")] UnknownLength, - /// Content type error - #[display(fmt = "Content type error")] + + /// Content type error. + #[display(fmt = "Content type error.")] ContentType, - /// Parse error - #[display(fmt = "Parse error")] + + /// Parse error. + #[display(fmt = "Parse error.")] Parse, - /// Payload error - #[display(fmt = "Error that occur during reading payload: {}", _0)] + + /// Payload error. + #[display(fmt = "Error that occur during reading payload: {}.", _0)] Payload(PayloadError), } -impl std::error::Error for UrlencodedError {} - /// Return `BadRequest` for `UrlencodedError` impl ResponseError for UrlencodedError { fn status_code(&self) -> StatusCode { @@ -115,16 +117,14 @@ impl ResponseError for PathError { } } -/// A set of errors that can occur during parsing query strings -#[derive(Debug, Display, From)] +/// A set of errors that can occur during parsing query strings. +#[derive(Debug, Display, Error, From)] pub enum QueryPayloadError { - /// Deserialize error + /// Query deserialize error. #[display(fmt = "Query deserialize error: {}", _0)] Deserialize(serde::de::value::Error), } -impl std::error::Error for QueryPayloadError {} - /// Return `BadRequest` for `QueryPayloadError` impl ResponseError for QueryPayloadError { fn status_code(&self) -> StatusCode { diff --git a/src/extract.rs b/src/extract.rs index 5916b1bc5..4081188ef 100644 --- a/src/extract.rs +++ b/src/extract.rs @@ -1,34 +1,37 @@ //! Request extractors -use std::future::Future; -use std::pin::Pin; -use std::task::{Context, Poll}; -use actix_http::error::Error; -use futures_util::future::{ready, Ready}; -use futures_util::ready; +use std::{ + future::Future, + pin::Pin, + task::{Context, Poll}, +}; -use crate::dev::Payload; -use crate::request::HttpRequest; +use futures_util::{ + future::{ready, Ready}, + ready, +}; + +use crate::{dev::Payload, Error, HttpRequest}; /// Trait implemented by types that can be extracted from request. /// /// Types that implement this trait can be used with `Route` handlers. pub trait FromRequest: Sized { + /// Configuration for this extractor. + type Config: Default + 'static; + /// The associated error which can be returned. type Error: Into; - /// Future that resolves to a Self + /// Future that resolves to a Self. type Future: Future>; - /// Configuration for this extractor - type Config: Default + 'static; - - /// Convert request to a Self + /// Create a Self from request parts asynchronously. fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future; - /// Convert request to a Self + /// Create a Self from request head asynchronously. /// - /// This method uses `Payload::None` as payload stream. + /// This method is short for `T::from_request(req, &mut Payload::None)`. fn extract(req: &HttpRequest) -> Self::Future { Self::from_request(req, &mut Payload::None) } diff --git a/src/handler.rs b/src/handler.rs index 30cc59842..47656cd84 100644 --- a/src/handler.rs +++ b/src/handler.rs @@ -135,7 +135,6 @@ where { Extract(#[pin] T::Future, Option, F), Handle(#[pin] R, Option), - Respond(#[pin] ::Future, Option), } impl Future for HandlerServiceFuture @@ -168,13 +167,8 @@ where } HandlerProj::Handle(fut, req) => { let res = ready!(fut.poll(cx)); - let fut = res.respond_to(req.as_ref().unwrap()); - let state = HandlerServiceFuture::Respond(fut, req.take()); - self.as_mut().set(state); - } - HandlerProj::Respond(fut, req) => { - let res = ready!(fut.poll(cx)).unwrap_or_else(|e| e.into().into()); let req = req.take().unwrap(); + let res = res.respond_to(&req); return Poll::Ready(Ok(ServiceResponse::new(req, res))); } } diff --git a/src/lib.rs b/src/lib.rs index 88eae44bf..fa4e70aec 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -6,7 +6,8 @@ //! use actix_web::{get, web, App, HttpServer, Responder}; //! //! #[get("/{id}/{name}/index.html")] -//! async fn index(web::Path((id, name)): web::Path<(u32, String)>) -> impl Responder { +//! async fn index(path: web::Path<(u32, String)>) -> impl Responder { +//! let (id, name) = path.into_inner(); //! format!("Hello {}! id:{}", name, id) //! } //! @@ -90,7 +91,7 @@ mod scope; mod server; mod service; pub mod test; -mod types; +pub(crate) mod types; pub mod web; pub use actix_http::Response as HttpResponse; @@ -106,6 +107,7 @@ pub use crate::responder::Responder; pub use crate::route::Route; pub use crate::scope::Scope; pub use crate::server::HttpServer; +// TODO: is exposing the error directly really needed pub use crate::types::{Either, EitherExtractError}; pub mod dev { diff --git a/src/middleware/logger.rs b/src/middleware/logger.rs index cdbd5e485..276265a58 100644 --- a/src/middleware/logger.rs +++ b/src/middleware/logger.rs @@ -15,7 +15,7 @@ use std::{ use actix_service::{Service, Transform}; use bytes::Bytes; use futures_util::future::{ok, Ready}; -use log::debug; +use log::{debug, warn}; use regex::{Regex, RegexSet}; use time::OffsetDateTime; @@ -188,9 +188,8 @@ where for unit in &self.0.format.0 { // missing request replacement function diagnostic if let FormatText::CustomRequest(label, None) = unit { - debug!( - "No custom request replacement function was registered for label {} in\ - logger format.", + warn!( + "No custom request replacement function was registered for label \"{}\".", label ); } diff --git a/src/request.rs b/src/request.rs index 452c88c9c..437d07b6e 100644 --- a/src/request.rs +++ b/src/request.rs @@ -8,6 +8,7 @@ use actix_router::{Path, Url}; use futures_util::future::{ok, Ready}; use smallvec::SmallVec; +use crate::app_service::AppInitServiceState; use crate::config::AppConfig; use crate::error::UrlGenerationError; use crate::extract::FromRequest; @@ -17,9 +18,10 @@ use crate::rmap::ResourceMap; #[derive(Clone)] /// An HTTP Request pub struct HttpRequest { - // *. Rc is used exclusively and NO Weak - // is allowed anywhere in the code. Weak pointer is purposely ignored when - // doing Rc's ref counter check. + /// # Panics + /// `Rc` is used exclusively and NO `Weak` + /// is allowed anywhere in the code. Weak pointer is purposely ignored when + /// doing `Rc`'s ref counter check. Expect panics if this invariant is violated. pub(crate) inner: Rc, } @@ -28,8 +30,7 @@ pub(crate) struct HttpRequestInner { pub(crate) path: Path, pub(crate) payload: Payload, pub(crate) app_data: SmallVec<[Rc; 4]>, - rmap: Rc, - config: AppConfig, + app_state: Rc, } impl HttpRequest { @@ -38,8 +39,7 @@ impl HttpRequest { path: Path, head: Message, payload: Payload, - rmap: Rc, - config: AppConfig, + app_state: Rc, app_data: Rc, ) -> HttpRequest { let mut data = SmallVec::<[Rc; 4]>::new(); @@ -50,8 +50,7 @@ impl HttpRequest { head, path, payload, - rmap, - config, + app_state, app_data: data, }), } @@ -138,7 +137,7 @@ impl HttpRequest { /// Returns a None when no resource is fully matched, including default services. #[inline] pub fn match_pattern(&self) -> Option { - self.inner.rmap.match_pattern(self.path()) + self.resource_map().match_pattern(self.path()) } /// The resource name that matched the path. Useful for logging and metrics. @@ -146,7 +145,7 @@ impl HttpRequest { /// Returns a None when no resource is fully matched, including default services. #[inline] pub fn match_name(&self) -> Option<&str> { - self.inner.rmap.match_name(self.path()) + self.resource_map().match_name(self.path()) } /// Request extensions @@ -188,7 +187,7 @@ impl HttpRequest { U: IntoIterator, I: AsRef, { - self.inner.rmap.url_for(&self, name, elements) + self.resource_map().url_for(&self, name, elements) } /// Generate url for named resource @@ -203,7 +202,7 @@ impl HttpRequest { #[inline] /// Get a reference to a `ResourceMap` of current application. pub fn resource_map(&self) -> &ResourceMap { - &self.inner.rmap + &self.app_state().rmap() } /// Peer socket address @@ -223,13 +222,13 @@ impl HttpRequest { /// borrowed. #[inline] pub fn connection_info(&self) -> Ref<'_, ConnectionInfo> { - ConnectionInfo::get(self.head(), &*self.app_config()) + ConnectionInfo::get(self.head(), self.app_config()) } /// App config #[inline] pub fn app_config(&self) -> &AppConfig { - &self.inner.config + self.app_state().config() } /// Get an application data object stored with `App::data` or `App::app_data` @@ -249,6 +248,11 @@ impl HttpRequest { None } + + #[inline] + fn app_state(&self) -> &AppInitServiceState { + &*self.inner.app_state + } } impl HttpMessage for HttpRequest { @@ -284,7 +288,7 @@ impl Drop for HttpRequest { // This relies on no Weak exists anywhere.(There is none) if let Some(inner) = Rc::get_mut(&mut self.inner) { - if inner.config.pool().is_available() { + if inner.app_state.pool().is_available() { // clear additional app_data and keep the root one for reuse. inner.app_data.truncate(1); // inner is borrowed mut here. get head's Extension mutably @@ -293,7 +297,7 @@ impl Drop for HttpRequest { // a re-borrow of pool is necessary here. let req = self.inner.clone(); - self.inner.config.pool().push(req); + self.app_state().pool().push(req); } } } @@ -384,7 +388,7 @@ impl HttpRequestPool { /// Re-use a previously allocated (but now completed/discarded) HttpRequest object. #[inline] - pub(crate) fn get(&self) -> Option { + pub(crate) fn pop(&self) -> Option { self.inner .borrow_mut() .pop() @@ -556,7 +560,7 @@ mod tests { let mut srv = init_service(App::new().service(web::resource("/").to( |req: HttpRequest| { HttpResponse::Ok() - .set_header("pool_cap", req.inner.config.pool().cap) + .set_header("pool_cap", req.app_state().pool().cap) .finish() }, ))) diff --git a/src/resource.rs b/src/resource.rs index 7d53ef936..843237079 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -1,18 +1,18 @@ use std::cell::RefCell; use std::fmt; use std::future::Future; -use std::pin::Pin; use std::rc::Rc; -use std::task::{Context, Poll}; +use std::task::Poll; use actix_http::{Error, Extensions, Response}; use actix_router::IntoPattern; use actix_service::boxed::{self, BoxService, BoxServiceFactory}; use actix_service::{ - apply, apply_fn_factory, IntoServiceFactory, Service, ServiceFactory, + apply, apply_fn_factory, fn_service, IntoServiceFactory, Service, ServiceFactory, ServiceFactoryExt, Transform, }; use futures_core::future::LocalBoxFuture; +use futures_util::future::join_all; use crate::data::Data; use crate::dev::{insert_slash, AppService, HttpServiceFactory, ResourceDef}; @@ -20,7 +20,7 @@ use crate::extract::FromRequest; use crate::guard::Guard; use crate::handler::Handler; use crate::responder::Responder; -use crate::route::{CreateRouteService, Route, RouteService}; +use crate::route::{Route, RouteService}; use crate::service::{ServiceRequest, ServiceResponse}; type HttpService = BoxService; @@ -53,9 +53,9 @@ pub struct Resource { rdef: Vec, name: Option, routes: Vec, - data: Option, + app_data: Option, guards: Vec>, - default: Rc>>>, + default: HttpNewService, factory_ref: Rc>>, } @@ -70,8 +70,10 @@ impl Resource { endpoint: ResourceEndpoint::new(fref.clone()), factory_ref: fref, guards: Vec::new(), - data: None, - default: Rc::new(RefCell::new(None)), + app_data: None, + default: boxed::factory(fn_service(|req: ServiceRequest| async { + Ok(req.into_response(Response::MethodNotAllowed().finish())) + })), } } } @@ -201,10 +203,10 @@ where /// /// Data of different types from parent contexts will still be accessible. pub fn app_data(mut self, data: U) -> Self { - if self.data.is_none() { - self.data = Some(Extensions::new()); + if self.app_data.is_none() { + self.app_data = Some(Extensions::new()); } - self.data.as_mut().unwrap().insert(data); + self.app_data.as_mut().unwrap().insert(data); self } @@ -274,7 +276,7 @@ where guards: self.guards, routes: self.routes, default: self.default, - data: self.data, + app_data: self.app_data, factory_ref: self.factory_ref, } } @@ -336,7 +338,7 @@ where guards: self.guards, routes: self.routes, default: self.default, - data: self.data, + app_data: self.app_data, factory_ref: self.factory_ref, } } @@ -356,11 +358,9 @@ where U::InitError: fmt::Debug, { // create and configure default resource - self.default = Rc::new(RefCell::new(Some(Rc::new(boxed::factory( - f.into_factory().map_init_err(|e| { - log::error!("Can not construct default service: {:?}", e) - }), - ))))); + self.default = boxed::factory(f.into_factory().map_init_err(|e| { + log::error!("Can not construct default service: {:?}", e) + })); self } @@ -391,7 +391,7 @@ where *rdef.name_mut() = name.clone(); } // custom app data storage - if let Some(ref mut ext) = self.data { + if let Some(ref mut ext) = self.app_data { config.set_service_data(ext); } @@ -412,7 +412,7 @@ where fn into_factory(self) -> T { *self.factory_ref.borrow_mut() = Some(ResourceFactory { routes: self.routes, - data: self.data.map(Rc::new), + app_data: self.app_data.map(Rc::new), default: self.default, }); @@ -422,8 +422,8 @@ where pub struct ResourceFactory { routes: Vec, - data: Option>, - default: Rc>>>, + app_data: Option>, + default: HttpNewService, } impl ServiceFactory for ResourceFactory { @@ -432,126 +432,60 @@ impl ServiceFactory for ResourceFactory { type Config = (); type Service = ResourceService; type InitError = (); - type Future = CreateResourceService; + type Future = LocalBoxFuture<'static, Result>; fn new_service(&self, _: ()) -> Self::Future { - let default_fut = if let Some(ref default) = *self.default.borrow() { - Some(default.new_service(())) - } else { - None - }; + // construct default service factory future. + let default_fut = self.default.new_service(()); - CreateResourceService { - fut: self - .routes - .iter() - .map(|route| CreateRouteServiceItem::Future(route.new_service(()))) - .collect(), - data: self.data.clone(), - default: None, - default_fut, - } - } -} + // construct route service factory futures + let factory_fut = + join_all(self.routes.iter().map(|route| route.new_service(()))); -enum CreateRouteServiceItem { - Future(CreateRouteService), - Service(RouteService), -} + let app_data = self.app_data.clone(); -pub struct CreateResourceService { - fut: Vec, - data: Option>, - default: Option, - default_fut: Option>>, -} + Box::pin(async move { + let default = default_fut.await?; + let routes = factory_fut + .await + .into_iter() + .collect::, _>>()?; -impl Future for CreateResourceService { - type Output = Result; - - fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - let mut done = true; - - if let Some(ref mut fut) = self.default_fut { - match Pin::new(fut).poll(cx)? { - Poll::Ready(default) => self.default = Some(default), - Poll::Pending => done = false, - } - } - - // poll http services - for item in &mut self.fut { - match item { - CreateRouteServiceItem::Future(ref mut fut) => match Pin::new(fut) - .poll(cx)? - { - Poll::Ready(route) => *item = CreateRouteServiceItem::Service(route), - Poll::Pending => { - done = false; - } - }, - CreateRouteServiceItem::Service(_) => continue, - }; - } - - if done { - let routes = self - .fut - .drain(..) - .map(|item| match item { - CreateRouteServiceItem::Service(service) => service, - CreateRouteServiceItem::Future(_) => unreachable!(), - }) - .collect(); - Poll::Ready(Ok(ResourceService { + Ok(ResourceService { + app_data, + default, routes, - data: self.data.clone(), - default: self.default.take(), - })) - } else { - Poll::Pending - } + }) + }) } } pub struct ResourceService { routes: Vec, - data: Option>, - default: Option, + app_data: Option>, + default: HttpService, } impl Service for ResourceService { type Response = ServiceResponse; type Error = Error; - type Future = LocalBoxFuture<'static, Result>; + type Future = LocalBoxFuture<'static, Result>; - fn poll_ready(&mut self, _: &mut Context<'_>) -> Poll> { - Poll::Ready(Ok(())) - } + actix_service::always_ready!(); fn call(&mut self, mut req: ServiceRequest) -> Self::Future { for route in self.routes.iter_mut() { if route.check(&mut req) { - if let Some(ref data) = self.data { - req.add_data_container(data.clone()); + if let Some(ref app_data) = self.app_data { + req.add_data_container(app_data.clone()); } return route.call(req); } } - if let Some(ref mut default) = self.default { - if let Some(ref data) = self.data { - req.add_data_container(data.clone()); - } - default.call(req) - } else { - let req = req.into_parts().0; - Box::pin(async { - Ok(ServiceResponse::new( - req, - Response::MethodNotAllowed().finish(), - )) - }) + if let Some(ref app_data) = self.app_data { + req.add_data_container(app_data.clone()); } + self.default.call(req) } } @@ -567,15 +501,15 @@ impl ResourceEndpoint { } impl ServiceFactory for ResourceEndpoint { - type Config = (); type Response = ServiceResponse; type Error = Error; - type InitError = (); + type Config = (); type Service = ResourceService; - type Future = CreateResourceService; + type InitError = (); + type Future = LocalBoxFuture<'static, Result>; fn new_service(&self, _: ()) -> Self::Future { - self.factory.borrow_mut().as_mut().unwrap().new_service(()) + self.factory.borrow().as_ref().unwrap().new_service(()) } } diff --git a/src/responder.rs b/src/responder.rs index 58e33f39d..9b33ac81a 100644 --- a/src/responder.rs +++ b/src/responder.rs @@ -1,33 +1,20 @@ use std::convert::TryFrom; -use std::future::Future; -use std::marker::PhantomData; -use std::pin::Pin; -use std::task::{Context, Poll}; use actix_http::error::InternalError; use actix_http::http::{ header::IntoHeaderValue, Error as HttpError, HeaderMap, HeaderName, StatusCode, }; -use actix_http::{Error, Response, ResponseBuilder}; +use actix_http::ResponseBuilder; use bytes::{Bytes, BytesMut}; -use futures_util::future::{err, ok, Either as EitherFuture, Ready}; -use futures_util::ready; -use pin_project::pin_project; -use crate::request::HttpRequest; +use crate::{Error, HttpRequest, HttpResponse}; /// Trait implemented by types that can be converted to a http response. /// /// Types that implement this trait can be used as the return type of a handler. pub trait Responder { - /// The associated error which can be returned. - type Error: Into; - - /// The future response value. - type Future: Future>; - - /// Convert itself to `AsyncResult` or `Error`. - fn respond_to(self, req: &HttpRequest) -> Self::Future; + /// Convert self to `HttpResponse`. + fn respond_to(self, req: &HttpRequest) -> HttpResponse; /// Override a status code for a Responder. /// @@ -76,29 +63,18 @@ pub trait Responder { } } -impl Responder for Response { - type Error = Error; - type Future = Ready>; - +impl Responder for HttpResponse { #[inline] - fn respond_to(self, _: &HttpRequest) -> Self::Future { - ok(self) + fn respond_to(self, _: &HttpRequest) -> HttpResponse { + self } } -impl Responder for Option -where - T: Responder, -{ - type Error = T::Error; - type Future = EitherFuture>>; - - fn respond_to(self, req: &HttpRequest) -> Self::Future { +impl Responder for Option { + fn respond_to(self, req: &HttpRequest) -> HttpResponse { match self { - Some(t) => EitherFuture::Left(t.respond_to(req)), - None => { - EitherFuture::Right(ok(Response::build(StatusCode::NOT_FOUND).finish())) - } + Some(t) => t.respond_to(req), + None => HttpResponse::build(StatusCode::NOT_FOUND).finish(), } } } @@ -108,109 +84,74 @@ where T: Responder, E: Into, { - type Error = Error; - type Future = EitherFuture< - ResponseFuture, - Ready>, - >; - - fn respond_to(self, req: &HttpRequest) -> Self::Future { + fn respond_to(self, req: &HttpRequest) -> HttpResponse { match self { - Ok(val) => EitherFuture::Left(ResponseFuture::new(val.respond_to(req))), - Err(e) => EitherFuture::Right(err(e.into())), + Ok(val) => val.respond_to(req), + Err(e) => HttpResponse::from_error(e.into()), } } } impl Responder for ResponseBuilder { - type Error = Error; - type Future = Ready>; - #[inline] - fn respond_to(mut self, _: &HttpRequest) -> Self::Future { - ok(self.finish()) + fn respond_to(mut self, _: &HttpRequest) -> HttpResponse { + self.finish() } } -impl Responder for (T, StatusCode) -where - T: Responder, -{ - type Error = T::Error; - type Future = CustomResponderFut; - - fn respond_to(self, req: &HttpRequest) -> Self::Future { - CustomResponderFut { - fut: self.0.respond_to(req), - status: Some(self.1), - headers: None, - } +impl Responder for (T, StatusCode) { + fn respond_to(self, req: &HttpRequest) -> HttpResponse { + let mut res = self.0.respond_to(req); + *res.status_mut() = self.1; + res } } impl Responder for &'static str { - type Error = Error; - type Future = Ready>; - - fn respond_to(self, _: &HttpRequest) -> Self::Future { - ok(Response::build(StatusCode::OK) - .content_type("text/plain; charset=utf-8") - .body(self)) + fn respond_to(self, _: &HttpRequest) -> HttpResponse { + HttpResponse::Ok() + .content_type(mime::TEXT_PLAIN_UTF_8) + .body(self) } } impl Responder for &'static [u8] { - type Error = Error; - type Future = Ready>; - - fn respond_to(self, _: &HttpRequest) -> Self::Future { - ok(Response::build(StatusCode::OK) - .content_type("application/octet-stream") - .body(self)) + fn respond_to(self, _: &HttpRequest) -> HttpResponse { + HttpResponse::Ok() + .content_type(mime::APPLICATION_OCTET_STREAM) + .body(self) } } impl Responder for String { - type Error = Error; - type Future = Ready>; - - fn respond_to(self, _: &HttpRequest) -> Self::Future { - ok(Response::build(StatusCode::OK) - .content_type("text/plain; charset=utf-8") - .body(self)) + fn respond_to(self, _: &HttpRequest) -> HttpResponse { + HttpResponse::Ok() + .content_type(mime::TEXT_PLAIN_UTF_8) + .body(self) } } impl<'a> Responder for &'a String { - type Error = Error; - type Future = Ready>; - - fn respond_to(self, _: &HttpRequest) -> Self::Future { - ok(Response::build(StatusCode::OK) - .content_type("text/plain; charset=utf-8") - .body(self)) + fn respond_to(self, _: &HttpRequest) -> HttpResponse { + HttpResponse::Ok() + .content_type(mime::TEXT_PLAIN_UTF_8) + .body(self) } } impl Responder for Bytes { - type Error = Error; - type Future = Ready>; - - fn respond_to(self, _: &HttpRequest) -> Self::Future { - ok(Response::build(StatusCode::OK) - .content_type("application/octet-stream") - .body(self)) + fn respond_to(self, _: &HttpRequest) -> HttpResponse { + HttpResponse::Ok() + .content_type(mime::APPLICATION_OCTET_STREAM) + .body(self) } } impl Responder for BytesMut { - type Error = Error; - type Future = Ready>; - - fn respond_to(self, _: &HttpRequest) -> Self::Future { - ok(Response::build(StatusCode::OK) - .content_type("application/octet-stream") - .body(self)) + fn respond_to(self, _: &HttpRequest) -> HttpResponse { + HttpResponse::Ok() + .content_type(mime::APPLICATION_OCTET_STREAM) + .body(self) } } @@ -290,45 +231,20 @@ impl CustomResponder { } impl Responder for CustomResponder { - type Error = T::Error; - type Future = CustomResponderFut; + fn respond_to(self, req: &HttpRequest) -> HttpResponse { + let mut res = self.responder.respond_to(req); - fn respond_to(self, req: &HttpRequest) -> Self::Future { - CustomResponderFut { - fut: self.responder.respond_to(req), - status: self.status, - headers: self.headers, - } - } -} - -#[pin_project] -pub struct CustomResponderFut { - #[pin] - fut: T::Future, - status: Option, - headers: Option, -} - -impl Future for CustomResponderFut { - type Output = Result; - - fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - let this = self.project(); - - let mut res = match ready!(this.fut.poll(cx)) { - Ok(res) => res, - Err(e) => return Poll::Ready(Err(e)), - }; - if let Some(status) = this.status.take() { + if let Some(status) = self.status { *res.status_mut() = status; } - if let Some(ref headers) = this.headers { + + if let Some(ref headers) = self.headers { for (k, v) in headers { res.headers_mut().insert(k.clone(), v.clone()); } } - Poll::Ready(Ok(res)) + + res } } @@ -336,40 +252,8 @@ impl Responder for InternalError where T: std::fmt::Debug + std::fmt::Display + 'static, { - type Error = Error; - type Future = Ready>; - - fn respond_to(self, _: &HttpRequest) -> Self::Future { - let err: Error = self.into(); - ok(err.into()) - } -} - -#[pin_project] -pub struct ResponseFuture { - #[pin] - fut: T, - _phantom: PhantomData, -} - -impl ResponseFuture { - pub fn new(fut: T) -> Self { - ResponseFuture { - fut, - _phantom: PhantomData, - } - } -} - -impl Future for ResponseFuture -where - T: Future>, - E: Into, -{ - type Output = Result; - - fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - Poll::Ready(ready!(self.project().fut.poll(cx)).map_err(|e| e.into())) + fn respond_to(self, _: &HttpRequest) -> HttpResponse { + HttpResponse::from_error(self.into()) } } @@ -382,7 +266,7 @@ pub(crate) mod tests { use crate::dev::{Body, ResponseBody}; use crate::http::{header::CONTENT_TYPE, HeaderValue, StatusCode}; use crate::test::{init_service, TestRequest}; - use crate::{error, web, App, HttpResponse}; + use crate::{error, web, App}; #[actix_rt::test] async fn test_option_responder() { @@ -441,7 +325,7 @@ pub(crate) mod tests { async fn test_responder() { let req = TestRequest::default().to_http_request(); - let resp: HttpResponse = "test".respond_to(&req).await.unwrap(); + let resp = "test".respond_to(&req); assert_eq!(resp.status(), StatusCode::OK); assert_eq!(resp.body().bin_ref(), b"test"); assert_eq!( @@ -449,7 +333,7 @@ pub(crate) mod tests { HeaderValue::from_static("text/plain; charset=utf-8") ); - let resp: HttpResponse = b"test".respond_to(&req).await.unwrap(); + let resp = b"test".respond_to(&req); assert_eq!(resp.status(), StatusCode::OK); assert_eq!(resp.body().bin_ref(), b"test"); assert_eq!( @@ -457,7 +341,7 @@ pub(crate) mod tests { HeaderValue::from_static("application/octet-stream") ); - let resp: HttpResponse = "test".to_string().respond_to(&req).await.unwrap(); + let resp = "test".to_string().respond_to(&req); assert_eq!(resp.status(), StatusCode::OK); assert_eq!(resp.body().bin_ref(), b"test"); assert_eq!( @@ -465,7 +349,7 @@ pub(crate) mod tests { HeaderValue::from_static("text/plain; charset=utf-8") ); - let resp: HttpResponse = (&"test".to_string()).respond_to(&req).await.unwrap(); + let resp = (&"test".to_string()).respond_to(&req); assert_eq!(resp.status(), StatusCode::OK); assert_eq!(resp.body().bin_ref(), b"test"); assert_eq!( @@ -473,8 +357,7 @@ pub(crate) mod tests { HeaderValue::from_static("text/plain; charset=utf-8") ); - let resp: HttpResponse = - Bytes::from_static(b"test").respond_to(&req).await.unwrap(); + let resp = Bytes::from_static(b"test").respond_to(&req); assert_eq!(resp.status(), StatusCode::OK); assert_eq!(resp.body().bin_ref(), b"test"); assert_eq!( @@ -482,10 +365,7 @@ pub(crate) mod tests { HeaderValue::from_static("application/octet-stream") ); - let resp: HttpResponse = BytesMut::from(b"test".as_ref()) - .respond_to(&req) - .await - .unwrap(); + let resp = BytesMut::from(b"test".as_ref()).respond_to(&req); assert_eq!(resp.status(), StatusCode::OK); assert_eq!(resp.body().bin_ref(), b"test"); assert_eq!( @@ -494,11 +374,8 @@ pub(crate) mod tests { ); // InternalError - let resp: HttpResponse = - error::InternalError::new("err", StatusCode::BAD_REQUEST) - .respond_to(&req) - .await - .unwrap(); + let resp = + error::InternalError::new("err", StatusCode::BAD_REQUEST).respond_to(&req); assert_eq!(resp.status(), StatusCode::BAD_REQUEST); } @@ -507,10 +384,7 @@ pub(crate) mod tests { let req = TestRequest::default().to_http_request(); // Result - let resp: HttpResponse = Ok::<_, Error>("test".to_string()) - .respond_to(&req) - .await - .unwrap(); + let resp = Ok::<_, Error>("test".to_string()).respond_to(&req); assert_eq!(resp.status(), StatusCode::OK); assert_eq!(resp.body().bin_ref(), b"test"); assert_eq!( @@ -520,9 +394,9 @@ pub(crate) mod tests { let res = Err::(error::InternalError::new("err", StatusCode::BAD_REQUEST)) - .respond_to(&req) - .await; - assert!(res.is_err()); + .respond_to(&req); + + assert_eq!(res.status(), StatusCode::BAD_REQUEST); } #[actix_rt::test] @@ -531,18 +405,15 @@ pub(crate) mod tests { let res = "test" .to_string() .with_status(StatusCode::BAD_REQUEST) - .respond_to(&req) - .await - .unwrap(); + .respond_to(&req); + assert_eq!(res.status(), StatusCode::BAD_REQUEST); assert_eq!(res.body().bin_ref(), b"test"); let res = "test" .to_string() .with_header("content-type", "json") - .respond_to(&req) - .await - .unwrap(); + .respond_to(&req); assert_eq!(res.status(), StatusCode::OK); assert_eq!(res.body().bin_ref(), b"test"); @@ -555,19 +426,14 @@ pub(crate) mod tests { #[actix_rt::test] async fn test_tuple_responder_with_status_code() { let req = TestRequest::default().to_http_request(); - let res = ("test".to_string(), StatusCode::BAD_REQUEST) - .respond_to(&req) - .await - .unwrap(); + let res = ("test".to_string(), StatusCode::BAD_REQUEST).respond_to(&req); assert_eq!(res.status(), StatusCode::BAD_REQUEST); assert_eq!(res.body().bin_ref(), b"test"); let req = TestRequest::default().to_http_request(); let res = ("test".to_string(), StatusCode::OK) .with_header("content-type", "json") - .respond_to(&req) - .await - .unwrap(); + .respond_to(&req); assert_eq!(res.status(), StatusCode::OK); assert_eq!(res.body().bin_ref(), b"test"); assert_eq!( diff --git a/src/scope.rs b/src/scope.rs index 419e572aa..2da4f5546 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -1,18 +1,18 @@ use std::cell::RefCell; use std::fmt; use std::future::Future; -use std::pin::Pin; use std::rc::Rc; -use std::task::{Context, Poll}; +use std::task::Poll; -use actix_http::{Extensions, Response}; -use actix_router::{ResourceDef, ResourceInfo, Router}; +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, }; use futures_core::future::LocalBoxFuture; +use futures_util::future::join_all; use crate::config::ServiceConfig; use crate::data::Data; @@ -61,10 +61,10 @@ type HttpNewService = BoxServiceFactory<(), ServiceRequest, ServiceResponse, Err pub struct Scope { endpoint: T, rdef: String, - data: Option, + app_data: Option, services: Vec>, guards: Vec>, - default: Rc>>>, + default: Option>, external: Vec, factory_ref: Rc>>, } @@ -76,10 +76,10 @@ impl Scope { Scope { endpoint: ScopeEndpoint::new(fref.clone()), rdef: path.to_string(), - data: None, + app_data: None, guards: Vec::new(), services: Vec::new(), - default: Rc::new(RefCell::new(None)), + default: None, external: Vec::new(), factory_ref: fref, } @@ -155,10 +155,10 @@ where /// /// Data of different types from parent contexts will still be accessible. pub fn app_data(mut self, data: U) -> Self { - if self.data.is_none() { - self.data = Some(Extensions::new()); + if self.app_data.is_none() { + self.app_data = Some(Extensions::new()); } - self.data.as_mut().unwrap().insert(data); + self.app_data.as_mut().unwrap().insert(data); self } @@ -201,15 +201,15 @@ where self.external.extend(cfg.external); if !cfg.data.is_empty() { - let mut data = self.data.unwrap_or_else(Extensions::new); + let mut data = self.app_data.unwrap_or_else(Extensions::new); for value in cfg.data.iter() { value.create(&mut data); } - self.data = Some(data); + self.app_data = Some(data); } - self.data + self.app_data .get_or_insert_with(Extensions::new) .extend(cfg.extensions); self @@ -295,11 +295,9 @@ where U::InitError: fmt::Debug, { // create and configure default resource - self.default = Rc::new(RefCell::new(Some(Rc::new(boxed::factory( - f.into_factory().map_init_err(|e| { - log::error!("Can not construct default service: {:?}", e) - }), - ))))); + self.default = Some(Rc::new(boxed::factory(f.into_factory().map_init_err( + |e| log::error!("Can not construct default service: {:?}", e), + )))); self } @@ -337,7 +335,7 @@ where Scope { endpoint: apply(mw, self.endpoint), rdef: self.rdef, - data: self.data, + app_data: self.app_data, guards: self.guards, services: self.services, default: self.default, @@ -397,7 +395,7 @@ where Scope { endpoint: apply_fn_factory(self.endpoint, mw), rdef: self.rdef, - data: self.data, + app_data: self.app_data, guards: self.guards, services: self.services, default: self.default, @@ -419,9 +417,7 @@ where { fn register(mut self, config: &mut AppService) { // update default resource if needed - if self.default.borrow().is_none() { - *self.default.borrow_mut() = Some(config.default_service()); - } + let default = self.default.unwrap_or_else(|| config.default_service()); // register nested services let mut cfg = config.clone_config(); @@ -437,14 +433,14 @@ where } // custom app data storage - if let Some(ref mut ext) = self.data { + if let Some(ref mut ext) = self.app_data { config.set_service_data(ext); } // complete scope pipeline creation *self.factory_ref.borrow_mut() = Some(ScopeFactory { - data: self.data.take().map(Rc::new), - default: self.default.clone(), + app_data: self.app_data.take().map(Rc::new), + default, services: cfg .into_services() .1 @@ -476,129 +472,65 @@ where } pub struct ScopeFactory { - data: Option>, + app_data: Option>, services: Rc<[(ResourceDef, HttpNewService, RefCell>)]>, - default: Rc>>>, + default: Rc, } impl ServiceFactory for ScopeFactory { - type Config = (); type Response = ServiceResponse; type Error = Error; - type InitError = (); + type Config = (); type Service = ScopeService; - type Future = ScopeFactoryResponse; + type InitError = (); + type Future = LocalBoxFuture<'static, Result>; fn new_service(&self, _: ()) -> Self::Future { - let default_fut = if let Some(ref default) = *self.default.borrow() { - Some(default.new_service(())) - } else { - None - }; + // construct default service factory future + let default_fut = self.default.new_service(()); - ScopeFactoryResponse { - fut: self - .services - .iter() - .map(|(path, service, guards)| { - CreateScopeServiceItem::Future( - Some(path.clone()), - guards.borrow_mut().take(), - service.new_service(()), - ) - }) - .collect(), - default: None, - data: self.data.clone(), - default_fut, - } - } -} + // construct all services factory future with it's resource def and guards. + let factory_fut = + join_all(self.services.iter().map(|(path, factory, guards)| { + let path = path.clone(); + let guards = guards.borrow_mut().take(); + let factory_fut = factory.new_service(()); + async move { + let service = factory_fut.await?; + Ok((path, guards, service)) + } + })); -/// Create scope service -#[doc(hidden)] -#[pin_project::pin_project] -pub struct ScopeFactoryResponse { - fut: Vec, - data: Option>, - default: Option, - default_fut: Option>>, -} + let app_data = self.app_data.clone(); -type HttpServiceFut = LocalBoxFuture<'static, Result>; + Box::pin(async move { + let default = default_fut.await?; -enum CreateScopeServiceItem { - Future(Option, Option, HttpServiceFut), - Service(ResourceDef, Option, HttpService), -} - -impl Future for ScopeFactoryResponse { - type Output = Result; - - fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - let mut done = true; - - if let Some(ref mut fut) = self.default_fut { - match Pin::new(fut).poll(cx)? { - Poll::Ready(default) => self.default = Some(default), - Poll::Pending => done = false, - } - } - - // poll http services - for item in &mut self.fut { - let res = match item { - CreateScopeServiceItem::Future( - ref mut path, - ref mut guards, - ref mut fut, - ) => match Pin::new(fut).poll(cx)? { - Poll::Ready(service) => { - Some((path.take().unwrap(), guards.take(), service)) - } - Poll::Pending => { - done = false; - None - } - }, - CreateScopeServiceItem::Service(_, _, _) => continue, - }; - - if let Some((path, guards, service)) = res { - *item = CreateScopeServiceItem::Service(path, guards, service); - } - } - - if done { - let router = self - .fut + // build router from the factory future result. + let router = factory_fut + .await + .into_iter() + .collect::, _>>()? .drain(..) - .fold(Router::build(), |mut router, item| { - match item { - CreateScopeServiceItem::Service(path, guards, service) => { - router.rdef(path, service).2 = guards; - } - CreateScopeServiceItem::Future(_, _, _) => unreachable!(), - } + .fold(Router::build(), |mut router, (path, guards, service)| { + router.rdef(path, service).2 = guards; router - }); - Poll::Ready(Ok(ScopeService { - data: self.data.clone(), - router: router.finish(), - default: self.default.take(), - _ready: None, - })) - } else { - Poll::Pending - } + }) + .finish(); + + Ok(ScopeService { + app_data, + router, + default, + }) + }) } } pub struct ScopeService { - data: Option>, + app_data: Option>, router: Router>>, - default: Option, - _ready: Option<(ServiceRequest, ResourceInfo)>, + default: HttpService, } impl Service for ScopeService { @@ -606,9 +538,7 @@ impl Service for ScopeService { type Error = Error; type Future = LocalBoxFuture<'static, Result>; - fn poll_ready(&mut self, _: &mut Context<'_>) -> Poll> { - Poll::Ready(Ok(())) - } + actix_service::always_ready!(); fn call(&mut self, mut req: ServiceRequest) -> Self::Future { let res = self.router.recognize_mut_checked(&mut req, |req, guards| { @@ -622,21 +552,14 @@ impl Service for ScopeService { true }); + if let Some(ref app_data) = self.app_data { + req.add_data_container(app_data.clone()); + } + if let Some((srv, _info)) = res { - if let Some(ref data) = self.data { - req.add_data_container(data.clone()); - } srv.call(req) - } else if let Some(ref mut default) = self.default { - if let Some(ref data) = self.data { - req.add_data_container(data.clone()); - } - default.call(req) } else { - let req = req.into_parts().0; - Box::pin(async { - Ok(ServiceResponse::new(req, Response::NotFound().finish())) - }) + self.default.call(req) } } } @@ -658,7 +581,7 @@ impl ServiceFactory for ScopeEndpoint { type Config = (); type Service = ScopeService; type InitError = (); - type Future = ScopeFactoryResponse; + type Future = LocalBoxFuture<'static, Result>; fn new_service(&self, _: ()) -> Self::Future { self.factory.borrow_mut().as_mut().unwrap().new_service(()) diff --git a/src/server.rs b/src/server.rs index 26089ccba..8bfb27b77 100644 --- a/src/server.rs +++ b/src/server.rs @@ -283,11 +283,7 @@ where lst, move || { let c = cfg.lock().unwrap(); - let cfg = AppConfig::new( - false, - addr, - c.host.clone().unwrap_or_else(|| format!("{}", addr)), - ); + let host = c.host.clone().unwrap_or_else(|| format!("{}", addr)); let svc = HttpService::build() .keep_alive(c.keep_alive) @@ -302,8 +298,10 @@ where svc }; - svc.finish(map_config(factory(), move |_| cfg.clone())) - .tcp() + svc.finish(map_config(factory(), move |_| { + AppConfig::new(false, addr, host.clone()) + })) + .tcp() }, )?; Ok(self) @@ -342,11 +340,7 @@ where lst, move || { let c = cfg.lock().unwrap(); - let cfg = AppConfig::new( - true, - addr, - c.host.clone().unwrap_or_else(|| format!("{}", addr)), - ); + let host = c.host.clone().unwrap_or_else(|| format!("{}", addr)); let svc = HttpService::build() .keep_alive(c.keep_alive) @@ -361,8 +355,10 @@ where svc }; - svc.finish(map_config(factory(), move |_| cfg.clone())) - .openssl(acceptor.clone()) + svc.finish(map_config(factory(), move |_| { + AppConfig::new(true, addr, host.clone()) + })) + .openssl(acceptor.clone()) }, )?; Ok(self) @@ -401,11 +397,7 @@ where lst, move || { let c = cfg.lock().unwrap(); - let cfg = AppConfig::new( - true, - addr, - c.host.clone().unwrap_or_else(|| format!("{}", addr)), - ); + let host = c.host.clone().unwrap_or_else(|| format!("{}", addr)); let svc = HttpService::build() .keep_alive(c.keep_alive) @@ -420,8 +412,10 @@ where svc }; - svc.finish(map_config(factory(), move |_| cfg.clone())) - .rustls(config.clone()) + svc.finish(map_config(factory(), move |_| { + AppConfig::new(true, addr, host.clone()) + })) + .rustls(config.clone()) }, )?; Ok(self) diff --git a/src/test.rs b/src/test.rs index c97c65f6f..271ed4505 100644 --- a/src/test.rs +++ b/src/test.rs @@ -27,6 +27,7 @@ use socket2::{Domain, Protocol, Socket, Type}; pub use actix_http::test::TestBuffer; +use crate::app_service::AppInitServiceState; use crate::config::AppConfig; use crate::data::Data; use crate::dev::{Body, MessageBody, Payload, Server}; @@ -541,12 +542,14 @@ impl TestRequest { head.peer_addr = self.peer_addr; self.path.get_mut().update(&head.uri); + let app_state = + AppInitServiceState::new(Rc::new(self.rmap), self.config.clone()); + ServiceRequest::new(HttpRequest::new( self.path, head, payload, - Rc::new(self.rmap), - self.config.clone(), + app_state, Rc::new(self.app_data), )) } @@ -562,14 +565,10 @@ impl TestRequest { head.peer_addr = self.peer_addr; self.path.get_mut().update(&head.uri); - HttpRequest::new( - self.path, - head, - payload, - Rc::new(self.rmap), - self.config.clone(), - Rc::new(self.app_data), - ) + let app_state = + AppInitServiceState::new(Rc::new(self.rmap), self.config.clone()); + + HttpRequest::new(self.path, head, payload, app_state, Rc::new(self.app_data)) } /// Complete request creation and generate `HttpRequest` and `Payload` instances @@ -578,12 +577,14 @@ impl TestRequest { head.peer_addr = self.peer_addr; self.path.get_mut().update(&head.uri); + let app_state = + AppInitServiceState::new(Rc::new(self.rmap), self.config.clone()); + let req = HttpRequest::new( self.path, head, Payload::None, - Rc::new(self.rmap), - self.config.clone(), + app_state, Rc::new(self.app_data), ); diff --git a/src/types/either.rs b/src/types/either.rs index 8a046d291..d72a14fd0 100644 --- a/src/types/either.rs +++ b/src/types/either.rs @@ -1,132 +1,170 @@ -use std::{ - future::Future, - pin::Pin, - task::{Context, Poll}, +//! For either helper, see [`Either`]. + +use bytes::Bytes; +use futures_util::{future::LocalBoxFuture, FutureExt, TryFutureExt}; + +use crate::{ + dev, + web::{Form, Json}, + Error, FromRequest, HttpRequest, HttpResponse, Responder, }; -use actix_http::{Error, Response}; -use bytes::Bytes; -use futures_util::{future::LocalBoxFuture, ready, FutureExt, TryFutureExt}; -use pin_project::pin_project; - -use crate::{dev, request::HttpRequest, FromRequest, Responder}; - -/// Combines two different responder types into a single type +/// Combines two extractor or responder types into a single type. /// -/// ```rust -/// use actix_web::{Either, Error, HttpResponse}; +/// Can be converted to and from an [`either::Either`]. /// -/// type RegisterResult = Either>; +/// # Extractor +/// Provides a mechanism for trying two extractors, a primary and a fallback. Useful for +/// "polymorphic payloads" where, for example, a form might be JSON or URL encoded. /// -/// fn index() -> RegisterResult { -/// if is_a_variant() { -/// // <- choose left variant -/// Either::A(HttpResponse::BadRequest().body("Bad data")) +/// It is important to note that this extractor, by necessity, buffers the entire request payload +/// as part of its implementation. Though, it does respect any `PayloadConfig` maximum size limits. +/// +/// ``` +/// use actix_web::{post, web, Either}; +/// use serde::Deserialize; +/// +/// #[derive(Deserialize)] +/// struct Info { +/// name: String, +/// } +/// +/// // handler that accepts form as JSON or form-urlencoded. +/// #[post("/")] +/// async fn index(form: Either, web::Form>) -> String { +/// let name: String = match form { +/// Either::Left(json) => json.name.to_owned(), +/// Either::Right(form) => form.name.to_owned(), +/// }; +/// +/// format!("Welcome {}!", name) +/// } +/// ``` +/// +/// # Responder +/// It may be desireable to use a concrete type for a response with multiple branches. As long as +/// both types implement `Responder`, so will the `Either` type, enabling it to be used as a +/// handler's return type. +/// +/// All properties of a response are determined by the Responder branch returned. +/// +/// ``` +/// use actix_web::{get, Either, Error, HttpResponse}; +/// +/// #[get("/")] +/// async fn index() -> Either<&'static str, Result> { +/// if 1 == 2 { +/// // respond with Left variant +/// Either::Left("Bad data") /// } else { -/// Either::B( -/// // <- Right variant +/// // respond with Right variant +/// Either::Right( /// Ok(HttpResponse::Ok() -/// .content_type("text/html") -/// .body("Hello!")) +/// .content_type(mime::TEXT_HTML) +/// .body("

Hello!

")) /// ) /// } /// } -/// # fn is_a_variant() -> bool { true } -/// # fn main() {} /// ``` #[derive(Debug, PartialEq)] -pub enum Either { - /// First branch of the type - A(A), - /// Second branch of the type - B(B), +pub enum Either { + /// A value of type `L`. + Left(L), + + /// A value of type `R`. + Right(R), +} + +impl Either, Json> { + pub fn into_inner(self) -> T { + match self { + Either::Left(form) => form.into_inner(), + Either::Right(form) => form.into_inner(), + } + } +} + +impl Either, Form> { + pub fn into_inner(self) -> T { + match self { + Either::Left(form) => form.into_inner(), + Either::Right(form) => form.into_inner(), + } + } +} + +impl From> for Either { + fn from(val: either::Either) -> Self { + match val { + either::Either::Left(l) => Either::Left(l), + either::Either::Right(r) => Either::Right(r), + } + } +} + +impl From> for either::Either { + fn from(val: Either) -> Self { + match val { + Either::Left(l) => either::Either::Left(l), + Either::Right(r) => either::Either::Right(r), + } + } } #[cfg(test)] -impl Either { - pub(self) fn unwrap_left(self) -> A { +impl Either { + pub(self) fn unwrap_left(self) -> L { match self { - Either::A(data) => data, - Either::B(_) => { - panic!("Cannot unwrap left branch. Either contains a right branch.") + Either::Left(data) => data, + Either::Right(_) => { + panic!("Cannot unwrap Left branch. Either contains an `R` type.") } } } - pub(self) fn unwrap_right(self) -> B { + pub(self) fn unwrap_right(self) -> R { match self { - Either::A(_) => { - panic!("Cannot unwrap right branch. Either contains a left branch.") + Either::Left(_) => { + panic!("Cannot unwrap Right branch. Either contains an `L` type.") } - Either::B(data) => data, + Either::Right(data) => data, } } } -impl Responder for Either +/// See [here](#responder) for example of usage as a handler return type. +impl Responder for Either where - A: Responder, - B: Responder, + L: Responder, + R: Responder, { - type Error = Error; - type Future = EitherResponder; - - fn respond_to(self, req: &HttpRequest) -> Self::Future { + fn respond_to(self, req: &HttpRequest) -> HttpResponse { match self { - Either::A(a) => EitherResponder::A(a.respond_to(req)), - Either::B(b) => EitherResponder::B(b.respond_to(req)), + Either::Left(a) => a.respond_to(req), + Either::Right(b) => b.respond_to(req), } } } -#[pin_project(project = EitherResponderProj)] -pub enum EitherResponder -where - A: Responder, - B: Responder, -{ - A(#[pin] A::Future), - B(#[pin] B::Future), -} - -impl Future for EitherResponder -where - A: Responder, - B: Responder, -{ - type Output = Result; - - fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - match self.project() { - EitherResponderProj::A(fut) => { - Poll::Ready(ready!(fut.poll(cx)).map_err(|e| e.into())) - } - EitherResponderProj::B(fut) => { - Poll::Ready(ready!(fut.poll(cx).map_err(|e| e.into()))) - } - } - } -} - -/// A composite error resulting from failure to extract an `Either`. +/// A composite error resulting from failure to extract an `Either`. /// /// The implementation of `Into` will return the payload buffering error or the /// error from the primary extractor. To access the fallback error, use a match clause. #[derive(Debug)] -pub enum EitherExtractError { +pub enum EitherExtractError { /// Error from payload buffering, such as exceeding payload max size limit. Bytes(Error), /// Error from primary extractor. - Extract(A, B), + Extract(L, R), } -impl From> for Error +impl From> for Error where - A: Into, - B: Into, + L: Into, + R: Into, { - fn from(err: EitherExtractError) -> Error { + fn from(err: EitherExtractError) -> Error { match err { EitherExtractError::Bytes(err) => err, EitherExtractError::Extract(a_err, _b_err) => a_err.into(), @@ -134,17 +172,13 @@ where } } -/// Provides a mechanism for trying two extractors, a primary and a fallback. Useful for -/// "polymorphic payloads" where, for example, a form might be JSON or URL encoded. -/// -/// It is important to note that this extractor, by necessity, buffers the entire request payload -/// as part of its implementation. Though, it does respect a `PayloadConfig`'s maximum size limit. -impl FromRequest for Either +/// See [here](#extractor) for example of usage as an extractor. +impl FromRequest for Either where - A: FromRequest + 'static, - B: FromRequest + 'static, + L: FromRequest + 'static, + R: FromRequest + 'static, { - type Error = EitherExtractError; + type Error = EitherExtractError; type Future = LocalBoxFuture<'static, Result>; type Config = (); @@ -153,32 +187,32 @@ where Bytes::from_request(req, payload) .map_err(EitherExtractError::Bytes) - .and_then(|bytes| bytes_to_a_or_b(req2, bytes)) + .and_then(|bytes| bytes_to_l_or_r(req2, bytes)) .boxed_local() } } -async fn bytes_to_a_or_b( +async fn bytes_to_l_or_r( req: HttpRequest, bytes: Bytes, -) -> Result, EitherExtractError> +) -> Result, EitherExtractError> where - A: FromRequest + 'static, - B: FromRequest + 'static, + L: FromRequest + 'static, + R: FromRequest + 'static, { let fallback = bytes.clone(); let a_err; let mut pl = payload_from_bytes(bytes); - match A::from_request(&req, &mut pl).await { - Ok(a_data) => return Ok(Either::A(a_data)), + 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, }; let mut pl = payload_from_bytes(fallback); - match B::from_request(&req, &mut pl).await { - Ok(b_data) => return Ok(Either::B(b_data)), + 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)), } } diff --git a/src/types/form.rs b/src/types/form.rs index 82ea73216..71680b19a 100644 --- a/src/types/form.rs +++ b/src/types/form.rs @@ -1,72 +1,68 @@ -//! Form extractor +//! For URL encoded form helper documentation, see [`Form`]. -use std::future::Future; -use std::pin::Pin; -use std::rc::Rc; -use std::task::{Context, Poll}; -use std::{fmt, ops}; +use std::{ + fmt, + future::Future, + ops, + pin::Pin, + rc::Rc, + task::{Context, Poll}, +}; -use actix_http::{Error, HttpMessage, Payload, Response}; +use actix_http::Payload; use bytes::BytesMut; use encoding_rs::{Encoding, UTF_8}; -use futures_util::future::{err, ok, FutureExt, LocalBoxFuture, Ready}; -use futures_util::StreamExt; -use serde::de::DeserializeOwned; -use serde::Serialize; +use futures_util::{ + future::{FutureExt, LocalBoxFuture}, + StreamExt, +}; +use serde::{de::DeserializeOwned, Serialize}; #[cfg(feature = "compress")] use crate::dev::Decompress; -use crate::error::UrlencodedError; -use crate::extract::FromRequest; -use crate::http::{ - header::{ContentType, CONTENT_LENGTH}, - StatusCode, +use crate::{ + error::UrlencodedError, extract::FromRequest, http::header::CONTENT_LENGTH, web, + Error, HttpMessage, HttpRequest, HttpResponse, Responder, }; -use crate::request::HttpRequest; -use crate::{responder::Responder, web}; -/// Form data helper (`application/x-www-form-urlencoded`) +/// URL encoded payload extractor and responder. /// -/// Can be use to extract url-encoded data from the request body, -/// or send url-encoded data as the response. +/// `Form` has two uses: URL encoded responses, and extracting typed data from URL request payloads. /// -/// ## Extract +/// # Extractor +/// To extract typed data from a request body, the inner type `T` must implement the +/// [`serde::Deserialize`] trait. /// -/// To extract typed information from request's body, the type `T` must -/// implement the `Deserialize` trait from *serde*. +/// Use [`FormConfig`] to configure extraction process. /// -/// [**FormConfig**](FormConfig) allows to configure extraction -/// process. -/// -/// ### Example -/// ```rust -/// use actix_web::web; -/// use serde_derive::Deserialize; +/// ``` +/// use actix_web::{post, web}; +/// use serde::Deserialize; /// /// #[derive(Deserialize)] -/// struct FormData { -/// username: String, +/// struct Info { +/// name: String, /// } /// -/// /// Extract form data using serde. -/// /// This handler get called only if content type is *x-www-form-urlencoded* -/// /// and content of the request could be deserialized to a `FormData` struct -/// fn index(form: web::Form) -> String { -/// format!("Welcome {}!", form.username) +/// // This handler is only called if: +/// // - request headers declare the content type as `application/x-www-form-urlencoded` +/// // - request payload is deserialized into a `Info` struct from the URL encoded format +/// #[post("/")] +/// async fn index(form: web::Form) -> String { +/// format!("Welcome {}!", form.name) /// } -/// # fn main() {} /// ``` /// -/// ## Respond +/// # Responder +/// The `Form` type also allows you to create URL encoded responses: +/// simply return a value of type Form where T is the type to be URL encoded. +/// The type must implement [`serde::Serialize`]. /// -/// The `Form` type also allows you to respond with well-formed url-encoded data: -/// simply return a value of type Form where T is the type to be url-encoded. -/// The type must implement `serde::Serialize`; +/// Responses use /// -/// ### Example -/// ```rust -/// use actix_web::*; -/// use serde_derive::Serialize; +/// ``` +/// use actix_web::{get, web}; +/// use serde::Serialize; /// /// #[derive(Serialize)] /// struct SomeForm { @@ -74,22 +70,23 @@ use crate::{responder::Responder, web}; /// age: u8 /// } /// -/// // Will return a 200 response with header -/// // `Content-Type: application/x-www-form-urlencoded` -/// // and body "name=actix&age=123" -/// fn index() -> web::Form { +/// // Response will have: +/// // - status: 200 OK +/// // - header: `Content-Type: application/x-www-form-urlencoded` +/// // - body: `name=actix&age=123` +/// #[get("/")] +/// async fn index() -> web::Form { /// web::Form(SomeForm { /// name: "actix".into(), /// age: 123 /// }) /// } -/// # fn main() {} /// ``` #[derive(PartialEq, Eq, PartialOrd, Ord)] pub struct Form(pub T); impl Form { - /// Deconstruct to an inner value + /// Unwrap into inner `T` value. pub fn into_inner(self) -> T { self.0 } @@ -109,6 +106,7 @@ impl ops::DerefMut for Form { } } +/// See [here](#extractor) for example of usage as an extractor. impl FromRequest for Form where T: DeserializeOwned + 'static, @@ -120,7 +118,7 @@ where #[inline] fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future { let req2 = req.clone(); - let (limit, err) = req + let (limit, err_handler) = req .app_data::() .or_else(|| { req.app_data::>() @@ -132,13 +130,10 @@ where UrlEncoded::new(req, payload) .limit(limit) .map(move |res| match res { - Err(e) => { - if let Some(err) = err { - Err((*err)(e, &req2)) - } else { - Err(e.into()) - } - } + Err(err) => match err_handler { + Some(err_handler) => Err((err_handler)(err, &req2)), + None => Err(err.into()), + }, Ok(item) => Ok(Form(item)), }) .boxed_local() @@ -157,49 +152,39 @@ impl fmt::Display for Form { } } +/// See [here](#responder) for example of usage as a handler return type. impl Responder for Form { - type Error = Error; - type Future = Ready>; - - fn respond_to(self, _: &HttpRequest) -> Self::Future { - let body = match serde_urlencoded::to_string(&self.0) { - Ok(body) => body, - Err(e) => return err(e.into()), - }; - - ok(Response::build(StatusCode::OK) - .set(ContentType::form_url_encoded()) - .body(body)) + fn respond_to(self, _: &HttpRequest) -> HttpResponse { + match serde_urlencoded::to_string(&self.0) { + Ok(body) => HttpResponse::Ok() + .content_type(mime::APPLICATION_WWW_FORM_URLENCODED) + .body(body), + Err(err) => HttpResponse::from_error(err.into()), + } } } -/// Form extractor configuration +/// [`Form`] extractor configuration. /// -/// ```rust -/// use actix_web::{web, App, FromRequest, Result}; -/// use serde_derive::Deserialize; +/// ``` +/// use actix_web::{post, web, App, FromRequest, Result}; +/// use serde::Deserialize; /// /// #[derive(Deserialize)] -/// struct FormData { +/// struct Info { /// username: String, /// } /// -/// /// Extract form data using serde. -/// /// Custom configuration is used for this handler, max payload size is 4k -/// async fn index(form: web::Form) -> Result { +/// // Custom `FormConfig` is applied to App. +/// // Max payload size for URL encoded forms is set to 4kB. +/// #[post("/")] +/// async fn index(form: web::Form) -> Result { /// Ok(format!("Welcome {}!", form.username)) /// } /// -/// fn main() { -/// let app = App::new().service( -/// web::resource("/index.html") -/// // change `Form` extractor configuration -/// .app_data( -/// web::FormConfig::default().limit(4097) -/// ) -/// .route(web::get().to(index)) -/// ); -/// } +/// App::new() +/// .app_data(web::FormConfig::default().limit(4096)) +/// .service(index); /// ``` #[derive(Clone)] pub struct FormConfig { @@ -208,7 +193,7 @@ pub struct FormConfig { } impl FormConfig { - /// Change max size of payload. By default max size is 16Kb + /// Set maximum accepted payload size. By default this limit is 16kB. pub fn limit(mut self, limit: usize) -> Self { self.limit = limit; self @@ -233,33 +218,30 @@ impl Default for FormConfig { } } -/// Future that resolves to a parsed urlencoded values. +/// Future that resolves to some `T` when parsed from a URL encoded payload. /// -/// Parse `application/x-www-form-urlencoded` encoded request's body. -/// Return `UrlEncoded` future. Form can be deserialized to any type that -/// implements `Deserialize` trait from *serde*. +/// Form can be deserialized from any type `T` that implements [`serde::Deserialize`]. /// -/// Returns error: -/// -/// * content type is not `application/x-www-form-urlencoded` -/// * content-length is greater than 32k -/// -pub struct UrlEncoded { +/// Returns error if: +/// - content type is not `application/x-www-form-urlencoded` +/// - content length is greater than [limit](UrlEncoded::limit()) +pub struct UrlEncoded { #[cfg(feature = "compress")] stream: Option>, #[cfg(not(feature = "compress"))] stream: Option, + limit: usize, length: Option, encoding: &'static Encoding, err: Option, - fut: Option>>, + fut: Option>>, } #[allow(clippy::borrow_interior_mutable_const)] -impl UrlEncoded { - /// Create a new future to URL encode a request - pub fn new(req: &HttpRequest, payload: &mut Payload) -> UrlEncoded { +impl UrlEncoded { + /// Create a new future to decode a URL encoded request payload. + pub fn new(req: &HttpRequest, payload: &mut Payload) -> Self { // check content type if req.content_type().to_lowercase() != "application/x-www-form-urlencoded" { return Self::err(UrlencodedError::ContentType); @@ -297,29 +279,29 @@ impl UrlEncoded { } } - fn err(e: UrlencodedError) -> Self { + fn err(err: UrlencodedError) -> Self { UrlEncoded { stream: None, limit: 32_768, fut: None, - err: Some(e), + err: Some(err), length: None, encoding: UTF_8, } } - /// Change max size of payload. By default max size is 256Kb + /// Set maximum accepted payload size. The default limit is 256kB. pub fn limit(mut self, limit: usize) -> Self { self.limit = limit; self } } -impl Future for UrlEncoded +impl Future for UrlEncoded where - U: DeserializeOwned + 'static, + T: DeserializeOwned + 'static, { - type Output = Result; + type Output = Result; fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { if let Some(ref mut fut) = self.fut { @@ -348,6 +330,7 @@ where while let Some(item) = stream.next().await { let chunk = item?; + if (body.len() + chunk.len()) > limit { return Err(UrlencodedError::Overflow { size: body.len() + chunk.len(), @@ -359,19 +342,21 @@ where } if encoding == UTF_8 { - serde_urlencoded::from_bytes::(&body) + serde_urlencoded::from_bytes::(&body) .map_err(|_| UrlencodedError::Parse) } else { let body = encoding .decode_without_bom_handling_and_without_replacement(&body) .map(|s| s.into_owned()) .ok_or(UrlencodedError::Parse)?; - serde_urlencoded::from_str::(&body) + + serde_urlencoded::from_str::(&body) .map_err(|_| UrlencodedError::Parse) } } .boxed_local(), ); + self.poll(cx) } } @@ -382,7 +367,10 @@ mod tests { use serde::{Deserialize, Serialize}; use super::*; - use crate::http::header::{HeaderValue, CONTENT_LENGTH, CONTENT_TYPE}; + use crate::http::{ + header::{HeaderValue, CONTENT_LENGTH, CONTENT_TYPE}, + StatusCode, + }; use crate::test::TestRequest; #[derive(Deserialize, Serialize, Debug, PartialEq)] @@ -493,7 +481,7 @@ mod tests { hello: "world".to_string(), counter: 123, }); - let resp = form.respond_to(&req).await.unwrap(); + let resp = form.respond_to(&req); assert_eq!(resp.status(), StatusCode::OK); assert_eq!( resp.headers().get(CONTENT_TYPE).unwrap(), @@ -519,6 +507,6 @@ mod tests { assert!(s.is_err()); let err_str = s.err().unwrap().to_string(); - assert!(err_str.contains("Urlencoded payload size is bigger")); + assert!(err_str.starts_with("URL encoded payload is larger")); } } diff --git a/src/types/json.rs b/src/types/json.rs index 74138ca56..edfb775f3 100644 --- a/src/types/json.rs +++ b/src/types/json.rs @@ -1,46 +1,44 @@ -//! Json extractor/responder +//! For JSON helper documentation, see [`Json`]. -use std::future::Future; -use std::marker::PhantomData; -use std::pin::Pin; -use std::sync::Arc; -use std::task::{Context, Poll}; -use std::{fmt, ops}; +use std::{ + fmt, + future::Future, + marker::PhantomData, + ops, + pin::Pin, + sync::Arc, + task::{Context, Poll}, +}; use bytes::BytesMut; -use futures_util::future::{ready, Ready}; -use futures_util::ready; -use futures_util::stream::Stream; -use serde::de::DeserializeOwned; -use serde::Serialize; +use futures_util::{ready, stream::Stream}; +use serde::{de::DeserializeOwned, Serialize}; -use actix_http::http::{header::CONTENT_LENGTH, StatusCode}; -use actix_http::{HttpMessage, Payload, Response}; +use actix_http::Payload; #[cfg(feature = "compress")] use crate::dev::Decompress; -use crate::error::{Error, JsonPayloadError}; -use crate::extract::FromRequest; -use crate::request::HttpRequest; -use crate::{responder::Responder, web}; +use crate::{ + error::{Error, JsonPayloadError}, + extract::FromRequest, + http::header::CONTENT_LENGTH, + request::HttpRequest, + web, HttpMessage, HttpResponse, Responder, +}; -/// Json helper +/// JSON extractor and responder. /// -/// Json can be used for two different purpose. First is for json response -/// generation and second is for extracting typed information from request's -/// payload. +/// `Json` has two uses: JSON responses, and extracting typed data from JSON request payloads. /// -/// To extract typed information from request's body, the type `T` must -/// implement the `Deserialize` trait from *serde*. +/// # Extractor +/// To extract typed data from a request body, the inner type `T` must implement the +/// [`serde::Deserialize`] trait. /// -/// [**JsonConfig**](JsonConfig) allows to configure extraction -/// process. +/// Use [`JsonConfig`] to configure extraction process. /// -/// ## Example -/// -/// ```rust -/// use actix_web::{web, App}; -/// use serde_derive::Deserialize; +/// ``` +/// use actix_web::{post, web, App}; +/// use serde::Deserialize; /// /// #[derive(Deserialize)] /// struct Info { @@ -48,43 +46,37 @@ use crate::{responder::Responder, web}; /// } /// /// /// deserialize `Info` from request's body +/// #[post("/")] /// async fn index(info: web::Json) -> String { /// format!("Welcome {}!", info.username) /// } -/// -/// fn main() { -/// let app = App::new().service( -/// web::resource("/index.html").route( -/// web::post().to(index)) -/// ); -/// } /// ``` /// -/// The `Json` type allows you to respond with well-formed JSON data: simply -/// return a value of type Json where T is the type of a structure -/// to serialize into *JSON*. The type `T` must implement the `Serialize` -/// trait from *serde*. +/// # Responder +/// The `Json` type JSON formatted responses. A handler may return a value of type +/// `Json` where `T` is the type of a structure to serialize into JSON. The type `T` must +/// implement [`serde::Serialize`]. /// -/// ```rust -/// use actix_web::*; -/// use serde_derive::Serialize; +/// ``` +/// use actix_web::{post, web, HttpRequest}; +/// use serde::Serialize; /// /// #[derive(Serialize)] -/// struct MyObj { +/// struct Info { /// name: String, /// } /// -/// fn index(req: HttpRequest) -> Result> { -/// Ok(web::Json(MyObj { -/// name: req.match_info().get("name").unwrap().to_string(), -/// })) +/// #[post("/{name}")] +/// async fn index(req: HttpRequest) -> web::Json { +/// web::Json(Info { +/// name: req.match_info().get("name").unwrap().to_owned(), +/// }) /// } -/// # fn main() {} /// ``` pub struct Json(pub T); impl Json { - /// Deconstruct to an inner value + /// Unwrap into inner `T` value. pub fn into_inner(self) -> T { self.0 } @@ -122,54 +114,21 @@ where } } +/// Creates response with OK status code, correct content type header, and serialized JSON payload. +/// +/// If serialization failed impl Responder for Json { - type Error = Error; - type Future = Ready>; - - fn respond_to(self, _: &HttpRequest) -> Self::Future { - let body = match serde_json::to_string(&self.0) { - Ok(body) => body, - Err(e) => return ready(Err(e.into())), - }; - - ready(Ok(Response::build(StatusCode::OK) - .content_type("application/json") - .body(body))) + fn respond_to(self, _: &HttpRequest) -> HttpResponse { + match serde_json::to_string(&self.0) { + Ok(body) => HttpResponse::Ok() + .content_type(mime::APPLICATION_JSON) + .body(body), + Err(err) => HttpResponse::from_error(err.into()), + } } } -/// Json extractor. Allow to extract typed information from request's -/// payload. -/// -/// To extract typed information from request's body, the type `T` must -/// implement the `Deserialize` trait from *serde*. -/// -/// [**JsonConfig**](JsonConfig) allows to configure extraction -/// process. -/// -/// ## Example -/// -/// ```rust -/// use actix_web::{web, App}; -/// use serde_derive::Deserialize; -/// -/// #[derive(Deserialize)] -/// struct Info { -/// username: String, -/// } -/// -/// /// deserialize `Info` from request's body -/// async fn index(info: web::Json) -> String { -/// format!("Welcome {}!", info.username) -/// } -/// -/// fn main() { -/// let app = App::new().service( -/// web::resource("/index.html").route( -/// web::post().to(index)) -/// ); -/// } -/// ``` +/// See [here](#extractor) for example of usage as an extractor. impl FromRequest for Json where T: DeserializeOwned + 'static, @@ -215,7 +174,7 @@ where let res = ready!(Pin::new(&mut this.fut).poll(cx)); let res = match res { - Err(e) => { + Err(err) => { let req = this.req.take().unwrap(); log::debug!( "Failed to deserialize Json from payload. \ @@ -223,10 +182,10 @@ where req.path() ); - if let Some(err) = this.err_handler.as_ref() { - Err((*err)(e, &req)) + if let Some(err_handler) = this.err_handler.as_ref() { + Err((*err_handler)(err, &req)) } else { - Err(e.into()) + Err(err.into()) } } Ok(data) => Ok(Json(data)), @@ -236,44 +195,39 @@ where } } -/// Json extractor configuration +/// `Json` extractor configuration. /// -/// # Example -/// -/// ```rust -/// use actix_web::{error, web, App, FromRequest, HttpResponse}; -/// use serde_derive::Deserialize; +/// # Usage +/// ``` +/// use actix_web::{error, post, web, App, FromRequest, HttpResponse}; +/// use serde::Deserialize; /// /// #[derive(Deserialize)] /// struct Info { -/// username: String, +/// name: String, /// } /// -/// /// deserialize `Info` from request's body, max payload size is 4kb +/// // `Json` extraction is bound by custom `JsonConfig` applied to App. +/// #[post("/")] /// async fn index(info: web::Json) -> String { -/// format!("Welcome {}!", info.username) +/// format!("Welcome {}!", info.name) /// } /// -/// fn main() { -/// let app = App::new().service( -/// web::resource("/index.html") -/// .app_data( -/// // Json extractor configuration for this resource. -/// web::JsonConfig::default() -/// .limit(4096) // Limit request payload size -/// .content_type(|mime| { // <- accept text/plain content type -/// mime.type_() == mime::TEXT && mime.subtype() == mime::PLAIN -/// }) -/// .error_handler(|err, req| { // <- create custom error response -/// error::InternalError::from_response( -/// err, HttpResponse::Conflict().finish()).into() -/// }) -/// ) -/// .route(web::post().to(index)) -/// ); -/// } +/// // custom `Json` extractor configuration +/// let json_cfg = web::JsonConfig::default() +/// // limit request payload size +/// .limit(4096) +/// // only accept text/plain content type +/// .content_type(|mime| mime == mime::TEXT_PLAIN) +/// // use custom error handler +/// .error_handler(|err, req| { +/// error::InternalError::from_response(err, HttpResponse::Conflict().finish()).into() +/// }); +/// +/// App::new() +/// .app_data(json_cfg) +/// .service(index); /// ``` -/// #[derive(Clone)] pub struct JsonConfig { limit: usize, @@ -282,13 +236,13 @@ pub struct JsonConfig { } impl JsonConfig { - /// Change max size of payload. By default max size is 32Kb + /// Set maximum accepted payload size. By default this limit is 32kB. pub fn limit(mut self, limit: usize) -> Self { self.limit = limit; self } - /// Set custom error handler + /// Set custom error handler. pub fn error_handler(mut self, f: F) -> Self where F: Fn(JsonPayloadError, &HttpRequest) -> Error + Send + Sync + 'static, @@ -297,7 +251,7 @@ impl JsonConfig { self } - /// Set predicate for allowed content types + /// Set predicate for allowed content types. pub fn content_type(mut self, predicate: F) -> Self where F: Fn(mime::Mime) -> bool + Send + Sync + 'static, @@ -328,15 +282,14 @@ impl Default for JsonConfig { } } -/// Request's payload json parser, it resolves to a deserialized `T` value. -/// This future could be used with `ServiceRequest` and `ServiceFromRequest`. +/// Future that resolves to some `T` when parsed from a JSON payload. /// -/// Returns error: +/// Form can be deserialized from any type `T` that implements [`serde::Deserialize`]. /// -/// * content type is not `application/json` -/// (unless specified in [`JsonConfig`]) -/// * content length is greater than 256k -pub enum JsonBody { +/// Returns error if: +/// - content type is not `application/json` +/// - content length is greater than [limit](JsonBody::limit()) +pub enum JsonBody { Error(Option), Body { limit: usize, @@ -346,17 +299,17 @@ pub enum JsonBody { #[cfg(not(feature = "compress"))] payload: Payload, buf: BytesMut, - _res: PhantomData, + _res: PhantomData, }, } -impl Unpin for JsonBody {} +impl Unpin for JsonBody {} -impl JsonBody +impl JsonBody where - U: DeserializeOwned + 'static, + T: DeserializeOwned + 'static, { - /// Create `JsonBody` for request. + /// Create a new future to decode a JSON request payload. #[allow(clippy::borrow_interior_mutable_const)] pub fn new( req: &HttpRequest, @@ -400,7 +353,7 @@ where } } - /// Change max size of payload. By default max size is 256Kb + /// Set maximum accepted payload size. The default limit is 256kB. pub fn limit(self, limit: usize) -> Self { match self { JsonBody::Body { @@ -428,11 +381,11 @@ where } } -impl Future for JsonBody +impl Future for JsonBody where - U: DeserializeOwned + 'static, + T: DeserializeOwned + 'static, { - type Output = Result; + type Output = Result; fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { let this = self.get_mut(); @@ -455,7 +408,7 @@ where } } None => { - let json = serde_json::from_slice::(&buf)?; + let json = serde_json::from_slice::(&buf)?; return Poll::Ready(Ok(json)); } } @@ -468,13 +421,17 @@ where #[cfg(test)] mod tests { use bytes::Bytes; - use serde_derive::{Deserialize, Serialize}; + use serde::{Deserialize, Serialize}; use super::*; - use crate::error::InternalError; - use crate::http::header::{self, HeaderValue, CONTENT_LENGTH, CONTENT_TYPE}; - use crate::test::{load_stream, TestRequest}; - use crate::HttpResponse; + use crate::{ + error::InternalError, + http::{ + header::{self, HeaderValue, CONTENT_LENGTH, CONTENT_TYPE}, + StatusCode, + }, + test::{load_stream, TestRequest}, + }; #[derive(Serialize, Deserialize, PartialEq, Debug)] struct MyObject { @@ -498,7 +455,7 @@ mod tests { let j = Json(MyObject { name: "test".to_string(), }); - let resp = j.respond_to(&req).await.unwrap(); + let resp = j.respond_to(&req); assert_eq!(resp.status(), StatusCode::OK); assert_eq!( resp.headers().get(header::CONTENT_TYPE).unwrap(), @@ -532,7 +489,7 @@ mod tests { .to_http_parts(); let s = Json::::from_request(&req, &mut pl).await; - let mut resp = Response::from_error(s.err().unwrap()); + let mut resp = HttpResponse::from_error(s.err().unwrap()); assert_eq!(resp.status(), StatusCode::BAD_REQUEST); let body = load_stream(resp.take_body()).await.unwrap(); diff --git a/src/types/mod.rs b/src/types/mod.rs index cedf86dd2..a062c351e 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -1,5 +1,6 @@ -//! Helper types +//! Common extractors and responders. +// TODO: review visibility mod either; pub(crate) mod form; pub(crate) mod json; diff --git a/src/types/path.rs b/src/types/path.rs index 640ff4346..9ee5106d0 100644 --- a/src/types/path.rs +++ b/src/types/path.rs @@ -1,70 +1,55 @@ -//! Path extractor -use std::sync::Arc; -use std::{fmt, ops}; +//! For path segment extractor documentation, see [`Path`]. + +use std::{fmt, ops, sync::Arc}; use actix_http::error::{Error, ErrorNotFound}; use actix_router::PathDeserializer; use futures_util::future::{ready, Ready}; use serde::de; -use crate::dev::Payload; -use crate::error::PathError; -use crate::request::HttpRequest; -use crate::FromRequest; +use crate::{dev::Payload, error::PathError, FromRequest, HttpRequest}; -#[derive(PartialEq, Eq, PartialOrd, Ord)] -/// Extract typed information from the request's path. +/// Extract typed data from request path segments. /// -/// [**PathConfig**](PathConfig) allows to configure extraction process. +/// Use [`PathConfig`] to configure extraction process. /// -/// ## Example +/// # Usage +/// ``` +/// use actix_web::{get, web}; /// -/// ```rust -/// use actix_web::{web, App}; -/// -/// /// extract path info from "/{username}/{count}/index.html" url -/// /// {username} - deserializes to a String -/// /// {count} - - deserializes to a u32 -/// async fn index(web::Path((username, count)): web::Path<(String, u32)>) -> String { -/// format!("Welcome {}! {}", username, count) -/// } -/// -/// fn main() { -/// let app = App::new().service( -/// web::resource("/{username}/{count}/index.html") // <- define path parameters -/// .route(web::get().to(index)) // <- register handler with `Path` extractor -/// ); +/// // extract path info from "/{name}/{count}/index.html" into tuple +/// // {name} - deserialize a String +/// // {count} - deserialize a u32 +/// #[get("/")] +/// async fn index(path: web::Path<(String, u32)>) -> String { +/// let (name, count) = path.into_inner(); +/// format!("Welcome {}! {}", name, count) /// } /// ``` /// -/// It is possible to extract path information to a specific type that -/// implements `Deserialize` trait from *serde*. +/// Path segments also can be deserialized into any type that implements [`serde::Deserialize`]. +/// Path segment labels will be matched with struct field names. /// -/// ```rust -/// use actix_web::{web, App, Error}; -/// use serde_derive::Deserialize; +/// ``` +/// use actix_web::{get, web}; +/// use serde::Deserialize; /// /// #[derive(Deserialize)] /// struct Info { -/// username: String, +/// name: String, /// } /// -/// /// extract `Info` from a path using serde -/// async fn index(info: web::Path) -> Result { -/// Ok(format!("Welcome {}!", info.username)) -/// } -/// -/// fn main() { -/// let app = App::new().service( -/// web::resource("/{username}/index.html") // <- define path parameters -/// .route(web::get().to(index)) // <- use handler with Path` extractor -/// ); +/// // extract `Info` from a path using serde +/// #[get("/")] +/// async fn index(info: web::Path) -> String { +/// format!("Welcome {}!", info.name) /// } /// ``` -pub struct Path(pub T); +#[derive(PartialEq, Eq, PartialOrd, Ord)] +pub struct Path(T); impl Path { - /// Deconstruct to an inner value + /// Unwrap into inner `T` value. pub fn into_inner(self) -> T { self.0 } @@ -108,52 +93,7 @@ impl fmt::Display for Path { } } -/// Extract typed information from the request's path. -/// -/// ## Example -/// -/// ```rust -/// use actix_web::{web, App}; -/// -/// /// extract path info from "/{username}/{count}/index.html" url -/// /// {username} - deserializes to a String -/// /// {count} - - deserializes to a u32 -/// async fn index(web::Path((username, count)): web::Path<(String, u32)>) -> String { -/// format!("Welcome {}! {}", username, count) -/// } -/// -/// fn main() { -/// let app = App::new().service( -/// web::resource("/{username}/{count}/index.html") // <- define path parameters -/// .route(web::get().to(index)) // <- register handler with `Path` extractor -/// ); -/// } -/// ``` -/// -/// It is possible to extract path information to a specific type that -/// implements `Deserialize` trait from *serde*. -/// -/// ```rust -/// use actix_web::{web, App, Error}; -/// use serde_derive::Deserialize; -/// -/// #[derive(Deserialize)] -/// struct Info { -/// username: String, -/// } -/// -/// /// extract `Info` from a path using serde -/// async fn index(info: web::Path) -> Result { -/// Ok(format!("Welcome {}!", info.username)) -/// } -/// -/// fn main() { -/// let app = App::new().service( -/// web::resource("/{username}/index.html") // <- define path parameters -/// .route(web::get().to(index)) // <- use handler with Path` extractor -/// ); -/// } -/// ``` +/// See [here](#usage) for example of usage as an extractor. impl FromRequest for Path where T: de::DeserializeOwned, @@ -191,10 +131,10 @@ where /// Path extractor configuration /// -/// ```rust +/// ``` /// use actix_web::web::PathConfig; /// use actix_web::{error, web, App, FromRequest, HttpResponse}; -/// use serde_derive::Deserialize; +/// use serde::Deserialize; /// /// #[derive(Deserialize, Debug)] /// enum Folder { @@ -249,7 +189,7 @@ impl Default for PathConfig { mod tests { use actix_router::ResourceDef; use derive_more::Display; - use serde_derive::Deserialize; + use serde::Deserialize; use super::*; use crate::test::TestRequest; diff --git a/src/types/payload.rs b/src/types/payload.rs index 14457176d..22528031c 100644 --- a/src/types/payload.rs +++ b/src/types/payload.rs @@ -1,57 +1,51 @@ -//! Payload/Bytes/String extractors -use std::future::Future; -use std::pin::Pin; -use std::str; -use std::task::{Context, Poll}; +//! Basic binary and string payload extractors. -use actix_http::error::{Error, ErrorBadRequest, PayloadError}; -use actix_http::HttpMessage; +use std::{ + future::Future, + pin::Pin, + str, + task::{Context, Poll}, +}; + +use actix_http::error::{ErrorBadRequest, PayloadError}; use bytes::{Bytes, BytesMut}; use encoding_rs::{Encoding, UTF_8}; use futures_core::stream::Stream; use futures_util::{ - future::{err, ok, Either, ErrInto, Ready, TryFutureExt as _}, + future::{ready, Either, ErrInto, Ready, TryFutureExt as _}, ready, }; use mime::Mime; -use crate::extract::FromRequest; -use crate::http::header; -use crate::request::HttpRequest; -use crate::{dev, web}; +use crate::{dev, http::header, web, Error, FromRequest, HttpMessage, HttpRequest}; -/// Payload extractor returns request 's payload stream. +/// Extract a request's raw payload stream. /// -/// ## Example +/// See [`PayloadConfig`] for important notes when using this advanced extractor. /// -/// ```rust -/// use actix_web::{web, error, App, Error, HttpResponse}; +/// # Usage +/// ``` /// use std::future::Future; -/// use futures_core::stream::Stream; -/// use futures_util::StreamExt; -/// /// extract binary data from request -/// async fn index(mut body: web::Payload) -> Result -/// { +/// use futures_util::stream::{Stream, StreamExt}; +/// use actix_web::{post, web}; +/// +/// // `body: web::Payload` parameter extracts raw payload stream from request +/// #[post("/")] +/// async fn index(mut body: web::Payload) -> actix_web::Result { +/// // for demonstration only; in a normal case use the `Bytes` extractor +/// // collect payload stream into a bytes object /// let mut bytes = web::BytesMut::new(); /// while let Some(item) = body.next().await { /// bytes.extend_from_slice(&item?); /// } /// -/// format!("Body {:?}!", bytes); -/// Ok(HttpResponse::Ok().finish()) -/// } -/// -/// fn main() { -/// let app = App::new().service( -/// web::resource("/index.html").route( -/// web::get().to(index)) -/// ); +/// Ok(format!("Request Body Bytes:\n{:?}", bytes)) /// } /// ``` pub struct Payload(pub crate::dev::Payload); impl Payload { - /// Deconstruct to a inner value + /// Unwrap to inner Payload type. pub fn into_inner(self) -> crate::dev::Payload { self.0 } @@ -69,35 +63,7 @@ impl Stream for Payload { } } -/// Get request's payload stream -/// -/// ## Example -/// -/// ```rust -/// use actix_web::{web, error, App, Error, HttpResponse}; -/// use std::future::Future; -/// use futures_core::stream::Stream; -/// use futures_util::StreamExt; -/// -/// /// extract binary data from request -/// async fn index(mut body: web::Payload) -> Result -/// { -/// let mut bytes = web::BytesMut::new(); -/// while let Some(item) = body.next().await { -/// bytes.extend_from_slice(&item?); -/// } -/// -/// format!("Body {:?}!", bytes); -/// Ok(HttpResponse::Ok().finish()) -/// } -/// -/// fn main() { -/// let app = App::new().service( -/// web::resource("/index.html").route( -/// web::get().to(index)) -/// ); -/// } -/// ``` +/// See [here](#usage) for example of usage as an extractor. impl FromRequest for Payload { type Config = PayloadConfig; type Error = Error; @@ -105,34 +71,25 @@ impl FromRequest for Payload { #[inline] fn from_request(_: &HttpRequest, payload: &mut dev::Payload) -> Self::Future { - ok(Payload(payload.take())) + ready(Ok(Payload(payload.take()))) } } -/// Request binary data from a request's payload. +/// Extract binary data from a request's payload. /// -/// Loads request's payload and construct Bytes instance. +/// Collects request payload stream into a [Bytes] instance. /// -/// [**PayloadConfig**](PayloadConfig) allows to configure -/// extraction process. +/// Use [`PayloadConfig`] to configure extraction process. /// -/// ## Example -/// -/// ```rust -/// use bytes::Bytes; -/// use actix_web::{web, App}; +/// # Usage +/// ``` +/// use actix_web::{post, web}; /// /// /// extract binary data from request -/// async fn index(body: Bytes) -> String { +/// #[post("/")] +/// async fn index(body: web::Bytes) -> String { /// format!("Body {:?}!", body) /// } -/// -/// fn main() { -/// let app = App::new().service( -/// web::resource("/index.html").route( -/// web::get().to(index)) -/// ); -/// } /// ``` impl FromRequest for Bytes { type Config = PayloadConfig; @@ -144,8 +101,8 @@ impl FromRequest for Bytes { // allow both Config and Data let cfg = PayloadConfig::from_req(req); - if let Err(e) = cfg.check_mimetype(req) { - return Either::Right(err(e)); + if let Err(err) = cfg.check_mimetype(req) { + return Either::Right(ready(Err(err))); } let limit = cfg.limit; @@ -161,26 +118,15 @@ impl FromRequest for Bytes { /// [**PayloadConfig**](PayloadConfig) allows to configure /// extraction process. /// -/// ## Example +/// # Usage +/// ``` +/// use actix_web::{post, web, FromRequest}; /// -/// ```rust -/// use actix_web::{web, App, FromRequest}; -/// -/// /// extract text data from request +/// // extract text data from request +/// #[post("/")] /// async fn index(text: String) -> String { /// format!("Body {}!", text) /// } -/// -/// fn main() { -/// let app = App::new().service( -/// web::resource("/index.html") -/// .app_data(String::configure(|cfg| { // <- limit size of the payload -/// cfg.limit(4096) -/// })) -/// .route(web::get().to(index)) // <- register handler with extractor params -/// ); -/// } -/// ``` impl FromRequest for String { type Config = PayloadConfig; type Error = Error; @@ -191,14 +137,14 @@ impl FromRequest for String { let cfg = PayloadConfig::from_req(req); // check content-type - if let Err(e) = cfg.check_mimetype(req) { - return Either::Right(err(e)); + if let Err(err) = cfg.check_mimetype(req) { + return Either::Right(ready(Err(err))); } // check charset let encoding = match req.encoding() { Ok(enc) => enc, - Err(e) => return Either::Right(err(e.into())), + Err(err) => return Either::Right(ready(Err(err.into()))), }; let limit = cfg.limit; let body_fut = HttpMessageBody::new(req, payload).limit(limit); @@ -238,11 +184,13 @@ fn bytes_to_string(body: Bytes, encoding: &'static Encoding) -> Result Self { Self { limit, @@ -258,14 +206,13 @@ impl PayloadConfig { } } - /// Change max size of payload. By default max size is 256Kb + /// Set maximum accepted payload size. The default limit is 256kB. pub fn limit(mut self, limit: usize) -> Self { self.limit = limit; self } - /// Set required mime-type of the request. By default mime type is not - /// enforced. + /// Set required mime type of the request. By default mime type is not enforced. pub fn mimetype(mut self, mt: Mime) -> Self { self.mimetype = Some(mt); self @@ -292,7 +239,7 @@ impl PayloadConfig { } /// Extract payload config from app data. Check both `T` and `Data`, in that order, and fall - /// back to the default payload config. + /// back to the default payload config if neither is found. fn from_req(req: &HttpRequest) -> &Self { req.app_data::() .or_else(|| req.app_data::>().map(|d| d.as_ref())) @@ -314,13 +261,10 @@ impl Default for PayloadConfig { } } -/// Future that resolves to a complete http message body. +/// Future that resolves to a complete HTTP body payload. /// -/// Load http message body. -/// -/// By default only 256Kb payload reads to a memory, then -/// `PayloadError::Overflow` get returned. Use `MessageBody::limit()` -/// method to change upper limit. +/// By default only 256kB payload is accepted before `PayloadError::Overflow` is returned. +/// Use `MessageBody::limit()` method to change upper limit. pub struct HttpMessageBody { limit: usize, length: Option, @@ -366,7 +310,7 @@ impl HttpMessageBody { } } - /// Change max size of payload. By default max size is 256Kb + /// Change max size of payload. By default max size is 256kB pub fn limit(mut self, limit: usize) -> Self { if let Some(l) = self.length { if l > limit { @@ -384,8 +328,8 @@ impl Future for HttpMessageBody { fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { let this = self.get_mut(); - if let Some(e) = this.err.take() { - return Poll::Ready(Err(e)); + if let Some(err) = this.err.take() { + return Poll::Ready(Err(err)); } loop { diff --git a/src/types/query.rs b/src/types/query.rs index 27df220fc..2cfc18c36 100644 --- a/src/types/query.rs +++ b/src/types/query.rs @@ -1,30 +1,27 @@ -//! Query extractor +//! For query parameter extractor documentation, see [`Query`]. -use std::sync::Arc; -use std::{fmt, ops}; +use std::{fmt, ops, sync::Arc}; -use actix_http::error::Error; use futures_util::future::{err, ok, Ready}; use serde::de; -use crate::dev::Payload; -use crate::error::QueryPayloadError; -use crate::extract::FromRequest; -use crate::request::HttpRequest; +use crate::{dev::Payload, error::QueryPayloadError, Error, FromRequest, HttpRequest}; /// Extract typed information from the request's query. /// -/// **Note**: A query string consists of unordered `key=value` pairs, therefore it cannot -/// be decoded into any type which depends upon data ordering e.g. tuples or tuple-structs. -/// Attempts to do so will *fail at runtime*. +/// To extract typed data from the URL query string, the inner type `T` must implement the +/// [`serde::Deserialize`] trait. /// -/// [**QueryConfig**](QueryConfig) allows to configure extraction process. +/// Use [`QueryConfig`] to configure extraction process. /// -/// ## Example +/// # Panics +/// A query string consists of unordered `key=value` pairs, therefore it cannot be decoded into any +/// type which depends upon data ordering (eg. tuples). Trying to do so will result in a panic. /// -/// ```rust -/// use actix_web::{web, App}; -/// use serde_derive::Deserialize; +/// # Usage +/// ``` +/// use actix_web::{get, web}; +/// use serde::Deserialize; /// /// #[derive(Debug, Deserialize)] /// pub enum ResponseType { @@ -38,35 +35,40 @@ use crate::request::HttpRequest; /// response_type: ResponseType, /// } /// -/// // Use `Query` extractor for query information (and destructure it within the signature). -/// // This handler gets called only if the request's query string contains `id` and `response_type` fields. -/// // The correct request for this handler would be `/index.html?id=64&response_type=Code"`. -/// async fn index(web::Query(info): web::Query) -> String { -/// format!("Authorization request for client with id={} and type={:?}!", info.id, info.response_type) -/// } -/// -/// fn main() { -/// let app = App::new().service( -/// web::resource("/index.html").route(web::get().to(index))); // <- use `Query` extractor +/// // Deserialize `AuthRequest` struct from query string. +/// // This handler gets called only if the request's query parameters contain both fields. +/// // A valid request path for this handler would be `/?id=64&response_type=Code"`. +/// #[get("/")] +/// async fn index(info: web::Query) -> String { +/// format!("Authorization request for id={} and type={:?}!", info.id, info.response_type) /// } /// ``` -#[derive(PartialEq, Eq, PartialOrd, Ord)] -pub struct Query(pub T); +#[derive(Clone, PartialEq, Eq, PartialOrd, Ord)] +pub struct Query(T); impl Query { - /// Deconstruct to a inner value + /// Unwrap into inner `T` value. pub fn into_inner(self) -> T { self.0 } - /// Get query parameters from the path + /// Deserialize `T` from a URL encoded query parameter string. + /// + /// ``` + /// # use std::collections::HashMap; + /// # use actix_web::web::Query; + /// let numbers = Query::>::from_query("one=1&two=2").unwrap(); + /// assert_eq!(numbers.get("one"), Some(&1)); + /// assert_eq!(numbers.get("two"), Some(&2)); + /// assert!(numbers.get("three").is_none()); + /// ``` pub fn from_query(query_str: &str) -> Result where T: de::DeserializeOwned, { serde_urlencoded::from_str::(query_str) - .map(|val| Ok(Query(val))) - .unwrap_or_else(move |e| Err(QueryPayloadError::Deserialize(e))) + .map(Self) + .map_err(QueryPayloadError::Deserialize) } } @@ -96,39 +98,7 @@ impl fmt::Display for Query { } } -/// Extract typed information from the request's query. -/// -/// ## Example -/// -/// ```rust -/// use actix_web::{web, App}; -/// use serde_derive::Deserialize; -/// -/// #[derive(Debug, Deserialize)] -/// pub enum ResponseType { -/// Token, -/// Code -/// } -/// -/// #[derive(Deserialize)] -/// pub struct AuthRequest { -/// id: u64, -/// response_type: ResponseType, -/// } -/// -/// // Use `Query` extractor for query information. -/// // This handler get called only if request's query contains `id` and `response_type` fields. -/// // The correct request for this handler would be `/index.html?id=64&response_type=Code"` -/// async fn index(info: web::Query) -> String { -/// format!("Authorization request for client with id={} and type={:?}!", info.id, info.response_type) -/// } -/// -/// fn main() { -/// let app = App::new().service( -/// web::resource("/index.html") -/// .route(web::get().to(index))); // <- use `Query` extractor -/// } -/// ``` +/// See [here](#usage) for example of usage as an extractor. impl FromRequest for Query where T: de::DeserializeOwned, @@ -141,7 +111,7 @@ where fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future { let error_handler = req .app_data::() - .map(|c| c.ehandler.clone()) + .map(|c| c.err_handler.clone()) .unwrap_or(None); serde_urlencoded::from_str::(req.query_string()) @@ -166,13 +136,12 @@ where } } -/// Query extractor configuration +/// Query extractor configuration. /// -/// ## Example -/// -/// ```rust -/// use actix_web::{error, web, App, FromRequest, HttpResponse}; -/// use serde_derive::Deserialize; +/// # Usage +/// ``` +/// use actix_web::{error, get, web, App, FromRequest, HttpResponse}; +/// use serde::Deserialize; /// /// #[derive(Deserialize)] /// struct Info { @@ -180,27 +149,25 @@ where /// } /// /// /// deserialize `Info` from request's querystring +/// #[get("/")] /// async fn index(info: web::Query) -> String { /// format!("Welcome {}!", info.username) /// } /// -/// fn main() { -/// let app = App::new().service( -/// web::resource("/index.html").app_data( -/// // change query extractor configuration -/// web::QueryConfig::default() -/// .error_handler(|err, req| { // <- create custom error response -/// error::InternalError::from_response( -/// err, HttpResponse::Conflict().finish()).into() -/// }) -/// ) -/// .route(web::post().to(index)) -/// ); -/// } +/// // custom `Query` extractor configuration +/// let query_cfg = web::QueryConfig::default() +/// // use custom error handler +/// .error_handler(|err, req| { +/// error::InternalError::from_response(err, HttpResponse::Conflict().finish()).into() +/// }); +/// +/// App::new() +/// .app_data(query_cfg) +/// .service(index); /// ``` #[derive(Clone)] pub struct QueryConfig { - ehandler: + err_handler: Option Error + Send + Sync>>, } @@ -210,14 +177,14 @@ impl QueryConfig { where F: Fn(QueryPayloadError, &HttpRequest) -> Error + Send + Sync + 'static, { - self.ehandler = Some(Arc::new(f)); + self.err_handler = Some(Arc::new(f)); self } } impl Default for QueryConfig { fn default() -> Self { - QueryConfig { ehandler: None } + QueryConfig { err_handler: None } } } @@ -225,7 +192,7 @@ impl Default for QueryConfig { mod tests { use actix_http::http::StatusCode; use derive_more::Display; - use serde_derive::Deserialize; + use serde::Deserialize; use super::*; use crate::error::InternalError; @@ -271,6 +238,17 @@ mod tests { assert_eq!(s.id, "test1"); } + #[actix_rt::test] + #[should_panic] + async fn test_tuple_panic() { + let req = TestRequest::with_uri("/?one=1&two=2").to_srv_request(); + let (req, mut pl) = req.into_parts(); + + Query::<(u32, u32)>::from_request(&req, &mut pl) + .await + .unwrap(); + } + #[actix_rt::test] async fn test_custom_error_responder() { let req = TestRequest::with_uri("/name/user1/") diff --git a/src/types/readlines.rs b/src/types/readlines.rs index f03235377..01aab64ab 100644 --- a/src/types/readlines.rs +++ b/src/types/readlines.rs @@ -1,17 +1,23 @@ -use std::borrow::Cow; -use std::pin::Pin; -use std::str; -use std::task::{Context, Poll}; +//! For request line reader documentation, see [`Readlines`]. + +use std::{ + borrow::Cow, + pin::Pin, + str, + task::{Context, Poll}, +}; use bytes::{Bytes, BytesMut}; use encoding_rs::{Encoding, UTF_8}; -use futures_util::stream::Stream; +use futures_core::{ready, stream::Stream}; -use crate::dev::Payload; -use crate::error::{PayloadError, ReadlinesError}; -use crate::HttpMessage; +use crate::{ + dev::Payload, + error::{PayloadError, ReadlinesError}, + HttpMessage, +}; -/// Stream to read request line by line. +/// Stream that reads request line by line. pub struct Readlines { stream: Payload, buff: BytesMut, @@ -43,7 +49,7 @@ where } } - /// Change max line size. By default max size is 256Kb + /// Set maximum accepted payload size. The default limit is 256kB. pub fn limit(mut self, limit: usize) -> Self { self.limit = limit; self @@ -108,9 +114,10 @@ where } this.checked_buff = true; } + // poll req for more bytes - match Pin::new(&mut this.stream).poll_next(cx) { - Poll::Ready(Some(Ok(mut bytes))) => { + match ready!(Pin::new(&mut this.stream).poll_next(cx)) { + Some(Ok(mut bytes)) => { // check if there is a newline in bytes let mut found: Option = None; for (ind, b) in bytes.iter().enumerate() { @@ -144,8 +151,8 @@ where this.buff.extend_from_slice(&bytes); Poll::Pending } - Poll::Pending => Poll::Pending, - Poll::Ready(None) => { + + None => { if this.buff.is_empty() { return Poll::Ready(None); } @@ -165,7 +172,8 @@ where this.buff.clear(); Poll::Ready(Some(Ok(line))) } - Poll::Ready(Some(Err(e))) => Poll::Ready(Some(Err(ReadlinesError::from(e)))), + + Some(Err(err)) => Poll::Ready(Some(Err(ReadlinesError::from(err)))), } } } diff --git a/src/web.rs b/src/web.rs index 39dfc450a..88071f551 100644 --- a/src/web.rs +++ b/src/web.rs @@ -280,5 +280,8 @@ where I: Send + 'static, E: Send + std::fmt::Debug + 'static, { - actix_threadpool::run(f).await + match actix_rt::task::spawn_blocking(f).await { + Ok(res) => res.map_err(BlockingError::Error), + Err(_) => Err(BlockingError::Canceled), + } }