From 0202d0bd0a9e793667f0aa90c23f2951a35d5a38 Mon Sep 17 00:00:00 2001
From: Artem Redkin <aredkin@apple.com>
Date: Wed, 11 Nov 2020 12:16:54 +0000
Subject: [PATCH] Fixes propagation of errors during TLS handshakre

Motivation:
Right now we only handle one type of SSL error: `.handshakeFailed`,
but in reality a multitude of errors can happen, for example, remote
party might just close connection that will in turn raise
`.uncleanShutdown` error, that will be dropped on the floor and
users will only get non-descriptive `NoResult` error.

Modifications:
Handle all types of SSL errors during handshake instead of just one.
Adds a test.

Result:
Closes #313
---
 Sources/AsyncHTTPClient/HTTPClient.swift      | 13 ++-----
 .../HTTPClientTests+XCTest.swift              |  1 +
 .../HTTPClientTests.swift                     | 37 +++++++++++++++++++
 3 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift
index 775254928..f5fe1693e 100644
--- a/Sources/AsyncHTTPClient/HTTPClient.swift
+++ b/Sources/AsyncHTTPClient/HTTPClient.swift
@@ -935,16 +935,9 @@ class TLSEventsHandler: ChannelInboundHandler, RemovableChannelHandler {
     }
 
     func errorCaught(context: ChannelHandlerContext, error: Error) {
-        if let sslError = error as? NIOSSLError {
-            switch sslError {
-            case .handshakeFailed:
-                self.completionPromise?.fail(error)
-                self.completionPromise = nil
-                context.pipeline.removeHandler(self, promise: nil)
-            default:
-                break
-            }
-        }
+        self.completionPromise?.fail(error)
+        self.completionPromise = nil
+        context.pipeline.removeHandler(self, promise: nil)
         context.fireErrorCaught(error)
     }
 
diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift
index 72df43dfa..655f31792 100644
--- a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift
+++ b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift
@@ -125,6 +125,7 @@ extension HTTPClientTests {
             ("testBodyUploadAfterEndFails", testBodyUploadAfterEndFails),
             ("testNoBytesSentOverBodyLimit", testNoBytesSentOverBodyLimit),
             ("testDoubleError", testDoubleError),
+            ("testSSLHandshakeErrorPropagation", testSSLHandshakeErrorPropagation),
         ]
     }
 }
diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift
index df2348e03..94f649aab 100644
--- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift
+++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift
@@ -2675,4 +2675,41 @@ class HTTPClientTests: XCTestCase {
         // we need to verify that second error on write after timeout does not lead to double-release.
         XCTAssertThrowsError(try self.defaultClient.execute(request: request, deadline: .now() + .milliseconds(2)).wait())
     }
+
+    func testSSLHandshakeErrorPropagation() throws {
+        class CloseHandler: ChannelInboundHandler {
+            typealias InboundIn = Any
+
+            func channelActive(context: ChannelHandlerContext) {
+                context.close(promise: nil)
+            }
+        }
+
+        let server = try ServerBootstrap(group: self.serverGroup)
+            .childChannelOption(ChannelOptions.autoRead, value: false)
+            .childChannelInitializer { channel in
+                channel.pipeline.addHandler(CloseHandler())
+            }
+            .bind(host: "127.0.0.1", port: 0)
+            .wait()
+
+        defer {
+            XCTAssertNoThrow(try server.close().wait())
+        }
+
+        let request = try Request(url: "https://localhost:\(server.localAddress!.port!)", method: .GET)
+        let task = self.defaultClient.execute(request: request, delegate: TestHTTPDelegate())
+
+        XCTAssertThrowsError(try task.wait()) { error in
+            #if os(Linux)
+                XCTAssertEqual(error as? NIOSSLError, NIOSSLError.uncleanShutdown)
+            #else
+                if isTestingNIOTS() {
+                    XCTAssertEqual((error as? AsyncHTTPClient.HTTPClient.NWTLSError).map { $0.status }, errSSLClosedNoNotify)
+                } else {
+                    XCTAssertEqual((error as? IOError).map { $0.errnoCode }, 54)
+                }
+            #endif
+        }
+    }
 }