Skip to content

Ignore unclean ssl shutdown errors #432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ final class HTTP1ClientChannelHandler: ChannelDuplexHandler {
requestLogger[metadataKey: "ahc-el"] = "\(self.connection.channel.eventLoop)"
self.logger = requestLogger

if let idleReadTimeout = newRequest.idleReadTimeout {
if let idleReadTimeout = newRequest.requestOptions.idleReadTimeout {
self.idleReadTimeoutStateMachine = .init(timeAmount: idleReadTimeout)
}
} else {
Expand Down Expand Up @@ -146,7 +146,11 @@ final class HTTP1ClientChannelHandler: ChannelDuplexHandler {
self.logger.debug("Request was scheduled on connection")
req.willExecuteRequest(self)

let action = self.state.runNewRequest(head: req.requestHead, metadata: req.requestFramingMetadata)
let action = self.state.runNewRequest(
head: req.requestHead,
metadata: req.requestFramingMetadata,
ignoreUncleanSSLShutdown: req.requestOptions.ignoreUncleanSSLShutdown
)
self.run(action, context: context)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,13 +154,18 @@ struct HTTP1ConnectionStateMachine {
}
}

mutating func runNewRequest(head: HTTPRequestHead, metadata: RequestFramingMetadata) -> Action {
mutating func runNewRequest(
head: HTTPRequestHead,
metadata: RequestFramingMetadata,
ignoreUncleanSSLShutdown: Bool
) -> Action {
guard case .idle = self.state else {
preconditionFailure("Invalid state")
}

var requestStateMachine = HTTPRequestStateMachine(
isChannelWritable: self.isChannelWritable
isChannelWritable: self.isChannelWritable,
ignoreUncleanSSLShutdown: ignoreUncleanSSLShutdown
)
let action = requestStateMachine.startRequest(head: head, metadata: metadata)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ final class HTTP2ClientRequestHandler: ChannelDuplexHandler {

private let eventLoop: EventLoop

private var state: HTTPRequestStateMachine = .init(isChannelWritable: false) {
private var state: HTTPRequestStateMachine = .init(isChannelWritable: false, ignoreUncleanSSLShutdown: false) {
willSet {
self.eventLoop.assertInEventLoop()
}
Expand All @@ -35,7 +35,7 @@ final class HTTP2ClientRequestHandler: ChannelDuplexHandler {

private var request: HTTPExecutableRequest? {
didSet {
if let newRequest = self.request, let idleReadTimeout = newRequest.idleReadTimeout {
if let newRequest = self.request, let idleReadTimeout = newRequest.requestOptions.idleReadTimeout {
self.idleReadTimeoutStateMachine = .init(timeAmount: idleReadTimeout)
} else {
self.idleReadTimeoutStateMachine = nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,8 @@ protocol HTTPExecutableRequest: AnyObject {
/// ``requestHeadSent``.
var requestFramingMetadata: RequestFramingMetadata { get }

/// The maximal `TimeAmount` that is allowed to pass between `channelRead`s from the Channel.
var idleReadTimeout: TimeAmount? { get }
/// Request specific configurations
var requestOptions: RequestOptions { get }

/// Will be called by the ChannelHandler to indicate that the request is going to be sent.
///
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import NIOCore
import NIOHTTP1
import NIOSSL

struct HTTPRequestStateMachine {
fileprivate enum State {
Expand Down Expand Up @@ -102,8 +103,11 @@ struct HTTPRequestStateMachine {

private var isChannelWritable: Bool

init(isChannelWritable: Bool) {
private let ignoreUncleanSSLShutdown: Bool

init(isChannelWritable: Bool, ignoreUncleanSSLShutdown: Bool) {
self.isChannelWritable = isChannelWritable
self.ignoreUncleanSSLShutdown = ignoreUncleanSSLShutdown
}

mutating func startRequest(head: HTTPRequestHead, metadata: RequestFramingMetadata) -> Action {
Expand Down Expand Up @@ -196,6 +200,12 @@ struct HTTPRequestStateMachine {
// the request failed, before it was sent onto the wire.
self.state = .failed(error)
return .failRequest(error, .none)

case .running(.streaming, .receivingBody),
.running(.endSent, .receivingBody)
where error as? NIOSSLError == .uncleanShutdown && self.ignoreUncleanSSLShutdown == true:
return .wait

case .running:
self.state = .failed(error)
return .failRequest(error, .close)
Expand Down
37 changes: 37 additions & 0 deletions Sources/AsyncHTTPClient/ConnectionPool/RequestOptions.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//===----------------------------------------------------------------------===//
//
// 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
//
//===----------------------------------------------------------------------===//

import NIOCore

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) {
self.idleReadTimeout = idleReadTimeout
self.ignoreUncleanSSLShutdown = ignoreUncleanSSLShutdown
}
}

extension RequestOptions {
static func fromClientConfiguration(_ configuration: HTTPClient.Configuration) -> Self {
RequestOptions(
idleReadTimeout: configuration.timeout.read,
ignoreUncleanSSLShutdown: configuration.ignoreUncleanSSLShutdown
)
}
}
6 changes: 3 additions & 3 deletions Sources/AsyncHTTPClient/RequestBag.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ final class RequestBag<Delegate: HTTPClientResponseDelegate> {

let connectionDeadline: NIODeadline

let idleReadTimeout: TimeAmount?
let requestOptions: RequestOptions

let requestHead: HTTPRequestHead
let requestFramingMetadata: RequestFramingMetadata
Expand All @@ -51,14 +51,14 @@ final class RequestBag<Delegate: HTTPClientResponseDelegate> {
task: HTTPClient.Task<Delegate.Response>,
redirectHandler: RedirectHandler<Delegate.Response>?,
connectionDeadline: NIODeadline,
idleReadTimeout: TimeAmount?,
requestOptions: RequestOptions,
delegate: Delegate) throws {
self.eventLoopPreference = eventLoopPreference
self.task = task
self.state = .init(redirectHandler: redirectHandler)
self.request = request
self.connectionDeadline = connectionDeadline
self.idleReadTimeout = idleReadTimeout
self.requestOptions = requestOptions
self.delegate = delegate

let (head, metadata) = try request.createRequestHead()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class HTTP1ClientChannelHandlerTests: XCTestCase {
task: .init(eventLoop: embedded.eventLoop, logger: testUtils.logger),
redirectHandler: nil,
connectionDeadline: .now() + .seconds(30),
idleReadTimeout: nil,
requestOptions: .forTests(),
delegate: delegate
))
guard let requestBag = maybeRequestBag else { return XCTFail("Expected to be able to create a request bag") }
Expand Down Expand Up @@ -126,7 +126,7 @@ class HTTP1ClientChannelHandlerTests: XCTestCase {
task: .init(eventLoop: embedded.eventLoop, logger: testUtils.logger),
redirectHandler: nil,
connectionDeadline: .now() + .seconds(30),
idleReadTimeout: .milliseconds(200),
requestOptions: .forTests(idleReadTimeout: .milliseconds(200)),
delegate: delegate
))
guard let requestBag = maybeRequestBag else { return XCTFail("Expected to be able to create a request bag") }
Expand Down Expand Up @@ -207,7 +207,7 @@ class HTTP1ClientChannelHandlerTests: XCTestCase {
task: .init(eventLoop: embedded.eventLoop, logger: testUtils.logger),
redirectHandler: nil,
connectionDeadline: .now() + .seconds(30),
idleReadTimeout: .milliseconds(200),
requestOptions: .forTests(idleReadTimeout: .milliseconds(200)),
delegate: delegate
))
guard let requestBag = maybeRequestBag else { return XCTFail("Expected to be able to create a request bag") }
Expand Down Expand Up @@ -253,7 +253,7 @@ class HTTP1ClientChannelHandlerTests: XCTestCase {
task: .init(eventLoop: embedded.eventLoop, logger: testUtils.logger),
redirectHandler: nil,
connectionDeadline: .now() + .seconds(30),
idleReadTimeout: .milliseconds(200),
requestOptions: .forTests(idleReadTimeout: .milliseconds(200)),
delegate: delegate
))
guard let requestBag = maybeRequestBag else { return XCTFail("Expected to be able to create a request bag") }
Expand Down
27 changes: 17 additions & 10 deletions Tests/AsyncHTTPClientTests/HTTP1ConnectionStateMachineTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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), .wait)
XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false), .wait)
XCTAssertEqual(state.writabilityChanged(writable: true), .sendRequestHead(requestHead, startBody: true))

let part0 = IOData.byteBuffer(ByteBuffer(bytes: [0]))
Expand Down Expand Up @@ -63,7 +63,8 @@ class HTTP1ConnectionStateMachineTests: XCTestCase {

let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
let metadata = RequestFramingMetadata(connectionClose: false, body: .none)
XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false)
XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false))

let responseHead = HTTPResponseHead(version: .http1_1, status: .ok, headers: ["content-length": "12"])
XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false))
Expand All @@ -90,7 +91,8 @@ 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)
XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false)
XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false))

let responseHead = HTTPResponseHead(version: .http1_1, status: .ok)
XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false))
Expand All @@ -105,7 +107,8 @@ 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)
XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false)
XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false))

