From 3d6ea7fe9b9027205ee88dd64c3d9c932240928e Mon Sep 17 00:00:00 2001 From: nerix Date: Sun, 26 Jun 2022 18:45:02 +0200 Subject: [PATCH 01/11] Improve documentation for `actix-web-actors` (#2788) --- actix-web-actors/Cargo.toml | 3 + actix-web-actors/src/context.rs | 56 +++++++++++++++- actix-web-actors/src/lib.rs | 55 ++++++++++++++++ actix-web-actors/src/ws.rs | 109 ++++++++++++++++++++++++++++---- 4 files changed, 210 insertions(+), 13 deletions(-) diff --git a/actix-web-actors/Cargo.toml b/actix-web-actors/Cargo.toml index c939f6ab5..284351ed3 100644 --- a/actix-web-actors/Cargo.toml +++ b/actix-web-actors/Cargo.toml @@ -29,6 +29,9 @@ tokio = { version = "1.13.1", features = ["sync"] } actix-rt = "2.2" actix-test = "0.1.0-beta.13" awc = { version = "3", default-features = false } +actix-web = { version = "4", features = ["macros"] } + +mime = "0.3" env_logger = "0.9" futures-util = { version = "0.3.7", default-features = false } diff --git a/actix-web-actors/src/context.rs b/actix-web-actors/src/context.rs index d83969ff7..f7b11c780 100644 --- a/actix-web-actors/src/context.rs +++ b/actix-web-actors/src/context.rs @@ -14,6 +14,58 @@ use futures_core::Stream; use tokio::sync::oneshot::Sender; /// Execution context for HTTP actors +/// +/// # Example +/// +/// A demonstration of [server-sent events](https://developer.mozilla.org/docs/Web/API/Server-sent_events) using actors: +/// +/// ```no_run +/// use std::time::Duration; +/// +/// use actix::{Actor, AsyncContext}; +/// use actix_web::{get, http::header, App, HttpResponse, HttpServer}; +/// use actix_web_actors::HttpContext; +/// use bytes::Bytes; +/// +/// struct MyActor { +/// count: usize, +/// } +/// +/// impl Actor for MyActor { +/// type Context = HttpContext; +/// +/// fn started(&mut self, ctx: &mut Self::Context) { +/// ctx.run_later(Duration::from_millis(100), Self::write); +/// } +/// } +/// +/// impl MyActor { +/// fn write(&mut self, ctx: &mut HttpContext) { +/// self.count += 1; +/// if self.count > 3 { +/// ctx.write_eof() +/// } else { +/// ctx.write(Bytes::from(format!("event: count\ndata: {}\n\n", self.count))); +/// ctx.run_later(Duration::from_millis(100), Self::write); +/// } +/// } +/// } +/// +/// #[get("/")] +/// async fn index() -> HttpResponse { +/// HttpResponse::Ok() +/// .insert_header(header::ContentType(mime::TEXT_EVENT_STREAM)) +/// .streaming(HttpContext::create(MyActor { count: 0 })) +/// } +/// +/// #[actix_web::main] +/// async fn main() -> std::io::Result<()> { +/// HttpServer::new(|| App::new().service(index)) +/// .bind(("127.0.0.1", 8080))? +/// .run() +/// .await +/// } +/// ``` pub struct HttpContext where A: Actor>, @@ -210,7 +262,7 @@ mod tests { type Context = HttpContext; fn started(&mut self, ctx: &mut Self::Context) { - ctx.run_later(Duration::from_millis(100), |slf, ctx| slf.write(ctx)); + ctx.run_later(Duration::from_millis(100), Self::write); } } @@ -221,7 +273,7 @@ mod tests { ctx.write_eof() } else { ctx.write(Bytes::from(format!("LINE-{}", self.count))); - ctx.run_later(Duration::from_millis(100), |slf, ctx| slf.write(ctx)); + ctx.run_later(Duration::from_millis(100), Self::write); } } } diff --git a/actix-web-actors/src/lib.rs b/actix-web-actors/src/lib.rs index 70c957020..106bc5202 100644 --- a/actix-web-actors/src/lib.rs +++ b/actix-web-actors/src/lib.rs @@ -1,4 +1,59 @@ //! Actix actors support for Actix Web. +//! +//! # Examples +//! +//! ```no_run +//! use actix::{Actor, StreamHandler}; +//! use actix_web::{get, web, App, Error, HttpRequest, HttpResponse, HttpServer}; +//! use actix_web_actors::ws; +//! +//! /// Define Websocket actor +//! struct MyWs; +//! +//! impl Actor for MyWs { +//! type Context = ws::WebsocketContext; +//! } +//! +//! /// Handler for ws::Message message +//! impl StreamHandler> for MyWs { +//! fn handle(&mut self, msg: Result, ctx: &mut Self::Context) { +//! match msg { +//! Ok(ws::Message::Ping(msg)) => ctx.pong(&msg), +//! Ok(ws::Message::Text(text)) => ctx.text(text), +//! Ok(ws::Message::Binary(bin)) => ctx.binary(bin), +//! _ => (), +//! } +//! } +//! } +//! +//! #[get("/ws")] +//! async fn index(req: HttpRequest, stream: web::Payload) -> Result { +//! ws::start(MyWs, &req, stream) +//! } +//! +//! #[actix_web::main] +//! async fn main() -> std::io::Result<()> { +//! HttpServer::new(|| App::new().service(index)) +//! .bind(("127.0.0.1", 8080))? +//! .run() +//! .await +//! } +//! ``` +//! +//! # Documentation & Community Resources +//! In addition to this API documentation, several other resources are available: +//! +//! * [Website & User Guide](https://actix.rs/) +//! * [Documentation for `actix_web`](actix_web) +//! * [Examples Repository](https://github.com/actix/examples) +//! * [Community Chat on Discord](https://discord.gg/NWpN5mmg3x) +//! +//! To get started navigating the API docs, you may consider looking at the following pages first: +//! +//! * [`ws`]: This module provides actor support for WebSockets. +//! +//! * [`HttpContext`]: This struct provides actor support for streaming HTTP responses. +//! #![deny(rust_2018_idioms, nonstandard_style)] #![warn(future_incompatible)] diff --git a/actix-web-actors/src/ws.rs b/actix-web-actors/src/ws.rs index 6fde10192..9a4880159 100644 --- a/actix-web-actors/src/ws.rs +++ b/actix-web-actors/src/ws.rs @@ -1,4 +1,60 @@ //! Websocket integration. +//! +//! # Examples +//! +//! ```no_run +//! use actix::{Actor, StreamHandler}; +//! use actix_web::{get, web, App, Error, HttpRequest, HttpResponse, HttpServer}; +//! use actix_web_actors::ws; +//! +//! /// Define Websocket actor +//! struct MyWs; +//! +//! impl Actor for MyWs { +//! type Context = ws::WebsocketContext; +//! } +//! +//! /// Handler for ws::Message message +//! impl StreamHandler> for MyWs { +//! fn handle(&mut self, msg: Result, ctx: &mut Self::Context) { +//! match msg { +//! Ok(ws::Message::Ping(msg)) => ctx.pong(&msg), +//! Ok(ws::Message::Text(text)) => ctx.text(text), +//! Ok(ws::Message::Binary(bin)) => ctx.binary(bin), +//! _ => (), +//! } +//! } +//! } +//! +//! #[get("/ws")] +//! async fn websocket(req: HttpRequest, stream: web::Payload) -> Result { +//! ws::start(MyWs, &req, stream) +//! } +//! +//! const MAX_FRAME_SIZE: usize = 16_384; // 16KiB +//! +//! #[get("/custom-ws")] +//! async fn custom_websocket(req: HttpRequest, stream: web::Payload) -> Result { +//! // Create a Websocket session with a specific max frame size, and protocols. +//! ws::WsResponseBuilder::new(MyWs, &req, stream) +//! .frame_size(MAX_FRAME_SIZE) +//! .protocols(&["A", "B"]) +//! .start() +//! } +//! +//! #[actix_web::main] +//! async fn main() -> std::io::Result<()> { +//! HttpServer::new(|| { +//! App::new() +//! .service(websocket) +//! .service(custom_websocket) +//! }) +//! .bind(("127.0.0.1", 8080))? +//! .run() +//! .await +//! } +//! ``` +//! use std::{ collections::VecDeque, @@ -41,20 +97,51 @@ use tokio::sync::oneshot; /// /// # Examples /// -/// Create a Websocket session response with default configuration. -/// ```ignore -/// WsResponseBuilder::new(WsActor, &req, stream).start() -/// ``` +/// ```no_run +/// # use actix::{Actor, StreamHandler}; +/// # use actix_web::{get, web, App, Error, HttpRequest, HttpResponse, HttpServer}; +/// # use actix_web_actors::ws; +/// # +/// # struct MyWs; +/// # +/// # impl Actor for MyWs { +/// # type Context = ws::WebsocketContext; +/// # } +/// # +/// # /// Handler for ws::Message message +/// # impl StreamHandler> for MyWs { +/// # fn handle(&mut self, msg: Result, ctx: &mut Self::Context) {} +/// # } +/// # +/// #[get("/ws")] +/// async fn websocket(req: HttpRequest, stream: web::Payload) -> Result { +/// ws::WsResponseBuilder::new(MyWs, &req, stream).start() +/// } /// -/// Create a Websocket session with a specific max frame size, [`Codec`], and protocols. -/// ```ignore /// const MAX_FRAME_SIZE: usize = 16_384; // 16KiB /// -/// ws::WsResponseBuilder::new(WsActor, &req, stream) -/// .codec(Codec::new()) -/// .protocols(&["A", "B"]) -/// .frame_size(MAX_FRAME_SIZE) -/// .start() +/// #[get("/custom-ws")] +/// async fn custom_websocket(req: HttpRequest, stream: web::Payload) -> Result { +/// // Create a Websocket session with a specific max frame size, codec, and protocols. +/// ws::WsResponseBuilder::new(MyWs, &req, stream) +/// .codec(actix_http::ws::Codec::new()) +/// // This will overwrite the codec's max frame-size +/// .frame_size(MAX_FRAME_SIZE) +/// .protocols(&["A", "B"]) +/// .start() +/// } +/// # +/// # #[actix_web::main] +/// # async fn main() -> std::io::Result<()> { +/// # HttpServer::new(|| { +/// # App::new() +/// # .service(websocket) +/// # .service(custom_websocket) +/// # }) +/// # .bind(("127.0.0.1", 8080))? +/// # .run() +/// # .await +/// # } /// ``` pub struct WsResponseBuilder<'a, A, T> where From f7d7d92984f0ed9204c7e69a8e16d0582a7efdcd Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Mon, 27 Jun 2022 03:12:36 +0100 Subject: [PATCH 02/11] address clippy lints --- actix-http/src/body/message_body.rs | 1 + actix-http/src/responses/head.rs | 4 ++-- actix-web/src/types/path.rs | 1 + actix-web/src/types/payload.rs | 4 ++-- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/actix-http/src/body/message_body.rs b/actix-http/src/body/message_body.rs index 9090e34d5..ab742b9cd 100644 --- a/actix-http/src/body/message_body.rs +++ b/actix-http/src/body/message_body.rs @@ -481,6 +481,7 @@ mod tests { assert_poll_next_none!(pl); } + #[allow(clippy::let_unit_value)] #[actix_rt::test] async fn test_unit() { let pl = (); diff --git a/actix-http/src/responses/head.rs b/actix-http/src/responses/head.rs index cb47c4b7a..01bca6c5b 100644 --- a/actix-http/src/responses/head.rs +++ b/actix-http/src/responses/head.rs @@ -237,7 +237,7 @@ mod tests { .await; let mut stream = net::TcpStream::connect(srv.addr()).unwrap(); - let _ = stream + stream .write_all(b"GET /camel HTTP/1.1\r\nConnection: Close\r\n\r\n") .unwrap(); let mut data = vec![]; @@ -251,7 +251,7 @@ mod tests { assert!(memmem::find(&data, b"content-length").is_none()); let mut stream = net::TcpStream::connect(srv.addr()).unwrap(); - let _ = stream + stream .write_all(b"GET /lower HTTP/1.1\r\nConnection: Close\r\n\r\n") .unwrap(); let mut data = vec![]; diff --git a/actix-web/src/types/path.rs b/actix-web/src/types/path.rs index 0fcac2c19..34c335ba9 100644 --- a/actix-web/src/types/path.rs +++ b/actix-web/src/types/path.rs @@ -183,6 +183,7 @@ mod tests { assert!(Path::::from_request(&req, &mut pl).await.is_err()); } + #[allow(clippy::let_unit_value)] #[actix_rt::test] async fn test_tuple_extract() { let resource = ResourceDef::new("/{key}/{value}/"); diff --git a/actix-web/src/types/payload.rs b/actix-web/src/types/payload.rs index b47a39e97..af195b867 100644 --- a/actix-web/src/types/payload.rs +++ b/actix-web/src/types/payload.rs @@ -113,7 +113,7 @@ pub struct BytesExtractFut { body_fut: HttpMessageBody, } -impl<'a> Future for BytesExtractFut { +impl Future for BytesExtractFut { type Output = Result; fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { @@ -167,7 +167,7 @@ pub struct StringExtractFut { encoding: &'static Encoding, } -impl<'a> Future for StringExtractFut { +impl Future for StringExtractFut { type Output = Result; fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { From 0dba6310c66c3a0fd5bc02b072fc1ec696814bbf Mon Sep 17 00:00:00 2001 From: Ulf Lilleengen Date: Mon, 27 Jun 2022 04:57:21 +0200 Subject: [PATCH 03/11] Expose option for setting TLS handshake timeout (#2752) Co-authored-by: Rob Ede --- actix-http/src/lib.rs | 3 ++ actix-http/src/service.rs | 65 +++++++++++++++++++++++++++++++- actix-http/tests/test_openssl.rs | 9 +++-- actix-http/tests/test_rustls.rs | 8 +++- actix-web/CHANGES.md | 7 +++- actix-web/src/server.rs | 39 ++++++++++++++++++- 6 files changed, 121 insertions(+), 10 deletions(-) diff --git a/actix-http/src/lib.rs b/actix-http/src/lib.rs index 360cb86fc..184049860 100644 --- a/actix-http/src/lib.rs +++ b/actix-http/src/lib.rs @@ -25,6 +25,7 @@ )] #![doc(html_logo_url = "https://actix.rs/img/logo.png")] #![doc(html_favicon_url = "https://actix.rs/favicon.ico")] +#![cfg_attr(docsrs, feature(doc_cfg))] pub use ::http::{uri, uri::Uri}; pub use ::http::{Method, StatusCode, Version}; @@ -69,6 +70,8 @@ pub use self::payload::{BoxedPayloadStream, Payload, PayloadStream}; pub use self::requests::{Request, RequestHead, RequestHeadType}; pub use self::responses::{Response, ResponseBuilder, ResponseHead}; pub use self::service::HttpService; +#[cfg(any(feature = "openssl", feature = "rustls"))] +pub use self::service::TlsAcceptorConfig; /// A major HTTP protocol version. #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] diff --git a/actix-http/src/service.rs b/actix-http/src/service.rs index f4fe625a3..27029cb8e 100644 --- a/actix-http/src/service.rs +++ b/actix-http/src/service.rs @@ -181,6 +181,25 @@ where } } +/// Configuration options used when accepting TLS connection. +#[cfg(any(feature = "openssl", feature = "rustls"))] +#[cfg_attr(docsrs, doc(cfg(any(feature = "openssl", feature = "rustls"))))] +#[derive(Debug, Default)] +pub struct TlsAcceptorConfig { + pub(crate) handshake_timeout: Option, +} + +#[cfg(any(feature = "openssl", feature = "rustls"))] +impl TlsAcceptorConfig { + /// Set TLS handshake timeout duration. + pub fn handshake_timeout(self, dur: std::time::Duration) -> Self { + Self { + handshake_timeout: Some(dur), + // ..self + } + } +} + #[cfg(feature = "openssl")] mod openssl { use actix_service::ServiceFactoryExt as _; @@ -230,7 +249,28 @@ mod openssl { Error = TlsError, InitError = (), > { - Acceptor::new(acceptor) + self.openssl_with_config(acceptor, TlsAcceptorConfig::default()) + } + + /// Create OpenSSL based service with custom TLS acceptor configuration. + pub fn openssl_with_config( + self, + acceptor: SslAcceptor, + tls_acceptor_config: TlsAcceptorConfig, + ) -> impl ServiceFactory< + TcpStream, + Config = (), + Response = (), + Error = TlsError, + InitError = (), + > { + let mut acceptor = Acceptor::new(acceptor); + + if let Some(handshake_timeout) = tls_acceptor_config.handshake_timeout { + acceptor.set_handshake_timeout(handshake_timeout); + } + + acceptor .map_init_err(|_| { unreachable!("TLS acceptor service factory does not error on init") }) @@ -293,8 +333,23 @@ mod rustls { { /// Create Rustls based service. pub fn rustls( + self, + config: ServerConfig, + ) -> impl ServiceFactory< + TcpStream, + Config = (), + Response = (), + Error = TlsError, + InitError = (), + > { + self.rustls_with_config(config, TlsAcceptorConfig::default()) + } + + /// Create Rustls based service with custom TLS acceptor configuration. + pub fn rustls_with_config( self, mut config: ServerConfig, + tls_acceptor_config: TlsAcceptorConfig, ) -> impl ServiceFactory< TcpStream, Config = (), @@ -306,7 +361,13 @@ mod rustls { protos.extend_from_slice(&config.alpn_protocols); config.alpn_protocols = protos; - Acceptor::new(config) + let mut acceptor = Acceptor::new(config); + + if let Some(handshake_timeout) = tls_acceptor_config.handshake_timeout { + acceptor.set_handshake_timeout(handshake_timeout); + } + + acceptor .map_init_err(|_| { unreachable!("TLS acceptor service factory does not error on init") }) diff --git a/actix-http/tests/test_openssl.rs b/actix-http/tests/test_openssl.rs index 35321ac98..b97b2e45b 100644 --- a/actix-http/tests/test_openssl.rs +++ b/actix-http/tests/test_openssl.rs @@ -2,13 +2,13 @@ extern crate tls_openssl as openssl; -use std::{convert::Infallible, io}; +use std::{convert::Infallible, io, time::Duration}; use actix_http::{ body::{BodyStream, BoxBody, SizedStream}, error::PayloadError, header::{self, HeaderValue}, - Error, HttpService, Method, Request, Response, StatusCode, Version, + Error, HttpService, Method, Request, Response, StatusCode, TlsAcceptorConfig, Version, }; use actix_http_test::test_server; use actix_service::{fn_service, ServiceFactoryExt}; @@ -89,7 +89,10 @@ async fn h2_1() -> io::Result<()> { assert_eq!(req.version(), Version::HTTP_2); ok::<_, Error>(Response::ok()) }) - .openssl(tls_config()) + .openssl_with_config( + tls_config(), + TlsAcceptorConfig::default().handshake_timeout(Duration::from_secs(5)), + ) .map_err(|_| ()) }) .await; diff --git a/actix-http/tests/test_rustls.rs b/actix-http/tests/test_rustls.rs index 550375296..2bbf1524b 100644 --- a/actix-http/tests/test_rustls.rs +++ b/actix-http/tests/test_rustls.rs @@ -8,13 +8,14 @@ use std::{ net::{SocketAddr, TcpStream as StdTcpStream}, sync::Arc, task::Poll, + time::Duration, }; use actix_http::{ body::{BodyStream, BoxBody, SizedStream}, error::PayloadError, header::{self, HeaderName, HeaderValue}, - Error, HttpService, Method, Request, Response, StatusCode, Version, + Error, HttpService, Method, Request, Response, StatusCode, TlsAcceptorConfig, Version, }; use actix_http_test::test_server; use actix_rt::pin; @@ -160,7 +161,10 @@ async fn h2_1() -> io::Result<()> { assert_eq!(req.version(), Version::HTTP_2); ok::<_, Error>(Response::ok()) }) - .rustls(tls_config()) + .rustls_with_config( + tls_config(), + TlsAcceptorConfig::default().handshake_timeout(Duration::from_secs(5)), + ) }) .await; diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md index 3fa2f8f21..0144cb912 100644 --- a/actix-web/CHANGES.md +++ b/actix-web/CHANGES.md @@ -1,12 +1,17 @@ # Changelog ## Unreleased - 2022-xx-xx -- Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency. ### Added - Add `ServiceRequest::{parts, request}()` getter methods. [#2786] +- Add configuration options for TLS handshake timeout via `HttpServer::{rustls, openssl}_with_config` methods. [#2752] +### Changed +- Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency. + +[#2752]: https://github.com/actix/actix-web/pull/2752 [#2786]: https://github.com/actix/actix-web/pull/2786 + ## 4.1.0 - 2022-06-11 ### Added - Add `ServiceRequest::extract()` to make it easier to use extractors when writing middlewares. [#2647] diff --git a/actix-web/src/server.rs b/actix-web/src/server.rs index 99812600c..169eafab0 100644 --- a/actix-web/src/server.rs +++ b/actix-web/src/server.rs @@ -18,6 +18,9 @@ use actix_tls::accept::openssl::reexports::{AlpnError, SslAcceptor, SslAcceptorB #[cfg(feature = "rustls")] use actix_tls::accept::rustls::reexports::ServerConfig as RustlsServerConfig; +#[cfg(any(feature = "openssl", feature = "rustls"))] +use actix_http::TlsAcceptorConfig; + use crate::{config::AppConfig, Error}; struct Socket { @@ -30,6 +33,8 @@ struct Config { keep_alive: KeepAlive, client_request_timeout: Duration, client_disconnect_timeout: Duration, + #[cfg(any(feature = "openssl", feature = "rustls"))] + tls_handshake_timeout: Option, } /// An HTTP Server. @@ -92,6 +97,8 @@ where keep_alive: KeepAlive::default(), client_request_timeout: Duration::from_secs(5), client_disconnect_timeout: Duration::from_secs(1), + #[cfg(any(feature = "rustls", feature = "openssl"))] + tls_handshake_timeout: None, })), backlog: 1024, sockets: Vec::new(), @@ -225,6 +232,24 @@ where self } + /// Set TLS handshake timeout. + /// + /// Defines a timeout for TLS handshake. If the TLS handshake does not complete + /// within this time, the connection is closed. + /// + /// By default handshake timeout is set to 3000 milliseconds. + #[cfg(any(feature = "openssl", feature = "rustls"))] + #[cfg_attr(docsrs, doc(cfg(any(feature = "openssl", feature = "rustls"))))] + pub fn tls_handshake_timeout(self, dur: Duration) -> Self { + self.config + .lock() + .unwrap() + .tls_handshake_timeout + .replace(dur); + + self + } + #[doc(hidden)] #[deprecated(since = "4.0.0", note = "Renamed to `client_disconnect_timeout`.")] pub fn client_shutdown(self, dur: u64) -> Self { @@ -376,10 +401,15 @@ where .into_factory() .map_err(|err| err.into().error_response()); + let acceptor_config = match c.tls_handshake_timeout { + Some(dur) => TlsAcceptorConfig::default().handshake_timeout(dur), + None => TlsAcceptorConfig::default(), + }; + svc.finish(map_config(fac, move |_| { AppConfig::new(true, host.clone(), addr) })) - .openssl(acceptor.clone()) + .openssl_with_config(acceptor.clone(), acceptor_config) })?; Ok(self) @@ -434,10 +464,15 @@ where .into_factory() .map_err(|err| err.into().error_response()); + let acceptor_config = match c.tls_handshake_timeout { + Some(dur) => TlsAcceptorConfig::default().handshake_timeout(dur), + None => TlsAcceptorConfig::default(), + }; + svc.finish(map_config(fac, move |_| { AppConfig::new(true, host.clone(), addr) })) - .rustls(config.clone()) + .rustls_with_config(config.clone(), acceptor_config) })?; Ok(self) From 06c79458012457b2ef6e106b7fbe283a63683e73 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Thu, 30 Jun 2022 09:19:16 +0100 Subject: [PATCH 04/11] retain previously set vary headers when using compress (#2798) * retain previously set vary headers when using compress --- actix-http/CHANGES.md | 8 +++++--- actix-http/src/encoding/encoder.rs | 2 +- actix-web/src/middleware/compress.rs | 25 +++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index dd6051b85..c9ab687f6 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -1,13 +1,15 @@ # Changes ## Unreleased - 2022-xx-xx -### Fixed -- Websocket parser no longer throws endless overflow errors after receiving an oversized frame. [#2790] - ### Changed - Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency. +### Fixed +- Websocket parser no longer throws endless overflow errors after receiving an oversized frame. [#2790] +- Retain previously set Vary headers when using compression encoder. [#2798] + [#2790]: https://github.com/actix/actix-web/pull/2790 +[#2798]: https://github.com/actix/actix-web/pull/2798 ## 3.1.0 - 2022-06-11 diff --git a/actix-http/src/encoding/encoder.rs b/actix-http/src/encoding/encoder.rs index 0bbb1c106..bbe53e8e8 100644 --- a/actix-http/src/encoding/encoder.rs +++ b/actix-http/src/encoding/encoder.rs @@ -257,7 +257,7 @@ fn update_head(encoding: ContentEncoding, head: &mut ResponseHead) { head.headers_mut() .insert(header::CONTENT_ENCODING, encoding.to_header_value()); head.headers_mut() - .insert(header::VARY, HeaderValue::from_static("accept-encoding")); + .append(header::VARY, HeaderValue::from_static("accept-encoding")); head.no_chunking(false); } diff --git a/actix-web/src/middleware/compress.rs b/actix-web/src/middleware/compress.rs index 4fdd74779..ed4291bed 100644 --- a/actix-web/src/middleware/compress.rs +++ b/actix-web/src/middleware/compress.rs @@ -251,6 +251,8 @@ static SUPPORTED_ENCODINGS: Lazy> = Lazy::new(|| { #[cfg(feature = "compress-gzip")] #[cfg(test)] mod tests { + use std::collections::HashSet; + use super::*; use crate::{middleware::DefaultHeaders, test, web, App}; @@ -305,4 +307,27 @@ mod tests { let bytes = test::read_body(res).await; assert_eq!(gzip_decode(bytes), DATA.as_bytes()); } + + #[actix_rt::test] + async fn retains_previously_set_vary_header() { + let app = test::init_service({ + App::new() + .wrap(Compress::default()) + .default_service(web::to(move || { + HttpResponse::Ok() + .insert_header((header::VARY, "x-test")) + .finish() + })) + }) + .await; + + let req = test::TestRequest::default() + .insert_header((header::ACCEPT_ENCODING, "gzip")) + .to_request(); + let res = test::call_service(&app, req).await; + assert_eq!(res.status(), StatusCode::OK); + let vary_headers = res.headers().get_all(header::VARY).collect::>(); + assert!(vary_headers.contains(&HeaderValue::from_static("x-test"))); + assert!(vary_headers.contains(&HeaderValue::from_static("accept-encoding"))); + } } From c6eba2da9b6ead6112433f7d2aaa1f2d19a22395 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 1 Jul 2022 06:16:17 +0100 Subject: [PATCH 05/11] prepare actix-http release 3.2.0 (#2801) --- actix-http/CHANGES.md | 3 +++ actix-http/Cargo.toml | 2 +- actix-http/README.md | 4 ++-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index c9ab687f6..024a731d8 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -1,6 +1,9 @@ # Changes ## Unreleased - 2022-xx-xx + + +## 3.2.0 - 2022-06-30 ### Changed - Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency. diff --git a/actix-http/Cargo.toml b/actix-http/Cargo.toml index 2a4966884..3fccbfa03 100644 --- a/actix-http/Cargo.toml +++ b/actix-http/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-http" -version = "3.1.0" +version = "3.2.0" authors = [ "Nikolay Kim ", "Rob Ede ", diff --git a/actix-http/README.md b/actix-http/README.md index 211f433e8..3179258af 100644 --- a/actix-http/README.md +++ b/actix-http/README.md @@ -3,11 +3,11 @@ > HTTP primitives for the Actix ecosystem. [![crates.io](https://img.shields.io/crates/v/actix-http?label=latest)](https://crates.io/crates/actix-http) -[![Documentation](https://docs.rs/actix-http/badge.svg?version=3.1.0)](https://docs.rs/actix-http/3.1.0) +[![Documentation](https://docs.rs/actix-http/badge.svg?version=3.2.0)](https://docs.rs/actix-http/3.2.0) ![Version](https://img.shields.io/badge/rustc-1.57+-ab6000.svg) ![MIT or Apache 2.0 licensed](https://img.shields.io/crates/l/actix-http.svg)
-[![dependency status](https://deps.rs/crate/actix-http/3.1.0/status.svg)](https://deps.rs/crate/actix-http/3.1.0) +[![dependency status](https://deps.rs/crate/actix-http/3.2.0/status.svg)](https://deps.rs/crate/actix-http/3.2.0) [![Download](https://img.shields.io/crates/d/actix-http.svg)](https://crates.io/crates/actix-http) [![Chat on Discord](https://img.shields.io/discord/771444961383153695?label=chat&logo=discord)](https://discord.gg/NWpN5mmg3x) From 8f9a12ed5d30c1f0a44cbcb5dfb0c3fecd1f6348 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 1 Jul 2022 08:23:40 +0100 Subject: [PATCH 06/11] fix parsing ambiguities for HTTP/1.0 requests (#2794) * fix HRS vuln when first CL header is 0 * ignore TE headers in http/1.0 reqs * update changelog * disallow HTTP/1.0 requests without a CL header * fix test * broken fix for http1.0 post requests --- actix-http/CHANGES.md | 4 + actix-http/src/h1/decoder.rs | 156 +++++++++++++++++++++++++++++++---- 2 files changed, 142 insertions(+), 18 deletions(-) diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index 024a731d8..5d441919d 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -1,6 +1,10 @@ # Changes ## Unreleased - 2022-xx-xx +### Fixed +- Fix parsing ambiguity in Transfer-Encoding and Content-Length headers for HTTP/1.0 requests. [#2794] + +[#2794]: https://github.com/actix/actix-web/pull/2794 ## 3.2.0 - 2022-06-30 diff --git a/actix-http/src/h1/decoder.rs b/actix-http/src/h1/decoder.rs index a9443997e..91a93d22c 100644 --- a/actix-http/src/h1/decoder.rs +++ b/actix-http/src/h1/decoder.rs @@ -46,6 +46,23 @@ pub(crate) enum PayloadLength { None, } +impl PayloadLength { + /// Returns true if variant is `None`. + fn is_none(&self) -> bool { + matches!(self, Self::None) + } + + /// Returns true if variant is represents zero-length (not none) payload. + fn is_zero(&self) -> bool { + matches!( + self, + PayloadLength::Payload(PayloadType::Payload(PayloadDecoder { + kind: Kind::Length(0) + })) + ) + } +} + pub(crate) trait MessageType: Sized { fn set_connection_type(&mut self, conn_type: Option); @@ -59,6 +76,7 @@ pub(crate) trait MessageType: Sized { &mut self, slice: &Bytes, raw_headers: &[HeaderIndex], + version: Version, ) -> Result { let mut ka = None; let mut has_upgrade_websocket = false; @@ -87,21 +105,23 @@ pub(crate) trait MessageType: Sized { return Err(ParseError::Header); } - header::CONTENT_LENGTH => match value.to_str() { - Ok(s) if s.trim().starts_with('+') => { - debug!("illegal Content-Length: {:?}", s); + header::CONTENT_LENGTH => match value.to_str().map(str::trim) { + Ok(val) if val.starts_with('+') => { + debug!("illegal Content-Length: {:?}", val); return Err(ParseError::Header); } - Ok(s) => { - if let Ok(len) = s.parse::() { - if len != 0 { - content_length = Some(len); - } + + Ok(val) => { + if let Ok(len) = val.parse::() { + // accept 0 lengths here and remove them in `decode` after all + // headers have been processed to prevent request smuggling issues + content_length = Some(len); } else { - debug!("illegal Content-Length: {:?}", s); + debug!("illegal Content-Length: {:?}", val); return Err(ParseError::Header); } } + Err(_) => { debug!("illegal Content-Length: {:?}", value); return Err(ParseError::Header); @@ -114,22 +134,23 @@ pub(crate) trait MessageType: Sized { return Err(ParseError::Header); } - header::TRANSFER_ENCODING => { + header::TRANSFER_ENCODING if version == Version::HTTP_11 => { seen_te = true; - if let Ok(s) = value.to_str().map(str::trim) { - if s.eq_ignore_ascii_case("chunked") { + if let Ok(val) = value.to_str().map(str::trim) { + if val.eq_ignore_ascii_case("chunked") { chunked = true; - } else if s.eq_ignore_ascii_case("identity") { + } else if val.eq_ignore_ascii_case("identity") { // allow silently since multiple TE headers are already checked } else { - debug!("illegal Transfer-Encoding: {:?}", s); + debug!("illegal Transfer-Encoding: {:?}", val); return Err(ParseError::Header); } } else { return Err(ParseError::Header); } } + // connection keep-alive state header::CONNECTION => { ka = if let Ok(conn) = value.to_str().map(str::trim) { @@ -146,6 +167,7 @@ pub(crate) trait MessageType: Sized { None }; } + header::UPGRADE => { if let Ok(val) = value.to_str().map(str::trim) { if val.eq_ignore_ascii_case("websocket") { @@ -153,19 +175,23 @@ pub(crate) trait MessageType: Sized { } } } + header::EXPECT => { let bytes = value.as_bytes(); if bytes.len() >= 4 && &bytes[0..4] == b"100-" { expect = true; } } + _ => {} } headers.append(name, value); } } + self.set_connection_type(ka); + if expect { self.set_expect() } @@ -249,7 +275,22 @@ impl MessageType for Request { let mut msg = Request::new(); // convert headers - let length = msg.set_headers(&src.split_to(len).freeze(), &headers[..h_len])?; + let mut length = + msg.set_headers(&src.split_to(len).freeze(), &headers[..h_len], ver)?; + + // disallow HTTP/1.0 POST requests that do not contain a Content-Length headers + // see https://datatracker.ietf.org/doc/html/rfc1945#section-7.2.2 + if ver == Version::HTTP_10 && method == Method::POST && length.is_none() { + debug!("no Content-Length specified for HTTP/1.0 POST request"); + return Err(ParseError::Header); + } + + // Remove CL value if 0 now that all headers and HTTP/1.0 special cases are processed. + // Protects against some request smuggling attacks. + // See https://github.com/actix/actix-web/issues/2767. + if length.is_zero() { + length = PayloadLength::None; + } // payload decoder let decoder = match length { @@ -337,7 +378,15 @@ impl MessageType for ResponseHead { msg.version = ver; // convert headers - let length = msg.set_headers(&src.split_to(len).freeze(), &headers[..h_len])?; + let mut length = + msg.set_headers(&src.split_to(len).freeze(), &headers[..h_len], ver)?; + + // Remove CL value if 0 now that all headers and HTTP/1.0 special cases are processed. + // Protects against some request smuggling attacks. + // See https://github.com/actix/actix-web/issues/2767. + if length.is_zero() { + length = PayloadLength::None; + } // message payload let decoder = if let PayloadLength::Payload(pl) = length { @@ -606,14 +655,40 @@ mod tests { } #[test] - fn test_parse_post() { - let mut buf = BytesMut::from("POST /test2 HTTP/1.0\r\n\r\n"); + fn parse_h10_post() { + let mut buf = BytesMut::from( + "POST /test1 HTTP/1.0\r\n\ + Content-Length: 3\r\n\ + \r\n\ + abc", + ); + + let mut reader = MessageDecoder::::default(); + let (req, _) = reader.decode(&mut buf).unwrap().unwrap(); + assert_eq!(req.version(), Version::HTTP_10); + assert_eq!(*req.method(), Method::POST); + assert_eq!(req.path(), "/test1"); + + let mut buf = BytesMut::from( + "POST /test2 HTTP/1.0\r\n\ + Content-Length: 0\r\n\ + \r\n", + ); let mut reader = MessageDecoder::::default(); let (req, _) = reader.decode(&mut buf).unwrap().unwrap(); assert_eq!(req.version(), Version::HTTP_10); assert_eq!(*req.method(), Method::POST); assert_eq!(req.path(), "/test2"); + + let mut buf = BytesMut::from( + "POST /test3 HTTP/1.0\r\n\ + \r\n", + ); + + let mut reader = MessageDecoder::::default(); + let err = reader.decode(&mut buf).unwrap_err(); + assert!(err.to_string().contains("Header")) } #[test] @@ -981,6 +1056,17 @@ mod tests { ); expect_parse_err!(&mut buf); + + let mut buf = BytesMut::from( + "GET / HTTP/1.1\r\n\ + Host: example.com\r\n\ + Content-Length: 0\r\n\ + Content-Length: 2\r\n\ + \r\n\ + ab", + ); + + expect_parse_err!(&mut buf); } #[test] @@ -996,6 +1082,40 @@ mod tests { expect_parse_err!(&mut buf); } + #[test] + fn hrs_te_http10() { + // in HTTP/1.0 transfer encoding is ignored and must therefore contain a CL header + + let mut buf = BytesMut::from( + "POST / HTTP/1.0\r\n\ + Host: example.com\r\n\ + Transfer-Encoding: chunked\r\n\ + \r\n\ + 3\r\n\ + aaa\r\n\ + 0\r\n\ + ", + ); + + expect_parse_err!(&mut buf); + } + + #[test] + fn hrs_cl_and_te_http10() { + // in HTTP/1.0 transfer encoding is simply ignored so it's fine to have both + + let mut buf = BytesMut::from( + "GET / HTTP/1.0\r\n\ + Host: example.com\r\n\ + Content-Length: 3\r\n\ + Transfer-Encoding: chunked\r\n\ + \r\n\ + 000", + ); + + parse_ready!(&mut buf); + } + #[test] fn hrs_unknown_transfer_encoding() { let mut buf = BytesMut::from( From 7e990e423fdf4b6b4b8cf24a7bc412351d8e995b Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 1 Jul 2022 08:24:45 +0100 Subject: [PATCH 07/11] add http/1.0 GET parsing tests --- actix-http/src/h1/decoder.rs | 40 ++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/actix-http/src/h1/decoder.rs b/actix-http/src/h1/decoder.rs index 91a93d22c..c718f26a8 100644 --- a/actix-http/src/h1/decoder.rs +++ b/actix-http/src/h1/decoder.rs @@ -654,6 +654,46 @@ mod tests { assert_eq!(req.path(), "/test"); } + #[test] + fn parse_h10_get() { + let mut buf = BytesMut::from( + "GET /test1 HTTP/1.0\r\n\ + \r\n\ + abc", + ); + + let mut reader = MessageDecoder::::default(); + let (req, _) = reader.decode(&mut buf).unwrap().unwrap(); + assert_eq!(req.version(), Version::HTTP_10); + assert_eq!(*req.method(), Method::GET); + assert_eq!(req.path(), "/test1"); + + let mut buf = BytesMut::from( + "GET /test2 HTTP/1.0\r\n\ + Content-Length: 0\r\n\ + \r\n", + ); + + let mut reader = MessageDecoder::::default(); + let (req, _) = reader.decode(&mut buf).unwrap().unwrap(); + assert_eq!(req.version(), Version::HTTP_10); + assert_eq!(*req.method(), Method::GET); + assert_eq!(req.path(), "/test2"); + + let mut buf = BytesMut::from( + "GET /test3 HTTP/1.0\r\n\ + Content-Length: 3\r\n\ + \r\n + abc", + ); + + let mut reader = MessageDecoder::::default(); + let (req, _) = reader.decode(&mut buf).unwrap().unwrap(); + assert_eq!(req.version(), Version::HTTP_10); + assert_eq!(*req.method(), Method::GET); + assert_eq!(req.path(), "/test3"); + } + #[test] fn parse_h10_post() { let mut buf = BytesMut::from( From e524fc86eaef37c36b40c25c20b9880893c76508 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 1 Jul 2022 09:03:57 +0100 Subject: [PATCH 08/11] add HTTP/0.9 rejection test --- actix-http/src/h1/decoder.rs | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/actix-http/src/h1/decoder.rs b/actix-http/src/h1/decoder.rs index c718f26a8..edfc00fd6 100644 --- a/actix-http/src/h1/decoder.rs +++ b/actix-http/src/h1/decoder.rs @@ -654,12 +654,32 @@ mod tests { assert_eq!(req.path(), "/test"); } + #[test] + fn parse_h09_reject() { + let mut buf = BytesMut::from( + "GET /test1 HTTP/0.9\r\n\ + \r\n", + ); + + let mut reader = MessageDecoder::::default(); + reader.decode(&mut buf).unwrap_err(); + + let mut buf = BytesMut::from( + "POST /test2 HTTP/0.9\r\n\ + Content-Length: 3\r\n\ + \r\n + abc", + ); + + let mut reader = MessageDecoder::::default(); + reader.decode(&mut buf).unwrap_err(); + } + #[test] fn parse_h10_get() { let mut buf = BytesMut::from( "GET /test1 HTTP/1.0\r\n\ - \r\n\ - abc", + \r\n", ); let mut reader = MessageDecoder::::default(); From 226ea696ce43283d259cdea316fcc9efca6e25b9 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 1 Jul 2022 10:19:28 +0100 Subject: [PATCH 09/11] update dev deps --- actix-http/Cargo.toml | 4 ++-- actix-web/Cargo.toml | 6 +++--- awc/Cargo.toml | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/actix-http/Cargo.toml b/actix-http/Cargo.toml index 3fccbfa03..abb1e4603 100644 --- a/actix-http/Cargo.toml +++ b/actix-http/Cargo.toml @@ -108,10 +108,10 @@ env_logger = "0.9" futures-util = { version = "0.3.7", default-features = false, features = ["alloc"] } memchr = "2.4" once_cell = "1.9" -rcgen = "0.8" +rcgen = "0.9" regex = "1.3" rustversion = "1" -rustls-pemfile = "0.2" +rustls-pemfile = "1" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" static_assertions = "1" diff --git a/actix-web/Cargo.toml b/actix-web/Cargo.toml index 8cdf0f611..c58e1604b 100644 --- a/actix-web/Cargo.toml +++ b/actix-web/Cargo.toml @@ -105,14 +105,14 @@ actix-test = { version = "0.1.0-beta.13", features = ["openssl", "rustls"] } awc = { version = "3", features = ["openssl"] } brotli = "3.3.3" -const-str = "0.3" +const-str = "0.4" criterion = { version = "0.3", features = ["html_reports"] } env_logger = "0.9" flate2 = "1.0.13" futures-util = { version = "0.3.7", default-features = false, features = ["std"] } rand = "0.8" -rcgen = "0.8" -rustls-pemfile = "0.2" +rcgen = "0.9" +rustls-pemfile = "1" serde = { version = "1.0", features = ["derive"] } static_assertions = "1" tls-openssl = { package = "openssl", version = "0.10.9" } diff --git a/awc/Cargo.toml b/awc/Cargo.toml index ba0fc14e3..1a69fd49e 100644 --- a/awc/Cargo.toml +++ b/awc/Cargo.toml @@ -102,13 +102,13 @@ actix-utils = "3" actix-web = { version = "4", features = ["openssl"] } brotli = "3.3.3" -const-str = "0.3" +const-str = "0.4" env_logger = "0.9" flate2 = "1.0.13" futures-util = { version = "0.3.7", default-features = false } static_assertions = "1.1" -rcgen = "0.8" -rustls-pemfile = "0.2" +rcgen = "0.9" +rustls-pemfile = "1" tokio = { version = "1.13.1", features = ["rt-multi-thread", "macros"] } zstd = "0.11" From df5257c3730ccf270dbe0799501826fc2d4a1072 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 1 Jul 2022 10:21:46 +0100 Subject: [PATCH 10/11] update trust dns resolver --- awc/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awc/Cargo.toml b/awc/Cargo.toml index 1a69fd49e..9da103cb0 100644 --- a/awc/Cargo.toml +++ b/awc/Cargo.toml @@ -90,7 +90,7 @@ cookie = { version = "0.16", features = ["percent-encode"], optional = true } tls-openssl = { package = "openssl", version = "0.10.9", optional = true } tls-rustls = { package = "rustls", version = "0.20.0", optional = true, features = ["dangerous_configuration"] } -trust-dns-resolver = { version = "0.20.0", optional = true } +trust-dns-resolver = { version = "0.21", optional = true } [dev-dependencies] actix-http = { version = "3", features = ["openssl"] } From b62f1b4ef74fc28881bdf070e35395065189bd26 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 1 Jul 2022 12:40:00 +0100 Subject: [PATCH 11/11] migrate deprecated method in docs --- actix-router/src/resource.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actix-router/src/resource.rs b/actix-router/src/resource.rs index bc082273c..3c6754aeb 100644 --- a/actix-router/src/resource.rs +++ b/actix-router/src/resource.rs @@ -649,7 +649,7 @@ impl ResourceDef { /// resource.capture_match_info_fn( /// path, /// // when env var is not set, reject when path contains "admin" - /// |res| !(!admin_allowed && res.path().contains("admin")), + /// |path| !(!admin_allowed && path.as_str().contains("admin")), /// ) /// } ///