diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index 718f67922..d9bef53cd 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -2,7 +2,9 @@ ## Unreleased -- Properly wake Payload receivers when feeding errors or EOF +- Properly wake Payload receivers when feeding errors or EOF. +- Add `ServiceConfigBuilder` type to facilitate future configuration extensions. +- Add a configuration option to allow/disallow half closed connections in HTTP/1. This defaults to allow, reverting the change made in 3.11.1. ## 3.11.1 diff --git a/actix-http/src/builder.rs b/actix-http/src/builder.rs index 95da8ac62..09b379e87 100644 --- a/actix-http/src/builder.rs +++ b/actix-http/src/builder.rs @@ -19,6 +19,7 @@ pub struct HttpServiceBuilder { client_disconnect_timeout: Duration, secure: bool, local_addr: Option, + h1_allow_half_closed: bool, expect: X, upgrade: Option, on_connect_ext: Option>>, @@ -40,6 +41,7 @@ where client_disconnect_timeout: Duration::ZERO, secure: false, local_addr: None, + h1_allow_half_closed: true, // dispatcher parts expect: ExpectHandler, @@ -124,6 +126,18 @@ where self.client_disconnect_timeout(dur) } + /// Sets whether HTTP/1 connections should support half-closures. + /// + /// Clients can choose to shutdown their writer-side of the connection after completing their + /// request and while waiting for the server response. Setting this to `false` will cause the + /// server to abort the connection handling as soon as it detects an EOF from the client. + /// + /// The default behavior is to allow, i.e. `true` + pub fn h1_allow_half_closed(mut self, allow: bool) -> Self { + self.h1_allow_half_closed = allow; + self + } + /// Provide service for `EXPECT: 100-Continue` support. /// /// Service get called with request that contains `EXPECT` header. @@ -142,6 +156,7 @@ where client_disconnect_timeout: self.client_disconnect_timeout, secure: self.secure, local_addr: self.local_addr, + h1_allow_half_closed: self.h1_allow_half_closed, expect: expect.into_factory(), upgrade: self.upgrade, on_connect_ext: self.on_connect_ext, @@ -166,6 +181,7 @@ where client_disconnect_timeout: self.client_disconnect_timeout, secure: self.secure, local_addr: self.local_addr, + h1_allow_half_closed: self.h1_allow_half_closed, expect: self.expect, upgrade: Some(upgrade.into_factory()), on_connect_ext: self.on_connect_ext, @@ -201,6 +217,7 @@ where .client_disconnect_timeout(self.client_disconnect_timeout) .secure(self.secure) .local_addr(self.local_addr) + .h1_allow_half_closed(self.h1_allow_half_closed) .build(); H1Service::with_config(cfg, service.into_factory()) @@ -226,6 +243,7 @@ where .client_disconnect_timeout(self.client_disconnect_timeout) .secure(self.secure) .local_addr(self.local_addr) + .h1_allow_half_closed(self.h1_allow_half_closed) .build(); crate::h2::H2Service::with_config(cfg, service.into_factory()) @@ -248,6 +266,7 @@ where .client_disconnect_timeout(self.client_disconnect_timeout) .secure(self.secure) .local_addr(self.local_addr) + .h1_allow_half_closed(self.h1_allow_half_closed) .build(); HttpService::with_config(cfg, service.into_factory()) diff --git a/actix-http/src/config.rs b/actix-http/src/config.rs index 840abeaaf..469974930 100644 --- a/actix-http/src/config.rs +++ b/actix-http/src/config.rs @@ -23,6 +23,7 @@ impl ServiceConfigBuilder { /// - 0 seconds for the client shutdown timeout /// - secure value of `false` /// - [`None`] for the local address setting + /// - Allow for half closed HTTP/1 connections pub fn new() -> Self { Self::default() } @@ -58,6 +59,16 @@ impl ServiceConfigBuilder { self } + /// Sets whether HTTP/1 connections should support half-closures. + /// + /// Clients can choose to shutdown their writer-side of the connection after completing their + /// request and while waiting for the server response. Setting this to `false` will cause the + /// server to abort the connection handling as soon as it detects an EOF from the client + pub fn h1_allow_half_closed(mut self, allow: bool) -> Self { + self.inner.h1_allow_half_closed = allow; + self + } + /// Builds a [`ServiceConfig`] from this [`ServiceConfigBuilder`] instance pub fn build(self) -> ServiceConfig { ServiceConfig(Rc::new(self.inner)) @@ -84,6 +95,7 @@ struct Inner { secure: bool, local_addr: Option, date_service: DateService, + h1_allow_half_closed: bool, } impl Default for Inner { @@ -95,6 +107,7 @@ impl Default for Inner { secure: false, local_addr: None, date_service: DateService::new(), + h1_allow_half_closed: true, } } } @@ -121,6 +134,7 @@ impl ServiceConfig { secure, local_addr, date_service: DateService::new(), + h1_allow_half_closed: true, })) } @@ -171,6 +185,15 @@ impl ServiceConfig { (timeout != Duration::ZERO).then(|| self.now() + timeout) } + /// Whether HTTP/1 connections should support half-closures. + /// + /// Clients can choose to shutdown their writer-side of the connection after completing their + /// request and while waiting for the server response. If this configuration is `false`, the + /// server will abort the connection handling as soon as it detects an EOF from the client + pub fn h1_allow_half_closed(&self) -> bool { + self.0.h1_allow_half_closed + } + pub(crate) fn now(&self) -> Instant { self.0.date_service.now() } diff --git a/actix-http/src/h1/dispatcher.rs b/actix-http/src/h1/dispatcher.rs index 3f0b78af4..41266659a 100644 --- a/actix-http/src/h1/dispatcher.rs +++ b/actix-http/src/h1/dispatcher.rs @@ -1181,8 +1181,16 @@ where let inner_p = inner.as_mut().project(); let state_is_none = inner_p.state.is_none(); - // read half is closed; we do not process any responses - if inner_p.flags.contains(Flags::READ_DISCONNECT) { + // If the read-half is closed, we start the shutdown procedure if either is + // true: + // + // - state is [`State::None`], which means that we're done with request + // processing, so if the client closed its writer-side it means that it won't + // send more requests. + // - The user requested to not allow half-closures + if inner_p.flags.contains(Flags::READ_DISCONNECT) + && (!inner_p.config.h1_allow_half_closed() || state_is_none) + { trace!("read half closed; start shutdown"); inner_p.flags.insert(Flags::SHUTDOWN); } diff --git a/actix-web/src/server.rs b/actix-web/src/server.rs index 0717f5bc6..2bd7c4463 100644 --- a/actix-web/src/server.rs +++ b/actix-web/src/server.rs @@ -31,6 +31,7 @@ struct Config { keep_alive: KeepAlive, client_request_timeout: Duration, client_disconnect_timeout: Duration, + h1_allow_half_closed: bool, #[allow(dead_code)] // only dead when no TLS features are enabled tls_handshake_timeout: Option, } @@ -116,6 +117,7 @@ where keep_alive: KeepAlive::default(), client_request_timeout: Duration::from_secs(5), client_disconnect_timeout: Duration::from_secs(1), + h1_allow_half_closed: true, tls_handshake_timeout: None, })), backlog: 1024, @@ -257,6 +259,18 @@ where self.client_disconnect_timeout(Duration::from_millis(dur)) } + /// Sets whether HTTP/1 connections should support half-closures. + /// + /// Clients can choose to shutdown their writer-side of the connection after completing their + /// request and while waiting for the server response. Setting this to `false` will cause the + /// server to abort the connection handling as soon as it detects an EOF from the client. + /// + /// The default behavior is to allow, i.e. `true` + pub fn h1_allow_half_closed(self, allow: bool) -> Self { + self.config.lock().unwrap().h1_allow_half_closed = allow; + self + } + /// Sets function that will be called once before each connection is handled. /// /// It will receive a `&std::any::Any`, which contains underlying connection type and an @@ -558,6 +572,7 @@ where .keep_alive(cfg.keep_alive) .client_request_timeout(cfg.client_request_timeout) .client_disconnect_timeout(cfg.client_disconnect_timeout) + .h1_allow_half_closed(cfg.h1_allow_half_closed) .local_addr(addr); if let Some(handler) = on_connect_fn.clone() { @@ -602,6 +617,7 @@ where .keep_alive(cfg.keep_alive) .client_request_timeout(cfg.client_request_timeout) .client_disconnect_timeout(cfg.client_disconnect_timeout) + .h1_allow_half_closed(cfg.h1_allow_half_closed) .local_addr(addr); if let Some(handler) = on_connect_fn.clone() { @@ -677,6 +693,7 @@ where let svc = HttpService::build() .keep_alive(c.keep_alive) .client_request_timeout(c.client_request_timeout) + .h1_allow_half_closed(c.h1_allow_half_closed) .client_disconnect_timeout(c.client_disconnect_timeout); let svc = if let Some(handler) = on_connect_fn.clone() { @@ -728,6 +745,7 @@ where let svc = HttpService::build() .keep_alive(c.keep_alive) .client_request_timeout(c.client_request_timeout) + .h1_allow_half_closed(c.h1_allow_half_closed) .client_disconnect_timeout(c.client_disconnect_timeout); let svc = if let Some(handler) = on_connect_fn.clone() { @@ -794,6 +812,7 @@ where let svc = HttpService::build() .keep_alive(c.keep_alive) .client_request_timeout(c.client_request_timeout) + .h1_allow_half_closed(c.h1_allow_half_closed) .client_disconnect_timeout(c.client_disconnect_timeout); let svc = if let Some(handler) = on_connect_fn.clone() { @@ -860,6 +879,7 @@ where let svc = HttpService::build() .keep_alive(c.keep_alive) .client_request_timeout(c.client_request_timeout) + .h1_allow_half_closed(c.h1_allow_half_closed) .client_disconnect_timeout(c.client_disconnect_timeout); let svc = if let Some(handler) = on_connect_fn.clone() { @@ -927,6 +947,7 @@ where .keep_alive(c.keep_alive) .client_request_timeout(c.client_request_timeout) .client_disconnect_timeout(c.client_disconnect_timeout) + .h1_allow_half_closed(c.h1_allow_half_closed) .local_addr(addr); let svc = if let Some(handler) = on_connect_fn.clone() { @@ -995,6 +1016,7 @@ where .keep_alive(c.keep_alive) .client_request_timeout(c.client_request_timeout) .client_disconnect_timeout(c.client_disconnect_timeout) + .h1_allow_half_closed(c.h1_allow_half_closed) .finish(map_config(fac, move |_| config.clone())), ) }, @@ -1036,6 +1058,7 @@ where let mut svc = HttpService::build() .keep_alive(c.keep_alive) .client_request_timeout(c.client_request_timeout) + .h1_allow_half_closed(c.h1_allow_half_closed) .client_disconnect_timeout(c.client_disconnect_timeout); if let Some(handler) = on_connect_fn.clone() {