let responseHead = HTTPResponseHead(version: .http1_0, status: .ok, headers: ["content-length": "4"])
XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false))
Expand All @@ -120,7 +123,8 @@ 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)
XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false)
XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false))

let responseHead = HTTPResponseHead(version: .http1_0, status: .ok, headers: ["content-length": "4", "connection": "keep-alive"])
XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false))
Expand All @@ -136,7 +140,8 @@ 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)
XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false)
XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false))

let responseHead = HTTPResponseHead(version: .http1_1, status: .ok, headers: ["connection": "close"])
XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false))
Expand Down Expand Up @@ -164,7 +169,8 @@ class HTTP1ConnectionStateMachineTests: XCTestCase {

let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
let metadata = RequestFramingMetadata(connectionClose: false, body: .none)
XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false)
XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false))

XCTAssertEqual(state.channelInactive(), .failRequest(HTTPClientError.remoteConnectionClosed, .none))
}
Expand All @@ -175,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), .wait)
XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false), .wait)
XCTAssertEqual(state.writabilityChanged(writable: true), .sendRequestHead(requestHead, startBody: true))

let part0 = IOData.byteBuffer(ByteBuffer(bytes: [0]))
Expand Down Expand Up @@ -219,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), .wait)
XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false), .wait)
XCTAssertEqual(state.requestCancelled(closeConnection: false), .failRequest(HTTPClientError.cancelled, .informConnectionIsIdle))
}

