Add connection backpressure and timeouts by gjcairo · Pull Request #72 · swift-server/swift-http-server

gjcairo · 2026-03-30T14:25:50Z

This PR adds connection-level backpressure and timeout support to the server.

A new optional maxConnections field on NIOHTTPServerConfiguration lets users cap the number of concurrent connections: when the limit is reached, the server stops accepting new connections by gating NIO read events on the server channel until existing connections close.

Alongside this, a new ConnectionTimeouts configuration provides idle, read-header, and-read body timeouts to evict slow or misbehaving connections, to mitigate attacks against the connection limit. Timeouts are enabled by default with reasonable values (60s/30s/60s respectively); the connection limit is opt-in.

Both features work across HTTP/1.1 and HTTP/2, with timeout handlers split appropriately between connection and stream levels for HTTP/2.

aryan-25 · 2026-03-31T09:15:16Z

+    func addTimeoutHandlers(to channel: any Channel) throws {
+        let timeouts = self.configuration.connectionTimeouts
+
+        if let idle = timeouts.idle {


Looks like these three if blocks are identical to the method bodies below. We can probably call into those methods here.

aryan-25 · 2026-03-31T09:23:24Z

+        if let idle = self.configuration.connectionTimeouts.idle {
+            let idleTimeAmount = TimeAmount(idle)
+            try channel.pipeline.syncOperations.addHandler(
+                IdleStateHandler(readTimeout: idleTimeAmount, writeTimeout: idleTimeAmount)


Should we allow both the read and write idle timeouts to be configurable?

aryan-25 · 2026-03-31T09:28:30Z

+            try await NIOHTTPServerTests.withServer(
+                server: server,
+                serverHandler: HTTPServerClosureRequestHandler { _, _, reader, responseSender in
+                    _ = try await reader.consumeAndConclude { bodyReader in


We can use NIOHTTPServerTests.echoResponse here.

aryan-25 · 2026-03-31T09:28:34Z

+                                    )
+                                    try await outbound.write(.end(nil))
+
+                                    var iter = inbound.makeAsyncIterator()


We can use NIOHTTPServerTests.validateResponse here.

aryan-25 · 2026-03-31T09:41:53Z

+    }
+
+    func read(context: ChannelHandlerContext) {
+        if self.activeConnections <= self.maxConnections {


Is this channel handler guaranteed to only prevent the call to read() once?

Because channel.read() is only called once upon close (line 48).

I might be misunderstanding your point, but the point of this handler is not to prevent the call to read just once - it's to make sure we're always below the set limit. That might mean calling read multiple times whenever we fall below the threshold, and withhold forwards of reads when we're above it.

fabianfett · 2026-04-01T08:23:44Z

+        let loopBoundContext = NIOLoopBound(context, eventLoop: context.eventLoop)
+        let eventLoop = context.eventLoop
+        childChannel.closeFuture.whenComplete { _ in
+            eventLoop.execute {


why do we need to hop here? we already should be on the correct EL.

fabianfett · 2026-04-01T08:28:35Z

+///
+/// The timeout starts when the channel becomes active and is cancelled when
+/// a `.head` part is received.
+final class ReadHeaderTimeoutHandler: ChannelInboundHandler, RemovableChannelHandler {


While it is totally fair to implement this in additional ChannelHanders, I strongly advise against it for performance reasons. We should really try to reduce pipeline overhead and pipeline modifications.

I've consolidated these into two handlers instead. Is that what you had in mind?

fabianfett · 2026-04-21T06:09:59Z

+    /// ## Configuration keys:
+    /// - `idle` (int, optional, default: nil): Maximum time in seconds a connection can remain idle.
+    /// - `readHeader` (int, optional, default: nil): Maximum time in seconds to receive request headers.
+    /// - `readBody` (int, optional, default: nil): Maximum time in seconds to receive the request body.


the full body? or is this between body part? please make this more explicit.

fabianfett · 2026-04-21T06:11:18Z

+        /// Maximum time allowed to receive the complete request body
+        /// after headers have been received. `nil` means no timeout.
+        public var readBody: Duration?


is this use-full? this means that this has to account for the longest possible request. I think something like the time between body parts is more appropriate?

That's a fair question. Go's http package only has a single timeout from connection establishment till connection end (so, the time it takes to read everything, including headers and request body).

I think having a timeout only between body parts could open the door to attacks where a client opens a bunch of connections and slowly trickles tiny bits of data just before the timeout fires to keep the connection alive.

We could have both, but we already have the idle timeout which keeps track of all reads or writes, so it's somewhat covered. I suppose we could remove the idle timeout and instead add timeouts for the time between reads and a separate one for time between writes if we want more granularity (while also keeping this readBody timeout). What do you think?

I'm in favor of the separate read and write timeouts. Because if the client doesn't accept any writes, we are also in a bad situation.

My question remains, how do you set a reasonable readBody timeout?

Also the readBody can easily be achieved using structured concurrency. We should consider if we really need this in the api.

This is btw. not a connectiontimeout but more a request timeout.

Have you thought of doing a rate based check when reading the body. Expect a minimum number of bytes to have been received over a defined period of time.

fabianfett · 2026-04-21T06:11:51Z

+    public struct ConnectionTimeouts: Sendable {
+        /// Maximum time a connection can remain idle (no data read or written)
+        /// before being closed. `nil` means no idle timeout.
+        public var idle: Duration?


please call out, that this is only used after the first request. before the first request, it is readHeader.

This timeout runs from connection establishment and is reset every time something is read or written, it's independent of the readHeader timeout.

okay, so after connection establishment, there are two timers: readHeader and idle? Isn't this wasteful?

fabianfett · 2026-04-21T06:14:08Z

        supportedHTTPVersions: Set<HTTPVersion>,
        transportSecurity: TransportSecurity,
-        backpressureStrategy: BackPressureStrategy = .defaults
+        backpressureStrategy: BackPressureStrategy = .defaults,
+        maxConnections: Int? = nil,
+        connectionTimeouts: ConnectionTimeouts = .defaults


are we sure we want to have this initializer? it feels like we would need to extend that, whenever a new property is added? I think having the init with just first three is more appropriate. All others can be changed via properties, if needed.

fabianfett · 2026-04-21T06:15:30Z

@@ -24,6 +25,9 @@ enum NIOHTTPServerConfigurationError: Error, CustomStringConvertible {

        case .incompatibleTransportSecurity:
            "Invalid configuration: only HTTP/1.1 can be served over plaintext. `transportSecurity` must be set to (m)TLS for serving HTTP/2."


just as a drive by notice: I think we should also support, h/2 over plaintext, shouldn't we?

Yeah we're tracking that work.

fabianfett · 2026-04-21T07:50:13Z

+/// - When `.end` is received, the body timeout is cancelled.
+///
+/// If either timeout fires, the connection is closed.
+final class RequestTimeoutHandler: ChannelInboundHandler, RemovableChannelHandler {


we 1000% should combine those two handlers into just one. We should try to reduce the total number of ChannelHandlers as much as possible.

I did it this way because for H2, we only want the idle handler in the connection channel, and the read handler in the stream channel, and merging them made it more confusing to read IMO as it would mean passing nil timeouts depending on which channel we're adding the handler to. I understand merging them would improve the performance of H1 requests though since we'd be adding a single handler instead of 2. I can make the change if we want to sacrifice some readability for performance in that case though.

I can make the change if we want to sacrifice some readability for performance in that case though.

This would mean that we currently also translate H2 to H1 to add timeouts. I think we need specialized handlers here.

fabianfett

A new round of reviews.

fabianfett · 2026-05-11T13:11:30Z

+    public struct ConnectionTimeouts: Sendable {
+        /// Maximum time a connection can remain idle (no data read or written)
+        /// before being closed. `nil` means no idle timeout.
+        public var idle: Duration?


okay, so after connection establishment, there are two timers: readHeader and idle? Isn't this wasteful?

fabianfett · 2026-05-11T13:14:42Z

+        /// Maximum time allowed to receive the complete request body
+        /// after headers have been received. `nil` means no timeout.
+        public var readBody: Duration?


I'm in favor of the separate read and write timeouts. Because if the client doesn't accept any writes, we are also in a bad situation.

My question remains, how do you set a reasonable readBody timeout?

Also the readBody can easily be achieved using structured concurrency. We should consider if we really need this in the api.

fabianfett · 2026-05-11T13:15:13Z

+        /// Maximum time allowed to receive the complete request body
+        /// after headers have been received. `nil` means no timeout.
+        public var readBody: Duration?


This is btw. not a connectiontimeout but more a request timeout.

fabianfett · 2026-05-11T13:16:12Z

+            idle: Duration? = Self.defaultIdle,
+            readHeader: Duration? = Self.defaultReadHeader,
+            readBody: Duration? = Self.defaultReadBody


if we add another timeout we have to add another initializer. Why not just add a .init() and users can set values using the vars?

fabianfett · 2026-05-11T13:21:14Z

+        let boundContext = NIOLoopBound(context, eventLoop: context.eventLoop)
+        self.scheduledTimeout = context.eventLoop.scheduleTask(in: self.timeout) {


I think we should use assumeIsolated() here over NIOLoopBound. This allows us to only check the correct EL once. When calling assumeIsolated()

fabianfett · 2026-05-11T13:22:43Z

+/// - When `.end` is received, the body timeout is cancelled.
+///
+/// If either timeout fires, the connection is closed.
+final class RequestTimeoutHandler: ChannelInboundHandler, RemovableChannelHandler {


I can make the change if we want to sacrifice some readability for performance in that case though.

This would mean that we currently also translate H2 to H1 to add timeouts. I think we need specialized handlers here.

Add connection backpressure and timeouts

2fb47cc

gjcairo added the ⚠️ semver/major Breaks existing public API. label Mar 30, 2026

Fix warnings

5e0742e

gjcairo marked this pull request as ready for review March 30, 2026 14:36

gjcairo requested a review from aryan-25 March 30, 2026 14:36

aryan-25 reviewed Mar 31, 2026

View reviewed changes

gjcairo added 2 commits March 31, 2026 13:11

Fix small bugs in ConnectionLimitHandler

7569b3b

Fix warnings

12ebb34

fabianfett reviewed Apr 1, 2026

View reviewed changes

Comment thread Sources/NIOHTTPServer/ConnectionLimitHandler.swift

fabianfett reviewed Apr 1, 2026

View reviewed changes

fabianfett requested changes Apr 1, 2026

View reviewed changes

gjcairo added 10 commits April 14, 2026 11:20

Don't coalesce SwiftConfiguration keys

4944464

Move timeout configuration funcs to Channel extension

92ea289

Consolidate timeout handlers

42f3ecf

Avoid unnecessary EL hop

676da34

Only forward read on ConnectionLimitHandler if there was a read pending

26ee0d3

EL hop was actually necessary

4495e5b

Remove some duplication in tests

0a312ab

Add missing conformance to handler

6915d49

Fix SwiftConfiguration tests

3aeb28d

Fix E2E tests

0a86d08

gjcairo requested review from aryan-25 and fabianfett April 14, 2026 15:52

fabianfett reviewed Apr 21, 2026

View reviewed changes

Comment thread Sources/NIOHTTPServer/Configuration/NIOHTTPServer+SwiftConfiguration.swift Outdated

fabianfett reviewed Apr 21, 2026

View reviewed changes

Comment thread Sources/NIOHTTPServer/NIOHTTPServer.swift Outdated

fabianfett reviewed Apr 21, 2026

View reviewed changes

Comment thread Sources/NIOHTTPServer/NIOHTTPServer+SecureUpgrade.swift Outdated

fabianfett reviewed Apr 21, 2026

View reviewed changes

PR changes

0634fee

gjcairo requested a review from fabianfett May 11, 2026 10:21

fabianfett reviewed May 11, 2026

View reviewed changes

		@@ -24,6 +25,9 @@ enum NIOHTTPServerConfigurationError: Error, CustomStringConvertible {

		case .incompatibleTransportSecurity:
		"Invalid configuration: only HTTP/1.1 can be served over plaintext. `transportSecurity` must be set to (m)TLS for serving HTTP/2."

		let boundContext = NIOLoopBound(context, eventLoop: context.eventLoop)
		self.scheduledTimeout = context.eventLoop.scheduleTask(in: self.timeout) {

Conversation

gjcairo commented Mar 30, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gjcairo May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fabianfett left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

gjcairo May 6, 2026 •

edited

Loading