[HTTP2] Internal HTTPVersion configuration (#463)

Author: dnadoba

Diff for: Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift

@@ -31,19 +31,14 @@ extension HTTPConnectionPool {
         let tlsConfiguration: TLSConfiguration
         let sslContextCache: SSLContextCache
 
-        // This property can be removed once we enable true http/2 support
-        let allowHTTP2Connections: Bool
-
         init(key: ConnectionPool.Key,
              tlsConfiguration: TLSConfiguration?,
              clientConfiguration: HTTPClient.Configuration,
-             sslContextCache: SSLContextCache,
-             allowHTTP2Connections: Bool = false) {
+             sslContextCache: SSLContextCache) {
             self.key = key
             self.clientConfiguration = clientConfiguration
             self.sslContextCache = sslContextCache
             self.tlsConfiguration = tlsConfiguration ?? clientConfiguration.tlsConfiguration ?? .makeClientConfiguration()
-            self.allowHTTP2Connections = allowHTTP2Connections
         }
     }
 }
@@ -303,13 +298,14 @@ extension HTTPConnectionPool.ConnectionFactory {
             return channel.eventLoop.makeSucceededFuture(.http1_1(channel))
         case .https:
             var tlsConfig = self.tlsConfiguration
-            // since we can support h2, we need to advertise this in alpn
-            if self.allowHTTP2Connections {
+            switch self.clientConfiguration.httpVersion.configuration {
+            case .automatic:
+                // since we can support h2, we need to advertise this in alpn
                 // "ProtocolNameList" contains the list of protocols advertised by the
                 // client, in descending order of preference.
                 // https://datatracker.ietf.org/doc/html/rfc7301#section-3.1
                 tlsConfig.applicationProtocols = ["h2", "http/1.1"]
-            } else {
+            case .http1Only:
                 tlsConfig.applicationProtocols = ["http/1.1"]
             }
             let tlsEventHandler = TLSEventsHandler(deadline: deadline)
@@ -407,14 +403,15 @@ extension HTTPConnectionPool.ConnectionFactory {
 
     private func makeTLSBootstrap(deadline: NIODeadline, eventLoop: EventLoop, logger: Logger)
         -> EventLoopFuture<NIOClientTCPBootstrapProtocol> {
-        // since we can support h2, we need to advertise this in alpn
         var tlsConfig = self.tlsConfiguration
-        if self.allowHTTP2Connections {
+        switch self.clientConfiguration.httpVersion.configuration {
+        case .automatic:
+            // since we can support h2, we need to advertise this in alpn
             // "ProtocolNameList" contains the list of protocols advertised by the
             // client, in descending order of preference.
             // https://datatracker.ietf.org/doc/html/rfc7301#section-3.1
             tlsConfig.applicationProtocols = ["h2", "http/1.1"]
-        } else {
+        case .http1Only:
             tlsConfig.applicationProtocols = ["http/1.1"]
         }
 

Diff for: Sources/AsyncHTTPClient/HTTPClient.swift

@@ -603,20 +603,51 @@ public class HTTPClient {
         /// Ignore TLS unclean shutdown error, defaults to `false`.
         public var ignoreUncleanSSLShutdown: Bool
 
-        public init(tlsConfiguration: TLSConfiguration? = nil,
-                    redirectConfiguration: RedirectConfiguration? = nil,
-                    timeout: Timeout = Timeout(),
-                    connectionPool: ConnectionPool = ConnectionPool(),
-                    proxy: Proxy? = nil,
-                    ignoreUncleanSSLShutdown: Bool = false,
-                    decompression: Decompression = .disabled) {
+        // TODO: make public
+        // TODO: set to automatic by default
+        /// HTTP/2 is by default disabled
+        internal var httpVersion: HTTPVersion
+
+        public init(
+            tlsConfiguration: TLSConfiguration? = nil,
+            redirectConfiguration: RedirectConfiguration? = nil,
+            timeout: Timeout = Timeout(),
+            connectionPool: ConnectionPool = ConnectionPool(),
+            proxy: Proxy? = nil,
+            ignoreUncleanSSLShutdown: Bool = false,
+            decompression: Decompression = .disabled
+        ) {
+            self.init(
+                tlsConfiguration: tlsConfiguration,
+                redirectConfiguration: redirectConfiguration,
+                timeout: timeout, connectionPool: connectionPool,
+                proxy: proxy,
+                ignoreUncleanSSLShutdown: ignoreUncleanSSLShutdown,
+                decompression: decompression,
+                // TODO: set to automatic by default
+                httpVersion: .http1Only
+            )
+        }
+
+        // TODO: make public
+        internal init(
+            tlsConfiguration: TLSConfiguration? = nil,
+            redirectConfiguration: RedirectConfiguration? = nil,
+            timeout: Timeout = Timeout(),
+            connectionPool: ConnectionPool = ConnectionPool(),
+            proxy: Proxy? = nil,
+            ignoreUncleanSSLShutdown: Bool = false,
+            decompression: Decompression = .disabled,
+            httpVersion: HTTPVersion
+        ) {
             self.tlsConfiguration = tlsConfiguration
             self.redirectConfiguration = redirectConfiguration ?? RedirectConfiguration()
             self.timeout = timeout
             self.connectionPool = connectionPool
             self.proxy = proxy
             self.ignoreUncleanSSLShutdown = ignoreUncleanSSLShutdown
             self.decompression = decompression
+            self.httpVersion = httpVersion
         }
 
         public init(tlsConfiguration: TLSConfiguration? = nil,
@@ -830,6 +861,22 @@ extension HTTPClient.Configuration {
             self.concurrentHTTP1ConnectionsPerHostSoftLimit = concurrentHTTP1ConnectionsPerHostSoftLimit
         }
     }
+
+    // TODO: make this struct and its static properties public
+    internal struct HTTPVersion {
+        internal enum Configuration {
+            case http1Only
+            case automatic
+        }
+
+        /// we only use HTTP/1, even if the server would supports HTTP/2
+        internal static let http1Only: Self = .init(configuration: .http1Only)
+
+        /// HTTP/2 is used if we connect to a server with HTTPS and the server supports HTTP/2, otherwise we use HTTP/1
+        internal static let automatic: Self = .init(configuration: .automatic)
+
+        internal var configuration: Configuration
+    }
 }
 
 /// Possible client errors.

Diff for: Tests/AsyncHTTPClientTests/HTTP2ConnectionTests.swift

@@ -253,9 +253,8 @@ class TestConnectionCreator {
         let factory = HTTPConnectionPool.ConnectionFactory(
             key: .init(request),
             tlsConfiguration: tlsConfiguration,
-            clientConfiguration: .init(),
-            sslContextCache: .init(),
-            allowHTTP2Connections: true
+            clientConfiguration: .init(httpVersion: .automatic),
+            sslContextCache: .init()
         )
 
         let promise = try self.lock.withLock { () -> EventLoopPromise<HTTP1Connection> in
@@ -295,9 +294,8 @@ class TestConnectionCreator {
         let factory = HTTPConnectionPool.ConnectionFactory(
             key: .init(request),
             tlsConfiguration: tlsConfiguration,
-            clientConfiguration: .init(),
-            sslContextCache: .init(),
-            allowHTTP2Connections: true
+            clientConfiguration: .init(httpVersion: .automatic),
+            sslContextCache: .init()
         )
 
         let promise = try self.lock.withLock { () -> EventLoopPromise<HTTP2Connection> in