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-31T08:15:15Z

+    ///
+    /// ## Configuration keys:
+    /// - `idle` (int, optional, default: 60): Maximum time in seconds a connection can remain idle.
+    ///   Set to `null` to disable.


"Set to null to disable" isn't true; each of these options will get nil-coalesced to the default value if the key isn't specified in the config.

I changed the docs but also removed the coalescing from the SwiftConfiguration initialiser - I think it's clearer that if you set things to nil then you don't want anything.

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:25:07Z

+                let context = loopBoundContext.value
+                self.activeConnections -= 1
+                if self.activeConnections <= self.maxConnections {
+                    context.read()


I don't think we should read here automatically. I think we should only read here, if we have seen a read call before that we didn't immediately forward. Other channels in the pipeline might want to stop backpressure for their own reasons. This auto read call assumes we are the only channel in the pipeline.

Good point, will make this change

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?

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.

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.

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

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

		@@ -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."

Conversation

gjcairo commented Mar 30, 2026

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!

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!

fabianfett Apr 1, 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!

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!

Uh oh!

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

Development

Uh oh!

3 participants

fabianfett Apr 1, 2026 •

edited

Loading

gjcairo May 6, 2026 •

edited

Loading