diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTP1.1/HTTP1ClientChannelHandler.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTP1.1/HTTP1ClientChannelHandler.swift index fcd59bc3d..15544dc4a 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/HTTP1.1/HTTP1ClientChannelHandler.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTP1.1/HTTP1ClientChannelHandler.swift @@ -153,8 +153,7 @@ final class HTTP1ClientChannelHandler: ChannelDuplexHandler { let action = self.state.runNewRequest( head: req.requestHead, - metadata: req.requestFramingMetadata, - ignoreUncleanSSLShutdown: req.requestOptions.ignoreUncleanSSLShutdown + metadata: req.requestFramingMetadata ) self.run(action, context: context) } diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTP1.1/HTTP1ConnectionStateMachine.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTP1.1/HTTP1ConnectionStateMachine.swift index 32cbe500c..614ba712a 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/HTTP1.1/HTTP1ConnectionStateMachine.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTP1.1/HTTP1ConnectionStateMachine.swift @@ -156,17 +156,13 @@ struct HTTP1ConnectionStateMachine { mutating func runNewRequest( head: HTTPRequestHead, - metadata: RequestFramingMetadata, - ignoreUncleanSSLShutdown: Bool + metadata: RequestFramingMetadata ) -> Action { guard case .idle = self.state else { preconditionFailure("Invalid state") } - var requestStateMachine = HTTPRequestStateMachine( - isChannelWritable: self.isChannelWritable, - ignoreUncleanSSLShutdown: ignoreUncleanSSLShutdown - ) + var requestStateMachine = HTTPRequestStateMachine(isChannelWritable: self.isChannelWritable) let action = requestStateMachine.startRequest(head: head, metadata: metadata) // by default we assume a persistent connection. however in `requestVerified`, we read the diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2ClientRequestHandler.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2ClientRequestHandler.swift index 48419d01a..09d2815f3 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2ClientRequestHandler.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2ClientRequestHandler.swift @@ -24,7 +24,7 @@ final class HTTP2ClientRequestHandler: ChannelDuplexHandler { private let eventLoop: EventLoop - private var state: HTTPRequestStateMachine = .init(isChannelWritable: false, ignoreUncleanSSLShutdown: false) { + private var state: HTTPRequestStateMachine = .init(isChannelWritable: false) { willSet { self.eventLoop.assertInEventLoop() } diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift index f723fa12e..32866697d 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift @@ -103,11 +103,8 @@ struct HTTPRequestStateMachine { private var isChannelWritable: Bool - private let ignoreUncleanSSLShutdown: Bool - - init(isChannelWritable: Bool, ignoreUncleanSSLShutdown: Bool) { + init(isChannelWritable: Bool) { self.isChannelWritable = isChannelWritable - self.ignoreUncleanSSLShutdown = ignoreUncleanSSLShutdown } mutating func startRequest(head: HTTPRequestHead, metadata: RequestFramingMetadata) -> Action { @@ -201,17 +198,45 @@ struct HTTPRequestStateMachine { self.state = .failed(error) return .failRequest(error, .none) - case .running(.streaming, .receivingBody), - .running(.endSent, .receivingBody) - where error as? NIOSSLError == .uncleanShutdown && self.ignoreUncleanSSLShutdown == true: + case .running(.streaming, .waitingForHead), + .running(.endSent, .waitingForHead) where error as? NIOSSLError == .uncleanShutdown: + // if we received a NIOSSL.uncleanShutdown before we got an answer we should handle + // this like a normal connection close. We will receive a call to channelInactive after + // this error. return .wait + case .running(.streaming, .receivingBody(let responseHead, _)), + .running(.endSent, .receivingBody(let responseHead, _)) where error as? NIOSSLError == .uncleanShutdown: + // This code is only reachable for request and responses, which we expect to have a body. + // We depend on logic from the HTTPResponseDecoder here. The decoder will emit an + // HTTPResponsePart.end right after the HTTPResponsePart.head, for every request with a + // CONNECT or HEAD method and every response with a 1xx, 204 or 304 response status. + // + // For this reason we only need to check the "content-length" or "transfer-encoding" + // headers here to determine if we are potentially in an EOF terminated response. + + if responseHead.headers.contains(name: "content-length") || responseHead.headers.contains(name: "transfer-encoding") { + // If we have already received the response head, the parser will ensure that we + // receive a complete response, if the content-length or transfer-encoding header + // was set. In this case we can ignore the NIOSSLError.uncleanShutdown. We will see + // a HTTPParserError very soon. + return .wait + } + + // If the response is EOF terminated, we need to rely on a clean tls shutdown to be sure + // we have received all necessary bytes. For this reason we forward the uncleanShutdown + // error to the user. + self.state = .failed(error) + return .failRequest(error, .close) + case .running: self.state = .failed(error) return .failRequest(error, .close) + case .finished, .failed: // ignore error return .wait + case .modifying: preconditionFailure("Invalid state: \(self.state)") } diff --git a/Sources/AsyncHTTPClient/ConnectionPool/RequestOptions.swift b/Sources/AsyncHTTPClient/ConnectionPool/RequestOptions.swift index 98f92b661..2092498d8 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/RequestOptions.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/RequestOptions.swift @@ -18,20 +18,15 @@ struct RequestOptions { /// The maximal `TimeAmount` that is allowed to pass between `channelRead`s from the Channel. var idleReadTimeout: TimeAmount? - /// Should `NIOSSLError.uncleanShutdown` be forwarded to the user in HTTP/1 mode. - var ignoreUncleanSSLShutdown: Bool - - init(idleReadTimeout: TimeAmount?, ignoreUncleanSSLShutdown: Bool) { + init(idleReadTimeout: TimeAmount?) { self.idleReadTimeout = idleReadTimeout - self.ignoreUncleanSSLShutdown = ignoreUncleanSSLShutdown } } extension RequestOptions { static func fromClientConfiguration(_ configuration: HTTPClient.Configuration) -> Self { RequestOptions( - idleReadTimeout: configuration.timeout.read, - ignoreUncleanSSLShutdown: configuration.ignoreUncleanSSLShutdown + idleReadTimeout: configuration.timeout.read ) } } diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index d6544ec7d..b5a4e7b2d 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -601,7 +601,11 @@ public class HTTPClient { /// Enables automatic body decompression. Supported algorithms are gzip and deflate. public var decompression: Decompression /// Ignore TLS unclean shutdown error, defaults to `false`. - public var ignoreUncleanSSLShutdown: Bool + @available(*, deprecated, message: "AsyncHTTPClient now correctly supports handling unexpected SSL connection drops. This property is ignored") + public var ignoreUncleanSSLShutdown: Bool { + get { false } + set {} + } // TODO: make public // TODO: set to automatic by default @@ -645,7 +649,6 @@ public class HTTPClient { self.timeout = timeout self.connectionPool = connectionPool self.proxy = proxy - self.ignoreUncleanSSLShutdown = ignoreUncleanSSLShutdown self.decompression = decompression self.httpVersion = httpVersion } diff --git a/Tests/AsyncHTTPClientTests/HTTP1ConnectionStateMachineTests.swift b/Tests/AsyncHTTPClientTests/HTTP1ConnectionStateMachineTests.swift index 28bfa0509..1cf7722b1 100644 --- a/Tests/AsyncHTTPClientTests/HTTP1ConnectionStateMachineTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTP1ConnectionStateMachineTests.swift @@ -25,7 +25,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase { let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: ["content-length": "4"]) let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(4)) - XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false), .wait) + XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata), .wait) XCTAssertEqual(state.writabilityChanged(writable: true), .sendRequestHead(requestHead, startBody: true)) let part0 = IOData.byteBuffer(ByteBuffer(bytes: [0])) @@ -63,7 +63,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase { let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) - let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false) + let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata) XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false)) let responseHead = HTTPResponseHead(version: .http1_1, status: .ok, headers: ["content-length": "12"]) @@ -91,7 +91,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase { XCTAssertEqual(state.channelActive(isWritable: true), .fireChannelActive) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/", headers: ["connection": "close"]) let metadata = RequestFramingMetadata(connectionClose: true, body: .none) - let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false) + let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata) XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false)) let responseHead = HTTPResponseHead(version: .http1_1, status: .ok) @@ -107,7 +107,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase { XCTAssertEqual(state.channelActive(isWritable: true), .fireChannelActive) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) - let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false) + let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata) XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false)) let responseHead = HTTPResponseHead(version: .http1_0, status: .ok, headers: ["content-length": "4"]) @@ -123,7 +123,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase { XCTAssertEqual(state.channelActive(isWritable: true), .fireChannelActive) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) - let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false) + let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata) XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false)) let responseHead = HTTPResponseHead(version: .http1_0, status: .ok, headers: ["content-length": "4", "connection": "keep-alive"]) @@ -140,7 +140,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase { XCTAssertEqual(state.writabilityChanged(writable: true), .wait) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) - let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false) + let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata) XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false)) let responseHead = HTTPResponseHead(version: .http1_1, status: .ok, headers: ["connection": "close"]) @@ -169,7 +169,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase { let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) - let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false) + let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata) XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false)) XCTAssertEqual(state.channelInactive(), .failRequest(HTTPClientError.remoteConnectionClosed, .none)) @@ -181,7 +181,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase { let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: ["content-length": "4"]) let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(4)) - XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false), .wait) + XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata), .wait) XCTAssertEqual(state.writabilityChanged(writable: true), .sendRequestHead(requestHead, startBody: true)) let part0 = IOData.byteBuffer(ByteBuffer(bytes: [0])) @@ -225,7 +225,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase { XCTAssertEqual(state.channelActive(isWritable: false), .fireChannelActive) let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: ["content-length": "4"]) let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(4)) - XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false), .wait) + XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata), .wait) XCTAssertEqual(state.requestCancelled(closeConnection: false), .failRequest(HTTPClientError.cancelled, .informConnectionIsIdle)) } @@ -234,7 +234,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase { XCTAssertEqual(state.channelActive(isWritable: true), .fireChannelActive) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) - let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false) + let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata) XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false)) let responseHead = HTTPResponseHead(version: .http1_1, status: .ok) XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false)) @@ -249,7 +249,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase { XCTAssertEqual(state.channelActive(isWritable: true), .fireChannelActive) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) - let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false) + let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata) XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false)) let responseHead = HTTPResponseHead(version: .http1_1, status: .switchingProtocols) XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false)) @@ -261,7 +261,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase { XCTAssertEqual(state.channelActive(isWritable: true), .fireChannelActive) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) - let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false) + let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata) XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false)) let responseHead = HTTPResponseHead(version: .http1_1, status: .init(statusCode: 103, reasonPhrase: "Early Hints")) XCTAssertEqual(state.channelRead(.head(responseHead)), .wait) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift b/Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift index 15baf0cc1..c83bca0b7 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift @@ -931,103 +931,6 @@ final class ConnectionsCountHandler: ChannelInboundHandler { } } -internal class HttpBinForSSLUncleanShutdown { - let group = MultiThreadedEventLoopGroup(numberOfThreads: 1) - let serverChannel: Channel - - var port: Int { - return Int(self.serverChannel.localAddress!.port!) - } - - init(channelPromise: EventLoopPromise? = nil) { - self.serverChannel = try! ServerBootstrap(group: self.group) - .serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1) - .childChannelOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1) - .childChannelInitializer { channel in - let requestDecoder = HTTPRequestDecoder() - return channel.pipeline.addHandler(ByteToMessageHandler(requestDecoder)).flatMap { - let configuration = TLSConfiguration.makeServerConfiguration(certificateChain: [.certificate(try! NIOSSLCertificate(bytes: Array(cert.utf8), format: .pem))], - privateKey: .privateKey(try! NIOSSLPrivateKey(bytes: Array(key.utf8), format: .pem))) - let context = try! NIOSSLContext(configuration: configuration) - return channel.pipeline.addHandler(NIOSSLServerHandler(context: context), name: "NIOSSLServerHandler", position: .first).flatMap { - channel.pipeline.addHandler(HttpBinForSSLUncleanShutdownHandler(channelPromise: channelPromise)) - } - } - }.bind(host: "127.0.0.1", port: 0).wait() - } - - func shutdown() { - try! self.group.syncShutdownGracefully() - } -} - -internal final class HttpBinForSSLUncleanShutdownHandler: ChannelInboundHandler { - typealias InboundIn = HTTPServerRequestPart - typealias OutboundOut = ByteBuffer - - let channelPromise: EventLoopPromise? - - init(channelPromise: EventLoopPromise? = nil) { - self.channelPromise = channelPromise - } - - func channelRead(context: ChannelHandlerContext, data: NIOAny) { - switch self.unwrapInboundIn(data) { - case .head(let req): - self.channelPromise?.succeed(context.channel) - - let response: String? - switch req.uri { - case "/nocontentlength": - response = """ - HTTP/1.1 200 OK\r\n\ - Connection: close\r\n\ - \r\n\ - foo - """ - case "/nocontent": - response = """ - HTTP/1.1 204 OK\r\n\ - Connection: close\r\n\ - \r\n - """ - case "/noresponse": - response = nil - case "/wrongcontentlength": - response = """ - HTTP/1.1 200 OK\r\n\ - Connection: close\r\n\ - Content-Length: 6\r\n\ - \r\n\ - foo - """ - default: - response = """ - HTTP/1.1 404 OK\r\n\ - Connection: close\r\n\ - Content-Length: 9\r\n\ - \r\n\ - Not Found - """ - } - - if let response = response { - var buffer = context.channel.allocator.buffer(capacity: response.count) - buffer.writeString(response) - context.writeAndFlush(self.wrapOutboundOut(buffer), promise: nil) - } - - context.channel.pipeline.removeHandler(name: "NIOSSLServerHandler").whenSuccess { - context.close(promise: nil) - } - case .body: - () - case .end: - () - } - } -} - internal final class CloseWithoutClosingServerHandler: ChannelInboundHandler { typealias InboundIn = HTTPServerRequestPart typealias OutboundOut = HTTPServerResponsePart diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift index daf06ad88..42395e2cb 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift @@ -59,14 +59,6 @@ extension HTTPClientTests { ("testProxyPlaintextWithCorrectlyAuthorization", testProxyPlaintextWithCorrectlyAuthorization), ("testProxyPlaintextWithIncorrectlyAuthorization", testProxyPlaintextWithIncorrectlyAuthorization), ("testUploadStreaming", testUploadStreaming), - ("testNoContentLengthForSSLUncleanShutdown", testNoContentLengthForSSLUncleanShutdown), - ("testNoContentLengthWithIgnoreErrorForSSLUncleanShutdown", testNoContentLengthWithIgnoreErrorForSSLUncleanShutdown), - ("testCorrectContentLengthForSSLUncleanShutdown", testCorrectContentLengthForSSLUncleanShutdown), - ("testNoContentForSSLUncleanShutdown", testNoContentForSSLUncleanShutdown), - ("testNoResponseForSSLUncleanShutdown", testNoResponseForSSLUncleanShutdown), - ("testNoResponseWithIgnoreErrorForSSLUncleanShutdown", testNoResponseWithIgnoreErrorForSSLUncleanShutdown), - ("testWrongContentLengthForSSLUncleanShutdown", testWrongContentLengthForSSLUncleanShutdown), - ("testWrongContentLengthWithIgnoreErrorForSSLUncleanShutdown", testWrongContentLengthWithIgnoreErrorForSSLUncleanShutdown), ("testEventLoopArgument", testEventLoopArgument), ("testDecompression", testDecompression), ("testDecompressionLimit", testDecompressionLimit), diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 2a9a21dd8..df5cd489b 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -721,168 +721,6 @@ class HTTPClientTests: XCTestCase { XCTAssertEqual("12344321", data.data) } - func testNoContentLengthForSSLUncleanShutdown() throws { - // NIOTS deals with ssl unclean shutdown internally - guard !isTestingNIOTS() else { return } - - let localHTTPBin = HttpBinForSSLUncleanShutdown() - let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), - configuration: HTTPClient.Configuration(certificateVerification: .none)) - - defer { - XCTAssertNoThrow(try localClient.syncShutdown()) - localHTTPBin.shutdown() - } - - XCTAssertThrowsError(try localClient.get(url: "https://localhost:\(localHTTPBin.port)/nocontentlength").wait(), "Should fail") { error in - guard case let error = error as? NIOSSLError, error == .uncleanShutdown else { - return XCTFail("Should fail with NIOSSLError.uncleanShutdown") - } - } - } - - func testNoContentLengthWithIgnoreErrorForSSLUncleanShutdown() throws { - // NIOTS deals with ssl unclean shutdown internally - guard !isTestingNIOTS() else { return } - - let localHTTPBin = HttpBinForSSLUncleanShutdown() - let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), - configuration: HTTPClient.Configuration(certificateVerification: .none, ignoreUncleanSSLShutdown: true)) - - defer { - XCTAssertNoThrow(try localClient.syncShutdown()) - localHTTPBin.shutdown() - } - - let response = try localClient.get(url: "https://localhost:\(localHTTPBin.port)/nocontentlength").wait() - let bytes = response.body.flatMap { $0.getData(at: 0, length: $0.readableBytes) } - let string = String(decoding: bytes!, as: UTF8.self) - - XCTAssertEqual(.ok, response.status) - XCTAssertEqual("foo", string) - } - - func testCorrectContentLengthForSSLUncleanShutdown() throws { - // NIOTS deals with ssl unclean shutdown internally - guard !isTestingNIOTS() else { return } - - let localHTTPBin = HttpBinForSSLUncleanShutdown() - let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), - configuration: HTTPClient.Configuration(certificateVerification: .none)) - - defer { - XCTAssertNoThrow(try localClient.syncShutdown()) - localHTTPBin.shutdown() - } - - let response = try localClient.get(url: "https://localhost:\(localHTTPBin.port)/").wait() - let bytes = response.body.flatMap { $0.getData(at: 0, length: $0.readableBytes) } - let string = String(decoding: bytes!, as: UTF8.self) - - XCTAssertEqual(.notFound, response.status) - XCTAssertEqual("Not Found", string) - } - - func testNoContentForSSLUncleanShutdown() throws { - // NIOTS deals with ssl unclean shutdown internally - guard !isTestingNIOTS() else { return } - - let localHTTPBin = HttpBinForSSLUncleanShutdown() - let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), - configuration: HTTPClient.Configuration(certificateVerification: .none)) - - defer { - XCTAssertNoThrow(try localClient.syncShutdown()) - localHTTPBin.shutdown() - } - - let response = try localClient.get(url: "https://localhost:\(localHTTPBin.port)/nocontent").wait() - - XCTAssertEqual(.noContent, response.status) - XCTAssertEqual(response.body, nil) - } - - func testNoResponseForSSLUncleanShutdown() throws { - // NIOTS deals with ssl unclean shutdown internally - guard !isTestingNIOTS() else { return } - - let localHTTPBin = HttpBinForSSLUncleanShutdown() - let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), - configuration: HTTPClient.Configuration(certificateVerification: .none)) - - defer { - XCTAssertNoThrow(try localClient.syncShutdown()) - localHTTPBin.shutdown() - } - - XCTAssertThrowsError(try localClient.get(url: "https://localhost:\(localHTTPBin.port)/noresponse").wait(), "Should fail") { error in - guard case let sslError = error as? NIOSSLError, sslError == .uncleanShutdown else { - return XCTFail("Should fail with NIOSSLError.uncleanShutdown") - } - } - } - - func testNoResponseWithIgnoreErrorForSSLUncleanShutdown() throws { - // NIOTS deals with ssl unclean shutdown internally - guard !isTestingNIOTS() else { return } - - let localHTTPBin = HttpBinForSSLUncleanShutdown() - let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), - configuration: HTTPClient.Configuration(certificateVerification: .none, ignoreUncleanSSLShutdown: true)) - - defer { - XCTAssertNoThrow(try localClient.syncShutdown()) - localHTTPBin.shutdown() - } - - XCTAssertThrowsError(try localClient.get(url: "https://localhost:\(localHTTPBin.port)/noresponse").wait(), "Should fail") { error in - guard case let sslError = error as? NIOSSLError, sslError == .uncleanShutdown else { - return XCTFail("Should fail with NIOSSLError.uncleanShutdown") - } - } - } - - func testWrongContentLengthForSSLUncleanShutdown() throws { - // NIOTS deals with ssl unclean shutdown internally - guard !isTestingNIOTS() else { return } - - let localHTTPBin = HttpBinForSSLUncleanShutdown() - let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), - configuration: HTTPClient.Configuration(certificateVerification: .none)) - - defer { - XCTAssertNoThrow(try localClient.syncShutdown()) - localHTTPBin.shutdown() - } - - XCTAssertThrowsError(try localClient.get(url: "https://localhost:\(localHTTPBin.port)/wrongcontentlength").wait(), "Should fail") { error in - XCTAssertEqual(.uncleanShutdown, error as? NIOSSLError) - } - } - - func testWrongContentLengthWithIgnoreErrorForSSLUncleanShutdown() throws { - // NIOTS deals with ssl unclean shutdown internally - guard !isTestingNIOTS() else { return } - - let localHTTPBin = HttpBinForSSLUncleanShutdown() - let localClient = HTTPClient( - eventLoopGroupProvider: .shared(self.clientGroup), - configuration: HTTPClient.Configuration( - certificateVerification: .none, - ignoreUncleanSSLShutdown: true - ) - ) - - defer { - XCTAssertNoThrow(try localClient.syncShutdown()) - localHTTPBin.shutdown() - } - - XCTAssertThrowsError(try localClient.get(url: "https://localhost:\(localHTTPBin.port)/wrongcontentlength").wait()) { - XCTAssertEqual($0 as? HTTPParserError, .invalidEOFState) - } - } - func testEventLoopArgument() throws { let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), configuration: HTTPClient.Configuration(redirectConfiguration: .follow(max: 10, allowCycles: true))) @@ -3011,7 +2849,6 @@ class HTTPClientTests: XCTestCase { func testRequestSpecificTLS() throws { let configuration = HTTPClient.Configuration(tlsConfiguration: nil, timeout: .init(), - ignoreUncleanSSLShutdown: false, decompression: .disabled) let localHTTPBin = HTTPBin(.http1_1(ssl: true)) let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), diff --git a/Tests/AsyncHTTPClientTests/HTTPClientUncleanSSLConnectionShutdownTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPClientUncleanSSLConnectionShutdownTests+XCTest.swift new file mode 100644 index 000000000..d95346673 --- /dev/null +++ b/Tests/AsyncHTTPClientTests/HTTPClientUncleanSSLConnectionShutdownTests+XCTest.swift @@ -0,0 +1,36 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the AsyncHTTPClient open source project +// +// Copyright (c) 2018-2019 Apple Inc. and the AsyncHTTPClient project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of AsyncHTTPClient project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// +// +// HTTPClientUncleanSSLConnectionShutdownTests+XCTest.swift +// +import XCTest + +/// +/// NOTE: This file was generated by generate_linux_tests.rb +/// +/// Do NOT edit this file directly as it will be regenerated automatically when needed. +/// + +extension HTTPClientUncleanSSLConnectionShutdownTests { + static var allTests: [(String, (HTTPClientUncleanSSLConnectionShutdownTests) -> () throws -> Void)] { + return [ + ("testEOFFramedSuccess", testEOFFramedSuccess), + ("testContentLength", testContentLength), + ("testContentLengthButTruncated", testContentLengthButTruncated), + ("testTransferEncoding", testTransferEncoding), + ("testTransferEncodingButTruncated", testTransferEncodingButTruncated), + ("testConnectionDrop", testConnectionDrop), + ] + } +} diff --git a/Tests/AsyncHTTPClientTests/HTTPClientUncleanSSLConnectionShutdownTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientUncleanSSLConnectionShutdownTests.swift new file mode 100644 index 000000000..854d9092c --- /dev/null +++ b/Tests/AsyncHTTPClientTests/HTTPClientUncleanSSLConnectionShutdownTests.swift @@ -0,0 +1,321 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the AsyncHTTPClient open source project +// +// Copyright (c) 2021 Apple Inc. and the AsyncHTTPClient project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of AsyncHTTPClient project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +// These tests may require some more context: +// +// TLS (SSL) encrypts, validates, and authenticates all the data that goes through the connection. +// That is a fantastic property to have and solves most issues. TLS however still runs over TCP and +// the control packets of TCP are not encrypted. This means that the one thing an attacker can still +// do on a TLS connection is to close it. The attacker could send RST or FIN packets the other peer +// and the receiving peer has no means to verify if this RST/FIN packet is actually coming from the +// other peer (as opposed to an attacker). +// +// To fix this problem, TLS introduces a close_notify message that is send over connection as +// encrypted data. So if the other peer receives a close_notify it knows that it was truly sent by +// the other peer (and not an attacker). A well behaving peer would then reply to the close_notify +// with its own close_notify and after that both peers can close the TCP connection because they +// know that the respective other peer knows they're okay closing it. +// +// Okay, but what's the issue with having a connection just close. Wouldn't I notice that part of +// the data is missing? The answer is it depends. Many protocols actually send to the other peer how +// much data they will send before sending the data. And they also have a well defined +// "end message". If you're using such a protocol, then an "unclean shutdown" (which is you have +// received a RST/FIN without a close_notify) is totally harmless. The higher level protocol allows +// you to distinguish between a truncated message (when there's still outstanding data) and a close +// after a completed message. +// +// The reason SwiftNIO sends you a NIOSSLError.uncleanShutdown if it sees a connection closing +// without a prior close_notify is because it doesn't know what protocol you're implementing. Maybe +// the protocol you speak doesn't transmit length information so a truncated message cannot be told +// apart from a complete message. +// +// Let's go into some example protocols and their behaviour regarding framing: +// +// - With HTTP/2 the other peer always knows how much data to expect, so an unclean shutdown is +// totally harmless, the error can be ignored. +// - With HTTP/1, the situation is much more complicated: HTTP/1 when using the content-length +// header is unaffected by truncation attacks because we know the content length. So if the +// connection closes before we have received that many bytes, we know it was a truncated message +// (either by an attacker injecting a FIN/RST, or by a software/network problem somewhere along +// the way) +// - HTTP/1 with transfer-encoding: chunked is also unaffected by truncation attacks because each +// chunk sends its length and there's a special "END" chunk (0\r\n\r\n). Unfortunately HTTP/1 can +// be used without either transfer-encoding: chunked or content-length. It then runs in the "EOF +// framing" mode which means that the message ends when we receive a connection close 😢 . Very +// bad, if HTTP/1 is used in "EOF framing" mode, then an unclean shutdown is actually a real +// error because we cannot tell a truncated message apart from a complete message. +// +// From @weissi in https://github.com/swift-server/async-http-client/issues/238 + +import AsyncHTTPClient +import Logging +import NIOCore +import NIOHTTP1 +import NIOPosix +import NIOSSL +import XCTest + +final class HTTPClientUncleanSSLConnectionShutdownTests: XCTestCase { + func testEOFFramedSuccess() { + let httpBin = HTTPBinForSSLUncleanShutdown() + defer { XCTAssertNoThrow(try httpBin.shutdown()) } + + let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1) + defer { XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully()) } + + let client = HTTPClient( + eventLoopGroupProvider: .shared(eventLoopGroup), + configuration: .init(certificateVerification: .none) + ) + defer { XCTAssertNoThrow(try client.syncShutdown()) } + + XCTAssertThrowsError(try client.get(url: "https://localhost:\(httpBin.port)/nocontentlength").wait()) { + XCTAssertEqual($0 as? NIOSSLError, .uncleanShutdown) + } + } + + func testContentLength() { + let httpBin = HTTPBinForSSLUncleanShutdown() + defer { XCTAssertNoThrow(try httpBin.shutdown()) } + + let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1) + defer { XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully()) } + + let client = HTTPClient( + eventLoopGroupProvider: .shared(eventLoopGroup), + configuration: .init(certificateVerification: .none) + ) + defer { XCTAssertNoThrow(try client.syncShutdown()) } + + var response: HTTPClient.Response? + XCTAssertNoThrow(response = try client.get(url: "https://localhost:\(httpBin.port)/contentlength").wait()) + XCTAssertEqual(response?.status, .notFound) + XCTAssertEqual(response?.headers["content-length"].first, "9") + } + + func testContentLengthButTruncated() { + let httpBin = HTTPBinForSSLUncleanShutdown() + defer { XCTAssertNoThrow(try httpBin.shutdown()) } + + let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1) + defer { XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully()) } + + let client = HTTPClient( + eventLoopGroupProvider: .shared(eventLoopGroup), + configuration: .init(certificateVerification: .none) + ) + defer { XCTAssertNoThrow(try client.syncShutdown()) } + + XCTAssertThrowsError(try client.get(url: "https://localhost:\(httpBin.port)/wrongcontentlength").wait()) { + XCTAssertEqual($0 as? HTTPParserError, .invalidEOFState) + } + } + + func testTransferEncoding() { + let httpBin = HTTPBinForSSLUncleanShutdown() + defer { XCTAssertNoThrow(try httpBin.shutdown()) } + + let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1) + defer { XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully()) } + + let client = HTTPClient( + eventLoopGroupProvider: .shared(eventLoopGroup), + configuration: .init(certificateVerification: .none) + ) + defer { XCTAssertNoThrow(try client.syncShutdown()) } + + var response: HTTPClient.Response? + XCTAssertNoThrow(response = try client.get(url: "https://localhost:\(httpBin.port)/transferencoding").wait()) + XCTAssertEqual(response?.status, .ok) + XCTAssertEqual(response?.headers["transfer-encoding"].first, "chunked") + XCTAssertEqual(response?.body, ByteBuffer(string: "foo")) + } + + func testTransferEncodingButTruncated() { + let httpBin = HTTPBinForSSLUncleanShutdown() + defer { XCTAssertNoThrow(try httpBin.shutdown()) } + + let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1) + defer { XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully()) } + + let client = HTTPClient( + eventLoopGroupProvider: .shared(eventLoopGroup), + configuration: .init(certificateVerification: .none) + ) + defer { XCTAssertNoThrow(try client.syncShutdown()) } + + XCTAssertThrowsError(try client.get(url: "https://localhost:\(httpBin.port)/transferencodingtruncated").wait()) { + XCTAssertEqual($0 as? HTTPParserError, .invalidEOFState) + } + } + + func testConnectionDrop() { + let httpBin = HTTPBinForSSLUncleanShutdown() + defer { XCTAssertNoThrow(try httpBin.shutdown()) } + + let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1) + defer { XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully()) } + + let client = HTTPClient( + eventLoopGroupProvider: .shared(eventLoopGroup), + configuration: .init(certificateVerification: .none) + ) + defer { XCTAssertNoThrow(try client.syncShutdown()) } + + XCTAssertThrowsError(try client.get(url: "https://localhost:\(httpBin.port)/noresponse").wait()) { + XCTAssertEqual($0 as? HTTPClientError, .remoteConnectionClosed) + } + } +} + +final class HTTPBinForSSLUncleanShutdown { + let group = MultiThreadedEventLoopGroup(numberOfThreads: 1) + let serverChannel: Channel + + var port: Int { + return Int(self.serverChannel.localAddress!.port!) + } + + init() { + let configuration = TLSConfiguration.makeServerConfiguration( + certificateChain: [.certificate(TestTLS.certificate)], + privateKey: .privateKey(TestTLS.privateKey) + ) + let context = try! NIOSSLContext(configuration: configuration) + + self.serverChannel = try! ServerBootstrap(group: self.group) + .serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1) + .childChannelOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1) + .childChannelInitializer { channel in + do { + let requestDecoder = HTTPRequestDecoder() + let sync = channel.pipeline.syncOperations + + try sync.addHandler(ConnectionForceCloser()) + try sync.addHandler(NIOSSLServerHandler(context: context)) + try sync.addHandler(ByteToMessageHandler(requestDecoder)) + try sync.addHandler(HTTPBinForSSLUncleanShutdownHandler()) + return channel.eventLoop.makeSucceededVoidFuture() + } catch { + return channel.eventLoop.makeFailedFuture(error) + } + }.bind(host: "127.0.0.1", port: 0).wait() + } + + func shutdown() throws { + try self.group.syncShutdownGracefully() + } +} + +private final class HTTPBinForSSLUncleanShutdownHandler: ChannelInboundHandler { + typealias InboundIn = HTTPServerRequestPart + typealias OutboundOut = ByteBuffer + + init() {} + + func channelRead(context: ChannelHandlerContext, data: NIOAny) { + switch self.unwrapInboundIn(data) { + case .head(let req): + let response: String? + switch req.uri { + case "/nocontentlength": + response = """ + HTTP/1.1 200 OK\r\n\ + Connection: close\r\n\ + \r\n\ + foo + """ + + case "/nocontent": + response = """ + HTTP/1.1 204 OK\r\n\ + Connection: close\r\n\ + \r\n + """ + + case "/noresponse": + response = nil + + case "/wrongcontentlength": + response = """ + HTTP/1.1 200 OK\r\n\ + Connection: close\r\n\ + Content-Length: 6\r\n\ + \r\n\ + foo + """ + + case "/transferencoding": + response = """ + HTTP/1.1 200 OK\r\n\ + Connection: close\r\n\ + Transfer-Encoding: chunked\r\n\ + \r\n\ + 3\r\n\ + foo\r\n\ + 0\r\n\ + \r\n + """ + + case "/transferencodingtruncated": + response = """ + HTTP/1.1 200 OK\r\n\ + Connection: close\r\n\ + Transfer-Encoding: chunked\r\n\ + \r\n\ + 12\r\n\ + foo + """ + + default: + response = """ + HTTP/1.1 404 OK\r\n\ + Connection: close\r\n\ + Content-Length: 9\r\n\ + \r\n\ + Not Found + """ + } + + if let response = response { + var buffer = context.channel.allocator.buffer(capacity: response.count) + buffer.writeString(response) + context.writeAndFlush(self.wrapOutboundOut(buffer), promise: nil) + } + + context.triggerUserOutboundEvent(ConnectionForceCloser.CloseEvent(), promise: nil) + case .body: + () + case .end: + () + } + } +} + +private final class ConnectionForceCloser: ChannelOutboundHandler { + typealias OutboundIn = NIOAny + + struct CloseEvent {} + + init() {} + + func triggerUserOutboundEvent(context: ChannelHandlerContext, event: Any, promise: EventLoopPromise?) { + switch event { + case is CloseEvent: + context.close(promise: promise) + default: + context.triggerUserOutboundEvent(event, promise: promise) + } + } +} diff --git a/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests+XCTest.swift index dacaf1a67..3dd6c7b30 100644 --- a/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests+XCTest.swift @@ -54,7 +54,6 @@ extension HTTPRequestStateMachineTests { ("testCanReadHTTP1_0ResponseWithBody", testCanReadHTTP1_0ResponseWithBody), ("testFailHTTP1_0RequestThatIsStillUploading", testFailHTTP1_0RequestThatIsStillUploading), ("testFailHTTP1RequestWithoutContentLengthWithNIOSSLErrorUncleanShutdown", testFailHTTP1RequestWithoutContentLengthWithNIOSSLErrorUncleanShutdown), - ("testFailHTTP1RequestWithoutContentLengthWithNIOSSLErrorUncleanShutdownButIgnoreIt", testFailHTTP1RequestWithoutContentLengthWithNIOSSLErrorUncleanShutdownButIgnoreIt), ("testFailHTTP1RequestWithContentLengthWithNIOSSLErrorUncleanShutdownButIgnoreIt", testFailHTTP1RequestWithContentLengthWithNIOSSLErrorUncleanShutdownButIgnoreIt), ("testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForDemand", testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForDemand), ("testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForRead", testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForRead), diff --git a/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift b/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift index 74c538de0..05530824c 100644 --- a/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift @@ -20,7 +20,7 @@ import XCTest class HTTPRequestStateMachineTests: XCTestCase { func testSimpleGETRequest() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false)) @@ -34,7 +34,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testPOSTRequestWithWriterBackpressure() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: HTTPHeaders([("content-length", "4")])) let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(4)) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: true)) @@ -68,7 +68,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testPOSTContentLengthIsTooLong() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: HTTPHeaders([("content-length", "4")])) let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(4)) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: true)) @@ -85,7 +85,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testPOSTContentLengthIsTooShort() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: HTTPHeaders([("content-length", "8")])) let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(8)) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: true)) @@ -101,7 +101,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testRequestBodyStreamIsCancelledIfServerRespondsWith301() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: HTTPHeaders([("content-length", "12")])) let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(12)) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: true)) @@ -126,7 +126,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testRequestBodyStreamIsCancelledIfServerRespondsWith301WhileWriteBackpressure() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: HTTPHeaders([("content-length", "12")])) let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(12)) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: true)) @@ -151,7 +151,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testRequestBodyStreamIsContinuedIfServerRespondsWith200() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: HTTPHeaders([("content-length", "12")])) let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(12)) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: true)) @@ -171,7 +171,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testRequestBodyStreamIsContinuedIfServerSendHeadWithStatus200() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: HTTPHeaders([("content-length", "12")])) let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(12)) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: true)) @@ -192,7 +192,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testRequestIsFailedIfRequestBodySizeIsWrongEvenAfterServerRespondedWith200() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: HTTPHeaders([("content-length", "12")])) let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(12)) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: true)) @@ -211,7 +211,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testRequestIsFailedIfRequestBodySizeIsWrongEvenAfterServerSendHeadWithStatus200() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: HTTPHeaders([("content-length", "12")])) let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(12)) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: true)) @@ -229,7 +229,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testRequestIsNotSendUntilChannelIsWritable() { - var state = HTTPRequestStateMachine(isChannelWritable: false, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: false) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .wait) @@ -245,7 +245,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testConnectionBecomesInactiveWhileWaitingForWritable() { - var state = HTTPRequestStateMachine(isChannelWritable: false, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: false) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .wait) @@ -253,7 +253,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testResponseReadingWithBackpressure() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false)) @@ -280,7 +280,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testChannelReadCompleteTriggersButNoBodyDataWasReceivedSoFar() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false)) @@ -307,7 +307,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testResponseReadingWithBackpressureEndOfResponseAllowsReadEventsToTriggerDirectly() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false)) @@ -338,12 +338,12 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testCancellingARequestInStateInitializedKeepsTheConnectionAlive() { - var state = HTTPRequestStateMachine(isChannelWritable: false, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: false) XCTAssertEqual(state.requestCancelled(), .failRequest(HTTPClientError.cancelled, .none)) } func testCancellingARequestBeforeBeingSendKeepsTheConnectionAlive() { - var state = HTTPRequestStateMachine(isChannelWritable: false, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: false) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .wait) @@ -351,7 +351,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testConnectionBecomesWritableBeforeFirstRequest() { - var state = HTTPRequestStateMachine(isChannelWritable: false, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: false) XCTAssertEqual(state.writabilityChanged(writable: true), .wait) // --- sending request @@ -369,7 +369,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testCancellingARequestThatIsSent() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false)) @@ -377,7 +377,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testRemoteSuddenlyClosesTheConnection() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/", headers: .init([("content-length", "4")])) let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(4)) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: true)) @@ -386,7 +386,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testReadTimeoutLeadsToFailureWithEverythingAfterBeingIgnored() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false)) @@ -403,7 +403,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testResponseWithStatus1XXAreIgnored() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false)) @@ -419,7 +419,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testReadTimeoutThatFiresToLateIsIgnored() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false)) @@ -431,7 +431,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testCancellationThatIsInvokedToLateIsIgnored() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false)) @@ -443,17 +443,17 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testErrorWhileRunningARequestClosesTheStream() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false)) - XCTAssertEqual(state.errorHappened(NIOSSLError.uncleanShutdown), .failRequest(NIOSSLError.uncleanShutdown, .close)) + XCTAssertEqual(state.errorHappened(HTTPParserError.invalidChunkSize), .failRequest(HTTPParserError.invalidChunkSize, .close)) XCTAssertEqual(state.requestCancelled(), .wait, "A cancellation that happens to late is ignored") } func testCanReadHTTP1_0ResponseWithoutBody() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false)) @@ -469,7 +469,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testCanReadHTTP1_0ResponseWithBody() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false)) @@ -487,7 +487,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testFailHTTP1_0RequestThatIsStillUploading() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .stream) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: true)) @@ -507,7 +507,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testFailHTTP1RequestWithoutContentLengthWithNIOSSLErrorUncleanShutdown() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false)) @@ -522,26 +522,8 @@ class HTTPRequestStateMachineTests: XCTestCase { XCTAssertEqual(state.channelInactive(), .wait) } - func testFailHTTP1RequestWithoutContentLengthWithNIOSSLErrorUncleanShutdownButIgnoreIt() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: true) - let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") - let metadata = RequestFramingMetadata(connectionClose: false, body: .none) - XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false)) - - let responseHead = HTTPResponseHead(version: .http1_1, status: .ok) - let body = ByteBuffer(string: "foo bar") - XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false)) - XCTAssertEqual(state.demandMoreResponseBodyParts(), .wait) - XCTAssertEqual(state.read(), .read) - XCTAssertEqual(state.channelRead(.body(body)), .wait) - XCTAssertEqual(state.channelReadComplete(), .forwardResponseBodyParts([body])) - XCTAssertEqual(state.errorHappened(NIOSSLError.uncleanShutdown), .wait) - XCTAssertEqual(state.channelRead(.end(nil)), .succeedRequest(.close, [])) - XCTAssertEqual(state.channelInactive(), .wait) - } - func testFailHTTP1RequestWithContentLengthWithNIOSSLErrorUncleanShutdownButIgnoreIt() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: true) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false)) @@ -559,7 +541,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForDemand() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false)) @@ -580,7 +562,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForRead() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false)) @@ -601,7 +583,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForReadAndDemand() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false)) @@ -621,7 +603,7 @@ class HTTPRequestStateMachineTests: XCTestCase { } func testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForReadAndDemandMultipleTimes() { - var state = HTTPRequestStateMachine(isChannelWritable: true, ignoreUncleanSSLShutdown: false) + var state = HTTPRequestStateMachine(isChannelWritable: true) let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/") let metadata = RequestFramingMetadata(connectionClose: false, body: .none) XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false)) diff --git a/Tests/AsyncHTTPClientTests/RequestBagTests.swift b/Tests/AsyncHTTPClientTests/RequestBagTests.swift index d1c6d6c87..1acde7732 100644 --- a/Tests/AsyncHTTPClientTests/RequestBagTests.swift +++ b/Tests/AsyncHTTPClientTests/RequestBagTests.swift @@ -588,10 +588,9 @@ class MockTaskQueuer: HTTPRequestScheduler { } extension RequestOptions { - static func forTests(idleReadTimeout: TimeAmount? = nil, ignoreUncleanSSLShutdown: Bool = false) -> Self { + static func forTests(idleReadTimeout: TimeAmount? = nil) -> Self { RequestOptions( - idleReadTimeout: idleReadTimeout, - ignoreUncleanSSLShutdown: ignoreUncleanSSLShutdown + idleReadTimeout: idleReadTimeout ) } } diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index 2cbddd362..cae37086d 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -40,6 +40,7 @@ import XCTest testCase(HTTPClientReproTests.allTests), testCase(HTTPClientSOCKSTests.allTests), testCase(HTTPClientTests.allTests), + testCase(HTTPClientUncleanSSLConnectionShutdownTests.allTests), testCase(HTTPConnectionPoolTests.allTests), testCase(HTTPConnectionPool_FactoryTests.allTests), testCase(HTTPConnectionPool_HTTP1ConnectionsTests.allTests),