From 2a8c650f2c4ccc5cafcfba01e85109a0388e604f Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 14 May 2021 16:40:00 +0100 Subject: [PATCH] move internalerror to actix web (#2215) --- Cargo.toml | 1 + actix-http/CHANGES.md | 4 + actix-http/Cargo.toml | 1 - actix-http/src/body/body.rs | 93 ++++----- actix-http/src/body/mod.rs | 2 +- actix-http/src/error.rs | 311 +------------------------------ actix-http/src/helpers.rs | 3 +- actix-http/tests/test_client.rs | 17 +- actix-http/tests/test_openssl.rs | 17 +- actix-http/tests/test_rustls.rs | 20 +- actix-http/tests/test_server.rs | 41 ++-- src/data.rs | 13 +- src/error/internal.rs | 304 ++++++++++++++++++++++++++++++ src/{error.rs => error/mod.rs} | 4 + src/request_data.rs | 4 +- src/responder.rs | 3 +- src/types/path.rs | 8 +- src/types/payload.rs | 7 +- 18 files changed, 456 insertions(+), 397 deletions(-) create mode 100644 src/error/internal.rs rename src/{error.rs => error/mod.rs} (99%) diff --git a/Cargo.toml b/Cargo.toml index 75b5e3a8e..5aa302333 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -82,6 +82,7 @@ language-tags = "0.3" once_cell = "1.5" log = "0.4" mime = "0.3" +paste = "1" pin-project = "1.0.0" regex = "1.4" serde = { version = "1.0", features = ["derive"] } diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index 31ad277de..ec7d8ee3b 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -2,6 +2,7 @@ ## Unreleased - 2021-xx-xx ### Added +* Alias `body::Body` as `body::AnyBody`. [#2215] * `BoxAnyBody`: a boxed message body with boxed errors. [#2183] * Re-export `http` crate's `Error` type as `error::HttpError`. [#2171] * Re-export `StatusCode`, `Method`, `Version` and `Uri` at the crate root. [#2171] @@ -25,12 +26,15 @@ * Error field from `Response` and `Response::error`. [#2205] * `impl Future` for `Response`. [#2201] * `Response::take_body` and old `Response::into_body` method that casted body type. [#2201] +* `InternalError` and all the error types it constructed. [#2215] +* Conversion (`impl Into`) of `Response` and `ResponseBuilder` to `Error`. [#2215] [#2171]: https://github.com/actix/actix-web/pull/2171 [#2183]: https://github.com/actix/actix-web/pull/2183 [#2196]: https://github.com/actix/actix-web/pull/2196 [#2201]: https://github.com/actix/actix-web/pull/2201 [#2205]: https://github.com/actix/actix-web/pull/2205 +[#2215]: https://github.com/actix/actix-web/pull/2215 ## 3.0.0-beta.6 - 2021-04-17 diff --git a/actix-http/Cargo.toml b/actix-http/Cargo.toml index 638557807..1f7df39a6 100644 --- a/actix-http/Cargo.toml +++ b/actix-http/Cargo.toml @@ -62,7 +62,6 @@ local-channel = "0.1" once_cell = "1.5" log = "0.4" mime = "0.3" -paste = "1" percent-encoding = "2.1" pin-project = "1.0.0" pin-project-lite = "0.2" diff --git a/actix-http/src/body/body.rs b/actix-http/src/body/body.rs index 4c95bd31a..3fda8ae11 100644 --- a/actix-http/src/body/body.rs +++ b/actix-http/src/body/body.rs @@ -13,9 +13,10 @@ use crate::error::Error; use super::{BodySize, BodyStream, MessageBody, MessageBodyMapErr, SizedStream}; +pub type Body = AnyBody; + /// Represents various types of HTTP message body. -// #[deprecated(since = "4.0.0", note = "Use body types directly.")] -pub enum Body { +pub enum AnyBody { /// Empty response. `Content-Length` header is not set. None, @@ -29,14 +30,14 @@ pub enum Body { Message(BoxAnyBody), } -impl Body { +impl AnyBody { /// Create body from slice (copy) - pub fn from_slice(s: &[u8]) -> Body { - Body::Bytes(Bytes::copy_from_slice(s)) + pub fn from_slice(s: &[u8]) -> Self { + Self::Bytes(Bytes::copy_from_slice(s)) } /// Create body from generic message body. - pub fn from_message(body: B) -> Body + pub fn from_message(body: B) -> Self where B: MessageBody + 'static, B::Error: Into>, @@ -45,15 +46,15 @@ impl Body { } } -impl MessageBody for Body { +impl MessageBody for AnyBody { type Error = Error; fn size(&self) -> BodySize { match self { - Body::None => BodySize::None, - Body::Empty => BodySize::Empty, - Body::Bytes(ref bin) => BodySize::Sized(bin.len() as u64), - Body::Message(ref body) => body.size(), + AnyBody::None => BodySize::None, + AnyBody::Empty => BodySize::Empty, + AnyBody::Bytes(ref bin) => BodySize::Sized(bin.len() as u64), + AnyBody::Message(ref body) => body.size(), } } @@ -62,9 +63,9 @@ impl MessageBody for Body { cx: &mut Context<'_>, ) -> Poll>> { match self.get_mut() { - Body::None => Poll::Ready(None), - Body::Empty => Poll::Ready(None), - Body::Bytes(ref mut bin) => { + AnyBody::None => Poll::Ready(None), + AnyBody::Empty => Poll::Ready(None), + AnyBody::Bytes(ref mut bin) => { let len = bin.len(); if len == 0 { Poll::Ready(None) @@ -74,7 +75,7 @@ impl MessageBody for Body { } // TODO: MSRV 1.51: poll_map_err - Body::Message(body) => match ready!(body.as_pin_mut().poll_next(cx)) { + AnyBody::Message(body) => match ready!(body.as_pin_mut().poll_next(cx)) { Some(Err(err)) => Poll::Ready(Some(Err(err.into()))), Some(Ok(val)) => Poll::Ready(Some(Ok(val))), None => Poll::Ready(None), @@ -83,100 +84,100 @@ impl MessageBody for Body { } } -impl PartialEq for Body { +impl PartialEq for AnyBody { fn eq(&self, other: &Body) -> bool { match *self { - Body::None => matches!(*other, Body::None), - Body::Empty => matches!(*other, Body::Empty), - Body::Bytes(ref b) => match *other { - Body::Bytes(ref b2) => b == b2, + AnyBody::None => matches!(*other, AnyBody::None), + AnyBody::Empty => matches!(*other, AnyBody::Empty), + AnyBody::Bytes(ref b) => match *other { + AnyBody::Bytes(ref b2) => b == b2, _ => false, }, - Body::Message(_) => false, + AnyBody::Message(_) => false, } } } -impl fmt::Debug for Body { +impl fmt::Debug for AnyBody { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match *self { - Body::None => write!(f, "Body::None"), - Body::Empty => write!(f, "Body::Empty"), - Body::Bytes(ref b) => write!(f, "Body::Bytes({:?})", b), - Body::Message(_) => write!(f, "Body::Message(_)"), + AnyBody::None => write!(f, "AnyBody::None"), + AnyBody::Empty => write!(f, "AnyBody::Empty"), + AnyBody::Bytes(ref b) => write!(f, "AnyBody::Bytes({:?})", b), + AnyBody::Message(_) => write!(f, "AnyBody::Message(_)"), } } } -impl From<&'static str> for Body { +impl From<&'static str> for AnyBody { fn from(s: &'static str) -> Body { - Body::Bytes(Bytes::from_static(s.as_ref())) + AnyBody::Bytes(Bytes::from_static(s.as_ref())) } } -impl From<&'static [u8]> for Body { +impl From<&'static [u8]> for AnyBody { fn from(s: &'static [u8]) -> Body { - Body::Bytes(Bytes::from_static(s)) + AnyBody::Bytes(Bytes::from_static(s)) } } -impl From> for Body { +impl From> for AnyBody { fn from(vec: Vec) -> Body { - Body::Bytes(Bytes::from(vec)) + AnyBody::Bytes(Bytes::from(vec)) } } -impl From for Body { +impl From for AnyBody { fn from(s: String) -> Body { s.into_bytes().into() } } -impl From<&'_ String> for Body { +impl From<&'_ String> for AnyBody { fn from(s: &String) -> Body { - Body::Bytes(Bytes::copy_from_slice(AsRef::<[u8]>::as_ref(&s))) + AnyBody::Bytes(Bytes::copy_from_slice(AsRef::<[u8]>::as_ref(&s))) } } -impl From> for Body { +impl From> for AnyBody { fn from(s: Cow<'_, str>) -> Body { match s { - Cow::Owned(s) => Body::from(s), + Cow::Owned(s) => AnyBody::from(s), Cow::Borrowed(s) => { - Body::Bytes(Bytes::copy_from_slice(AsRef::<[u8]>::as_ref(s))) + AnyBody::Bytes(Bytes::copy_from_slice(AsRef::<[u8]>::as_ref(s))) } } } } -impl From for Body { +impl From for AnyBody { fn from(s: Bytes) -> Body { - Body::Bytes(s) + AnyBody::Bytes(s) } } -impl From for Body { +impl From for AnyBody { fn from(s: BytesMut) -> Body { - Body::Bytes(s.freeze()) + AnyBody::Bytes(s.freeze()) } } -impl From> for Body +impl From> for AnyBody where S: Stream> + 'static, { fn from(s: SizedStream) -> Body { - Body::from_message(s) + AnyBody::from_message(s) } } -impl From> for Body +impl From> for AnyBody where S: Stream> + 'static, E: Into + 'static, { fn from(s: BodyStream) -> Body { - Body::from_message(s) + AnyBody::from_message(s) } } diff --git a/actix-http/src/body/mod.rs b/actix-http/src/body/mod.rs index cdfcd226b..11aff039e 100644 --- a/actix-http/src/body/mod.rs +++ b/actix-http/src/body/mod.rs @@ -15,7 +15,7 @@ mod response_body; mod size; mod sized_stream; -pub use self::body::{Body, BoxAnyBody}; +pub use self::body::{AnyBody, Body, BoxAnyBody}; pub use self::body_stream::BodyStream; pub use self::message_body::MessageBody; pub(crate) use self::message_body::MessageBodyMapErr; diff --git a/actix-http/src/error.rs b/actix-http/src/error.rs index 20b2a2d75..92efd572d 100644 --- a/actix-http/src/error.rs +++ b/actix-http/src/error.rs @@ -1,7 +1,6 @@ //! Error and Result module use std::{ - cell::RefCell, error::Error as StdError, fmt, io::{self, Write as _}, @@ -14,7 +13,7 @@ use derive_more::{Display, Error, From}; use http::{header, uri::InvalidUri, StatusCode}; use serde::de::value::Error as DeError; -use crate::{body::Body, helpers::Writer, Response, ResponseBuilder}; +use crate::{body::Body, helpers::Writer, Response}; pub use http::Error as HttpError; @@ -121,20 +120,6 @@ impl From for Error { } } -/// Convert Response to a Error -impl From> for Error { - fn from(res: Response) -> Error { - InternalError::from_response("", res).into() - } -} - -/// Convert ResponseBuilder to a Error -impl From for Error { - fn from(mut res: ResponseBuilder) -> Error { - InternalError::from_response("", res.finish()).into() - } -} - #[derive(Debug, Display, Error)] #[display(fmt = "Unknown Error")] struct UnitError; @@ -449,179 +434,12 @@ mod content_type_test_impls { } } -/// Return `BadRequest` for `ContentTypeError` impl ResponseError for ContentTypeError { fn status_code(&self) -> StatusCode { StatusCode::BAD_REQUEST } } -/// Helper type that can wrap any error and generate custom response. -/// -/// In following example any `io::Error` will be converted into "BAD REQUEST" -/// response as opposite to *INTERNAL SERVER ERROR* which is defined by -/// default. -/// -/// ``` -/// # use std::io; -/// # use actix_http::{error, Request}; -/// fn index(req: Request) -> Result<&'static str, actix_http::Error> { -/// Err(error::ErrorBadRequest(io::Error::new(io::ErrorKind::Other, "error"))) -/// } -/// ``` -pub struct InternalError { - cause: T, - status: InternalErrorType, -} - -enum InternalErrorType { - Status(StatusCode), - Response(RefCell>>), -} - -impl InternalError { - /// Create `InternalError` instance - pub fn new(cause: T, status: StatusCode) -> Self { - InternalError { - cause, - status: InternalErrorType::Status(status), - } - } - - /// Create `InternalError` with predefined `Response`. - pub fn from_response(cause: T, response: Response) -> Self { - InternalError { - cause, - status: InternalErrorType::Response(RefCell::new(Some(response))), - } - } -} - -impl fmt::Debug for InternalError -where - T: fmt::Debug + 'static, -{ - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Debug::fmt(&self.cause, f) - } -} - -impl fmt::Display for InternalError -where - T: fmt::Display + 'static, -{ - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Display::fmt(&self.cause, f) - } -} - -impl ResponseError for InternalError -where - T: fmt::Debug + fmt::Display + 'static, -{ - fn status_code(&self) -> StatusCode { - match self.status { - InternalErrorType::Status(st) => st, - InternalErrorType::Response(ref resp) => { - if let Some(resp) = resp.borrow().as_ref() { - resp.head().status - } else { - StatusCode::INTERNAL_SERVER_ERROR - } - } - } - } - - fn error_response(&self) -> Response { - match self.status { - InternalErrorType::Status(st) => { - let mut res = Response::new(st); - let mut buf = BytesMut::new(); - let _ = write!(Writer(&mut buf), "{}", self); - res.headers_mut().insert( - header::CONTENT_TYPE, - header::HeaderValue::from_static("text/plain; charset=utf-8"), - ); - res.set_body(Body::from(buf)) - } - InternalErrorType::Response(ref resp) => { - if let Some(resp) = resp.borrow_mut().take() { - resp - } else { - Response::new(StatusCode::INTERNAL_SERVER_ERROR) - } - } - } - } -} - -macro_rules! error_helper { - ($name:ident, $status:ident) => { - paste::paste! { - #[doc = "Helper function that wraps any error and generates a `" $status "` response."] - #[allow(non_snake_case)] - pub fn $name(err: T) -> Error - where - T: fmt::Debug + fmt::Display + 'static, - { - InternalError::new(err, StatusCode::$status).into() - } - } - } -} - -error_helper!(ErrorBadRequest, BAD_REQUEST); -error_helper!(ErrorUnauthorized, UNAUTHORIZED); -error_helper!(ErrorPaymentRequired, PAYMENT_REQUIRED); -error_helper!(ErrorForbidden, FORBIDDEN); -error_helper!(ErrorNotFound, NOT_FOUND); -error_helper!(ErrorMethodNotAllowed, METHOD_NOT_ALLOWED); -error_helper!(ErrorNotAcceptable, NOT_ACCEPTABLE); -error_helper!( - ErrorProxyAuthenticationRequired, - PROXY_AUTHENTICATION_REQUIRED -); -error_helper!(ErrorRequestTimeout, REQUEST_TIMEOUT); -error_helper!(ErrorConflict, CONFLICT); -error_helper!(ErrorGone, GONE); -error_helper!(ErrorLengthRequired, LENGTH_REQUIRED); -error_helper!(ErrorPayloadTooLarge, PAYLOAD_TOO_LARGE); -error_helper!(ErrorUriTooLong, URI_TOO_LONG); -error_helper!(ErrorUnsupportedMediaType, UNSUPPORTED_MEDIA_TYPE); -error_helper!(ErrorRangeNotSatisfiable, RANGE_NOT_SATISFIABLE); -error_helper!(ErrorImATeapot, IM_A_TEAPOT); -error_helper!(ErrorMisdirectedRequest, MISDIRECTED_REQUEST); -error_helper!(ErrorUnprocessableEntity, UNPROCESSABLE_ENTITY); -error_helper!(ErrorLocked, LOCKED); -error_helper!(ErrorFailedDependency, FAILED_DEPENDENCY); -error_helper!(ErrorUpgradeRequired, UPGRADE_REQUIRED); -error_helper!(ErrorPreconditionFailed, PRECONDITION_FAILED); -error_helper!(ErrorPreconditionRequired, PRECONDITION_REQUIRED); -error_helper!(ErrorTooManyRequests, TOO_MANY_REQUESTS); -error_helper!( - ErrorRequestHeaderFieldsTooLarge, - REQUEST_HEADER_FIELDS_TOO_LARGE -); -error_helper!( - ErrorUnavailableForLegalReasons, - UNAVAILABLE_FOR_LEGAL_REASONS -); -error_helper!(ErrorExpectationFailed, EXPECTATION_FAILED); -error_helper!(ErrorInternalServerError, INTERNAL_SERVER_ERROR); -error_helper!(ErrorNotImplemented, NOT_IMPLEMENTED); -error_helper!(ErrorBadGateway, BAD_GATEWAY); -error_helper!(ErrorServiceUnavailable, SERVICE_UNAVAILABLE); -error_helper!(ErrorGatewayTimeout, GATEWAY_TIMEOUT); -error_helper!(ErrorHttpVersionNotSupported, HTTP_VERSION_NOT_SUPPORTED); -error_helper!(ErrorVariantAlsoNegotiates, VARIANT_ALSO_NEGOTIATES); -error_helper!(ErrorInsufficientStorage, INSUFFICIENT_STORAGE); -error_helper!(ErrorLoopDetected, LOOP_DETECTED); -error_helper!(ErrorNotExtended, NOT_EXTENDED); -error_helper!( - ErrorNetworkAuthenticationRequired, - NETWORK_AUTHENTICATION_REQUIRED -); - #[cfg(test)] mod tests { use super::*; @@ -718,13 +536,6 @@ mod tests { from!(httparse::Error::Version => ParseError::Version); } - #[test] - fn test_internal_error() { - let err = InternalError::from_response(ParseError::Method, Response::ok()); - let resp: Response = err.error_response(); - assert_eq!(resp.status(), StatusCode::OK); - } - #[test] fn test_error_casting() { let err = PayloadError::Overflow; @@ -734,124 +545,4 @@ mod tests { let not_err = resp_err.downcast_ref::(); assert!(not_err.is_none()); } - - #[test] - fn test_error_helpers() { - let res: Response = ErrorBadRequest("err").into(); - assert_eq!(res.status(), StatusCode::BAD_REQUEST); - - let res: Response = ErrorUnauthorized("err").into(); - assert_eq!(res.status(), StatusCode::UNAUTHORIZED); - - let res: Response = ErrorPaymentRequired("err").into(); - assert_eq!(res.status(), StatusCode::PAYMENT_REQUIRED); - - let res: Response = ErrorForbidden("err").into(); - assert_eq!(res.status(), StatusCode::FORBIDDEN); - - let res: Response = ErrorNotFound("err").into(); - assert_eq!(res.status(), StatusCode::NOT_FOUND); - - let res: Response = ErrorMethodNotAllowed("err").into(); - assert_eq!(res.status(), StatusCode::METHOD_NOT_ALLOWED); - - let res: Response = ErrorNotAcceptable("err").into(); - assert_eq!(res.status(), StatusCode::NOT_ACCEPTABLE); - - let res: Response = ErrorProxyAuthenticationRequired("err").into(); - assert_eq!(res.status(), StatusCode::PROXY_AUTHENTICATION_REQUIRED); - - let res: Response = ErrorRequestTimeout("err").into(); - assert_eq!(res.status(), StatusCode::REQUEST_TIMEOUT); - - let res: Response = ErrorConflict("err").into(); - assert_eq!(res.status(), StatusCode::CONFLICT); - - let res: Response = ErrorGone("err").into(); - assert_eq!(res.status(), StatusCode::GONE); - - let res: Response = ErrorLengthRequired("err").into(); - assert_eq!(res.status(), StatusCode::LENGTH_REQUIRED); - - let res: Response = ErrorPreconditionFailed("err").into(); - assert_eq!(res.status(), StatusCode::PRECONDITION_FAILED); - - let res: Response = ErrorPayloadTooLarge("err").into(); - assert_eq!(res.status(), StatusCode::PAYLOAD_TOO_LARGE); - - let res: Response = ErrorUriTooLong("err").into(); - assert_eq!(res.status(), StatusCode::URI_TOO_LONG); - - let res: Response = ErrorUnsupportedMediaType("err").into(); - assert_eq!(res.status(), StatusCode::UNSUPPORTED_MEDIA_TYPE); - - let res: Response = ErrorRangeNotSatisfiable("err").into(); - assert_eq!(res.status(), StatusCode::RANGE_NOT_SATISFIABLE); - - let res: Response = ErrorExpectationFailed("err").into(); - assert_eq!(res.status(), StatusCode::EXPECTATION_FAILED); - - let res: Response = ErrorImATeapot("err").into(); - assert_eq!(res.status(), StatusCode::IM_A_TEAPOT); - - let res: Response = ErrorMisdirectedRequest("err").into(); - assert_eq!(res.status(), StatusCode::MISDIRECTED_REQUEST); - - let res: Response = ErrorUnprocessableEntity("err").into(); - assert_eq!(res.status(), StatusCode::UNPROCESSABLE_ENTITY); - - let res: Response = ErrorLocked("err").into(); - assert_eq!(res.status(), StatusCode::LOCKED); - - let res: Response = ErrorFailedDependency("err").into(); - assert_eq!(res.status(), StatusCode::FAILED_DEPENDENCY); - - let res: Response = ErrorUpgradeRequired("err").into(); - assert_eq!(res.status(), StatusCode::UPGRADE_REQUIRED); - - let res: Response = ErrorPreconditionRequired("err").into(); - assert_eq!(res.status(), StatusCode::PRECONDITION_REQUIRED); - - let res: Response = ErrorTooManyRequests("err").into(); - assert_eq!(res.status(), StatusCode::TOO_MANY_REQUESTS); - - let res: Response = ErrorRequestHeaderFieldsTooLarge("err").into(); - assert_eq!(res.status(), StatusCode::REQUEST_HEADER_FIELDS_TOO_LARGE); - - let res: Response = ErrorUnavailableForLegalReasons("err").into(); - assert_eq!(res.status(), StatusCode::UNAVAILABLE_FOR_LEGAL_REASONS); - - let res: Response = ErrorInternalServerError("err").into(); - assert_eq!(res.status(), StatusCode::INTERNAL_SERVER_ERROR); - - let res: Response = ErrorNotImplemented("err").into(); - assert_eq!(res.status(), StatusCode::NOT_IMPLEMENTED); - - let res: Response = ErrorBadGateway("err").into(); - assert_eq!(res.status(), StatusCode::BAD_GATEWAY); - - let res: Response = ErrorServiceUnavailable("err").into(); - assert_eq!(res.status(), StatusCode::SERVICE_UNAVAILABLE); - - let res: Response = ErrorGatewayTimeout("err").into(); - assert_eq!(res.status(), StatusCode::GATEWAY_TIMEOUT); - - let res: Response = ErrorHttpVersionNotSupported("err").into(); - assert_eq!(res.status(), StatusCode::HTTP_VERSION_NOT_SUPPORTED); - - let res: Response = ErrorVariantAlsoNegotiates("err").into(); - assert_eq!(res.status(), StatusCode::VARIANT_ALSO_NEGOTIATES); - - let res: Response = ErrorInsufficientStorage("err").into(); - assert_eq!(res.status(), StatusCode::INSUFFICIENT_STORAGE); - - let res: Response = ErrorLoopDetected("err").into(); - assert_eq!(res.status(), StatusCode::LOOP_DETECTED); - - let res: Response = ErrorNotExtended("err").into(); - assert_eq!(res.status(), StatusCode::NOT_EXTENDED); - - let res: Response = ErrorNetworkAuthenticationRequired("err").into(); - assert_eq!(res.status(), StatusCode::NETWORK_AUTHENTICATION_REQUIRED); - } } diff --git a/actix-http/src/helpers.rs b/actix-http/src/helpers.rs index b00ca307e..34bb989f9 100644 --- a/actix-http/src/helpers.rs +++ b/actix-http/src/helpers.rs @@ -41,7 +41,8 @@ pub fn write_content_length(n: u64, buf: &mut B) { buf.put_slice(b"\r\n"); } -// TODO: bench why this is needed +// TODO: bench why this is needed vs Buf::writer +/// An `io` writer for a `BufMut` that should only be used once and on an empty buffer. pub(crate) struct Writer<'a, B>(pub &'a mut B); impl<'a, B> io::Write for Writer<'a, B> diff --git a/actix-http/tests/test_client.rs b/actix-http/tests/test_client.rs index 0a06d90e5..4bd7dbe14 100644 --- a/actix-http/tests/test_client.rs +++ b/actix-http/tests/test_client.rs @@ -1,10 +1,11 @@ use actix_http::{ - error, http, http::StatusCode, HttpMessage, HttpService, Request, Response, + http, http::StatusCode, HttpMessage, HttpService, Request, Response, ResponseError, }; use actix_http_test::test_server; use actix_service::ServiceFactoryExt; use actix_utils::future; use bytes::Bytes; +use derive_more::{Display, Error}; use futures_util::StreamExt as _; const STR: &str = "Hello World Hello World Hello World Hello World Hello World \ @@ -92,6 +93,16 @@ async fn test_with_query_parameter() { assert!(response.status().is_success()); } +#[derive(Debug, Display, Error)] +#[display(fmt = "expect failed")] +struct ExpectFailed; + +impl ResponseError for ExpectFailed { + fn status_code(&self) -> StatusCode { + StatusCode::EXPECTATION_FAILED + } +} + #[actix_rt::test] async fn test_h1_expect() { let srv = test_server(move || { @@ -100,7 +111,7 @@ async fn test_h1_expect() { if req.headers().contains_key("AUTH") { Ok(req) } else { - Err(error::ErrorExpectationFailed("expect failed")) + Err(ExpectFailed) } }) .h1(|req: Request| async move { @@ -134,7 +145,7 @@ async fn test_h1_expect() { let response = request.send_body("expect body").await.unwrap(); assert_eq!(response.status(), StatusCode::EXPECTATION_FAILED); - // test exepct would continue + // test expect would continue let request = srv .request(http::Method::GET, srv.url("/")) .insert_header(("Expect", "100-continue")) diff --git a/actix-http/tests/test_openssl.rs b/actix-http/tests/test_openssl.rs index 7cbd58518..d3a3bea3b 100644 --- a/actix-http/tests/test_openssl.rs +++ b/actix-http/tests/test_openssl.rs @@ -6,17 +6,18 @@ use std::io; use actix_http::{ body::{Body, SizedStream}, - error::{ErrorBadRequest, PayloadError}, + error::PayloadError, http::{ header::{self, HeaderName, HeaderValue}, Method, StatusCode, Version, }, - Error, HttpMessage, HttpService, Request, Response, + Error, HttpMessage, HttpService, Request, Response, ResponseError, }; use actix_http_test::test_server; use actix_service::{fn_service, ServiceFactoryExt}; use actix_utils::future::{err, ok, ready}; use bytes::{Bytes, BytesMut}; +use derive_more::{Display, Error}; use futures_core::Stream; use futures_util::stream::{once, StreamExt as _}; use openssl::{ @@ -401,11 +402,21 @@ async fn test_h2_response_http_error_handling() { assert_eq!(bytes, Bytes::from_static(b"failed to parse header value")); } +#[derive(Debug, Display, Error)] +#[display(fmt = "error")] +struct BadRequest; + +impl ResponseError for BadRequest { + fn status_code(&self) -> StatusCode { + StatusCode::BAD_REQUEST + } +} + #[actix_rt::test] async fn test_h2_service_error() { let mut srv = test_server(move || { HttpService::build() - .h2(|_| err::, Error>(ErrorBadRequest("error"))) + .h2(|_| err::, _>(BadRequest)) .openssl(tls_config()) .map_err(|_| ()) }) diff --git a/actix-http/tests/test_rustls.rs b/actix-http/tests/test_rustls.rs index a122ab847..2382d1ad3 100644 --- a/actix-http/tests/test_rustls.rs +++ b/actix-http/tests/test_rustls.rs @@ -4,18 +4,18 @@ extern crate tls_rustls as rustls; use actix_http::{ body::{Body, SizedStream}, - error::{self, PayloadError}, + error::PayloadError, http::{ header::{self, HeaderName, HeaderValue}, Method, StatusCode, Version, }, - Error, HttpService, Request, Response, + Error, HttpService, Request, Response, ResponseError, }; use actix_http_test::test_server; use actix_service::{fn_factory_with_config, fn_service}; use actix_utils::future::{err, ok}; - use bytes::{Bytes, BytesMut}; +use derive_more::{Display, Error}; use futures_core::Stream; use futures_util::stream::{once, StreamExt as _}; use rustls::{ @@ -417,11 +417,21 @@ async fn test_h2_response_http_error_handling() { assert_eq!(bytes, Bytes::from_static(b"failed to parse header value")); } +#[derive(Debug, Display, Error)] +#[display(fmt = "error")] +struct BadRequest; + +impl ResponseError for BadRequest { + fn status_code(&self) -> StatusCode { + StatusCode::BAD_REQUEST + } +} + #[actix_rt::test] async fn test_h2_service_error() { let mut srv = test_server(move || { HttpService::build() - .h2(|_| err::, Error>(error::ErrorBadRequest("error"))) + .h2(|_| err::, _>(BadRequest)) .rustls(tls_config()) }) .await; @@ -438,7 +448,7 @@ async fn test_h2_service_error() { async fn test_h1_service_error() { let mut srv = test_server(move || { HttpService::build() - .h1(|_| err::, Error>(error::ErrorBadRequest("error"))) + .h1(|_| err::, _>(BadRequest)) .rustls(tls_config()) }) .await; diff --git a/actix-http/tests/test_server.rs b/actix-http/tests/test_server.rs index 9b8b039c3..abfda249c 100644 --- a/actix-http/tests/test_server.rs +++ b/actix-http/tests/test_server.rs @@ -2,23 +2,22 @@ use std::io::{Read, Write}; use std::time::Duration; use std::{net, thread}; +use actix_http::{ + body::{Body, SizedStream}, + http::{self, header, StatusCode}, + Error, HttpService, KeepAlive, Request, Response, +}; +use actix_http::{HttpMessage, ResponseError}; use actix_http_test::test_server; use actix_rt::time::sleep; use actix_service::fn_service; use actix_utils::future::{err, ok, ready}; use bytes::Bytes; +use derive_more::{Display, Error}; use futures_util::stream::{once, StreamExt as _}; use futures_util::FutureExt as _; use regex::Regex; -use actix_http::HttpMessage; -use actix_http::{ - body::{Body, SizedStream}, - error, - http::{self, header, StatusCode}, - Error, HttpService, KeepAlive, Request, Response, -}; - #[actix_rt::test] async fn test_h1() { let srv = test_server(|| { @@ -58,6 +57,16 @@ async fn test_h1_2() { assert!(response.status().is_success()); } +#[derive(Debug, Display, Error)] +#[display(fmt = "expect failed")] +struct ExpectFailed; + +impl ResponseError for ExpectFailed { + fn status_code(&self) -> StatusCode { + StatusCode::PRECONDITION_FAILED + } +} + #[actix_rt::test] async fn test_expect_continue() { let srv = test_server(|| { @@ -66,7 +75,7 @@ async fn test_expect_continue() { if req.head().uri.query() == Some("yes=") { ok(req) } else { - err(error::ErrorPreconditionFailed("error")) + err(ExpectFailed) } })) .finish(|_| ok::<_, ()>(Response::ok())) @@ -96,7 +105,7 @@ async fn test_expect_continue_h1() { if req.head().uri.query() == Some("yes=") { ok(req) } else { - err(error::ErrorPreconditionFailed("error")) + err(ExpectFailed) } }) })) @@ -647,11 +656,21 @@ async fn test_h1_response_http_error_handling() { assert_eq!(bytes, Bytes::from_static(b"failed to parse header value")); } +#[derive(Debug, Display, Error)] +#[display(fmt = "error")] +struct BadRequest; + +impl ResponseError for BadRequest { + fn status_code(&self) -> StatusCode { + StatusCode::BAD_REQUEST + } +} + #[actix_rt::test] async fn test_h1_service_error() { let mut srv = test_server(|| { HttpService::build() - .h1(|_| err::, _>(error::ErrorBadRequest("error"))) + .h1(|_| err::, _>(BadRequest)) .tcp() }) .await; diff --git a/src/data.rs b/src/data.rs index bd9b88301..d63c15580 100644 --- a/src/data.rs +++ b/src/data.rs @@ -1,16 +1,13 @@ -use std::any::type_name; -use std::ops::Deref; -use std::sync::Arc; +use std::{any::type_name, ops::Deref, sync::Arc}; -use actix_http::error::{Error, ErrorInternalServerError}; -use actix_http::Extensions; +use actix_http::{error::Error, Extensions}; use actix_utils::future::{err, ok, Ready}; use futures_core::future::LocalBoxFuture; use serde::Serialize; -use crate::dev::Payload; -use crate::extract::FromRequest; -use crate::request::HttpRequest; +use crate::{ + dev::Payload, error::ErrorInternalServerError, extract::FromRequest, request::HttpRequest, +}; /// Data factory. pub(crate) trait DataFactory { diff --git a/src/error/internal.rs b/src/error/internal.rs new file mode 100644 index 000000000..23b7dc31e --- /dev/null +++ b/src/error/internal.rs @@ -0,0 +1,304 @@ +use std::{cell::RefCell, fmt, io::Write as _}; + +use actix_http::{body::Body, header, Response, StatusCode}; +use bytes::{BufMut as _, BytesMut}; + +use crate::{Error, HttpResponse, ResponseError}; + +/// Wraps errors to alter the generated response status code. +/// +/// In following example, the `io::Error` is wrapped into `ErrorBadRequest` which will generate a +/// response with the 400 Bad Request status code instead of the usual status code generated by +/// an `io::Error`. +/// +/// # Examples +/// ``` +/// # use std::io; +/// # use actix_web::{error, HttpRequest}; +/// async fn handler_error() -> Result { +/// let err = io::Error::new(io::ErrorKind::Other, "error"); +/// Err(error::ErrorBadRequest(err)) +/// } +/// ``` +pub struct InternalError { + cause: T, + status: InternalErrorType, +} + +enum InternalErrorType { + Status(StatusCode), + Response(RefCell>), +} + +impl InternalError { + /// Constructs an `InternalError` with given status code. + pub fn new(cause: T, status: StatusCode) -> Self { + InternalError { + cause, + status: InternalErrorType::Status(status), + } + } + + /// Constructs an `InternalError` with pre-defined response. + pub fn from_response(cause: T, response: HttpResponse) -> Self { + InternalError { + cause, + status: InternalErrorType::Response(RefCell::new(Some(response))), + } + } +} + +impl fmt::Debug for InternalError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.cause.fmt(f) + } +} + +impl fmt::Display for InternalError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.cause.fmt(f) + } +} + +impl ResponseError for InternalError +where + T: fmt::Debug + fmt::Display, +{ + fn status_code(&self) -> StatusCode { + match self.status { + InternalErrorType::Status(st) => st, + InternalErrorType::Response(ref resp) => { + if let Some(resp) = resp.borrow().as_ref() { + resp.head().status + } else { + StatusCode::INTERNAL_SERVER_ERROR + } + } + } + } + + fn error_response(&self) -> Response { + match self.status { + InternalErrorType::Status(status) => { + let mut res = Response::new(status); + let mut buf = BytesMut::new().writer(); + let _ = write!(buf, "{}", self); + + res.headers_mut().insert( + header::CONTENT_TYPE, + header::HeaderValue::from_static("text/plain; charset=utf-8"), + ); + res.set_body(Body::from(buf.into_inner())).into() + } + + InternalErrorType::Response(ref resp) => { + if let Some(resp) = resp.borrow_mut().take() { + resp.into() + } else { + Response::new(StatusCode::INTERNAL_SERVER_ERROR) + } + } + } + } +} + +macro_rules! error_helper { + ($name:ident, $status:ident) => { + paste::paste! { + #[doc = "Helper function that wraps any error and generates a `" $status "` response."] + #[allow(non_snake_case)] + pub fn $name(err: T) -> Error + where + T: fmt::Debug + fmt::Display + 'static, + { + InternalError::new(err, StatusCode::$status).into() + } + } + } +} + +error_helper!(ErrorBadRequest, BAD_REQUEST); +error_helper!(ErrorUnauthorized, UNAUTHORIZED); +error_helper!(ErrorPaymentRequired, PAYMENT_REQUIRED); +error_helper!(ErrorForbidden, FORBIDDEN); +error_helper!(ErrorNotFound, NOT_FOUND); +error_helper!(ErrorMethodNotAllowed, METHOD_NOT_ALLOWED); +error_helper!(ErrorNotAcceptable, NOT_ACCEPTABLE); +error_helper!( + ErrorProxyAuthenticationRequired, + PROXY_AUTHENTICATION_REQUIRED +); +error_helper!(ErrorRequestTimeout, REQUEST_TIMEOUT); +error_helper!(ErrorConflict, CONFLICT); +error_helper!(ErrorGone, GONE); +error_helper!(ErrorLengthRequired, LENGTH_REQUIRED); +error_helper!(ErrorPayloadTooLarge, PAYLOAD_TOO_LARGE); +error_helper!(ErrorUriTooLong, URI_TOO_LONG); +error_helper!(ErrorUnsupportedMediaType, UNSUPPORTED_MEDIA_TYPE); +error_helper!(ErrorRangeNotSatisfiable, RANGE_NOT_SATISFIABLE); +error_helper!(ErrorImATeapot, IM_A_TEAPOT); +error_helper!(ErrorMisdirectedRequest, MISDIRECTED_REQUEST); +error_helper!(ErrorUnprocessableEntity, UNPROCESSABLE_ENTITY); +error_helper!(ErrorLocked, LOCKED); +error_helper!(ErrorFailedDependency, FAILED_DEPENDENCY); +error_helper!(ErrorUpgradeRequired, UPGRADE_REQUIRED); +error_helper!(ErrorPreconditionFailed, PRECONDITION_FAILED); +error_helper!(ErrorPreconditionRequired, PRECONDITION_REQUIRED); +error_helper!(ErrorTooManyRequests, TOO_MANY_REQUESTS); +error_helper!( + ErrorRequestHeaderFieldsTooLarge, + REQUEST_HEADER_FIELDS_TOO_LARGE +); +error_helper!( + ErrorUnavailableForLegalReasons, + UNAVAILABLE_FOR_LEGAL_REASONS +); +error_helper!(ErrorExpectationFailed, EXPECTATION_FAILED); +error_helper!(ErrorInternalServerError, INTERNAL_SERVER_ERROR); +error_helper!(ErrorNotImplemented, NOT_IMPLEMENTED); +error_helper!(ErrorBadGateway, BAD_GATEWAY); +error_helper!(ErrorServiceUnavailable, SERVICE_UNAVAILABLE); +error_helper!(ErrorGatewayTimeout, GATEWAY_TIMEOUT); +error_helper!(ErrorHttpVersionNotSupported, HTTP_VERSION_NOT_SUPPORTED); +error_helper!(ErrorVariantAlsoNegotiates, VARIANT_ALSO_NEGOTIATES); +error_helper!(ErrorInsufficientStorage, INSUFFICIENT_STORAGE); +error_helper!(ErrorLoopDetected, LOOP_DETECTED); +error_helper!(ErrorNotExtended, NOT_EXTENDED); +error_helper!( + ErrorNetworkAuthenticationRequired, + NETWORK_AUTHENTICATION_REQUIRED +); + +#[cfg(test)] +mod tests { + use actix_http::{error::ParseError, Response}; + + use super::*; + + #[test] + fn test_internal_error() { + let err = InternalError::from_response(ParseError::Method, HttpResponse::Ok().finish()); + let resp: Response = err.error_response(); + assert_eq!(resp.status(), StatusCode::OK); + } + + #[test] + fn test_error_helpers() { + let res: Response = ErrorBadRequest("err").into(); + assert_eq!(res.status(), StatusCode::BAD_REQUEST); + + let res: Response = ErrorUnauthorized("err").into(); + assert_eq!(res.status(), StatusCode::UNAUTHORIZED); + + let res: Response = ErrorPaymentRequired("err").into(); + assert_eq!(res.status(), StatusCode::PAYMENT_REQUIRED); + + let res: Response = ErrorForbidden("err").into(); + assert_eq!(res.status(), StatusCode::FORBIDDEN); + + let res: Response = ErrorNotFound("err").into(); + assert_eq!(res.status(), StatusCode::NOT_FOUND); + + let res: Response = ErrorMethodNotAllowed("err").into(); + assert_eq!(res.status(), StatusCode::METHOD_NOT_ALLOWED); + + let res: Response = ErrorNotAcceptable("err").into(); + assert_eq!(res.status(), StatusCode::NOT_ACCEPTABLE); + + let res: Response = ErrorProxyAuthenticationRequired("err").into(); + assert_eq!(res.status(), StatusCode::PROXY_AUTHENTICATION_REQUIRED); + + let res: Response = ErrorRequestTimeout("err").into(); + assert_eq!(res.status(), StatusCode::REQUEST_TIMEOUT); + + let res: Response = ErrorConflict("err").into(); + assert_eq!(res.status(), StatusCode::CONFLICT); + + let res: Response = ErrorGone("err").into(); + assert_eq!(res.status(), StatusCode::GONE); + + let res: Response = ErrorLengthRequired("err").into(); + assert_eq!(res.status(), StatusCode::LENGTH_REQUIRED); + + let res: Response = ErrorPreconditionFailed("err").into(); + assert_eq!(res.status(), StatusCode::PRECONDITION_FAILED); + + let res: Response = ErrorPayloadTooLarge("err").into(); + assert_eq!(res.status(), StatusCode::PAYLOAD_TOO_LARGE); + + let res: Response = ErrorUriTooLong("err").into(); + assert_eq!(res.status(), StatusCode::URI_TOO_LONG); + + let res: Response = ErrorUnsupportedMediaType("err").into(); + assert_eq!(res.status(), StatusCode::UNSUPPORTED_MEDIA_TYPE); + + let res: Response = ErrorRangeNotSatisfiable("err").into(); + assert_eq!(res.status(), StatusCode::RANGE_NOT_SATISFIABLE); + + let res: Response = ErrorExpectationFailed("err").into(); + assert_eq!(res.status(), StatusCode::EXPECTATION_FAILED); + + let res: Response = ErrorImATeapot("err").into(); + assert_eq!(res.status(), StatusCode::IM_A_TEAPOT); + + let res: Response = ErrorMisdirectedRequest("err").into(); + assert_eq!(res.status(), StatusCode::MISDIRECTED_REQUEST); + + let res: Response = ErrorUnprocessableEntity("err").into(); + assert_eq!(res.status(), StatusCode::UNPROCESSABLE_ENTITY); + + let res: Response = ErrorLocked("err").into(); + assert_eq!(res.status(), StatusCode::LOCKED); + + let res: Response = ErrorFailedDependency("err").into(); + assert_eq!(res.status(), StatusCode::FAILED_DEPENDENCY); + + let res: Response = ErrorUpgradeRequired("err").into(); + assert_eq!(res.status(), StatusCode::UPGRADE_REQUIRED); + + let res: Response = ErrorPreconditionRequired("err").into(); + assert_eq!(res.status(), StatusCode::PRECONDITION_REQUIRED); + + let res: Response = ErrorTooManyRequests("err").into(); + assert_eq!(res.status(), StatusCode::TOO_MANY_REQUESTS); + + let res: Response = ErrorRequestHeaderFieldsTooLarge("err").into(); + assert_eq!(res.status(), StatusCode::REQUEST_HEADER_FIELDS_TOO_LARGE); + + let res: Response = ErrorUnavailableForLegalReasons("err").into(); + assert_eq!(res.status(), StatusCode::UNAVAILABLE_FOR_LEGAL_REASONS); + + let res: Response = ErrorInternalServerError("err").into(); + assert_eq!(res.status(), StatusCode::INTERNAL_SERVER_ERROR); + + let res: Response = ErrorNotImplemented("err").into(); + assert_eq!(res.status(), StatusCode::NOT_IMPLEMENTED); + + let res: Response = ErrorBadGateway("err").into(); + assert_eq!(res.status(), StatusCode::BAD_GATEWAY); + + let res: Response = ErrorServiceUnavailable("err").into(); + assert_eq!(res.status(), StatusCode::SERVICE_UNAVAILABLE); + + let res: Response = ErrorGatewayTimeout("err").into(); + assert_eq!(res.status(), StatusCode::GATEWAY_TIMEOUT); + + let res: Response = ErrorHttpVersionNotSupported("err").into(); + assert_eq!(res.status(), StatusCode::HTTP_VERSION_NOT_SUPPORTED); + + let res: Response = ErrorVariantAlsoNegotiates("err").into(); + assert_eq!(res.status(), StatusCode::VARIANT_ALSO_NEGOTIATES); + + let res: Response = ErrorInsufficientStorage("err").into(); + assert_eq!(res.status(), StatusCode::INSUFFICIENT_STORAGE); + + let res: Response = ErrorLoopDetected("err").into(); + assert_eq!(res.status(), StatusCode::LOOP_DETECTED); + + let res: Response = ErrorNotExtended("err").into(); + assert_eq!(res.status(), StatusCode::NOT_EXTENDED); + + let res: Response = ErrorNetworkAuthenticationRequired("err").into(); + assert_eq!(res.status(), StatusCode::NETWORK_AUTHENTICATION_REQUIRED); + } +} diff --git a/src/error.rs b/src/error/mod.rs similarity index 99% rename from src/error.rs rename to src/error/mod.rs index a5a245693..7be9f501b 100644 --- a/src/error.rs +++ b/src/error/mod.rs @@ -9,6 +9,10 @@ use url::ParseError as UrlParseError; use crate::http::StatusCode; +mod internal; + +pub use self::internal::*; + /// A convenience [`Result`](std::result::Result) for Actix Web operations. /// /// This type alias is generally used to avoid writing out `actix_http::Error` directly. diff --git a/src/request_data.rs b/src/request_data.rs index 60471cbf9..559d6ecbf 100644 --- a/src/request_data.rs +++ b/src/request_data.rs @@ -1,9 +1,9 @@ use std::{any::type_name, ops::Deref}; -use actix_http::error::{Error, ErrorInternalServerError}; +use actix_http::error::Error; use actix_utils::future::{err, ok, Ready}; -use crate::{dev::Payload, FromRequest, HttpRequest}; +use crate::{dev::Payload, error::ErrorInternalServerError, FromRequest, HttpRequest}; /// Request-local data extractor. /// diff --git a/src/responder.rs b/src/responder.rs index 2393d046b..8bf8d9ea0 100644 --- a/src/responder.rs +++ b/src/responder.rs @@ -2,12 +2,11 @@ use std::{borrow::Cow, fmt}; use actix_http::{ body::Body, - error::InternalError, http::{header::IntoHeaderPair, Error as HttpError, HeaderMap, StatusCode}, }; use bytes::{Bytes, BytesMut}; -use crate::{Error, HttpRequest, HttpResponse, HttpResponseBuilder}; +use crate::{error::InternalError, Error, HttpRequest, HttpResponse, HttpResponseBuilder}; /// Trait implemented by types that can be converted to an HTTP response. /// diff --git a/src/types/path.rs b/src/types/path.rs index 50e2cb510..59a107a7e 100644 --- a/src/types/path.rs +++ b/src/types/path.rs @@ -2,12 +2,16 @@ use std::{fmt, ops, sync::Arc}; -use actix_http::error::{Error, ErrorNotFound}; +use actix_http::error::Error; use actix_router::PathDeserializer; use actix_utils::future::{ready, Ready}; use serde::de; -use crate::{dev::Payload, error::PathError, FromRequest, HttpRequest}; +use crate::{ + dev::Payload, + error::{ErrorNotFound, PathError}, + FromRequest, HttpRequest, +}; /// Extract typed data from request path segments. /// diff --git a/src/types/payload.rs b/src/types/payload.rs index f88800855..d69e0a126 100644 --- a/src/types/payload.rs +++ b/src/types/payload.rs @@ -7,14 +7,17 @@ use std::{ task::{Context, Poll}, }; -use actix_http::error::{ErrorBadRequest, PayloadError}; +use actix_http::error::PayloadError; use actix_utils::future::{ready, Either, Ready}; use bytes::{Bytes, BytesMut}; use encoding_rs::{Encoding, UTF_8}; use futures_core::{ready, stream::Stream}; use mime::Mime; -use crate::{dev, http::header, web, Error, FromRequest, HttpMessage, HttpRequest}; +use crate::{ + dev, error::ErrorBadRequest, http::header, web, Error, FromRequest, HttpMessage, + HttpRequest, +}; /// Extract a request's raw payload stream. ///