From e9db0531d14fa52984312084aee1e7bcbb617f2a Mon Sep 17 00:00:00 2001 From: Aryan Shah Date: Mon, 3 Aug 2026 18:16:01 +0100 Subject: [PATCH] Do not re-schedule request head timeout for HTTP/2 and HTTP/3 --- .../NIOHTTPServer/NIOHTTPServer+HTTP1_1.swift | 5 +- .../NIOHTTPServer/NIOHTTPServer+HTTP3.swift | 5 +- .../NIOHTTPServer+SecureUpgrade.swift | 3 +- Sources/NIOHTTPServer/NIOHTTPServer.swift | 28 +++++- Sources/NIOHTTPServer/TimeoutHandlers.swift | 19 +++- .../TimeoutHandlerTests.swift | 96 ++++++++++++++++--- 6 files changed, 132 insertions(+), 24 deletions(-) diff --git a/Sources/NIOHTTPServer/NIOHTTPServer+HTTP1_1.swift b/Sources/NIOHTTPServer/NIOHTTPServer+HTTP1_1.swift index 57a98b2..da735d5 100644 --- a/Sources/NIOHTTPServer/NIOHTTPServer+HTTP1_1.swift +++ b/Sources/NIOHTTPServer/NIOHTTPServer+HTTP1_1.swift @@ -172,7 +172,10 @@ extension NIOHTTPServer { channel.pipeline.configureHTTPServerPipeline().flatMapThrowing { try channel.pipeline.syncOperations.addHandler(HTTP1ToHTTPServerCodec(secure: isSecure)) try channel.pipeline.syncOperations.addHandler(HTTPKeepAliveHandler()) - try channel.pipeline.syncOperations.addTimeoutHandlers(self.configuration.connectionTimeouts) + try channel.pipeline.syncOperations.addTimeoutHandlers( + self.configuration.connectionTimeouts, + expectMultipleRequests: true + ) return try NIOAsyncChannel( wrappingChannelSynchronously: channel, diff --git a/Sources/NIOHTTPServer/NIOHTTPServer+HTTP3.swift b/Sources/NIOHTTPServer/NIOHTTPServer+HTTP3.swift index b531fc8..edff7e2 100644 --- a/Sources/NIOHTTPServer/NIOHTTPServer+HTTP3.swift +++ b/Sources/NIOHTTPServer/NIOHTTPServer+HTTP3.swift @@ -262,7 +262,10 @@ extension NIOHTTPServer { /// Configures the pipeline for an inbound HTTP/3 stream channel and wraps it in a `NIOAsyncChannel`. func setupHTTP3Stream(streamChannel: any Channel) throws -> NIOAsyncChannel { - try streamChannel.pipeline.syncOperations.addReadTimeoutHandlers(self.configuration.connectionTimeouts) + try streamChannel.pipeline.syncOperations.addReadTimeoutHandlers( + self.configuration.connectionTimeouts, + expectMultipleRequests: false + ) return try NIOAsyncChannel( wrappingChannelSynchronously: streamChannel, diff --git a/Sources/NIOHTTPServer/NIOHTTPServer+SecureUpgrade.swift b/Sources/NIOHTTPServer/NIOHTTPServer+SecureUpgrade.swift index 5a5bbc4..db88208 100644 --- a/Sources/NIOHTTPServer/NIOHTTPServer+SecureUpgrade.swift +++ b/Sources/NIOHTTPServer/NIOHTTPServer+SecureUpgrade.swift @@ -291,7 +291,8 @@ extension NIOHTTPServer { // Add read header and body timeouts per-stream for HTTP/2 try http2StreamChannel.pipeline.syncOperations.addReadTimeoutHandlers( - self.configuration.connectionTimeouts + self.configuration.connectionTimeouts, + expectMultipleRequests: false ) return try NIOAsyncChannel( diff --git a/Sources/NIOHTTPServer/NIOHTTPServer.swift b/Sources/NIOHTTPServer/NIOHTTPServer.swift index 37c0bf2..0a90557 100644 --- a/Sources/NIOHTTPServer/NIOHTTPServer.swift +++ b/Sources/NIOHTTPServer/NIOHTTPServer.swift @@ -442,9 +442,17 @@ extension ChannelPipeline.SynchronousOperations { /// Adds timeout handlers (idle, read header, read body) to the channel pipeline. /// /// Only handlers for non-nil timeouts are installed. - func addTimeoutHandlers(_ timeouts: NIOHTTPServerConfiguration.ConnectionTimeouts) throws { + /// + /// - Parameters: + /// - timeouts: The configured connection timeouts. Only handlers for non-nil timeouts are installed. + /// - expectMultipleRequests: Whether the channel can receive more than one request. Pass `true` for an HTTP/1.1 + /// connection channel (for keep-alive), and `false` for an HTTP/2 or HTTP/3 stream channel. + func addTimeoutHandlers( + _ timeouts: NIOHTTPServerConfiguration.ConnectionTimeouts, + expectMultipleRequests: Bool + ) throws { try self.addIdleTimeoutHandlers(timeouts) - try self.addReadTimeoutHandlers(timeouts) + try self.addReadTimeoutHandlers(timeouts, expectMultipleRequests: expectMultipleRequests) } /// Adds the connection idle timeout handler to the channel. Used by HTTP/1.1 connection channels. HTTP/2 delegates @@ -459,12 +467,24 @@ extension ChannelPipeline.SynchronousOperations { } /// Adds header and body read timeout handlers to the channel. - func addReadTimeoutHandlers(_ timeouts: NIOHTTPServerConfiguration.ConnectionTimeouts) throws { + /// + /// - Parameters: + /// - timeouts: The configured connection timeouts. No handler is installed if both read timeouts are `nil`. + /// - expectMultipleRequests: Whether the channel can receive more than one request. Pass `true` for an HTTP/1.1 + /// connection channel (for keep-alive), and `false` for an HTTP/2 or HTTP/3 stream channel. + func addReadTimeoutHandlers( + _ timeouts: NIOHTTPServerConfiguration.ConnectionTimeouts, + expectMultipleRequests: Bool + ) throws { let readHeader = timeouts.readHeader.map { TimeAmount($0) } let readBody = timeouts.readBody.map { TimeAmount($0) } if readHeader != nil || readBody != nil { try self.addHandler( - RequestTimeoutHandler(readHeaderTimeout: readHeader, readBodyTimeout: readBody) + RequestTimeoutHandler( + readHeaderTimeout: readHeader, + readBodyTimeout: readBody, + expectMultipleRequests: expectMultipleRequests + ) ) } } diff --git a/Sources/NIOHTTPServer/TimeoutHandlers.swift b/Sources/NIOHTTPServer/TimeoutHandlers.swift index 0d2c93d..c40dc0e 100644 --- a/Sources/NIOHTTPServer/TimeoutHandlers.swift +++ b/Sources/NIOHTTPServer/TimeoutHandlers.swift @@ -85,9 +85,8 @@ final class ConnectionIdleTimeoutHandler: ChannelDuplexHandler, RemovableChannel /// State machine: /// - On channel active, a header timeout is scheduled (if configured). /// - When `.head` is received, the header timeout is cancelled and a body timeout is scheduled (if configured). -/// - When `.end` is received, the body timeout is cancelled and the header timeout is rescheduled so that the next -/// request on a keep-alive connection is also protected. (For HTTP/2 and HTTP/3 streams this is a no-op in practice: -/// each stream sees only one request and is closed shortly after `.end`.) +/// - When `.end` is received, the body timeout is cancelled and, for HTTP/1.1, the header timeout is rescheduled so +/// that the next request on a keep-alive connection is also protected. /// /// If either timeout fires, the connection is closed. final class RequestTimeoutHandler: ChannelInboundHandler, RemovableChannelHandler { @@ -97,9 +96,19 @@ final class RequestTimeoutHandler: ChannelInboundHandler, RemovableChannelHandle private let readBodyTimeout: TimeAmount? private var scheduledTimeout: Scheduled? - init(readHeaderTimeout: TimeAmount?, readBodyTimeout: TimeAmount?) { + private let expectMultipleRequests: Bool + + /// - Parameters: + /// - readHeaderTimeout: How long to wait for a request head to arrive before closing the channel. Pass `nil` to + /// not enforce a request header timeout. + /// - readBodyTimeout: How long to wait for a request body to complete (once the head has arrived) before closing + /// the channel. Pass `nil` to not enforce a request body timeout. + /// - expectMultipleRequests: Whether the channel can receive more than one request. Pass `true` for an HTTP/1.1 + /// connection channel (for keep-alive), or `false` for an HTTP/2 or HTTP/3 stream channel. + init(readHeaderTimeout: TimeAmount?, readBodyTimeout: TimeAmount?, expectMultipleRequests: Bool) { self.readHeaderTimeout = readHeaderTimeout self.readBodyTimeout = readBodyTimeout + self.expectMultipleRequests = expectMultipleRequests } func handlerAdded(context: ChannelHandlerContext) { @@ -131,7 +140,7 @@ final class RequestTimeoutHandler: ChannelInboundHandler, RemovableChannelHandle self.scheduledTimeout?.cancel() self.scheduledTimeout = nil // Re-arm the header timer so the next request on this connection is also protected. - if let readHeaderTimeout { + if self.expectMultipleRequests, let readHeaderTimeout { self.scheduleTimeout(readHeaderTimeout, context: context) } } diff --git a/Tests/NIOHTTPServerTests/TimeoutHandlerTests.swift b/Tests/NIOHTTPServerTests/TimeoutHandlerTests.swift index bd164fa..44fd295 100644 --- a/Tests/NIOHTTPServerTests/TimeoutHandlerTests.swift +++ b/Tests/NIOHTTPServerTests/TimeoutHandlerTests.swift @@ -161,7 +161,11 @@ struct RequestTimeoutHandlerTests { @Test("Headers received within timeout — connection stays open") func headersReceivedWithinTimeout() throws { let channel = EmbeddedChannel() - let handler = RequestTimeoutHandler(readHeaderTimeout: .seconds(5), readBodyTimeout: nil) + let handler = RequestTimeoutHandler( + readHeaderTimeout: .seconds(5), + readBodyTimeout: nil, + expectMultipleRequests: true + ) try channel.pipeline.syncOperations.addHandler(handler) try channel.connect(to: .init(ipAddress: "127.0.0.1", port: 8080)).wait() @@ -177,7 +181,11 @@ struct RequestTimeoutHandlerTests { @Test("Headers not received within timeout — connection closed") func headersNotReceivedWithinTimeout() throws { let channel = EmbeddedChannel() - let handler = RequestTimeoutHandler(readHeaderTimeout: .seconds(5), readBodyTimeout: nil) + let handler = RequestTimeoutHandler( + readHeaderTimeout: .seconds(5), + readBodyTimeout: nil, + expectMultipleRequests: true + ) try channel.pipeline.syncOperations.addHandler(handler) try channel.connect(to: .init(ipAddress: "127.0.0.1", port: 8080)).wait() @@ -192,7 +200,11 @@ struct RequestTimeoutHandlerTests { @Test("Body completed within timeout — connection stays open") func bodyCompletedWithinTimeout() throws { let channel = EmbeddedChannel() - let handler = RequestTimeoutHandler(readHeaderTimeout: nil, readBodyTimeout: .seconds(5)) + let handler = RequestTimeoutHandler( + readHeaderTimeout: nil, + readBodyTimeout: .seconds(5), + expectMultipleRequests: true + ) try channel.pipeline.syncOperations.addHandler(handler) try channel.connect(to: .init(ipAddress: "127.0.0.1", port: 8080)).wait() @@ -210,7 +222,11 @@ struct RequestTimeoutHandlerTests { @Test("Body not completed within timeout — connection closed") func bodyNotCompletedWithinTimeout() throws { let channel = EmbeddedChannel() - let handler = RequestTimeoutHandler(readHeaderTimeout: nil, readBodyTimeout: .seconds(5)) + let handler = RequestTimeoutHandler( + readHeaderTimeout: nil, + readBodyTimeout: .seconds(5), + expectMultipleRequests: true + ) try channel.pipeline.syncOperations.addHandler(handler) try channel.connect(to: .init(ipAddress: "127.0.0.1", port: 8080)).wait() @@ -226,7 +242,11 @@ struct RequestTimeoutHandlerTests { @Test("Body parts do not reset timeout") func bodyPartsDoNotResetTimeout() throws { let channel = EmbeddedChannel() - let handler = RequestTimeoutHandler(readHeaderTimeout: nil, readBodyTimeout: .seconds(5)) + let handler = RequestTimeoutHandler( + readHeaderTimeout: nil, + readBodyTimeout: .seconds(5), + expectMultipleRequests: true + ) try channel.pipeline.syncOperations.addHandler(handler) try channel.connect(to: .init(ipAddress: "127.0.0.1", port: 8080)).wait() @@ -251,7 +271,11 @@ struct RequestTimeoutHandlerTests { @Test("Both timeouts configured — header then body") func bothTimeoutsHeaderThenBody() throws { let channel = EmbeddedChannel() - let handler = RequestTimeoutHandler(readHeaderTimeout: .seconds(5), readBodyTimeout: .seconds(10)) + let handler = RequestTimeoutHandler( + readHeaderTimeout: .seconds(5), + readBodyTimeout: .seconds(10), + expectMultipleRequests: true + ) try channel.pipeline.syncOperations.addHandler(handler) try channel.connect(to: .init(ipAddress: "127.0.0.1", port: 8080)).wait() @@ -280,7 +304,11 @@ struct RequestTimeoutHandlerTests { @Test("Both timeouts configured — header timeout fires") func bothTimeoutsHeaderFires() throws { let channel = EmbeddedChannel() - let handler = RequestTimeoutHandler(readHeaderTimeout: .seconds(5), readBodyTimeout: .seconds(10)) + let handler = RequestTimeoutHandler( + readHeaderTimeout: .seconds(5), + readBodyTimeout: .seconds(10), + expectMultipleRequests: true + ) try channel.pipeline.syncOperations.addHandler(handler) try channel.connect(to: .init(ipAddress: "127.0.0.1", port: 8080)).wait() @@ -293,7 +321,11 @@ struct RequestTimeoutHandlerTests { @Test("Both timeouts configured — body timeout fires") func bothTimeoutsBodyFires() throws { let channel = EmbeddedChannel() - let handler = RequestTimeoutHandler(readHeaderTimeout: .seconds(5), readBodyTimeout: .seconds(10)) + let handler = RequestTimeoutHandler( + readHeaderTimeout: .seconds(5), + readBodyTimeout: .seconds(10), + expectMultipleRequests: true + ) try channel.pipeline.syncOperations.addHandler(handler) try channel.connect(to: .init(ipAddress: "127.0.0.1", port: 8080)).wait() @@ -309,7 +341,11 @@ struct RequestTimeoutHandlerTests { @Test("Header timeout is re-armed after end so subsequent requests are protected") func headerTimeoutRearmedAfterEnd() throws { let channel = EmbeddedChannel() - let handler = RequestTimeoutHandler(readHeaderTimeout: .seconds(5), readBodyTimeout: nil) + let handler = RequestTimeoutHandler( + readHeaderTimeout: .seconds(5), + readBodyTimeout: nil, + expectMultipleRequests: true + ) try channel.pipeline.syncOperations.addHandler(handler) try channel.connect(to: .init(ipAddress: "127.0.0.1", port: 8080)).wait() @@ -325,12 +361,40 @@ struct RequestTimeoutHandlerTests { #expect(!channel.isActive) } + @Test("Header timeout is not re-scheduled after request end when expectMultipleRequests=false") + func headerTimeoutNotRescheduledAfterEndWhenSingleRequestExpected() throws { + let channel = EmbeddedChannel() + let handler = RequestTimeoutHandler( + readHeaderTimeout: .seconds(5), + readBodyTimeout: nil, + expectMultipleRequests: false + ) + try channel.pipeline.syncOperations.addHandler(handler) + + try channel.connect(to: .init(ipAddress: "127.0.0.1", port: 8080)).wait() + + // Write a single request on this channel within the timeout window. + let head = HTTPRequest(method: .get, scheme: "http", authority: "", path: "/") + try channel.writeInbound(HTTPRequestPart.head(head)) + try channel.writeInbound(HTTPRequestPart.end(nil)) + + // Advance time forward beyond the configured timeout. + channel.embeddedEventLoop.advanceTime(by: .seconds(6)) + + // The handler shouldn't close the channel if the timeout wasn't rescheduled. + #expect(channel.isActive) + } + // MARK: - Cleanup @Test("Cleanup on handler removal during header phase") func cleanupOnHandlerRemovalDuringHeaderPhase() throws { let channel = EmbeddedChannel() - let handler = RequestTimeoutHandler(readHeaderTimeout: .seconds(5), readBodyTimeout: .seconds(5)) + let handler = RequestTimeoutHandler( + readHeaderTimeout: .seconds(5), + readBodyTimeout: .seconds(5), + expectMultipleRequests: true + ) try channel.pipeline.syncOperations.addHandler(handler) try channel.connect(to: .init(ipAddress: "127.0.0.1", port: 8080)).wait() @@ -345,7 +409,11 @@ struct RequestTimeoutHandlerTests { @Test("Cleanup on handler removal during body phase") func cleanupOnHandlerRemovalDuringBodyPhase() throws { let channel = EmbeddedChannel() - let handler = RequestTimeoutHandler(readHeaderTimeout: .seconds(5), readBodyTimeout: .seconds(5)) + let handler = RequestTimeoutHandler( + readHeaderTimeout: .seconds(5), + readBodyTimeout: .seconds(5), + expectMultipleRequests: true + ) try channel.pipeline.syncOperations.addHandler(handler) try channel.connect(to: .init(ipAddress: "127.0.0.1", port: 8080)).wait() @@ -372,7 +440,11 @@ struct RequestTimeoutHandlerTests { try channel.connect(to: .init(ipAddress: "127.0.0.1", port: 8080)).wait() #expect(channel.isActive) - let handler = RequestTimeoutHandler(readHeaderTimeout: .seconds(5), readBodyTimeout: nil) + let handler = RequestTimeoutHandler( + readHeaderTimeout: .seconds(5), + readBodyTimeout: nil, + expectMultipleRequests: true + ) try channel.pipeline.syncOperations.addHandler(handler) // The header timer must have been armed in `handlerAdded`, so the connection closes once the