Skip to content

Commit df1217e

Browse files
fabianfettfabianfett
committed
Handle NIOSSLError.uncleanShutdown correctly
1 parent 47c7a7b commit df1217e

16 files changed

+442
-396
lines changed

Diff for: ‎Sources/AsyncHTTPClient/ConnectionPool/HTTP1.1/HTTP1ClientChannelHandler.swift

+1-2
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,7 @@ final class HTTP1ClientChannelHandler: ChannelDuplexHandler {
153153

154154
let action = self.state.runNewRequest(
155155
head: req.requestHead,
156-
metadata: req.requestFramingMetadata,
157-
ignoreUncleanSSLShutdown: req.requestOptions.ignoreUncleanSSLShutdown
156+
metadata: req.requestFramingMetadata
158157
)
159158
self.run(action, context: context)
160159
}

Diff for: ‎Sources/AsyncHTTPClient/ConnectionPool/HTTP1.1/HTTP1ConnectionStateMachine.swift

+2-6
Original file line numberDiff line numberDiff line change
@@ -156,17 +156,13 @@ struct HTTP1ConnectionStateMachine {
156156

157157
mutating func runNewRequest(
158158
head: HTTPRequestHead,
159-
metadata: RequestFramingMetadata,
160-
ignoreUncleanSSLShutdown: Bool
159+
metadata: RequestFramingMetadata
161160
) -> Action {
162161
guard case .idle = self.state else {
163162
preconditionFailure("Invalid state")
164163
}
165164

166-
var requestStateMachine = HTTPRequestStateMachine(
167-
isChannelWritable: self.isChannelWritable,
168-
ignoreUncleanSSLShutdown: ignoreUncleanSSLShutdown
169-
)
165+
var requestStateMachine = HTTPRequestStateMachine(isChannelWritable: self.isChannelWritable)
170166
let action = requestStateMachine.startRequest(head: head, metadata: metadata)
171167

172168
// by default we assume a persistent connection. however in `requestVerified`, we read the

Diff for: ‎Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2ClientRequestHandler.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ final class HTTP2ClientRequestHandler: ChannelDuplexHandler {
2424

2525
private let eventLoop: EventLoop
2626

27-
private var state: HTTPRequestStateMachine = .init(isChannelWritable: false, ignoreUncleanSSLShutdown: false) {
27+
private var state: HTTPRequestStateMachine = .init(isChannelWritable: false) {
2828
willSet {
2929
self.eventLoop.assertInEventLoop()
3030
}

Diff for: ‎Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift

+24-7
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,8 @@ struct HTTPRequestStateMachine {
103103

104104
private var isChannelWritable: Bool
105105

106-
private let ignoreUncleanSSLShutdown: Bool
107-
108-
init(isChannelWritable: Bool, ignoreUncleanSSLShutdown: Bool) {
106+
init(isChannelWritable: Bool) {
109107
self.isChannelWritable = isChannelWritable
110-
self.ignoreUncleanSSLShutdown = ignoreUncleanSSLShutdown
111108
}
112109

113110
mutating func startRequest(head: HTTPRequestHead, metadata: RequestFramingMetadata) -> Action {
@@ -201,17 +198,37 @@ struct HTTPRequestStateMachine {
201198
self.state = .failed(error)
202199
return .failRequest(error, .none)
203200

204-
case .running(.streaming, .receivingBody),
205-
.running(.endSent, .receivingBody)
206-
where error as? NIOSSLError == .uncleanShutdown && self.ignoreUncleanSSLShutdown == true:
201+
case .running(.streaming, .waitingForHead),
202+
.running(.endSent, .waitingForHead) where error as? NIOSSLError == .uncleanShutdown:
203+
// if we received a NIOSSL.uncleanShutdown before we got an answer we should handle
204+
// this like a normal connection close. We will receive a call to channelInactive after
205+
// this error.
207206
return .wait
208207

208+
case .running(.streaming, .receivingBody(let responseHead, _)),
209+
.running(.endSent, .receivingBody(let responseHead, _)) where error as? NIOSSLError == .uncleanShutdown:
210+
211+
// if we have already received the response head, the parser will ensure that we receive
212+
// the complete response. we can ignore this error. we might see a HTTPParserError very
213+
// soon.
214+
if responseHead.headers.contains(name: "content-length") || responseHead.headers.contains(name: "transfer-encoding") {
215+
return .wait
216+
}
217+
218+
// if the response is EOF terminated, we need to rely on a clean tls shutdown to be sure
219+
// we have received all necessary bytes. For this reason we forward the uncleanShutdown
220+
// error to the user.
221+
self.state = .failed(error)
222+
return .failRequest(error, .close)
223+
209224
case .running:
210225
self.state = .failed(error)
211226
return .failRequest(error, .close)
227+
212228
case .finished, .failed:
213229
// ignore error
214230
return .wait
231+
215232
case .modifying:
216233
preconditionFailure("Invalid state: \(self.state)")
217234
}

Diff for: ‎Sources/AsyncHTTPClient/ConnectionPool/RequestOptions.swift

+2-7
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,15 @@ struct RequestOptions {
1818
/// The maximal `TimeAmount` that is allowed to pass between `channelRead`s from the Channel.
1919
var idleReadTimeout: TimeAmount?
2020

21-
/// Should `NIOSSLError.uncleanShutdown` be forwarded to the user in HTTP/1 mode.
22-
var ignoreUncleanSSLShutdown: Bool
23-
24-
init(idleReadTimeout: TimeAmount?, ignoreUncleanSSLShutdown: Bool) {
21+
init(idleReadTimeout: TimeAmount?) {
2522
self.idleReadTimeout = idleReadTimeout
26-
self.ignoreUncleanSSLShutdown = ignoreUncleanSSLShutdown
2723
}
2824
}
2925

3026
extension RequestOptions {
3127
static func fromClientConfiguration(_ configuration: HTTPClient.Configuration) -> Self {
3228
RequestOptions(
33-
idleReadTimeout: configuration.timeout.read,
34-
ignoreUncleanSSLShutdown: configuration.ignoreUncleanSSLShutdown
29+
idleReadTimeout: configuration.timeout.read
3530
)
3631
}
3732
}

Diff for: ‎Sources/AsyncHTTPClient/HTTPClient.swift

+5-2
Original file line numberDiff line numberDiff line change
@@ -601,7 +601,11 @@ public class HTTPClient {
601601
/// Enables automatic body decompression. Supported algorithms are gzip and deflate.
602602
public var decompression: Decompression
603603
/// Ignore TLS unclean shutdown error, defaults to `false`.
604-
public var ignoreUncleanSSLShutdown: Bool
604+
@available(*, deprecated, message: "AsyncHTTPClient now correctly supports handling unexpected SSL connection drops. This property is ignored")
605+
public var ignoreUncleanSSLShutdown: Bool {
606+
get { false }
607+
set {}
608+
}
605609

606610
// TODO: make public
607611
// TODO: set to automatic by default
@@ -645,7 +649,6 @@ public class HTTPClient {
645649
self.timeout = timeout
646650
self.connectionPool = connectionPool
647651
self.proxy = proxy
648-
self.ignoreUncleanSSLShutdown = ignoreUncleanSSLShutdown
649652
self.decompression = decompression
650653
self.httpVersion = httpVersion
651654
}

Diff for: ‎Tests/AsyncHTTPClientTests/HTTP1ConnectionStateMachineTests.swift

+12-12
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase {
2525

2626
let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: ["content-length": "4"])
2727
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(4))
28-
XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false), .wait)
28+
XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata), .wait)
2929
XCTAssertEqual(state.writabilityChanged(writable: true), .sendRequestHead(requestHead, startBody: true))
3030

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

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

6969
let responseHead = HTTPResponseHead(version: .http1_1, status: .ok, headers: ["content-length": "12"])
@@ -91,7 +91,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase {
9191
XCTAssertEqual(state.channelActive(isWritable: true), .fireChannelActive)
9292
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/", headers: ["connection": "close"])
9393
let metadata = RequestFramingMetadata(connectionClose: true, body: .none)
94-
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false)
94+
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata)
9595
XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false))
9696

9797
let responseHead = HTTPResponseHead(version: .http1_1, status: .ok)
@@ -107,7 +107,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase {
107107
XCTAssertEqual(state.channelActive(isWritable: true), .fireChannelActive)
108108
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
109109
let metadata = RequestFramingMetadata(connectionClose: false, body: .none)
110-
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false)
110+
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata)
111111
XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false))
112112

