From 909b41cefa7f680bc355066131494f34232e837a Mon Sep 17 00:00:00 2001 From: Gus Cairo Date: Wed, 27 May 2026 10:23:55 +0100 Subject: [PATCH 1/2] Add missing HTTPKeepAliveHandler on H1 secure upgrade path --- .../NIOHTTPServer+SecureUpgrade.swift | 1 + .../HTTPKeepAliveHandlerTests.swift | 86 +++++++++++++++++++ .../NIOHTTPServerTests.swift | 9 +- 3 files changed, 95 insertions(+), 1 deletion(-) diff --git a/Sources/NIOHTTPServer/NIOHTTPServer+SecureUpgrade.swift b/Sources/NIOHTTPServer/NIOHTTPServer+SecureUpgrade.swift index 0fd8451..7cfac9b 100644 --- a/Sources/NIOHTTPServer/NIOHTTPServer+SecureUpgrade.swift +++ b/Sources/NIOHTTPServer/NIOHTTPServer+SecureUpgrade.swift @@ -205,6 +205,7 @@ extension NIOHTTPServer { channel.pipeline.configureHTTPServerPipeline().flatMap { _ in channel.eventLoop.makeCompletedFuture { try channel.pipeline.syncOperations.addHandler(HTTP1ToHTTPServerCodec(secure: true)) + try channel.pipeline.syncOperations.addHandler(HTTPKeepAliveHandler()) return try NIOAsyncChannel( wrappingChannelSynchronously: channel, diff --git a/Tests/NIOHTTPServerTests/HTTPKeepAliveHandlerTests.swift b/Tests/NIOHTTPServerTests/HTTPKeepAliveHandlerTests.swift index 7b3aac8..70ec9ac 100644 --- a/Tests/NIOHTTPServerTests/HTTPKeepAliveHandlerTests.swift +++ b/Tests/NIOHTTPServerTests/HTTPKeepAliveHandlerTests.swift @@ -491,4 +491,90 @@ struct HTTPKeepAliveHandlerTests { } ) } + + /// Verifies that the keep-alive handler is also present on the secure upgrade + /// HTTP/1.1 pipeline. This mirrors `testShortResponseBeforeRequestEnd` but runs + /// over TLS: if the keep-alive handler isn't wired into the secure pipeline, + /// the response head will be flushed without `Connection: close` and this test + /// will fail. + @available(anyAppleOS 26.0, *) + @Test("Server sends head+end before request .end over TLS — Connection: close in header") + func testShortResponseBeforeRequestEndOverTLS() async throws { + let serverChain = try TestCA.makeSelfSignedChain() + let server = NIOHTTPServer( + logger: self.serverLogger, + configuration: try .init( + bindTarget: .hostAndPort(host: "127.0.0.1", port: 0), + supportedHTTPVersions: [.http1_1], + transportSecurity: .tls( + credentials: .inMemory( + certificateChain: serverChain.chain, + privateKey: serverChain.privateKey + ) + ) + ) + ) + + try await NIOHTTPServerTests.withServer( + server: server, + serverHandler: HTTPServerClosureRequestHandler { _, _, reader, sender in + let _ = try await reader.consumeAndConclude { partsReader in + var partsReader = partsReader + try await partsReader.read { _ in } + } + let writer = try await sender.send( + .init(status: .ok, headerFields: [.contentLength: "0"]) + ) + try await writer.writeAndConclude("".utf8.span, finalElement: nil) + }, + body: { serverAddress in + let clientChannel = try await ClientBootstrap(group: .singletonMultiThreadedEventLoopGroup) + .connectToTestSecureUpgradeHTTPServer( + at: serverAddress, + trustRoots: serverChain.chain, + applicationProtocol: HTTPVersion.http1_1.alpnIdentifier + ) + let client = try await NIOHTTPServerTests.unwrapNegotiatedChannel(clientChannel, .http1_1) + + try await client.executeThenClose { inbound, outbound in + try await outbound.write( + .head(.init(method: .post, scheme: "https", authority: "", path: "/")) + ) + try await outbound.write(.body(ByteBuffer(string: "x"))) + + var responseIterator = inbound.makeAsyncIterator() + let headPart = try await responseIterator.next() + guard case .head(let response) = headPart else { + Issue.record("Expected .head, got \(String(describing: headPart))") + return + } + #expect(response.status == .ok) + #expect( + response.headerFields[.connection] == "close", + "Expected Connection: close, got headers: \(response.headerFields)" + ) + + var sawEnd = false + while !sawEnd { + let part = try await responseIterator.next() + switch part { + case .body: + continue + case .end: + sawEnd = true + case .none: + Issue.record("Stream ended before response .end") + return + case .head: + Issue.record("Unexpected second .head: \(String(describing: part))") + return + } + } + + let next = try await responseIterator.next() + #expect(next == nil, "Expected channel to be closed; got \(String(describing: next))") + } + } + ) + } } diff --git a/Tests/NIOHTTPServerTests/NIOHTTPServerTests.swift b/Tests/NIOHTTPServerTests/NIOHTTPServerTests.swift index efb78aa..d314fc3 100644 --- a/Tests/NIOHTTPServerTests/NIOHTTPServerTests.swift +++ b/Tests/NIOHTTPServerTests/NIOHTTPServerTests.swift @@ -356,8 +356,15 @@ struct NIOHTTPServerTests { try await outbound.write(.head(.init(method: .post, scheme: "https", authority: "", path: "/"))) var responseIterator = inbound.makeAsyncIterator() + // For HTTP/1.1, the keep-alive handler flushes the response head with + // `Connection: close` because a body part is written before the request + // `.end` arrives. HTTP/2 has no equivalent header. + var expectedHead = Self.responseHead(status: .ok, for: httpVersion) + if httpVersion == .http1_1 { + expectedHead.headerFields[.connection] = "close" + } let head = try await responseIterator.next() - #expect(head == .head(Self.responseHead(status: .ok, for: httpVersion))) + #expect(head == .head(expectedHead)) for i in 1...5 { let body = ByteBuffer(bytes: [UInt8(i)]) From 76b18fb23408a6a657bdb7957a3941a6198560e8 Mon Sep 17 00:00:00 2001 From: Gus Cairo Date: Wed, 27 May 2026 15:53:18 +0100 Subject: [PATCH 2/2] Remove duplicated method --- .../NIOHTTPServer/NIOHTTPServer+HTTP1_1.swift | 12 ++++--- .../NIOHTTPServer+SecureUpgrade.swift | 36 ++++++++----------- .../TestingChannelServer+HTTP1.swift | 5 +-- 3 files changed, 24 insertions(+), 29 deletions(-) diff --git a/Sources/NIOHTTPServer/NIOHTTPServer+HTTP1_1.swift b/Sources/NIOHTTPServer/NIOHTTPServer+HTTP1_1.swift index 625078a..83affad 100644 --- a/Sources/NIOHTTPServer/NIOHTTPServer+HTTP1_1.swift +++ b/Sources/NIOHTTPServer/NIOHTTPServer+HTTP1_1.swift @@ -83,12 +83,13 @@ extension NIOHTTPServer { case .hostAndPort(let host, let port): let serverChannel = try await bootstrap.bind(host: host, port: port) { channel in - self.setupHTTP1_1ConnectionChildChannel( + self.setupHTTP1_1Connection( channel: channel, asyncChannelConfiguration: .init( backPressureStrategy: .init(self.configuration.backpressureStrategy), isOutboundHalfClosureEnabled: true - ) + ), + isSecure: false ) } serverChannels.append(serverChannel) @@ -109,12 +110,13 @@ extension NIOHTTPServer { return serverChannels } - func setupHTTP1_1ConnectionChildChannel( + func setupHTTP1_1Connection( channel: any Channel, - asyncChannelConfiguration: NIOAsyncChannel.Configuration + asyncChannelConfiguration: NIOAsyncChannel.Configuration, + isSecure: Bool ) -> EventLoopFuture> { channel.pipeline.configureHTTPServerPipeline().flatMapThrowing { - try channel.pipeline.syncOperations.addHandler(HTTP1ToHTTPServerCodec(secure: false)) + try channel.pipeline.syncOperations.addHandler(HTTP1ToHTTPServerCodec(secure: isSecure)) try channel.pipeline.syncOperations.addHandler(HTTPKeepAliveHandler()) return try NIOAsyncChannel( diff --git a/Sources/NIOHTTPServer/NIOHTTPServer+SecureUpgrade.swift b/Sources/NIOHTTPServer/NIOHTTPServer+SecureUpgrade.swift index 7cfac9b..854da77 100644 --- a/Sources/NIOHTTPServer/NIOHTTPServer+SecureUpgrade.swift +++ b/Sources/NIOHTTPServer/NIOHTTPServer+SecureUpgrade.swift @@ -199,26 +199,7 @@ extension NIOHTTPServer { return serverChannels } - private func http1ConnectionInitializer( - channel: any Channel - ) -> EventLoopFuture> { - channel.pipeline.configureHTTPServerPipeline().flatMap { _ in - channel.eventLoop.makeCompletedFuture { - try channel.pipeline.syncOperations.addHandler(HTTP1ToHTTPServerCodec(secure: true)) - try channel.pipeline.syncOperations.addHandler(HTTPKeepAliveHandler()) - - return try NIOAsyncChannel( - wrappingChannelSynchronously: channel, - configuration: .init( - backPressureStrategy: .init(self.configuration.backpressureStrategy), - isOutboundHalfClosureEnabled: true - ) - ) - } - } - } - - private func http2ConnectionInitializer( + private func setupHTTP2Connection( channel: any Channel, configuration: NIOHTTPServerConfiguration.HTTP2 ) -> EventLoopFuture< @@ -303,10 +284,21 @@ extension NIOHTTPServer { NIOTypedApplicationProtocolNegotiationHandler { result in switch (result, http2Config) { case (.negotiated("http/1.1"), _): - return self.http1ConnectionInitializer(channel: channel).map { .http1_1($0) } + return self.setupHTTP1_1Connection( + channel: channel, + asyncChannelConfiguration: .init( + backPressureStrategy: .init(self.configuration.backpressureStrategy), + isOutboundHalfClosureEnabled: true + ), + isSecure: true + ) + .map { .http1_1($0) } case (.negotiated("h2"), .some(let http2Config)): - return self.http2ConnectionInitializer(channel: channel, configuration: http2Config).map { .http2($0) } + return self.setupHTTP2Connection( + channel: channel, + configuration: http2Config + ).map { .http2($0) } case (.negotiated, _), (.fallback, _): // The negotiated result was an unsupported protocol, or ALPN negotiation failed / never took place. diff --git a/Tests/NIOHTTPServerTests/Utilities/TestingChannelClientServer/TestingChannelServer+HTTP1.swift b/Tests/NIOHTTPServerTests/Utilities/TestingChannelClientServer/TestingChannelServer+HTTP1.swift index 3d90ff3..2d3c5f6 100644 --- a/Tests/NIOHTTPServerTests/Utilities/TestingChannelClientServer/TestingChannelServer+HTTP1.swift +++ b/Tests/NIOHTTPServerTests/Utilities/TestingChannelClientServer/TestingChannelServer+HTTP1.swift @@ -79,9 +79,10 @@ struct TestingChannelHTTP1Server { let serverTestConnectionChannel = try await NIOAsyncTestingChannel.createActiveChannel() // Set up the required channel handlers on `serverTestConnectionChannel` - let serverAsyncConnectionChannel = try await self.server.setupHTTP1_1ConnectionChildChannel( + let serverAsyncConnectionChannel = try await self.server.setupHTTP1_1Connection( channel: serverTestConnectionChannel, - asyncChannelConfiguration: .init() + asyncChannelConfiguration: .init(), + isSecure: false ).get() // Write the connection channel to the server channel to simulate an incoming connection