Skip to content

Commit 6c4d28a

Browse files
rnrornro
committed
promise changes in tests, review changes
- include responseHEAD in invalid response errors - remove precondition in favor of error in event of an unexpected proxy response
1 parent e2203ca commit 6c4d28a

File tree

2 files changed

+62
-42
lines changed

2 files changed

+62
-42
lines changed

‎Sources/NIOExtras/HTTP1ProxyConnectHandler.swift

Lines changed: 40 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public final class NIOHTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableC
3939
private let targetPort: Int
4040
private let headers: HTTPHeaders
4141
private let deadline: NIODeadline
42-
private let promise: EventLoopPromise<Void>?
42+
private let promise: EventLoopPromise<Void>
4343

4444
/// Creates a new ``NIOHTTP1ProxyConnectHandler`` that issues a CONNECT request to a proxy server
4545
/// and instructs the server to connect to `targetHost`.
@@ -53,7 +53,7 @@ public final class NIOHTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableC
5353
targetPort: Int,
5454
headers: HTTPHeaders,
5555
deadline: NIODeadline,
56-
promise: EventLoopPromise<Void>?) {
56+
promise: EventLoopPromise<Void>) {
5757
self.targetHost = targetHost
5858
self.targetPort = targetPort
5959
self.headers = headers
@@ -62,7 +62,9 @@ public final class NIOHTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableC
6262
}
6363

6464
public func handlerAdded(context: ChannelHandlerContext) {
65-
self.sendConnect(context: context)
65+
if context.channel.isActive {
66+
self.sendConnect(context: context)
67+
}
6668
}
6769

6870
public func handlerRemoved(context: ChannelHandlerContext) {
@@ -71,7 +73,7 @@ public final class NIOHTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableC
7173
break
7274
case .initialized, .connectSent, .headReceived:
7375
self.state = .failed(Error.noResult)
74-
self.promise?.fail(Error.noResult)
76+
self.promise.fail(Error.noResult)
7577
}
7678
}
7779

@@ -143,23 +145,26 @@ public final class NIOHTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableC
143145
}
144146

145147
private func handleHTTPHeadReceived(_ head: HTTPResponseHead, context: ChannelHandlerContext) {
146-
guard case .connectSent(let scheduled) = self.state else {
147-
preconditionFailure("HTTPDecoder should throw an error, if we have not send a request")
148-
}
149-
150-
switch head.status.code {
151-
case 200..<300:
152-
// Any 2xx (Successful) response indicates that the sender (and all
153-
// inbound proxies) will switch to tunnel mode immediately after the
154-
// blank line that concludes the successful response's header section
155-
self.state = .headReceived(scheduled)
156-
case 407:
157-
self.failWithError(Error.proxyAuthenticationRequired, context: context)
158-
159-
default:
160-
// Any response other than a successful response indicates that the tunnel
161-
// has not yet been formed and that the connection remains governed by HTTP.
162-
self.failWithError(Error.invalidProxyResponse, context: context)
148+
switch self.state {
149+
case .connectSent(let scheduled):
150+
switch head.status.code {
151+
case 200..<300:
152+
// Any 2xx (Successful) response indicates that the sender (and all
153+
// inbound proxies) will switch to tunnel mode immediately after the
154+
// blank line that concludes the successful response's header section
155+
self.state = .headReceived(scheduled)
156+
case 407:
157+
self.failWithError(Error.proxyAuthenticationRequired, context: context)
158+
159+
default:
160+
// Any response other than a successful response indicates that the tunnel
161+
// has not yet been formed and that the connection remains governed by HTTP.
162+
self.failWithError(Error.invalidProxyResponseHead(head), context: context)
163+
}
164+
case .failed, .completed:
165+
break
166+
case .initialized, .headReceived:
167+
preconditionFailure("Invalid state: \(self.state)")
163168
}
164169
}
165170

@@ -182,7 +187,7 @@ public final class NIOHTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableC
182187
case .headReceived(let timeout):
183188
timeout.cancel()
184189
self.state = .completed
185-
self.promise?.succeed(())
190+
self.promise.succeed(())
186191

187192
case .failed:
188193
// ran into an error before... ignore this one
@@ -194,31 +199,35 @@ public final class NIOHTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableC
194199

195200
private func failWithError(_ error: Error, context: ChannelHandlerContext, closeConnection: Bool = true) {
196201
self.state = .failed(error)
197-
self.promise?.fail(error)
202+
self.promise.fail(error)
198203
context.fireErrorCaught(error)
199204
if closeConnection {
200205
context.close(mode: .all, promise: nil)
201206
}
202207
}
203208

204209
/// Error types for ``HTTP1ProxyConnectHandler``
205-
public struct Error: Swift.Error, CustomStringConvertible, Equatable {
206-
fileprivate enum ErrorEnum: String {
210+
public struct Error: Swift.Error, Equatable {
211+
212+
fileprivate enum ErrorEnum: Equatable {
207213
case proxyAuthenticationRequired
214+
case invalidProxyResponseHead(head: HTTPResponseHead)
208215
case invalidProxyResponse
209216
case remoteConnectionClosed
210217
case httpProxyHandshakeTimeout
211218
case noResult
212219
}
213220
fileprivate let error: ErrorEnum
214221

215-
/// return as String
216-
public var description: String { return error.rawValue }
217-
218222
/// Proxy response status `407` indicates that authentication is required
219223
public static let proxyAuthenticationRequired = Error(error: .proxyAuthenticationRequired)
220224

221-
/// Proxy response contains unexpected status or body
225+
/// Proxy response contains unexpected status
226+
public static func invalidProxyResponseHead(_ head: HTTPResponseHead) -> Error {
227+
Error(error: .invalidProxyResponseHead(head: head))
228+
}
229+
230+
/// Proxy response contains unexpected body
222231
public static let invalidProxyResponse = Error(error: .invalidProxyResponse)
223232

224233
/// Connection has been closed for ongoing request
@@ -229,4 +238,5 @@ public final class NIOHTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableC
229238

230239
/// Handler was removed before we received a result for the request
231240
public static let noResult = Error(error: .noResult)
232-
}}
241+
}
242+
}

