Fixed testProxyTLS()

The ssl handler has to be added after the proxy handler has run. Resurrected handshakePromise.

Author: adam-fowler

Sources/AsyncHTTPClient/ConnectionPool.swift

@@ -373,15 +373,42 @@ final class ConnectionPool {
             }
         }
 
+        // if the configuration includes a proxy and requires TLS, TLS will not have been enabled yet. This
+        // returns whether we need to call addSSLHandlerIfNeeded().
+        private func requiresSSLHandler(on eventLoop: EventLoop) -> Bool {
+            // if a proxy is not set return false, otherwise for non-TS situation return true, if TS is available and
+            // either we don't have an NIOTSEventLoop or the scheme is HTTPS then return true
+            if self.configuration.proxy != nil {
+                #if canImport(Network)
+                if #available(OSX 10.14, iOS 12.0, tvOS 12.0, watchOS 6.0, *)
+                {
+                    if !(eventLoop is NIOTSEventLoop) {
+                        return true
+                    }
+                    if self.key.scheme == .https {
+                        return true
+                    }
+                } else {
+                    return true
+                }
+                #else
+                return true
+                #endif
+            }
+            return false
+        }
+        
         private func makeConnection(on eventLoop: EventLoop) -> EventLoopFuture<Connection> {
             self.activityPrecondition(expected: [.opened])
             let address = HTTPClient.resolveAddress(host: self.key.host, port: self.key.port, proxy: self.configuration.proxy)
+            let requiresTLS = self.key.scheme == .https
             let bootstrap : NIOClientTCPBootstrap
             do {
-                bootstrap = try NIOClientTCPBootstrap.makeHTTPClientBootstrapBase(on: eventLoop, host: key.host, port: key.port, requiresTLS: self.key.scheme == .https, configuration: self.configuration)
+                bootstrap = try NIOClientTCPBootstrap.makeHTTPClientBootstrapBase(on: eventLoop, host: key.host, port: key.port, requiresTLS: requiresTLS, configuration: self.configuration)
             } catch {
                 return eventLoop.makeFailedFuture(error)
             }
+            let handshakePromise = eventLoop.makePromise(of: Void.self)
 
             let channel: EventLoopFuture<Channel>
             switch self.key.scheme {
@@ -392,16 +419,25 @@ final class ConnectionPool {
             }
 
             return channel.flatMap { channel -> EventLoopFuture<ConnectionPool.Connection> in
-                
-                channel.pipeline.addHTTPClientHandlers(leftOverBytesStrategy: .forwardBytes).map {
-                    let connection = Connection(key: self.key, channel: channel, parentPool: self.parentPool)
-                    connection.isLeased = true
-                    return connection
+                if self.requiresSSLHandler(on: eventLoop) {
+                    channel.pipeline.addSSLHandlerIfNeeded(for: self.key, tlsConfiguration: self.configuration.tlsConfiguration, handshakePromise: handshakePromise)
+                } else {
+                    handshakePromise.succeed(())
+                }
+                return handshakePromise.futureResult.flatMap {
+                   channel.pipeline.addHTTPClientHandlers(leftOverBytesStrategy: .forwardBytes)
+                }.map {
+                   let connection = Connection(key: self.key, channel: channel, parentPool: self.parentPool)
+                   connection.isLeased = true
+                   return connection
                 }
             }.map { connection in
                 self.configureCloseCallback(of: connection)
                 return connection
             }.flatMapError { error in
+                // This promise may not have been completed if we reach this
+                // so we fail it to avoid any leak
+                handshakePromise.fail(error)
                 let action = self.parentPool.connectionProvidersLock.withLock {
                     self.stateLock.withLock {
                         self.state.failedConnectionAction()

Sources/AsyncHTTPClient/Utils.swift

@@ -49,36 +49,61 @@ public final class HTTPClientCopyingDelegate: HTTPClientResponseDelegate {
 }
 
 extension ClientBootstrap {
-    fileprivate static func makeBootstrap(on eventLoop: EventLoop, host: String, requiresTLS: Bool, configuration: HTTPClient.Configuration) throws -> NIOClientTCPBootstrap {
+    fileprivate static func makeBootstrap(
+        on eventLoop: EventLoop,
+        host: String,
+        requiresTLS: Bool,
+        configuration: HTTPClient.Configuration
+    ) throws -> NIOClientTCPBootstrap {
         let tlsConfiguration = configuration.tlsConfiguration ?? TLSConfiguration.forClient()
         let sslContext = try NIOSSLContext(configuration: tlsConfiguration)
-        let tlsProvider = try NIOSSLClientTLSProvider<ClientBootstrap>(context: sslContext, serverHostname: (!requiresTLS || host.isIPAddress) ? nil : host)
+        let hostname = (!requiresTLS || host.isIPAddress) ? nil : host
+        let tlsProvider = try NIOSSLClientTLSProvider<ClientBootstrap>(context: sslContext, serverHostname: hostname)
         return NIOClientTCPBootstrap(ClientBootstrap(group: eventLoop), tls: tlsProvider)
     }
 }
 
 extension NIOClientTCPBootstrap {
     /// create a TCP Bootstrap based off what type of `EventLoop` has been passed to the function.
-    fileprivate static func makeBootstrap(on eventLoop: EventLoop, host: String, requiresTLS: Bool, configuration: HTTPClient.Configuration) throws -> NIOClientTCPBootstrap {
+    fileprivate static func makeBootstrap(
+        on eventLoop: EventLoop,
+        host: String,
+        requiresTLS: Bool,
+        configuration: HTTPClient.Configuration
+    ) throws -> NIOClientTCPBootstrap {
         let bootstrap: NIOClientTCPBootstrap
         #if canImport(Network)
         if #available(OSX 10.14, iOS 12.0, tvOS 12.0, watchOS 6.0, *), eventLoop is NIOTSEventLoop {
-            let tlsProvider = NIOTSClientTLSProvider(tlsOptions: .init())
-            bootstrap = NIOClientTCPBootstrap(NIOTSConnectionBootstrap(group: eventLoop), tls: tlsProvider)
+            if configuration.proxy != nil, requiresTLS {
+                let tlsConfiguration = configuration.tlsConfiguration ?? TLSConfiguration.forClient()
+                let sslContext = try NIOSSLContext(configuration: tlsConfiguration)
+                let hostname = (!requiresTLS || host.isIPAddress) ? nil : host
+                bootstrap = try NIOClientTCPBootstrap(NIOTSConnectionBootstrap(group: eventLoop), tls: NIOSSLClientTLSProvider(context: sslContext, serverHostname: hostname))
+            } else {
+                let tlsProvider = NIOTSClientTLSProvider(tlsOptions: .init())
+                bootstrap = NIOClientTCPBootstrap(NIOTSConnectionBootstrap(group: eventLoop), tls: tlsProvider)
+            }
         } else {
             bootstrap = try ClientBootstrap.makeBootstrap(on: eventLoop, host: host, requiresTLS: requiresTLS, configuration: configuration)
         }
         #else
         bootstrap = try ClientBootstrap.makeBootstrap(on: eventLoop, host: host, requiresTLS: requiresTLS, configuration: configuration)
         #endif
 
-        if requiresTLS {
+        // don't enable TLS if we have a proxy, this will be enabled later on
+        if requiresTLS, configuration.proxy == nil {
             return bootstrap.enableTLS()
         }
         return bootstrap
     }
 
-    static func makeHTTPClientBootstrapBase(on eventLoop: EventLoop, host: String, port: Int, requiresTLS: Bool, configuration: HTTPClient.Configuration) throws -> NIOClientTCPBootstrap {
+    static func makeHTTPClientBootstrapBase(
+        on eventLoop: EventLoop,
+        host: String,
+        port: Int,
+        requiresTLS: Bool,
+        configuration: HTTPClient.Configuration
+    ) throws -> NIOClientTCPBootstrap {
         return try makeBootstrap(on: eventLoop, host: host, requiresTLS: requiresTLS, configuration: configuration)
             .channelOption(ChannelOptions.socket(SocketOptionLevel(IPPROTO_TCP), TCP_NODELAY), value: 1)
             .channelInitializer { channel in

Tests/AsyncHTTPClientTests/HTTPClientTests.swift

@@ -384,9 +384,6 @@ class HTTPClientTests: XCTestCase {
     }
 
     func testProxyTLS() throws {
-        XCTFail("Disabled test as it crashes");
-        return
-
         let httpBin = HTTPBin(simulateProxy: .tls)
         let httpClient = HTTPClient(
             eventLoopGroupProvider: .shared(self.clientGroup),