Expand All @@ -228,7 +234,8 @@ 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)
XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false)
XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false))
let responseHead = HTTPResponseHead(version: .http1_1, status: .ok)
XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false))
XCTAssertEqual(state.channelRead(.body(ByteBuffer(string: "Hello world!\n"))), .wait)
Expand Down
8 changes: 4 additions & 4 deletions Tests/AsyncHTTPClientTests/HTTP1ConnectionTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ class HTTP1ConnectionTests: XCTestCase {
task: task,
redirectHandler: nil,
connectionDeadline: .now() + .seconds(60),
idleReadTimeout: nil,
requestOptions: .forTests(),
delegate: ResponseAccumulator(request: request)
))
guard let requestBag = maybeRequestBag else { return XCTFail("Expected to be able to create a request bag.") }
Expand Down Expand Up @@ -223,7 +223,7 @@ class HTTP1ConnectionTests: XCTestCase {
task: .init(eventLoop: eventLoopGroup.next(), logger: logger),
redirectHandler: nil,
connectionDeadline: .now() + .seconds(30),
idleReadTimeout: nil,
requestOptions: .forTests(),
delegate: delegate
))
guard let requestBag = maybeRequestBag else { return XCTFail("Expected to be able to create a request bag") }
Expand Down Expand Up @@ -281,7 +281,7 @@ class HTTP1ConnectionTests: XCTestCase {
task: .init(eventLoop: eventLoopGroup.next(), logger: logger),
redirectHandler: nil,
connectionDeadline: .now() + .seconds(30),
idleReadTimeout: nil,
requestOptions: .forTests(),
delegate: delegate
))
guard let requestBag = maybeRequestBag else { return XCTFail("Expected to be able to create a request bag") }
Expand Down Expand Up @@ -348,7 +348,7 @@ class HTTP1ConnectionTests: XCTestCase {
task: .init(eventLoop: eventLoopGroup.next(), logger: logger),
redirectHandler: nil,
connectionDeadline: .now() + .seconds(30),
idleReadTimeout: nil,
requestOptions: .forTests(),
delegate: delegate
))
guard let requestBag = maybeRequestBag else { return XCTFail("Expected to be able to create a request bag") }
Expand Down
Loading