113113
let responseHead = HTTPResponseHead(version: .http1_0, status: .ok, headers: ["content-length": "4"])
@@ -123,7 +123,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase {
123123
XCTAssertEqual(state.channelActive(isWritable: true), .fireChannelActive)
124124
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
125125
let metadata = RequestFramingMetadata(connectionClose: false, body: .none)
126-
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false)
126+
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata)
127127
XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false))
128128

129129
let responseHead = HTTPResponseHead(version: .http1_0, status: .ok, headers: ["content-length": "4", "connection": "keep-alive"])
@@ -140,7 +140,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase {
140140
XCTAssertEqual(state.writabilityChanged(writable: true), .wait)
141141
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
142142
let metadata = RequestFramingMetadata(connectionClose: false, body: .none)
143-
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false)
143+
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata)
144144
XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false))
145145

146146
let responseHead = HTTPResponseHead(version: .http1_1, status: .ok, headers: ["connection": "close"])
@@ -169,7 +169,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase {
169169

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

175175
XCTAssertEqual(state.channelInactive(), .failRequest(HTTPClientError.remoteConnectionClosed, .none))
@@ -181,7 +181,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase {
181181

182182
let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: ["content-length": "4"])
183183
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(4))
184-
XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false), .wait)
184+
XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata), .wait)
185185
XCTAssertEqual(state.writabilityChanged(writable: true), .sendRequestHead(requestHead, startBody: true))
186186

