Skip to content

Commit 6099619

Browse files
fabianfettfabianfett
committed
SOCKS, HTTPProxy and TLS are now protected against timeouts
1 parent 55508b0 commit 6099619

7 files changed

+229
-47
lines changed

Diff for: ‎Sources/AsyncHTTPClient/ConnectionPool/ChannelHandler/HTTP1ProxyConnectHandler.swift

+60-21
Original file line numberDiff line numberDiff line change
@@ -22,35 +22,42 @@ final class HTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableChannelHand
2222

2323
enum State {
2424
// transitions to `.connectSent` or `.failed`
25-
case initialized(EventLoopPromise<Void>)
25+
case initialized
2626
// transitions to `.headReceived` or `.failed`
27-
case connectSent(EventLoopPromise<Void>)
27+
case connectSent(Scheduled<Void>)
2828
// transitions to `.completed` or `.failed`
29-
case headReceived(EventLoopPromise<Void>)
29+
case headReceived(Scheduled<Void>)
3030
// final error state
3131
case failed(Error)
3232
// final success state
3333
case completed
3434
}
3535

36-
private var state: State
36+
private var state: State = .initialized
3737

3838
let targetHost: String
3939
let targetPort: Int
4040
let proxyAuthorization: HTTPClient.Authorization?
41+
let deadline: NIODeadline
42+
43+
private var proxyEstablishedPromise: EventLoopPromise<Void>?
44+
var proxyEstablishedFuture: EventLoopFuture<Void>! {
45+
return self.proxyEstablishedPromise?.futureResult
46+
}
4147

4248
init(targetHost: String,
4349
targetPort: Int,
4450
proxyAuthorization: HTTPClient.Authorization?,
45-
connectPromise: EventLoopPromise<Void>) {
51+
deadline: NIODeadline) {
4652
self.targetHost = targetHost
4753
self.targetPort = targetPort
4854
self.proxyAuthorization = proxyAuthorization
49-
50-
self.state = .initialized(connectPromise)
55+
self.deadline = deadline
5156
}
5257

5358
func handlerAdded(context: ChannelHandlerContext) {
59+
self.proxyEstablishedPromise = context.eventLoop.makePromise(of: Void.self)
60+
5461
if context.channel.isActive {
5562
self.sendConnect(context: context)
5663
}
@@ -61,7 +68,9 @@ final class HTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableChannelHand
6168
case .failed, .completed:
6269
break
6370
case .initialized, .connectSent, .headReceived:
64-
preconditionFailure("Removing the handler, while connecting seems wrong")
71+
struct NoResult: Error {}
72+
self.state = .failed(NoResult())
73+
self.proxyEstablishedPromise?.fail(NoResult())
6574
}
6675
}
6776

@@ -71,10 +80,14 @@ final class HTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableChannelHand
7180

