Add SSL handler when needed in makeConnection

There was an issue where only the initial connection added the SSL Handler, but HTTP1ConnectionProvider wouldn't ever add the right handler if needed when it makes a new connection

Author: PopFlamingo

Sources/AsyncHTTPClient/ConnectionPool.swift

@@ -148,7 +148,7 @@ class ConnectionPool {
                 }
 
                 if request.useTLS {
-                    _ = channel.pipeline.addSSLHandlerIfNeeded(for: request, tlsConfiguration: self.configuration.tlsConfiguration).flatMap { _ in
+                    _ = channel.pipeline.addSSLHandlerIfNeeded(for: key, tlsConfiguration: self.configuration.tlsConfiguration).flatMap { _ in
                         channel.pipeline.configureHTTP2SecureUpgrade(h2PipelineConfigurator: h2PipelineConfigurator, http1PipelineConfigurator: http1PipelineConfigurator)
                     }
                 } else {
@@ -357,7 +357,9 @@ class ConnectionPool {
 
         private func makeConnection(preference: HTTPClient.EventLoopPreference) -> EventLoopFuture<Connection> {
             let bootstrap = ClientBootstrap.makeHTTPClientBootstrapBase(group: self.loopGroup, host: self.key.host, port: self.key.port, configuration: self.configuration) { channel in
-                channel.pipeline.addHTTPClientHandlers(leftOverBytesStrategy: .forwardBytes)
+                channel.pipeline.addSSLHandlerIfNeeded(for: self.key, tlsConfiguration: self.configuration.tlsConfiguration).flatMap {
+                    channel.pipeline.addHTTPClientHandlers(leftOverBytesStrategy: .forwardBytes)
+                }
             }
             let address = HTTPClient.resolveAddress(host: self.key.host, port: self.key.port, proxy: self.configuration.proxy)
             return bootstrap.connect(host: address.host, port: address.port).map { channel in

Sources/AsyncHTTPClient/HTTPClient.swift

@@ -358,15 +358,15 @@ extension ChannelPipeline {
         return addHandlers([encoder, decoder, handler])
     }
 
-    func addSSLHandlerIfNeeded(for request: HTTPClient.Request, tlsConfiguration: TLSConfiguration?) -> EventLoopFuture<Void> {
-        guard request.useTLS else {
+    func addSSLHandlerIfNeeded(for key: ConnectionPool.Key, tlsConfiguration: TLSConfiguration?) -> EventLoopFuture<Void> {
+        guard key.scheme == .https else {
             return self.eventLoop.makeSucceededFuture(())
         }
 
         do {
             let tlsConfiguration = tlsConfiguration ?? TLSConfiguration.forClient(applicationProtocols: NIOHTTP2SupportedALPNProtocols)
             let context = try NIOSSLContext(configuration: tlsConfiguration)
-            return self.addHandler(try NIOSSLClientHandler(context: context, serverHostname: request.host))
+            return self.addHandler(try NIOSSLClientHandler(context: context, serverHostname: key.host))
         } catch {
             return self.eventLoop.makeFailedFuture(error)
         }

Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift

@@ -57,6 +57,7 @@ extension HTTPClientTests {
             ("testWrongContentLengthForSSLUncleanShutdown", testWrongContentLengthForSSLUncleanShutdown),
             ("testWrongContentLengthWithIgnoreErrorForSSLUncleanShutdown", testWrongContentLengthWithIgnoreErrorForSSLUncleanShutdown),
             ("testEventLoopArgument", testEventLoopArgument),
+            ("testStressGetHttps", testStressGetHttps),
         ]
     }
 }

Tests/AsyncHTTPClientTests/HTTPClientTests.swift

@@ -562,4 +562,32 @@ class HTTPClientTests: XCTestCase {
         response = try httpClient.execute(request: request, delegate: delegate, eventLoop: .prefers(eventLoop)).wait()
         XCTAssertEqual(true, response)
     }
+
+    func testStressGetHttps() {
+        let httpBin = HTTPBin(ssl: true)
+        let httpClient = HTTPClient(eventLoopGroupProvider: .createNew,
+                                    configuration: HTTPClient.Configuration(certificateVerification: .none))
+        defer {
+            XCTAssertNoThrow(try httpClient.syncShutdown())
+            XCTAssertNoThrow(try httpBin.shutdown())
+        }
+
+        let eventLoop = httpClient.eventLoopGroup
+        let lastReqPromise = eventLoop.next().makePromise(of: Void.self)
+        let requestCount = 5000
+        for i in 1 ... requestCount {
+            httpClient.get(url: "https://localhost:\(httpBin.port)/get").whenComplete { result in
+                switch result {
+                case .success(let response):
+                    XCTAssertEqual(.ok, response.status)
+                case .failure(let error):
+                    XCTFail("\(error)")
+                }
+                if i == requestCount {
+                    lastReqPromise.succeed(())
+                }
+            }
+        }
+        try! lastReqPromise.futureResult.wait()
+    }
 }