diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2IdleHandler.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2IdleHandler.swift index 8978e1a86..c522b2425 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2IdleHandler.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2IdleHandler.swift @@ -168,7 +168,7 @@ extension HTTP2IdleHandler { mutating func settingsReceived(_ settings: HTTP2Settings) -> Action { switch self.state { - case .initialized, .closed: + case .initialized: preconditionFailure("Invalid state: \(self.state)") case .connected: @@ -188,12 +188,17 @@ extension HTTP2IdleHandler { case .closing: return .nothing + + case .closed: + // We may receive a Settings frame after we have called connection close, because of + // packages being delivered from the incoming buffer. + return .nothing } } mutating func goAwayReceived() -> Action { switch self.state { - case .initialized, .closed: + case .initialized: preconditionFailure("Invalid state: \(self.state)") case .connected: @@ -206,6 +211,11 @@ extension HTTP2IdleHandler { case .closing: return .notifyConnectionGoAwayReceived(close: false) + + case .closed: + // We may receive a GoAway frame after we have called connection close, because of + // packages being delivered from the incoming buffer. + return .nothing } } @@ -234,6 +244,9 @@ extension HTTP2IdleHandler { mutating func streamCreated() -> Action { switch self.state { + case .initialized, .connected: + preconditionFailure("Invalid state: \(self.state)") + case .active(var openStreams, let maxStreams): openStreams += 1 self.state = .active(openStreams: openStreams, maxStreams: maxStreams) @@ -246,13 +259,18 @@ extension HTTP2IdleHandler { self.state = .closing(openStreams: openStreams, maxStreams: maxStreams) return .nothing - case .initialized, .connected, .closed: - preconditionFailure("Invalid state: \(self.state)") + case .closed: + // We may receive a events after we have called connection close, because of + // internal races. We should just ignore these cases. + return .nothing } } mutating func streamClosed() -> Action { switch self.state { + case .initialized, .connected: + preconditionFailure("Invalid state: \(self.state)") + case .active(var openStreams, let maxStreams): openStreams -= 1 assert(openStreams >= 0) @@ -269,8 +287,10 @@ extension HTTP2IdleHandler { self.state = .closing(openStreams: openStreams, maxStreams: maxStreams) return .nothing - case .initialized, .connected, .closed: - preconditionFailure("Invalid state: \(self.state)") + case .closed: + // We may receive a events after we have called connection close, because of + // internal races. We should just ignore these cases. + return .nothing } } } diff --git a/Tests/AsyncHTTPClientTests/HTTP2IdleHandlerTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTP2IdleHandlerTests+XCTest.swift index 5c7021e23..1b9558105 100644 --- a/Tests/AsyncHTTPClientTests/HTTP2IdleHandlerTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTP2IdleHandlerTests+XCTest.swift @@ -35,6 +35,7 @@ extension HTTP2IdleHandlerTests { ("testCloseEventWhileNoOpenStreams", testCloseEventWhileNoOpenStreams), ("testCloseEventWhileThereAreOpenStreams", testCloseEventWhileThereAreOpenStreams), ("testGoAwayWhileThereAreOpenStreams", testGoAwayWhileThereAreOpenStreams), + ("testReceiveSettingsAndGoAwayAfterClientSideClose", testReceiveSettingsAndGoAwayAfterClientSideClose), ] } } diff --git a/Tests/AsyncHTTPClientTests/HTTP2IdleHandlerTests.swift b/Tests/AsyncHTTPClientTests/HTTP2IdleHandlerTests.swift index 57560b659..355969c6a 100644 --- a/Tests/AsyncHTTPClientTests/HTTP2IdleHandlerTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTP2IdleHandlerTests.swift @@ -225,6 +225,32 @@ class HTTP2IdleHandlerTests: XCTestCase { } } } + + func testReceiveSettingsAndGoAwayAfterClientSideClose() { + let delegate = MockHTTP2IdleHandlerDelegate() + let idleHandler = HTTP2IdleHandler(delegate: delegate, logger: Logger(label: "test")) + let embedded = EmbeddedChannel(handlers: [idleHandler]) + XCTAssertNoThrow(try embedded.connect(to: .makeAddressResolvingHost("localhost", port: 0)).wait()) + + let settingsFrame = HTTP2Frame(streamID: 0, payload: .settings(.settings([.init(parameter: .maxConcurrentStreams, value: 10)]))) + XCTAssertEqual(delegate.maxStreams, nil) + XCTAssertNoThrow(try embedded.writeInbound(settingsFrame)) + XCTAssertEqual(delegate.maxStreams, 10) + + XCTAssertTrue(embedded.isActive) + embedded.pipeline.triggerUserOutboundEvent(HTTPConnectionEvent.shutdownRequested, promise: nil) + XCTAssertFalse(embedded.isActive) + + let newSettingsFrame = HTTP2Frame(streamID: 0, payload: .settings(.settings([.init(parameter: .maxConcurrentStreams, value: 20)]))) + XCTAssertEqual(delegate.maxStreams, 10) + XCTAssertNoThrow(try embedded.writeInbound(newSettingsFrame)) + XCTAssertEqual(delegate.maxStreams, 10, "Expected message to not be forwarded.") + + let goAwayFrame = HTTP2Frame(streamID: HTTP2StreamID(0), payload: .goAway(lastStreamID: 2, errorCode: .http11Required, opaqueData: nil)) + XCTAssertEqual(delegate.goAwayReceived, false) + XCTAssertNoThrow(try embedded.writeInbound(goAwayFrame)) + XCTAssertEqual(delegate.goAwayReceived, false, "Expected go away to not be forwarded.") + } } class MockHTTP2IdleHandlerDelegate: HTTP2IdleHandlerDelegate {