‎Tests/NIOExtrasTests/HTTP1ProxyConnectHandlerTests.swift

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,13 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
2626
let socketAddress = try! SocketAddress.makeAddressResolvingHost("localhost", port: 0)
2727
XCTAssertNoThrow(try embedded.connect(to: socketAddress).wait())
2828

29+
let promise: EventLoopPromise<Void> = embedded.eventLoop.makePromise()
2930
let proxyConnectHandler = NIOHTTP1ProxyConnectHandler(
3031
targetHost: "swift.org",
3132
targetPort: 443,
3233
headers: [:],
33-
deadline: .now() + .seconds(10)
34+
deadline: .now() + .seconds(10),
35+
promise: promise
3436
)
3537

3638
XCTAssertNoThrow(try embedded.pipeline.syncOperations.addHandler(proxyConnectHandler))
@@ -50,7 +52,7 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
5052
XCTAssertNoThrow(try embedded.writeInbound(HTTPClientResponsePart.head(responseHead)))
5153
XCTAssertNoThrow(try embedded.writeInbound(HTTPClientResponsePart.end(nil)))
5254

53-
XCTAssertNoThrow(try XCTUnwrap(proxyConnectHandler.proxyEstablishedFuture).wait())
55+
XCTAssertNoThrow(try promise.futureResult.wait())
5456
}
5557

5658
func testProxyConnectWithAuthorization() {
@@ -59,11 +61,13 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
5961
let socketAddress = try! SocketAddress.makeAddressResolvingHost("localhost", port: 0)
6062
XCTAssertNoThrow(try embedded.connect(to: socketAddress).wait())
6163

64+
let promise: EventLoopPromise<Void> = embedded.eventLoop.makePromise()
6265
let proxyConnectHandler = NIOHTTP1ProxyConnectHandler(
6366
targetHost: "swift.org",
6467
targetPort: 443,
6568
headers: ["proxy-authorization" : "Basic abc123"],
66-
deadline: .now() + .seconds(10)
69+
deadline: .now() + .seconds(10),
70+
promise: promise
6771
)
6872

6973
XCTAssertNoThrow(try embedded.pipeline.syncOperations.addHandler(proxyConnectHandler))
@@ -83,7 +87,7 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
8387
XCTAssertNoThrow(try embedded.writeInbound(HTTPClientResponsePart.head(responseHead)))
8488
XCTAssertNoThrow(try embedded.writeInbound(HTTPClientResponsePart.end(nil)))
8589

86-
XCTAssertNoThrow(try XCTUnwrap(proxyConnectHandler.proxyEstablishedFuture).wait())
90+
XCTAssertNoThrow(try promise.futureResult.wait())
8791
}
8892