7281
func channelInactive(context: ChannelHandlerContext) {
7382
switch self.state {
74-
case .initialized(let promise), .connectSent(let promise), .headReceived(let promise):
83+
case .initialized:
84+
preconditionFailure("How can we receive a channelInactive before a channelActive?")
85+
case .connectSent(let timeout), .headReceived(let timeout):
86+
timeout.cancel()
7587
let error = HTTPClientError.remoteConnectionClosed
7688
self.state = .failed(error)
77-
promise.fail(error)
89+
self.proxyEstablishedPromise?.fail(error)
90+
context.fireErrorCaught(error)
7891
case .failed:
7992
break
8093
case .completed:
@@ -102,25 +115,31 @@ final class HTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableChannelHand
102115
case 407:
103116
let error = HTTPClientError.proxyAuthenticationRequired
104117
self.state = .failed(error)
105-
context.close(promise: nil)
106-
promise.fail(error)
118+
self.proxyEstablishedPromise?.fail(error)
119+
context.fireErrorCaught(error)
120+
context.close(mode: .all, promise: nil)
121+
107122
default:
108123
// Any response other than a successful response
109124
// indicates that the tunnel has not yet been formed and that the
110125
// connection remains governed by HTTP.
111126
let error = HTTPClientError.invalidProxyResponse
112127
self.state = .failed(error)
113-
context.close(promise: nil)
114-
promise.fail(error)
128+
self.proxyEstablishedPromise?.fail(error)
129+
context.fireErrorCaught(error)
130+
context.close(mode: .all, promise: nil)
115131
}
116132
case .body:
117133
switch self.state {
118-
case .headReceived(let promise):
134+
case .headReceived(let timeout):
135+
timeout.cancel()
119136
// we don't expect a body
120137
let error = HTTPClientError.invalidProxyResponse
121138
self.state = .failed(error)
122-
context.close(promise: nil)
123-
promise.fail(error)
139+
self.proxyEstablishedPromise?.fail(error)
140+
context.fireErrorCaught(error)
141+
context.close(mode: .all, promise: nil)
142+
124143
case .failed:
125144
// ran into an error before... ignore this one
126145
break
@@ -130,9 +149,11 @@ final class HTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableChannelHand
130149

131150
case .end:
132151
switch self.state {
133-
case .headReceived(let promise):
152+
case .headReceived(let timeout):
153+
timeout.cancel()
134154
self.state = .completed
135-
promise.succeed(())
155+
self.proxyEstablishedPromise?.succeed(())
156+
136157
case .failed:
137158
// ran into an error before... ignore this one
138159
break
@@ -143,12 +164,30 @@ final class HTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableChannelHand
143164
}
144165

145166
func sendConnect(context: ChannelHandlerContext) {
146-
guard case .initialized(let promise) = self.state else {
167+
guard case .initialized = self.state else {
147168
// we might run into this handler twice, once in handlerAdded and once in channelActive.
148169
return
149170
}
150171

151-
self.state = .connectSent(promise)
172+
let timeout = context.eventLoop.scheduleTask(deadline: self.deadline) {
173+
switch self.state {
174+
case .initialized:
175+
preconditionFailure("How can we have a scheduled timeout, if the connection is not even up?")
176+
177+
case .connectSent(let scheduled), .headReceived(let scheduled):
178+
scheduled.cancel()
179+
let error = HTTPClientError.httpProxyHandshakeTimeout
180+
self.state = .failed(error)
181+
self.proxyEstablishedPromise?.fail(error)
182+
context.fireErrorCaught(error)
183+
context.close(mode: .all, promise: nil)
184+
185+
case .failed, .completed:
186+
break
187+
}
188+
}
189+
190+
self.state = .connectSent(timeout)
152191

153192
var head = HTTPRequestHead(
154193
version: .init(major: 1, minor: 1),

Diff for: ‎Sources/AsyncHTTPClient/ConnectionPool/ChannelHandler/TLSEventsHandler.swift

+2-2
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ final class TLSEventsHandler: ChannelInboundHandler, RemovableChannelHandler {
2121
enum State {
2222
// transitions to channelActive or failed
2323
case initialized
24-
// transitions to socksEstablished or failed
24+
// transitions to tlsEstablished or failed
2525
case channelActive(Scheduled<Void>?)
2626
// final success state
2727
case tlsEstablished
@@ -109,7 +109,7 @@ final class TLSEventsHandler: ChannelInboundHandler, RemovableChannelHandler {
109109
case .initialized, .channelActive:
110110
// close the connection, if the handshake timed out
111111
context.close(mode: .all, promise: nil)
112-
let error = HTTPClientError.socksHandshakeTimeout
112+
let error = HTTPClientError.tlsHandshakeTimeout
113113
self.state = .failed(error)
114114
self.tlsEstablishedPromise?.fail(error)
115115
case .failed, .tlsEstablished:

Diff for: ‎Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift

+2-4
Original file line numberDiff line numberDiff line change
@@ -97,15 +97,13 @@ extension HTTPConnectionPool.ConnectionFactory {
9797
// upgraded to TLS before we send our first request.
9898
let bootstrap = self.makePlainBootstrap(eventLoop: eventLoop)
9999
return bootstrap.connect(host: proxy.host, port: proxy.port).flatMap { channel in
100-
let connectPromise = channel.eventLoop.makePromise(of: Void.self)
101-
102100
let encoder = HTTPRequestEncoder()
103101
let decoder = ByteToMessageHandler(HTTPResponseDecoder(leftOverBytesStrategy: .dropBytes))
104102
let proxyHandler = HTTP1ProxyConnectHandler(
105103
targetHost: self.key.host,
106104
targetPort: self.key.port,
107105
proxyAuthorization: proxy.authorization,
108-
connectPromise: connectPromise
106+
deadline: .now() + .seconds(10)
109107
)
110108

111109
do {
@@ -116,7 +114,7 @@ extension HTTPConnectionPool.ConnectionFactory {
116114
return channel.eventLoop.makeFailedFuture(error)
117115
}
118116

119-
return connectPromise.futureResult.flatMap {
117+
return proxyHandler.proxyEstablishedFuture.flatMap {
120118
channel.pipeline.removeHandler(proxyHandler).flatMap {
121119
channel.pipeline.removeHandler(decoder).flatMap {
122120
channel.pipeline.removeHandler(encoder)

Diff for: ‎Tests/AsyncHTTPClientTests/HTTP1ProxyConnectHandlerTests.swift

+15-20
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,14 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
2222
let embedded = EmbeddedChannel()
2323
defer { XCTAssertNoThrow(try embedded.finish(acceptAlreadyClosed: false)) }
2424

25-
let socketAddress = try! SocketAddress.makeAddressResolvingHost("localhost", port: 3000)
25+
let socketAddress = try! SocketAddress.makeAddressResolvingHost("localhost", port: 0)
2626
XCTAssertNoThrow(try embedded.connect(to: socketAddress).wait())
2727

28-
let connectPromise = embedded.eventLoop.makePromise(of: Void.self)
2928
let proxyConnectHandler = HTTP1ProxyConnectHandler(
3029
targetHost: "swift.org",
3130
targetPort: 443,
3231
proxyAuthorization: .none,
33-
connectPromise: connectPromise
32+
deadline: .now() + .seconds(10)
3433
)
3534

3635
XCTAssertNoThrow(try embedded.pipeline.syncOperations.addHandler(proxyConnectHandler))
@@ -50,21 +49,20 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
5049
XCTAssertNoThrow(try embedded.writeInbound(HTTPClientResponsePart.head(responseHead)))
5150
XCTAssertNoThrow(try embedded.writeInbound(HTTPClientResponsePart.end(nil)))
5251

53-
XCTAssertNoThrow(try connectPromise.futureResult.wait())
52+
XCTAssertNoThrow(try proxyConnectHandler.proxyEstablishedFuture.wait())
5453
}
5554

5655
func testProxyConnectWithAuthorization() {
5756
let embedded = EmbeddedChannel()
5857

59-
let socketAddress = try! SocketAddress.makeAddressResolvingHost("localhost", port: 3000)
58+
let socketAddress = try! SocketAddress.makeAddressResolvingHost("localhost", port: 0)
6059
XCTAssertNoThrow(try embedded.connect(to: socketAddress).wait())
6160

62-
let connectPromise = embedded.eventLoop.makePromise(of: Void.self)
6361
let proxyConnectHandler = HTTP1ProxyConnectHandler(
6462
targetHost: "swift.org",
6563
targetPort: 443,
6664
proxyAuthorization: .basic(credentials: "abc123"),
67-
connectPromise: connectPromise
65+
deadline: .now() + .seconds(10)
6866
)
6967

7068
XCTAssertNoThrow(try embedded.pipeline.syncOperations.addHandler(proxyConnectHandler))
@@ -84,21 +82,20 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
8482
XCTAssertNoThrow(try embedded.writeInbound(HTTPClientResponsePart.head(responseHead)))
8583
XCTAssertNoThrow(try embedded.writeInbound(HTTPClientResponsePart.end(nil)))
8684

87-
connectPromise.succeed(())
85+
XCTAssertNoThrow(try proxyConnectHandler.proxyEstablishedFuture.wait())
8886
}
8987

9088
func testProxyConnectWithoutAuthorizationFailure500() {
9189
let embedded = EmbeddedChannel()
9290

93-
let socketAddress = try! SocketAddress.makeAddressResolvingHost("localhost", port: 3000)
91+
let socketAddress = try! SocketAddress.makeAddressResolvingHost("localhost", port: 0)
9492
XCTAssertNoThrow(try embedded.connect(to: socketAddress).wait())
9593

96-
let connectPromise = embedded.eventLoop.makePromise(of: Void.self)
9794
let proxyConnectHandler = HTTP1ProxyConnectHandler(
9895
targetHost: "swift.org",
9996
targetPort: 443,
10097
proxyAuthorization: .none,
101-
connectPromise: connectPromise
98+
deadline: .now() + .seconds(10)
10299
)
103100

104101
XCTAssertNoThrow(try embedded.pipeline.syncOperations.addHandler(proxyConnectHandler))
@@ -119,23 +116,22 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
119116
XCTAssertEqual(embedded.isActive, false)
120117
XCTAssertNoThrow(try embedded.writeInbound(HTTPClientResponsePart.end(nil)))
121118

122-
XCTAssertThrowsError(try connectPromise.futureResult.wait()) { error in
119+
XCTAssertThrowsError(try proxyConnectHandler.proxyEstablishedFuture.wait()) { error in
123120
XCTAssertEqual(error as? HTTPClientError, .invalidProxyResponse)
124121
}
125122
}
126123

127124
func testProxyConnectWithoutAuthorizationButAuthorizationNeeded() {
128125
let embedded = EmbeddedChannel()
129126

130-
let socketAddress = try! SocketAddress.makeAddressResolvingHost("localhost", port: 3000)
127+
let socketAddress = try! SocketAddress.makeAddressResolvingHost("localhost", port: 0)
131128
XCTAssertNoThrow(try embedded.connect(to: socketAddress).wait())
132129

133-
let connectPromise = embedded.eventLoop.makePromise(of: Void.self)
134130
let proxyConnectHandler = HTTP1ProxyConnectHandler(
135131
targetHost: "swift.org",
136132
targetPort: 443,
137133
proxyAuthorization: .none,
138-
connectPromise: connectPromise
134+
deadline: .now() + .seconds(10)
139135
)
140136

141137
XCTAssertNoThrow(try embedded.pipeline.syncOperations.addHandler(proxyConnectHandler))
@@ -156,23 +152,22 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
156152
XCTAssertEqual(embedded.isActive, false)
157153
XCTAssertNoThrow(try embedded.writeInbound(HTTPClientResponsePart.end(nil)))
158154

159-
XCTAssertThrowsError(try connectPromise.futureResult.wait()) { error in
155+
XCTAssertThrowsError(try proxyConnectHandler.proxyEstablishedFuture.wait()) { error in
160156
XCTAssertEqual(error as? HTTPClientError, .proxyAuthenticationRequired)
161157
}
162158
}
163159

164160
func testProxyConnectReceivesBody() {
165161
let embedded = EmbeddedChannel()
166162

167-
let socketAddress = try! SocketAddress.makeAddressResolvingHost("localhost", port: 3000)
163+
let socketAddress = try! SocketAddress.makeAddressResolvingHost("localhost", port: 0)
168164
XCTAssertNoThrow(try embedded.connect(to: socketAddress).wait())
169165

170-
let connectPromise = embedded.eventLoop.makePromise(of: Void.self)
171166
let proxyConnectHandler = HTTP1ProxyConnectHandler(
172167
targetHost: "swift.org",
173168
targetPort: 443,
174169
proxyAuthorization: .none,
175-
connectPromise: connectPromise
170+
deadline: .now() + .seconds(10)
176171
)
177172

178173
XCTAssertNoThrow(try embedded.pipeline.syncOperations.addHandler(proxyConnectHandler))
@@ -193,7 +188,7 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
193188
XCTAssertEqual(embedded.isActive, false)
194189
XCTAssertNoThrow(try embedded.writeInbound(HTTPClientResponsePart.end(nil)))
195190

196-
XCTAssertThrowsError(try connectPromise.futureResult.wait()) { error in
191+
XCTAssertThrowsError(try proxyConnectHandler.proxyEstablishedFuture.wait()) { error in
197192
XCTAssertEqual(error as? HTTPClientError, .invalidProxyResponse)
198193
}
199194
}

Diff for: ‎Tests/AsyncHTTPClientTests/HTTPConnectionPool+FactoryTests+XCTest.swift

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the AsyncHTTPClient open source project
4+
//
5+
// Copyright (c) 2018-2019 Apple Inc. and the AsyncHTTPClient project authors
6+
// Licensed under Apache License v2.0
7+
//
8+
// See LICENSE.txt for license information
9+
// See CONTRIBUTORS.txt for the list of AsyncHTTPClient project authors
10+
//
11+
// SPDX-License-Identifier: Apache-2.0
12+
//
13+
//===----------------------------------------------------------------------===//
14+
//
15+
// HTTPConnectionPool+FactoryTests+XCTest.swift
16+
//
17+
import XCTest
18+
19+
///
20+
/// NOTE: This file was generated by generate_linux_tests.rb
21+
///
22+
/// Do NOT edit this file directly as it will be regenerated automatically when needed.
23+
///
24+
25+
extension HTTPConnectionPool_FactoryTests {
26+
static var allTests: [(String, (HTTPConnectionPool_FactoryTests) -> () throws -> Void)] {
27+
return [
28+
("testSOCKSConnectionCreationTimesoutIfRemoteIsUnresponsive", testSOCKSConnectionCreationTimesoutIfRemoteIsUnresponsive),
29+
("testHTTPProxyConnectionCreationTimesoutIfRemoteIsUnresponsive", testHTTPProxyConnectionCreationTimesoutIfRemoteIsUnresponsive),
30+
("testTLSConnectionCreationTimesoutIfRemoteIsUnresponsive", testTLSConnectionCreationTimesoutIfRemoteIsUnresponsive),
31+
]
32+
}
33+
}

0 commit comments

Comments
 (0)