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

Author: karwa

Sources/AsyncHTTPClient/ConnectionPool.swift

@@ -12,63 +12,30 @@
 //
 //===----------------------------------------------------------------------===//
 
-import enum NIOCore.SocketAddress
-
 enum ConnectionPool {
-    enum Host: 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 always have a non-nil 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)
-            }
-        }
-    }
-
     /// 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 host: Host
+        var connectionTarget: ConnectionTarget
         private var tlsConfiguration: BestEffortHashableTLSConfiguration?
 
         init(_ request: HTTPClient.Request) {
+            self.connectionTarget = request.connectionTarget
             switch request.scheme {
             case "http":
                 self.scheme = .http
-                self.host = Host(remoteHost: request.host, port: request.port)
             case "https":
                 self.scheme = .https
-                self.host = Host(remoteHost: request.host, port: request.port)
             case "unix":
                 self.scheme = .unix
-                self.host = .unixSocket(path: request.socketPath)
             case "http+unix":
                 self.scheme = .http_unix
-                self.host = .unixSocket(path: request.socketPath)
             case "https+unix":
                 self.scheme = .https_unix
-                self.host = .unixSocket(path: request.socketPath)
             default:
                 fatalError("HTTPClient.Request scheme should already be a valid one")
             }
@@ -108,7 +75,7 @@ enum ConnectionPool {
             self.tlsConfiguration?.hash(into: &hasher)
             let hash = hasher.finalize()
             var hostDescription = ""
-            switch host {
+            switch self.connectionTarget {
             case .ipAddress(let serialization, let addr):
                 hostDescription = "\(serialization):\(addr.port!)"
             case .domain(let domain, port: let port):

Sources/AsyncHTTPClient/ConnectionPool/ChannelHandler/HTTP1ProxyConnectHandler.swift

@@ -46,7 +46,7 @@ final class HTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableChannelHand
     }
 
     convenience
-    init(target: ConnectionPool.Host,
+    init(target: ConnectionTarget,
          proxyAuthorization: HTTPClient.Authorization?,
          deadline: NIODeadline) {
         let targetHost: String

Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift

@@ -198,7 +198,7 @@ extension HTTPConnectionPool.ConnectionFactory {
 
     private func makePlainChannel(deadline: NIODeadline, eventLoop: EventLoop) -> EventLoopFuture<Channel> {
         precondition(!self.key.scheme.requiresTLS, "Unexpected scheme")
-        return self.makePlainBootstrap(deadline: deadline, eventLoop: eventLoop).connect(host: self.key.host)
+        return self.makePlainBootstrap(deadline: deadline, eventLoop: eventLoop).connect(target: self.key.connectionTarget)
     }
 
     private func makeHTTPProxyChannel(
@@ -216,7 +216,7 @@ extension HTTPConnectionPool.ConnectionFactory {
             let encoder = HTTPRequestEncoder()
             let decoder = ByteToMessageHandler(HTTPResponseDecoder(leftOverBytesStrategy: .dropBytes))
             let proxyHandler = HTTP1ProxyConnectHandler(
-                target: self.key.host,
+                target: self.key.connectionTarget,
                 proxyAuthorization: proxy.authorization,
                 deadline: deadline
             )
@@ -255,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: SOCKSAddress(self.key.host))
+            let socksConnectHandler = SOCKSClientHandler(targetAddress: SOCKSAddress(self.key.connectionTarget))
             let socksEventHandler = SOCKSEventsHandler(deadline: deadline)
 
             do {
@@ -301,7 +301,7 @@ extension HTTPConnectionPool.ConnectionFactory {
             }
             let tlsEventHandler = TLSEventsHandler(deadline: deadline)
 
-            let sslServerHostname = self.key.host.sslServerHostname
+            let sslServerHostname = self.key.connectionTarget.sslServerHostname
             let sslContextFuture = self.sslContextCache.sslContext(
                 tlsConfiguration: tlsConfig,
                 eventLoop: channel.eventLoop,
@@ -364,7 +364,7 @@ extension HTTPConnectionPool.ConnectionFactory {
         )
 
         var channelFuture = bootstrapFuture.flatMap { bootstrap -> EventLoopFuture<Channel> in
-            return bootstrap.connect(host: self.key.host)
+            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`.
@@ -427,7 +427,7 @@ extension HTTPConnectionPool.ConnectionFactory {
         }
         #endif
 
-        let sslServerHostname = self.key.host.sslServerHostname
+        let sslServerHostname = self.key.connectionTarget.sslServerHostname
         let sslContextFuture = sslContextCache.sslContext(
             tlsConfiguration: tlsConfig,
             eventLoop: eventLoop,
@@ -481,7 +481,7 @@ extension ConnectionPool.Key.Scheme {
     }
 }
 
-extension ConnectionPool.Host {
+extension ConnectionTarget {
     fileprivate var sslServerHostname: String? {
         switch self {
         case .domain(let domain, _): return domain
@@ -491,7 +491,7 @@ extension ConnectionPool.Host {
 }
 
 extension SOCKSAddress {
-    fileprivate init(_ host: ConnectionPool.Host) {
+    fileprivate init(_ host: ConnectionTarget) {
         switch host {
         case .ipAddress(_, let address): self = .address(address)
         case .domain(let domain, let port): self = .domain(domain, port: port)
@@ -501,8 +501,8 @@ extension SOCKSAddress {
 }
 
 extension NIOClientTCPBootstrapProtocol {
-    func connect(host: ConnectionPool.Host) -> EventLoopFuture<Channel> {
-        switch host {
+    func connect(target: ConnectionTarget) -> EventLoopFuture<Channel> {
+        switch target {
         case .ipAddress(_, let socketAddress):
             return self.connect(to: socketAddress)
         case .domain(let domain, let port):

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")
     }
 }