8993
func testProxyConnectWithoutAuthorizationFailure500() {
@@ -92,11 +96,13 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
9296
let socketAddress = try! SocketAddress.makeAddressResolvingHost("localhost", port: 0)
9397
XCTAssertNoThrow(try embedded.connect(to: socketAddress).wait())
9498

99+
let promise: EventLoopPromise<Void> = embedded.eventLoop.makePromise()
95100
let proxyConnectHandler = NIOHTTP1ProxyConnectHandler(
96101
targetHost: "swift.org",
97102
targetPort: 443,
98103
headers: [:],
99-
deadline: .now() + .seconds(10)
104+
deadline: .now() + .seconds(10),
105+
promise: promise
100106
)
101107

102108
XCTAssertNoThrow(try embedded.pipeline.syncOperations.addHandler(proxyConnectHandler))
@@ -115,13 +121,13 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
115121
let responseHead = HTTPResponseHead(version: .http1_1, status: .internalServerError)
116122
// answering with 500 should lead to a triggered error in pipeline
117123
XCTAssertThrowsError(try embedded.writeInbound(HTTPClientResponsePart.head(responseHead))) {
118-
XCTAssertEqual($0 as? NIOHTTP1ProxyConnectHandler.Error, .invalidProxyResponse)
124+
XCTAssertEqual($0 as? NIOHTTP1ProxyConnectHandler.Error, .invalidProxyResponseHead(responseHead))
119125
}
120126
XCTAssertFalse(embedded.isActive, "Channel should be closed in response to the error")
121127
XCTAssertNoThrow(try embedded.writeInbound(HTTPClientResponsePart.end(nil)))
122128

123-
XCTAssertThrowsError(try XCTUnwrap(proxyConnectHandler.proxyEstablishedFuture).wait()) {
124-
XCTAssertEqual($0 as? NIOHTTP1ProxyConnectHandler.Error, .invalidProxyResponse)
129+
XCTAssertThrowsError(try promise.futureResult.wait()) {
130+
XCTAssertEqual($0 as? NIOHTTP1ProxyConnectHandler.Error, .invalidProxyResponseHead(responseHead))
125131
}
126132
}
127133

@@ -131,11 +137,13 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
131137
let socketAddress = try! SocketAddress.makeAddressResolvingHost("localhost", port: 0)
132138
XCTAssertNoThrow(try embedded.connect(to: socketAddress).wait())
133139

140+
let promise: EventLoopPromise<Void> = embedded.eventLoop.makePromise()
134141
let proxyConnectHandler = NIOHTTP1ProxyConnectHandler(
135142
targetHost: "swift.org",
136143
targetPort: 443,
137144
headers: [:],
138-
deadline: .now() + .seconds(10)
145+
deadline: .now() + .seconds(10),
146+
promise: promise
139147
)
140148

141149
XCTAssertNoThrow(try embedded.pipeline.syncOperations.addHandler(proxyConnectHandler))
@@ -159,7 +167,7 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
159167
XCTAssertFalse(embedded.isActive, "Channel should be closed in response to the error")
160168
XCTAssertNoThrow(try embedded.writeInbound(HTTPClientResponsePart.end(nil)))
161169

162-
XCTAssertThrowsError(try XCTUnwrap(proxyConnectHandler.proxyEstablishedFuture).wait()) {
170+
XCTAssertThrowsError(try promise.futureResult.wait()) {
163171
XCTAssertEqual($0 as? NIOHTTP1ProxyConnectHandler.Error, .proxyAuthenticationRequired)
164172
}
165173
}
@@ -170,11 +178,13 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
170178
let socketAddress = try! SocketAddress.makeAddressResolvingHost("localhost", port: 0)
171179
XCTAssertNoThrow(try embedded.connect(to: socketAddress).wait())
172180

181+
let promise: EventLoopPromise<Void> = embedded.eventLoop.makePromise()
173182
let proxyConnectHandler = NIOHTTP1ProxyConnectHandler(
174183
targetHost: "swift.org",
175184
targetPort: 443,
176185
headers: [:],
177-
deadline: .now() + .seconds(10)
186+
deadline: .now() + .seconds(10),
187+
promise: promise
178188
)
179189

180190
XCTAssertNoThrow(try embedded.pipeline.syncOperations.addHandler(proxyConnectHandler))
@@ -198,7 +208,7 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
198208
XCTAssertEqual(embedded.isActive, false)
199209
XCTAssertNoThrow(try embedded.writeInbound(HTTPClientResponsePart.end(nil)))
200210

201-
XCTAssertThrowsError(try XCTUnwrap(proxyConnectHandler.proxyEstablishedFuture).wait()) {
211+
XCTAssertThrowsError(try promise.futureResult.wait()) {
202212
XCTAssertEqual($0 as? NIOHTTP1ProxyConnectHandler.Error, .invalidProxyResponse)
203213
}
204214
}

0 commit comments

Comments
 (0)