187187
let part0 = IOData.byteBuffer(ByteBuffer(bytes: [0]))
@@ -225,7 +225,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase {
225225
XCTAssertEqual(state.channelActive(isWritable: false), .fireChannelActive)
226226
let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: ["content-length": "4"])
227227
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(4))
228-
XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false), .wait)
228+
XCTAssertEqual(state.runNewRequest(head: requestHead, metadata: metadata), .wait)
229229
XCTAssertEqual(state.requestCancelled(closeConnection: false), .failRequest(HTTPClientError.cancelled, .informConnectionIsIdle))
230230
}
231231

@@ -234,7 +234,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase {
234234
XCTAssertEqual(state.channelActive(isWritable: true), .fireChannelActive)
235235
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
236236
let metadata = RequestFramingMetadata(connectionClose: false, body: .none)
237-
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false)
237+
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata)
238238
XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false))
239239
let responseHead = HTTPResponseHead(version: .http1_1, status: .ok)
240240
XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false))
@@ -249,7 +249,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase {
249249
XCTAssertEqual(state.channelActive(isWritable: true), .fireChannelActive)
250250
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
251251
let metadata = RequestFramingMetadata(connectionClose: false, body: .none)
252-
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false)
252+
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata)
253253
XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false))
254254
let responseHead = HTTPResponseHead(version: .http1_1, status: .switchingProtocols)
255255
XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false))
@@ -261,7 +261,7 @@ class HTTP1ConnectionStateMachineTests: XCTestCase {
261261
XCTAssertEqual(state.channelActive(isWritable: true), .fireChannelActive)
262262
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
263263
let metadata = RequestFramingMetadata(connectionClose: false, body: .none)
264-
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata, ignoreUncleanSSLShutdown: false)
264+
let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata)
265265
XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, startBody: false))
266266
let responseHead = HTTPResponseHead(version: .http1_1, status: .init(statusCode: 103, reasonPhrase: "Early Hints"))
267267
XCTAssertEqual(state.channelRead(.head(responseHead)), .wait)

Diff for: ‎Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift

-8
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,6 @@ extension HTTPClientTests {
5959
("testProxyPlaintextWithCorrectlyAuthorization", testProxyPlaintextWithCorrectlyAuthorization),
6060
("testProxyPlaintextWithIncorrectlyAuthorization", testProxyPlaintextWithIncorrectlyAuthorization),
6161
("testUploadStreaming", testUploadStreaming),
62-
("testNoContentLengthForSSLUncleanShutdown", testNoContentLengthForSSLUncleanShutdown),
63-
("testNoContentLengthWithIgnoreErrorForSSLUncleanShutdown", testNoContentLengthWithIgnoreErrorForSSLUncleanShutdown),
64-
("testCorrectContentLengthForSSLUncleanShutdown", testCorrectContentLengthForSSLUncleanShutdown),
65-
("testNoContentForSSLUncleanShutdown", testNoContentForSSLUncleanShutdown),
66-
("testNoResponseForSSLUncleanShutdown", testNoResponseForSSLUncleanShutdown),
67-
("testNoResponseWithIgnoreErrorForSSLUncleanShutdown", testNoResponseWithIgnoreErrorForSSLUncleanShutdown),
68-
("testWrongContentLengthForSSLUncleanShutdown", testWrongContentLengthForSSLUncleanShutdown),
69-
("testWrongContentLengthWithIgnoreErrorForSSLUncleanShutdown", testWrongContentLengthWithIgnoreErrorForSSLUncleanShutdown),
7062
("testEventLoopArgument", testEventLoopArgument),
7163
("testDecompression", testDecompression),
7264
("testDecompressionLimit", testDecompressionLimit),

0 commit comments

Comments
 (0)