Introduce a ConnectionTarget enum (#501)

* Add a ConnectionPool.Host enum

* Move Host out as a top-level ConnectionTarget type, and use it in Request.

Author: karwa

Sources/AsyncHTTPClient/ConnectionPool.swift

@@ -15,11 +15,16 @@
 enum ConnectionPool {
     /// Used by the `ConnectionPool` to index its `HTTP1ConnectionProvider`s
     ///
-    /// A key is initialized from a `URL`, it uses the components to derive a hashed value
+    /// A key is initialized from a `Request`, it uses the components to derive a hashed value
     /// used by the `providers` dictionary to allow retrieving and creating
     /// connection providers associated to a certain request in constant time.
     struct Key: Hashable, CustomStringConvertible {
+        var scheme: Scheme
+        var connectionTarget: ConnectionTarget
+        private var tlsConfiguration: BestEffortHashableTLSConfiguration?
+
         init(_ request: HTTPClient.Request) {
+            self.connectionTarget = request.connectionTarget
             switch request.scheme {
             case "http":
                 self.scheme = .http
@@ -34,20 +39,11 @@ enum ConnectionPool {
             default:
                 fatalError("HTTPClient.Request scheme should already be a valid one")
             }
-            self.port = request.port
-            self.host = request.host
-            self.unixPath = request.socketPath
             if let tls = request.tlsConfiguration {
                 self.tlsConfiguration = BestEffortHashableTLSConfiguration(wrapping: tls)
             }
         }
 
-        var scheme: Scheme
-        var host: String
-        var port: Int
-        var unixPath: String
-        private var tlsConfiguration: BestEffortHashableTLSConfiguration?
-
         enum Scheme: Hashable {
             case http
             case https
@@ -78,13 +74,16 @@ enum ConnectionPool {
             var hasher = Hasher()
             self.tlsConfiguration?.hash(into: &hasher)
             let hash = hasher.finalize()
-            var path = ""
-            if self.unixPath != "" {
-                path = self.unixPath
-            } else {
-                path = "\(self.host):\(self.port)"
+            var hostDescription = ""
+            switch self.connectionTarget {
+            case .ipAddress(let serialization, let addr):
+                hostDescription = "\(serialization):\(addr.port!)"
+            case .domain(let domain, port: let port):
+                hostDescription = "\(domain):\(port)"
+            case .unixSocket(let socketPath):
+                hostDescription = socketPath
             }
-            return "\(self.scheme)://\(path) TLS-hash: \(hash)"
+            return "\(self.scheme)://\(hostDescription) TLS-hash: \(hash)"
         }
     }
 }

Sources/AsyncHTTPClient/ConnectionPool/ChannelHandler/HTTP1ProxyConnectHandler.swift

@@ -45,6 +45,30 @@ final class HTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableChannelHand
         return self.proxyEstablishedPromise?.futureResult
     }
 
+    convenience
+    init(target: ConnectionTarget,
+         proxyAuthorization: HTTPClient.Authorization?,
+         deadline: NIODeadline) {
+        let targetHost: String
+        let targetPort: Int
+        switch target {
+        case .ipAddress(serialization: let serialization, address: let address):
+            targetHost = serialization
+            targetPort = address.port!
+        case .domain(name: let domain, port: let port):
+            targetHost = domain
+            targetPort = port
+        case .unixSocket:
+            fatalError("Unix Domain Sockets do not support proxies")
+        }
+        self.init(
+            targetHost: targetHost,
+            targetPort: targetPort,
+            proxyAuthorization: proxyAuthorization,
+            deadline: deadline
+        )
+    }
+
     init(targetHost: String,
          targetPort: Int,
          proxyAuthorization: HTTPClient.Authorization?,

Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift

@@ -197,16 +197,8 @@ extension HTTPConnectionPool.ConnectionFactory {
     }
 
     private func makePlainChannel(deadline: NIODeadline, eventLoop: EventLoop) -> EventLoopFuture<Channel> {
-        let bootstrap = self.makePlainBootstrap(deadline: deadline, eventLoop: eventLoop)
-
-        switch self.key.scheme {
-        case .http:
-            return bootstrap.connect(host: self.key.host, port: self.key.port)
-        case .http_unix, .unix:
-            return bootstrap.connect(unixDomainSocketPath: self.key.unixPath)
-        case .https, .https_unix:
-            preconditionFailure("Unexpected scheme")
-        }
+        precondition(!self.key.scheme.requiresTLS, "Unexpected scheme")
+        return self.makePlainBootstrap(deadline: deadline, eventLoop: eventLoop).connect(target: self.key.connectionTarget)
     }
 
     private func makeHTTPProxyChannel(
@@ -224,8 +216,7 @@ extension HTTPConnectionPool.ConnectionFactory {
             let encoder = HTTPRequestEncoder()
             let decoder = ByteToMessageHandler(HTTPResponseDecoder(leftOverBytesStrategy: .dropBytes))
             let proxyHandler = HTTP1ProxyConnectHandler(
-                targetHost: self.key.host,
-                targetPort: self.key.port,
+                target: self.key.connectionTarget,
                 proxyAuthorization: proxy.authorization,
                 deadline: deadline
             )
@@ -264,7 +255,7 @@ extension HTTPConnectionPool.ConnectionFactory {
         // upgraded to TLS before we send our first request.
         let bootstrap = self.makePlainBootstrap(deadline: deadline, eventLoop: eventLoop)
         return bootstrap.connect(host: proxy.host, port: proxy.port).flatMap { channel in
-            let socksConnectHandler = SOCKSClientHandler(targetAddress: .domain(self.key.host, port: self.key.port))
+            let socksConnectHandler = SOCKSClientHandler(targetAddress: SOCKSAddress(self.key.connectionTarget))
             let socksEventHandler = SOCKSEventsHandler(deadline: deadline)
 
             do {
@@ -310,6 +301,7 @@ extension HTTPConnectionPool.ConnectionFactory {
             }
             let tlsEventHandler = TLSEventsHandler(deadline: deadline)
 
+            let sslServerHostname = self.key.connectionTarget.sslServerHostname
             let sslContextFuture = self.sslContextCache.sslContext(
                 tlsConfiguration: tlsConfig,
                 eventLoop: channel.eventLoop,
@@ -320,7 +312,7 @@ extension HTTPConnectionPool.ConnectionFactory {
                 do {
                     let sslHandler = try NIOSSLClientHandler(
                         context: sslContext,
-                        serverHostname: self.key.host
+                        serverHostname: sslServerHostname
                     )
                     try channel.pipeline.syncOperations.addHandler(sslHandler)
                     try channel.pipeline.syncOperations.addHandler(tlsEventHandler)
@@ -364,21 +356,15 @@ extension HTTPConnectionPool.ConnectionFactory {
     }
 
     private func makeTLSChannel(deadline: NIODeadline, eventLoop: EventLoop, logger: Logger) -> EventLoopFuture<(Channel, String?)> {
+        precondition(self.key.scheme.requiresTLS, "Unexpected scheme")
         let bootstrapFuture = self.makeTLSBootstrap(
             deadline: deadline,
             eventLoop: eventLoop,
             logger: logger
         )
 
         var channelFuture = bootstrapFuture.flatMap { bootstrap -> EventLoopFuture<Channel> in
-            switch self.key.scheme {
-            case .https:
-                return bootstrap.connect(host: self.key.host, port: self.key.port)
-            case .https_unix:
-                return bootstrap.connect(unixDomainSocketPath: self.key.unixPath)
-            case .http, .http_unix, .unix:
-                preconditionFailure("Unexpected scheme")
-            }
+            return bootstrap.connect(target: self.key.connectionTarget)
         }.flatMap { channel -> EventLoopFuture<(Channel, String?)> in
             // It is save to use `try!` here, since we are sure, that a `TLSEventsHandler` exists
             // within the pipeline. It is added in `makeTLSBootstrap`.
@@ -441,9 +427,7 @@ extension HTTPConnectionPool.ConnectionFactory {
         }
         #endif
 
-        let host = self.key.host
-        let hostname = (host.isIPAddress || host.isEmpty) ? nil : host
-
+        let sslServerHostname = self.key.connectionTarget.sslServerHostname
         let sslContextFuture = sslContextCache.sslContext(
             tlsConfiguration: tlsConfig,
             eventLoop: eventLoop,
@@ -458,7 +442,7 @@ extension HTTPConnectionPool.ConnectionFactory {
                         let sync = channel.pipeline.syncOperations
                         let sslHandler = try NIOSSLClientHandler(
                             context: sslContext,
-                            serverHostname: hostname
+                            serverHostname: sslServerHostname
                         )
                         let tlsEventHandler = TLSEventsHandler(deadline: deadline)
 
@@ -497,14 +481,34 @@ extension ConnectionPool.Key.Scheme {
     }
 }
 
-extension String {
-    fileprivate var isIPAddress: Bool {
-        var ipv4Addr = in_addr()
-        var ipv6Addr = in6_addr()
+extension ConnectionTarget {
+    fileprivate var sslServerHostname: String? {
+        switch self {
+        case .domain(let domain, _): return domain
+        case .ipAddress, .unixSocket: return nil
+        }
+    }
+}
+
+extension SOCKSAddress {
+    fileprivate init(_ host: ConnectionTarget) {
+        switch host {
+        case .ipAddress(_, let address): self = .address(address)
+        case .domain(let domain, let port): self = .domain(domain, port: port)
+        case .unixSocket: fatalError("Unix Domain Sockets are not supported by SOCKSAddress")
+        }
+    }
+}
 
-        return self.withCString { ptr in
-            inet_pton(AF_INET, ptr, &ipv4Addr) == 1 ||
-                inet_pton(AF_INET6, ptr, &ipv6Addr) == 1
+extension NIOClientTCPBootstrapProtocol {
+    func connect(target: ConnectionTarget) -> EventLoopFuture<Channel> {
+        switch target {
+        case .ipAddress(_, let socketAddress):
+            return self.connect(to: socketAddress)
+        case .domain(let domain, let port):
+            return self.connect(host: domain, port: port)
+        case .unixSocket(let path):
+            return self.connect(unixDomainSocketPath: path)
         }
     }
 }

Sources/AsyncHTTPClient/ConnectionTarget.swift

@@ -0,0 +1,42 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the AsyncHTTPClient open source project
+//
+// Copyright (c) 2019-2021 Apple Inc. and the AsyncHTTPClient project authors
+// Licensed under Apache License v2.0
+//
+// See LICENSE.txt for license information
+// See CONTRIBUTORS.txt for the list of AsyncHTTPClient project authors
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+//===----------------------------------------------------------------------===//
+
+import enum NIOCore.SocketAddress
+
+enum ConnectionTarget: Equatable, Hashable {
+    // We keep the IP address serialization precisely as it is in the URL.
+    // Some platforms have quirks in their implementations of 'ntop', for example
+    // writing IPv6 addresses as having embedded IPv4 sections (e.g. [::192.168.0.1] vs [::c0a8:1]).
+    // This serialization includes square brackets, so it is safe to write next to a port number.
+    // Note: `address` must have an explicit port.
+    case ipAddress(serialization: String, address: SocketAddress)
+    case domain(name: String, port: Int)
+    case unixSocket(path: String)
+
+    init(remoteHost: String, port: Int) {
+        if let addr = try? SocketAddress(ipAddress: remoteHost, port: port) {
+            switch addr {
+            case .v6:
+                self = .ipAddress(serialization: "[\(remoteHost)]", address: addr)
+            case .v4:
+                self = .ipAddress(serialization: remoteHost, address: addr)
+            case .unixDomainSocket:
+                fatalError("Expected a remote host")
+            }
+        } else {
+            precondition(!remoteHost.isEmpty, "HTTPClient.Request should already reject empty remote hostnames")
+            self = .domain(name: remoteHost, port: port)
+        }
+    }
+}

Sources/AsyncHTTPClient/HTTPHandler.swift

@@ -126,9 +126,7 @@ extension HTTPClient {
 
         static func deconstructURL(
             _ url: URL
-        ) throws -> (
-            kind: Kind, scheme: String, hostname: String, port: Int, socketPath: String, uri: String
-        ) {
+        ) throws -> (kind: Kind, scheme: String, connectionTarget: ConnectionTarget, uri: String) {
             guard let scheme = url.scheme?.lowercased() else {
                 throw HTTPClientError.emptyScheme
             }
@@ -138,20 +136,23 @@ extension HTTPClient {
                     throw HTTPClientError.emptyHost
                 }
                 let defaultPort = self.useTLS(scheme) ? 443 : 80
-                return (.host, scheme, host, url.port ?? defaultPort, "", url.uri)
+                let hostTarget = ConnectionTarget(remoteHost: host, port: url.port ?? defaultPort)
+                return (.host, scheme, hostTarget, url.uri)
             case "http+unix", "https+unix":
                 guard let socketPath = url.host, !socketPath.isEmpty else {
                     throw HTTPClientError.missingSocketPath
                 }
-                let (kind, defaultPort) = self.useTLS(scheme) ? (Kind.UnixScheme.https_unix, 443) : (.http_unix, 80)
-                return (.unixSocket(kind), scheme, "", url.port ?? defaultPort, socketPath, url.uri)
+                let socketTarget = ConnectionTarget.unixSocket(path: socketPath)
+                let kind = self.useTLS(scheme) ? Kind.UnixScheme.https_unix : .http_unix
+                return (.unixSocket(kind), scheme, socketTarget, url.uri)
             case "unix":
                 let socketPath = url.baseURL?.path ?? url.path
                 let uri = url.baseURL != nil ? url.uri : "/"
                 guard !socketPath.isEmpty else {
                     throw HTTPClientError.missingSocketPath
                 }
-                return (.unixSocket(.baseURL), scheme, "", url.port ?? 80, socketPath, uri)
+                let socketTarget = ConnectionTarget.unixSocket(path: socketPath)
+                return (.unixSocket(.baseURL), scheme, socketTarget, uri)
             default:
                 throw HTTPClientError.unsupportedScheme(url.scheme!)
             }
@@ -163,12 +164,8 @@ extension HTTPClient {
         public let url: URL
         /// Remote HTTP scheme, resolved from `URL`.
         public let scheme: String
-        /// Remote host, resolved from `URL`.
-        public let host: String
-        /// Resolved port.
-        public let port: Int
-        /// Socket path, resolved from `URL`.
-        let socketPath: String
+        /// The connection target, resolved from `URL`.
+        let connectionTarget: ConnectionTarget
         /// URI composed of the path and query, resolved from `URL`.
         let uri: String
         /// Request custom HTTP Headers, defaults to no headers.
@@ -255,7 +252,7 @@ extension HTTPClient {
         ///     - `emptyHost` if URL does not contains a host.
         ///     - `missingSocketPath` if URL does not contains a socketPath as an encoded host.
         public init(url: URL, method: HTTPMethod = .GET, headers: HTTPHeaders = HTTPHeaders(), body: Body? = nil, tlsConfiguration: TLSConfiguration?) throws {
-            (self.kind, self.scheme, self.host, self.port, self.socketPath, self.uri) = try Request.deconstructURL(url)
+            (self.kind, self.scheme, self.connectionTarget, self.uri) = try Request.deconstructURL(url)
             self.redirectState = nil
             self.url = url
             self.method = method
@@ -269,6 +266,24 @@ extension HTTPClient {
             return Request.useTLS(self.scheme)
         }
 
+        /// Remote host, resolved from `URL`.
+        public var host: String {
+            switch self.connectionTarget {
+            case .ipAddress(let serialization, _): return serialization
+            case .domain(let name, _): return name
+            case .unixSocket: return ""
+            }
+        }
+
+        /// Resolved port.
+        public var port: Int {
+            switch self.connectionTarget {
+            case .ipAddress(_, let address): return address.port!
+            case .domain(_, let port): return port
+            case .unixSocket: return Request.useTLS(self.scheme) ? 443 : 80
+            }
+        }
+
         func createRequestHead() throws -> (HTTPRequestHead, RequestFramingMetadata) {
             var head = HTTPRequestHead(
                 version: .http1_1,

Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift

@@ -447,27 +447,76 @@ class HTTPClientInternalTests: XCTestCase {
     func testInternalRequestURI() throws {
         let request1 = try Request(url: "https://someserver.com:8888/some/path?foo=bar")
         XCTAssertEqual(request1.kind, .host)
-        XCTAssertEqual(request1.socketPath, "")
+        XCTAssertEqual(request1.connectionTarget, .domain(name: "someserver.com", port: 8888))
         XCTAssertEqual(request1.uri, "/some/path?foo=bar")
 
         let request2 = try Request(url: "https://someserver.com")
         XCTAssertEqual(request2.kind, .host)
-        XCTAssertEqual(request2.socketPath, "")
+        XCTAssertEqual(request2.connectionTarget, .domain(name: "someserver.com", port: 443))
         XCTAssertEqual(request2.uri, "/")
 
         let request3 = try Request(url: "unix:///tmp/file")
         XCTAssertEqual(request3.kind, .unixSocket(.baseURL))
-        XCTAssertEqual(request3.socketPath, "/tmp/file")
+        XCTAssertEqual(request3.connectionTarget, .unixSocket(path: "/tmp/file"))
         XCTAssertEqual(request3.uri, "/")
 
         let request4 = try Request(url: "http+unix://%2Ftmp%2Ffile/file/path")
         XCTAssertEqual(request4.kind, .unixSocket(.http_unix))
-        XCTAssertEqual(request4.socketPath, "/tmp/file")
+        XCTAssertEqual(request4.connectionTarget, .unixSocket(path: "/tmp/file"))
         XCTAssertEqual(request4.uri, "/file/path")
 
         let request5 = try Request(url: "https+unix://%2Ftmp%2Ffile/file/path")
         XCTAssertEqual(request5.kind, .unixSocket(.https_unix))
-        XCTAssertEqual(request5.socketPath, "/tmp/file")
+        XCTAssertEqual(request5.connectionTarget, .unixSocket(path: "/tmp/file"))
         XCTAssertEqual(request5.uri, "/file/path")
+
+        let request6 = try Request(url: "https://127.0.0.1")
+        XCTAssertEqual(request6.kind, .host)
+        XCTAssertEqual(request6.connectionTarget, .ipAddress(
+            serialization: "127.0.0.1",
+            address: try! SocketAddress(ipAddress: "127.0.0.1", port: 443)
+        ))
+        XCTAssertEqual(request6.uri, "/")
+
+        let request7 = try Request(url: "https://0x7F.1:9999")
+        XCTAssertEqual(request7.kind, .host)
+        XCTAssertEqual(request7.connectionTarget, .domain(name: "0x7F.1", port: 9999))
+        XCTAssertEqual(request7.uri, "/")
+
+        let request8 = try Request(url: "http://[::1]")
+        XCTAssertEqual(request8.kind, .host)
+        XCTAssertEqual(request8.connectionTarget, .ipAddress(
+            serialization: "[::1]",
+            address: try! SocketAddress(ipAddress: "::1", port: 80)
+        ))
+        XCTAssertEqual(request8.uri, "/")
+
+        let request9 = try Request(url: "http://[763e:61d9::6ACA:3100:6274]:4242/foo/bar?baz")
+        XCTAssertEqual(request9.kind, .host)
+        XCTAssertEqual(request9.connectionTarget, .ipAddress(
+            serialization: "[763e:61d9::6ACA:3100:6274]",
+            address: try! SocketAddress(ipAddress: "763e:61d9::6aca:3100:6274", port: 4242)
+        ))
+        XCTAssertEqual(request9.uri, "/foo/bar?baz")
+
+        // Some systems have quirks in their implementations of 'ntop' which cause them to write
+        // certain IPv6 addresses with embedded IPv4 parts (e.g. "::192.168.0.1" vs "::c0a8:1").
+        // We want to make sure that our request formatting doesn't depend on the platform's quirks,
+        // so the serialization must be kept verbatim as it was given in the request.
+        let request10 = try Request(url: "http://[::c0a8:1]:4242/foo/bar?baz")
+        XCTAssertEqual(request10.kind, .host)
+        XCTAssertEqual(request10.connectionTarget, .ipAddress(
+            serialization: "[::c0a8:1]",
+            address: try! SocketAddress(ipAddress: "::c0a8:1", port: 4242)
+        ))
+        XCTAssertEqual(request9.uri, "/foo/bar?baz")
+
+        let request11 = try Request(url: "http://[::192.168.0.1]:4242/foo/bar?baz")
+        XCTAssertEqual(request11.kind, .host)
+        XCTAssertEqual(request11.connectionTarget, .ipAddress(
+            serialization: "[::192.168.0.1]",
+            address: try! SocketAddress(ipAddress: "::192.168.0.1", port: 4242)
+        ))
+        XCTAssertEqual(request11.uri, "/foo/bar?baz")
     }
 }