diff --git a/CHANGES.md b/CHANGES.md index f05137ab9..607e9d4f5 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -18,6 +18,10 @@ * Move multipart support to actix-multipart crate +### Fixed + +* Fix body propagation in Response::from_error. #760 + ## [1.0.0-alpha.3] - 2019-04-02 diff --git a/Cargo.toml b/Cargo.toml index 0c4d31374..609b2ff3f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,6 +29,7 @@ members = [ "awc", "actix-http", "actix-files", + "actix-framed", "actix-session", "actix-multipart", "actix-web-actors", diff --git a/actix-http/src/error.rs b/actix-http/src/error.rs index 6573c8ce6..92a046846 100644 --- a/actix-http/src/error.rs +++ b/actix-http/src/error.rs @@ -1,11 +1,13 @@ //! Error and Result module use std::cell::RefCell; +use std::io::Write; use std::str::Utf8Error; use std::string::FromUtf8Error; use std::{fmt, io, result}; pub use actix_threadpool::BlockingError; use actix_utils::timeout::TimeoutError; +use bytes::BytesMut; use derive_more::{Display, From}; use futures::Canceled; use http::uri::InvalidUri; @@ -17,7 +19,9 @@ use serde_urlencoded::ser::Error as FormError; use tokio_timer::Error as TimerError; // re-export for convinience +use crate::body::Body; pub use crate::cookie::ParseError as CookieParseError; +use crate::helpers::Writer; use crate::response::Response; /// A specialized [`Result`](https://doc.rust-lang.org/std/result/enum.Result.html) @@ -57,6 +61,18 @@ pub trait ResponseError: fmt::Debug + fmt::Display { fn error_response(&self) -> Response { Response::new(StatusCode::INTERNAL_SERVER_ERROR) } + + /// Constructs an error response + fn render_response(&self) -> Response { + let mut resp = self.error_response(); + let mut buf = BytesMut::new(); + let _ = write!(Writer(&mut buf), "{}", self); + resp.headers_mut().insert( + header::CONTENT_TYPE, + header::HeaderValue::from_static("text/plain"), + ); + resp.set_body(Body::from(buf)) + } } impl fmt::Display for Error { @@ -477,7 +493,16 @@ where { fn error_response(&self) -> Response { match self.status { - InternalErrorType::Status(st) => Response::new(st), + 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"), + ); + res.set_body(Body::from(buf)) + } InternalErrorType::Response(ref resp) => { if let Some(resp) = resp.borrow_mut().take() { resp @@ -487,6 +512,11 @@ where } } } + + /// Constructs an error response + fn render_response(&self) -> Response { + self.error_response() + } } /// Convert Response to a Error diff --git a/actix-http/src/helpers.rs b/actix-http/src/helpers.rs index e4ccd8aef..e8dbcd82a 100644 --- a/actix-http/src/helpers.rs +++ b/actix-http/src/helpers.rs @@ -1,6 +1,7 @@ +use std::{io, mem, ptr, slice}; + use bytes::{BufMut, BytesMut}; use http::Version; -use std::{mem, ptr, slice}; const DEC_DIGITS_LUT: &[u8] = b"0001020304050607080910111213141516171819\ 2021222324252627282930313233343536373839\ @@ -167,6 +168,18 @@ pub(crate) fn convert_usize(mut n: usize, bytes: &mut BytesMut) { } } +pub(crate) struct Writer<'a>(pub &'a mut BytesMut); + +impl<'a> io::Write for Writer<'a> { + fn write(&mut self, buf: &[u8]) -> io::Result { + self.0.extend_from_slice(buf); + Ok(buf.len()) + } + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/actix-http/src/response.rs b/actix-http/src/response.rs index 95e4e789b..6125ae1cb 100644 --- a/actix-http/src/response.rs +++ b/actix-http/src/response.rs @@ -1,7 +1,7 @@ //! Http response use std::cell::{Ref, RefMut}; use std::io::Write; -use std::{fmt, io, str}; +use std::{fmt, str}; use bytes::{BufMut, Bytes, BytesMut}; use futures::future::{ok, FutureResult, IntoFuture}; @@ -51,13 +51,9 @@ impl Response { /// Constructs an error response #[inline] pub fn from_error(error: Error) -> Response { - let mut resp = error.as_response_error().error_response(); - let mut buf = BytesMut::new(); - let _ = write!(Writer(&mut buf), "{}", error); - resp.headers_mut() - .insert(header::CONTENT_TYPE, HeaderValue::from_static("text/plain")); + let mut resp = error.as_response_error().render_response(); resp.error = Some(error); - resp.set_body(Body::from(buf)) + resp } /// Convert response to response with body @@ -309,18 +305,6 @@ impl<'a> Iterator for CookieIter<'a> { } } -pub struct Writer<'a>(pub &'a mut BytesMut); - -impl<'a> io::Write for Writer<'a> { - fn write(&mut self, buf: &[u8]) -> io::Result { - self.0.extend_from_slice(buf); - Ok(buf.len()) - } - fn flush(&mut self) -> io::Result<()> { - Ok(()) - } -} - /// An HTTP response builder /// /// This type can be used to construct an instance of `Response` through a diff --git a/src/app.rs b/src/app.rs index 802569458..f378572b2 100644 --- a/src/app.rs +++ b/src/app.rs @@ -64,7 +64,7 @@ where InitError = (), >, { - /// Set application data. Applicatin data could be accessed + /// Set application data. Application data could be accessed /// by using `Data` extractor where `T` is data type. /// /// **Note**: http server accepts an application factory rather than diff --git a/src/data.rs b/src/data.rs index 502dd6be8..7fb382f82 100644 --- a/src/data.rs +++ b/src/data.rs @@ -25,7 +25,7 @@ pub(crate) trait DataFactoryResult { /// during application configuration process /// with `App::data()` method. /// -/// Applicatin data could be accessed by using `Data` +/// Application data could be accessed by using `Data` /// extractor where `T` is data type. /// /// **Note**: http server accepts an application factory rather than diff --git a/src/error.rs b/src/error.rs index 74b890f0a..e9e225f22 100644 --- a/src/error.rs +++ b/src/error.rs @@ -31,7 +31,7 @@ pub enum UrlencodedError { #[display(fmt = "Can not decode chunked transfer encoding")] Chunked, /// Payload size is bigger than allowed. (default: 256kB) - #[display(fmt = "Urlencoded payload size is bigger than allowed. (default: 256kB)")] + #[display(fmt = "Urlencoded payload size is bigger than allowed (default: 256kB)")] Overflow, /// Payload size is now known #[display(fmt = "Payload size is now known")] @@ -66,7 +66,7 @@ impl ResponseError for UrlencodedError { #[derive(Debug, Display, From)] pub enum JsonPayloadError { /// Payload size is bigger than allowed. (default: 32kB) - #[display(fmt = "Json payload size is bigger than allowed.")] + #[display(fmt = "Json payload size is bigger than allowed")] Overflow, /// Content type error #[display(fmt = "Content type error")] diff --git a/src/types/json.rs b/src/types/json.rs index f001ee1f1..912561510 100644 --- a/src/types/json.rs +++ b/src/types/json.rs @@ -365,8 +365,10 @@ mod tests { use serde_derive::{Deserialize, Serialize}; use super::*; + use crate::error::InternalError; use crate::http::header; use crate::test::{block_on, TestRequest}; + use crate::HttpResponse; #[derive(Serialize, Deserialize, PartialEq, Debug)] struct MyObject { @@ -405,6 +407,37 @@ mod tests { assert_eq!(resp.body().bin_ref(), b"{\"name\":\"test\"}"); } + #[test] + fn test_custom_error_responder() { + let (req, mut pl) = TestRequest::default() + .header( + header::CONTENT_TYPE, + header::HeaderValue::from_static("application/json"), + ) + .header( + header::CONTENT_LENGTH, + header::HeaderValue::from_static("16"), + ) + .set_payload(Bytes::from_static(b"{\"name\": \"test\"}")) + .route_data(JsonConfig::default().limit(10).error_handler(|err, _| { + let msg = MyObject { + name: "invalid request".to_string(), + }; + let resp = HttpResponse::BadRequest() + .body(serde_json::to_string(&msg).unwrap()); + InternalError::from_response(err, resp).into() + })) + .to_http_parts(); + + let s = block_on(Json::::from_request(&req, &mut pl)); + let mut resp = Response::from_error(s.err().unwrap().into()); + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + + let body = block_on(resp.take_body().concat2()).unwrap(); + let msg: MyObject = serde_json::from_slice(&body).unwrap(); + assert_eq!(msg.name, "invalid request"); + } + #[test] fn test_extract() { let (req, mut pl) = TestRequest::default() @@ -443,7 +476,7 @@ mod tests { let s = block_on(Json::::from_request(&req, &mut pl)); assert!(format!("{}", s.err().unwrap()) - .contains("Json payload size is bigger than allowed.")); + .contains("Json payload size is bigger than allowed")); let (req, mut pl) = TestRequest::default() .header( diff --git a/test-server/src/lib.rs b/test-server/src/lib.rs index 3f77f3786..d83432df9 100644 --- a/test-server/src/lib.rs +++ b/test-server/src/lib.rs @@ -107,7 +107,7 @@ impl TestServer { TestServerRuntime { addr, rt, client } } - /// Get firat available unused address + /// Get first available unused address pub fn unused_addr() -> net::SocketAddr { let addr: net::SocketAddr = "127.0.0.1:0".parse().unwrap(); let socket = TcpBuilder::new_v4().unwrap();