diff --git a/Tests/AsyncHTTPClientTests/HTTP2ClientRequestHandlerTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTP2ClientRequestHandlerTests+XCTest.swift index 8fa219838..221a63211 100644 --- a/Tests/AsyncHTTPClientTests/HTTP2ClientRequestHandlerTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTP2ClientRequestHandlerTests+XCTest.swift @@ -30,6 +30,7 @@ extension HTTP2ClientRequestHandlerTests { ("testIdleReadTimeout", testIdleReadTimeout), ("testIdleReadTimeoutIsCanceledIfRequestIsCanceled", testIdleReadTimeoutIsCanceledIfRequestIsCanceled), ("testWriteHTTPHeadFails", testWriteHTTPHeadFails), + ("testChannelBecomesNonWritableDuringHeaderWrite", testChannelBecomesNonWritableDuringHeaderWrite), ] } } diff --git a/Tests/AsyncHTTPClientTests/HTTP2ClientRequestHandlerTests.swift b/Tests/AsyncHTTPClientTests/HTTP2ClientRequestHandlerTests.swift index e67529ad8..5dfce3f9d 100644 --- a/Tests/AsyncHTTPClientTests/HTTP2ClientRequestHandlerTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTP2ClientRequestHandlerTests.swift @@ -345,4 +345,33 @@ class HTTP2ClientRequestHandlerTests: XCTestCase { XCTAssertEqual(embedded.isActive, false) } } + + func testChannelBecomesNonWritableDuringHeaderWrite() throws { + try XCTSkipIf(true, "this currently fails and will be fixed in follow up PR") + final class ChangeWritabilityOnFlush: ChannelOutboundHandler { + typealias OutboundIn = Any + func flush(context: ChannelHandlerContext) { + context.flush() + (context.channel as! EmbeddedChannel).isWritable = false + context.fireChannelWritabilityChanged() + } + } + let eventLoopGroup = EmbeddedEventLoopGroup(loops: 1) + let eventLoop = eventLoopGroup.next() as! EmbeddedEventLoop + let handler = HTTP2ClientRequestHandler( + eventLoop: eventLoop + ) + let channel = EmbeddedChannel(handlers: [ + ChangeWritabilityOnFlush(), + handler, + ], loop: eventLoop) + try channel.connect(to: .init(ipAddress: "127.0.0.1", port: 80)).wait() + + let request = MockHTTPExecutableRequest() + // non empty body is important to trigger this bug as we otherwise finish the request in a single flush + request.requestFramingMetadata.body = .fixedSize(1) + request.raiseErrorIfUnimplementedMethodIsCalled = false + channel.writeAndFlush(request, promise: nil) + XCTAssertEqual(request.events.map(\.kind), [.willExecuteRequest, .requestHeadSent]) + } } diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift b/Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift index e50dab3b6..ca24cba1c 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift @@ -329,17 +329,32 @@ internal final class HTTPBin where // supports http1.1 connections only, which can be either plain text or encrypted case http1_1(ssl: Bool = false, compress: Bool = false) // supports http1.1 and http2 connections which must be always encrypted - case http2(compress: Bool) + case http2( + compress: Bool = false, + settings: HTTP2Settings? = nil + ) // supports request decompression and http response compression var compress: Bool { switch self { case .refuse: return false - case .http1_1(ssl: _, compress: let compress), .http2(compress: let compress): + case .http1_1(ssl: _, compress: let compress), .http2(compress: let compress, _): return compress } } + + var httpSettings: HTTP2Settings { + switch self { + case .http1_1, .http2(_, nil), .refuse: + return [ + HTTP2Setting(parameter: .maxConcurrentStreams, value: 10), + HTTP2Setting(parameter: .maxHeaderListSize, value: HPACKDecoder.defaultMaxHeaderListSize), + ] + case .http2(_, .some(let customSettings)): + return customSettings + } + } } enum Proxy { @@ -565,11 +580,7 @@ internal final class HTTPBin where // Successful upgrade to HTTP/2. Let the user configure the pipeline. let http2Handler = NIOHTTP2Handler( mode: .server, - initialSettings: [ - // TODO: make max concurrent streams configurable - HTTP2Setting(parameter: .maxConcurrentStreams, value: 10), - HTTP2Setting(parameter: .maxHeaderListSize, value: HPACKDecoder.defaultMaxHeaderListSize), - ] + initialSettings: self.mode.httpSettings ) let multiplexer = HTTP2StreamMultiplexer( mode: .server, diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift index f9ddb1c8b..6e84f9d29 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift @@ -144,6 +144,7 @@ extension HTTPClientTests { ("testMassiveDownload", testMassiveDownload), ("testShutdownWithFutures", testShutdownWithFutures), ("testMassiveHeaderHTTP1", testMassiveHeaderHTTP1), + ("testMassiveHeaderHTTP2", testMassiveHeaderHTTP2), ] } } diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 8e5d5fbfa..54d854bf0 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -3378,4 +3378,33 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { XCTAssertNoThrow(try defaultClient.execute(request: request).wait()) } + + func testMassiveHeaderHTTP2() throws { + try XCTSkipIf(true, "this currently crashes and will be fixed in follow up PR") + let bin = HTTPBin(.http2(settings: [ + .init(parameter: .maxConcurrentStreams, value: 100), + .init(parameter: .maxHeaderListSize, value: 1024 * 256), + .init(parameter: .maxFrameSize, value: 1024 * 256), + ])) + defer { XCTAssertNoThrow(try bin.shutdown()) } + + let client = HTTPClient( + eventLoopGroupProvider: .shared(clientGroup), + configuration: .init(certificateVerification: .none) + ) + + defer { XCTAssertNoThrow(try client.syncShutdown()) } + + var request = try HTTPClient.Request(url: bin.baseURL, method: .POST) + // add ~200 KB header + let headerValue = String(repeating: "0", count: 1024) + for headerID in 0..<200 { + request.headers.replaceOrAdd(name: "larg-header-\(headerID)", value: headerValue) + } + + // non empty body is important to trigger this bug as we otherwise finish the request in a single flush + request.body = .byteBuffer(ByteBuffer(bytes: [0])) + + XCTAssertNoThrow(try client.execute(request: request).wait()) + } }