diff --git a/CHANGES.md b/CHANGES.md index 5f8f489fc..f37f8b466 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,4 +1,13 @@ # Changes +## not released yet + +### Added + +* Add `middleware::Conditon` that conditionally enables another middleware + +### Fixed + +* h2 will use error response #1080 ## [1.0.7] - 2019-08-29 diff --git a/actix-http/src/h2/dispatcher.rs b/actix-http/src/h2/dispatcher.rs index 69c620e62..888f9065e 100644 --- a/actix-http/src/h2/dispatcher.rs +++ b/actix-http/src/h2/dispatcher.rs @@ -257,8 +257,8 @@ where } } Ok(Async::NotReady) => Ok(Async::NotReady), - Err(_e) => { - let res: Response = Response::InternalServerError().finish(); + Err(e) => { + let res: Response = e.into().into(); let (res, body) = res.replace_body(()); let mut send = send.take().unwrap(); diff --git a/actix-http/tests/test_rustls_server.rs b/actix-http/tests/test_rustls_server.rs index 32b33fce8..b74fd07bf 100644 --- a/actix-http/tests/test_rustls_server.rs +++ b/actix-http/tests/test_rustls_server.rs @@ -454,9 +454,9 @@ fn test_h2_service_error() { }); let response = srv.block_on(srv.sget("/").send()).unwrap(); - assert_eq!(response.status(), http::StatusCode::INTERNAL_SERVER_ERROR); + assert_eq!(response.status(), http::StatusCode::BAD_REQUEST); // read response let bytes = srv.load_body(response).unwrap(); - assert!(bytes.is_empty()); + assert_eq!(bytes, Bytes::from_static(b"error")); } diff --git a/actix-http/tests/test_ssl_server.rs b/actix-http/tests/test_ssl_server.rs index f0c82870d..897d92b37 100644 --- a/actix-http/tests/test_ssl_server.rs +++ b/actix-http/tests/test_ssl_server.rs @@ -449,11 +449,11 @@ fn test_h2_service_error() { }); let response = srv.block_on(srv.sget("/").send()).unwrap(); - assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR); + assert_eq!(response.status(), StatusCode::BAD_REQUEST); // read response let bytes = srv.load_body(response).unwrap(); - assert!(bytes.is_empty()); + assert_eq!(bytes, Bytes::from_static(b"error")); } #[test] diff --git a/actix-multipart/CHANGES.md b/actix-multipart/CHANGES.md index 27333f4c4..365dca286 100644 --- a/actix-multipart/CHANGES.md +++ b/actix-multipart/CHANGES.md @@ -1,4 +1,7 @@ # Changes +## [0.1.4] - 2019-xx-xx + +* Multipart handling now parses requests which do not end in CRLF #1038 ## [0.1.3] - 2019-08-18 diff --git a/actix-multipart/src/server.rs b/actix-multipart/src/server.rs index e2111bb7b..3312a580a 100644 --- a/actix-multipart/src/server.rs +++ b/actix-multipart/src/server.rs @@ -167,7 +167,7 @@ impl InnerMultipart { boundary: &str, ) -> Result, MultipartError> { // TODO: need to read epilogue - match payload.readline()? { + match payload.readline_or_eof()? { None => { if payload.eof { Ok(Some(true)) @@ -176,16 +176,15 @@ impl InnerMultipart { } } Some(chunk) => { - if chunk.len() == boundary.len() + 4 - && &chunk[..2] == b"--" - && &chunk[2..boundary.len() + 2] == boundary.as_bytes() - { + if chunk.len() < boundary.len() + 4 + || &chunk[..2] != b"--" + || &chunk[2..boundary.len() + 2] != boundary.as_bytes() { + Err(MultipartError::Boundary) + } else if &chunk[boundary.len() + 2..] == b"\r\n" { Ok(Some(false)) - } else if chunk.len() == boundary.len() + 6 - && &chunk[..2] == b"--" - && &chunk[2..boundary.len() + 2] == boundary.as_bytes() - && &chunk[boundary.len() + 2..boundary.len() + 4] == b"--" - { + } else if &chunk[boundary.len() + 2..boundary.len() + 4] == b"--" + && (chunk.len() == boundary.len() + 4 + || &chunk[boundary.len() + 4..] == b"\r\n") { Ok(Some(true)) } else { Err(MultipartError::Boundary) @@ -779,6 +778,14 @@ impl PayloadBuffer { self.read_until(b"\n") } + /// Read bytes until new line delimiter or eof + pub fn readline_or_eof(&mut self) -> Result, MultipartError> { + match self.readline() { + Err(MultipartError::Incomplete) if self.eof => Ok(Some(self.buf.take().freeze())), + line => line + } + } + /// Put unprocessed data back to the buffer pub fn unprocessed(&mut self, data: Bytes) { let buf = BytesMut::from(data); @@ -849,32 +856,65 @@ mod tests { (tx, rx.map_err(|_| panic!()).and_then(|res| res)) } + fn create_simple_request_with_header() -> (Bytes, HeaderMap) { + let bytes = Bytes::from( + "testasdadsad\r\n\ + --abbc761f78ff4d7cb7573b5a23f96ef0\r\n\ + Content-Disposition: form-data; name=\"file\"; filename=\"fn.txt\"\r\n\ + Content-Type: text/plain; charset=utf-8\r\nContent-Length: 4\r\n\r\n\ + test\r\n\ + --abbc761f78ff4d7cb7573b5a23f96ef0\r\n\ + Content-Type: text/plain; charset=utf-8\r\nContent-Length: 4\r\n\r\n\ + data\r\n\ + --abbc761f78ff4d7cb7573b5a23f96ef0--\r\n" + ); + let mut headers = HeaderMap::new(); + headers.insert( + header::CONTENT_TYPE, + header::HeaderValue::from_static( + "multipart/mixed; boundary=\"abbc761f78ff4d7cb7573b5a23f96ef0\"", + ), + ); + (bytes, headers) + } + + #[test] + fn test_multipart_no_end_crlf() { + run_on(|| { + let (sender, payload) = create_stream(); + let (bytes, headers) = create_simple_request_with_header(); + let bytes_stripped = bytes.slice_to(bytes.len()); // strip crlf + + sender.unbounded_send(Ok(bytes_stripped)).unwrap(); + drop(sender); // eof + + let mut multipart = Multipart::new(&headers, payload); + + match multipart.poll().unwrap() { + Async::Ready(Some(_)) => (), + _ => unreachable!(), + } + + match multipart.poll().unwrap() { + Async::Ready(Some(_)) => (), + _ => unreachable!(), + } + + match multipart.poll().unwrap() { + Async::Ready(None) => (), + _ => unreachable!(), + } + }) + } + #[test] fn test_multipart() { run_on(|| { let (sender, payload) = create_stream(); + let (bytes, headers) = create_simple_request_with_header(); - let bytes = Bytes::from( - "testasdadsad\r\n\ - --abbc761f78ff4d7cb7573b5a23f96ef0\r\n\ - Content-Disposition: form-data; name=\"file\"; filename=\"fn.txt\"\r\n\ - Content-Type: text/plain; charset=utf-8\r\nContent-Length: 4\r\n\r\n\ - test\r\n\ - --abbc761f78ff4d7cb7573b5a23f96ef0\r\n\ - Content-Type: text/plain; charset=utf-8\r\nContent-Length: 4\r\n\r\n\ - data\r\n\ - --abbc761f78ff4d7cb7573b5a23f96ef0--\r\n", - ); sender.unbounded_send(Ok(bytes)).unwrap(); - let mut headers = HeaderMap::new(); - headers.insert( - header::CONTENT_TYPE, - header::HeaderValue::from_static( - "multipart/mixed; boundary=\"abbc761f78ff4d7cb7573b5a23f96ef0\"", - ), - ); - let mut multipart = Multipart::new(&headers, payload); match multipart.poll().unwrap() { Async::Ready(Some(mut field)) => { @@ -925,28 +965,10 @@ mod tests { fn test_stream() { run_on(|| { let (sender, payload) = create_stream(); + let (bytes, headers) = create_simple_request_with_header(); - let bytes = Bytes::from( - "testasdadsad\r\n\ - --abbc761f78ff4d7cb7573b5a23f96ef0\r\n\ - Content-Disposition: form-data; name=\"file\"; filename=\"fn.txt\"\r\n\ - Content-Type: text/plain; charset=utf-8\r\n\r\n\ - test\r\n\ - --abbc761f78ff4d7cb7573b5a23f96ef0\r\n\ - Content-Type: text/plain; charset=utf-8\r\n\r\n\ - data\r\n\ - --abbc761f78ff4d7cb7573b5a23f96ef0--\r\n", - ); sender.unbounded_send(Ok(bytes)).unwrap(); - let mut headers = HeaderMap::new(); - headers.insert( - header::CONTENT_TYPE, - header::HeaderValue::from_static( - "multipart/mixed; boundary=\"abbc761f78ff4d7cb7573b5a23f96ef0\"", - ), - ); - let mut multipart = Multipart::new(&headers, payload); match multipart.poll().unwrap() { Async::Ready(Some(mut field)) => { diff --git a/awc/CHANGES.md b/awc/CHANGES.md index 8c48c64c8..27962a6f5 100644 --- a/awc/CHANGES.md +++ b/awc/CHANGES.md @@ -7,6 +7,13 @@ * Add `FrozenClientRequest` to support retries for sending HTTP requests +## [0.2.5] - 2019-09-06 + +### Changed + +* Ensure that the `Host` header is set when initiating a WebSocket client connection. + + ## [0.2.4] - 2019-08-13 ### Changed diff --git a/awc/src/ws.rs b/awc/src/ws.rs index 72c9a38bc..67be9e9d8 100644 --- a/awc/src/ws.rs +++ b/awc/src/ws.rs @@ -233,6 +233,10 @@ impl WebsocketsRequest { return Either::A(err(InvalidUrl::UnknownScheme.into())); } + if !self.head.headers.contains_key(header::HOST) { + self.head.headers.insert(header::HOST, HeaderValue::from_str(uri.host().unwrap()).unwrap()); + } + // set cookies if let Some(ref mut jar) = self.cookies { let mut cookie = String::new(); diff --git a/src/middleware/condition.rs b/src/middleware/condition.rs new file mode 100644 index 000000000..ddc5fdd42 --- /dev/null +++ b/src/middleware/condition.rs @@ -0,0 +1,143 @@ +//! `Middleware` for conditionally enables another middleware. +use actix_service::{Service, Transform}; +use futures::future::{ok, Either, FutureResult, Map}; +use futures::{Future, Poll}; + +/// `Middleware` for conditionally enables another middleware. +/// The controled middleware must not change the `Service` interfaces. +/// This means you cannot control such middlewares like `Logger` or `Compress`. +/// +/// ## Usage +/// +/// ```rust +/// use actix_web::middleware::{Condition, NormalizePath}; +/// use actix_web::App; +/// +/// fn main() { +/// let enable_normalize = std::env::var("NORMALIZE_PATH") == Ok("true".into()); +/// let app = App::new() +/// .wrap(Condition::new(enable_normalize, NormalizePath)); +/// } +/// ``` +pub struct Condition { + trans: T, + enable: bool, +} + +impl Condition { + pub fn new(enable: bool, trans: T) -> Self { + Self { trans, enable } + } +} + +impl Transform for Condition +where + S: Service, + T: Transform, +{ + type Request = S::Request; + type Response = S::Response; + type Error = S::Error; + type InitError = T::InitError; + type Transform = ConditionMiddleware; + type Future = Either< + Map Self::Transform>, + FutureResult, + >; + + fn new_transform(&self, service: S) -> Self::Future { + if self.enable { + let f = self + .trans + .new_transform(service) + .map(ConditionMiddleware::Enable as fn(T::Transform) -> Self::Transform); + Either::A(f) + } else { + Either::B(ok(ConditionMiddleware::Disable(service))) + } + } +} + +pub enum ConditionMiddleware { + Enable(E), + Disable(D), +} + +impl Service for ConditionMiddleware +where + E: Service, + D: Service, +{ + type Request = E::Request; + type Response = E::Response; + type Error = E::Error; + type Future = Either; + + fn poll_ready(&mut self) -> Poll<(), Self::Error> { + use ConditionMiddleware::*; + match self { + Enable(service) => service.poll_ready(), + Disable(service) => service.poll_ready(), + } + } + + fn call(&mut self, req: E::Request) -> Self::Future { + use ConditionMiddleware::*; + match self { + Enable(service) => Either::A(service.call(req)), + Disable(service) => Either::B(service.call(req)), + } + } +} + +#[cfg(test)] +mod tests { + use actix_service::IntoService; + + use super::*; + use crate::dev::{ServiceRequest, ServiceResponse}; + use crate::error::Result; + use crate::http::{header::CONTENT_TYPE, HeaderValue, StatusCode}; + use crate::middleware::errhandlers::*; + use crate::test::{self, TestRequest}; + use crate::HttpResponse; + + fn render_500(mut res: ServiceResponse) -> Result> { + res.response_mut() + .headers_mut() + .insert(CONTENT_TYPE, HeaderValue::from_static("0001")); + Ok(ErrorHandlerResponse::Response(res)) + } + + #[test] + fn test_handler_enabled() { + let srv = |req: ServiceRequest| { + req.into_response(HttpResponse::InternalServerError().finish()) + }; + + let mw = + ErrorHandlers::new().handler(StatusCode::INTERNAL_SERVER_ERROR, render_500); + + let mut mw = + test::block_on(Condition::new(true, mw).new_transform(srv.into_service())) + .unwrap(); + let resp = test::call_service(&mut mw, TestRequest::default().to_srv_request()); + assert_eq!(resp.headers().get(CONTENT_TYPE).unwrap(), "0001"); + } + #[test] + fn test_handler_disabled() { + let srv = |req: ServiceRequest| { + req.into_response(HttpResponse::InternalServerError().finish()) + }; + + let mw = + ErrorHandlers::new().handler(StatusCode::INTERNAL_SERVER_ERROR, render_500); + + let mut mw = + test::block_on(Condition::new(false, mw).new_transform(srv.into_service())) + .unwrap(); + + let resp = test::call_service(&mut mw, TestRequest::default().to_srv_request()); + assert_eq!(resp.headers().get(CONTENT_TYPE), None); + } +} diff --git a/src/middleware/mod.rs b/src/middleware/mod.rs index 814993f0c..311d0ee99 100644 --- a/src/middleware/mod.rs +++ b/src/middleware/mod.rs @@ -6,7 +6,9 @@ mod defaultheaders; pub mod errhandlers; mod logger; mod normalize; +mod condition; pub use self::defaultheaders::DefaultHeaders; pub use self::logger::Logger; pub use self::normalize::NormalizePath; +pub use self::condition::Condition;