Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HTTP2] Make HTTPVersion public and set to .automatic by default #473

Merged
merged 4 commits into from
Nov 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 6 additions & 32 deletions Sources/AsyncHTTPClient/HTTPClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -607,10 +607,8 @@ public class HTTPClient {
set {}
}

// TODO: make public
// TODO: set to automatic by default
/// HTTP/2 is by default disabled
internal var httpVersion: HTTPVersion
/// is set to `.automatic` by default which will use HTTP/2 if run over https and the server supports it, otherwise HTTP/1
public var httpVersion: HTTPVersion

public init(
tlsConfiguration: TLSConfiguration? = nil,
Expand All @@ -620,37 +618,14 @@ public class HTTPClient {
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.decompression = decompression
self.httpVersion = httpVersion
self.httpVersion = .automatic
}

public init(tlsConfiguration: TLSConfiguration? = nil,
Expand Down Expand Up @@ -865,18 +840,17 @@ extension HTTPClient.Configuration {
}
}

// TODO: make this struct and its static properties public
internal struct HTTPVersion {
public 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)
public 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)
public static let automatic: Self = .init(configuration: .automatic)

internal var configuration: Configuration
}
Expand Down
38 changes: 17 additions & 21 deletions Tests/AsyncHTTPClientTests/HTTP2ClientTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@
//
//===----------------------------------------------------------------------===//

// TODO: remove @testable once we officially support HTTP/2
@testable import AsyncHTTPClient // Tests that really need @testable go into HTTP2ClientInternalTests.swift
/* NOT @testable */ import AsyncHTTPClient // Tests that really need @testable go into HTTP2ClientInternalTests.swift
#if canImport(Network)
import Network
#endif
Expand All @@ -28,14 +27,13 @@ class HTTP2ClientTests: XCTestCase {
func makeDefaultHTTPClient(
eventLoopGroupProvider: HTTPClient.EventLoopGroupProvider = .createNew
) -> HTTPClient {
var tlsConfig = TLSConfiguration.makeClientConfiguration()
tlsConfig.certificateVerification = .none
var config = HTTPClient.Configuration()
config.tlsConfiguration = .clientDefault
config.tlsConfiguration?.certificateVerification = .none
config.httpVersion = .automatic
return HTTPClient(
eventLoopGroupProvider: eventLoopGroupProvider,
configuration: HTTPClient.Configuration(
tlsConfiguration: tlsConfig,
httpVersion: .automatic
),
configuration: config,
backgroundActivityLogger: Logger(label: "HTTPClient", factory: StreamLogHandler.standardOutput(label:))
)
}
Expand Down Expand Up @@ -138,14 +136,13 @@ class HTTP2ClientTests: XCTestCase {

let localHTTPBin = HTTPBin(.http2(compress: false))
let elg = MultiThreadedEventLoopGroup(numberOfThreads: numberOfWorkers)
var tlsConfig = TLSConfiguration.makeClientConfiguration()
tlsConfig.certificateVerification = .none
var config = HTTPClient.Configuration()
config.tlsConfiguration = .clientDefault
config.tlsConfiguration?.certificateVerification = .none
config.httpVersion = .automatic
let localClient = HTTPClient(
eventLoopGroupProvider: .shared(elg),
configuration: HTTPClient.Configuration(
tlsConfiguration: tlsConfig,
httpVersion: .automatic
),
configuration: config,
backgroundActivityLogger: Logger(label: "HTTPClient", factory: StreamLogHandler.standardOutput(label:))
)
defer {
Expand Down Expand Up @@ -303,15 +300,14 @@ class HTTP2ClientTests: XCTestCase {
let el1 = clientGroup.next()
let el2 = clientGroup.next()
defer { XCTAssertNoThrow(try clientGroup.syncShutdownGracefully()) }
var tlsConfig = TLSConfiguration.makeClientConfiguration()
tlsConfig.certificateVerification = .none
var config = HTTPClient.Configuration()
config.tlsConfiguration = .clientDefault
config.tlsConfiguration?.certificateVerification = .none
config.httpVersion = .automatic
config.timeout.connect = .milliseconds(1000)
let client = HTTPClient(
eventLoopGroupProvider: .shared(clientGroup),
configuration: HTTPClient.Configuration(
tlsConfiguration: tlsConfig,
timeout: .init(connect: .milliseconds(1000)),
httpVersion: .automatic
),
configuration: config,
backgroundActivityLogger: Logger(label: "HTTPClient", factory: StreamLogHandler.standardOutput(label:))
)
defer { XCTAssertNoThrow(try client.syncShutdown()) }
Expand Down
8 changes: 6 additions & 2 deletions Tests/AsyncHTTPClientTests/HTTP2ConnectionTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -250,10 +250,12 @@ class TestConnectionCreator {

var tlsConfiguration = TLSConfiguration.makeClientConfiguration()
tlsConfiguration.certificateVerification = .none
var config = HTTPClient.Configuration()
config.httpVersion = .automatic
let factory = HTTPConnectionPool.ConnectionFactory(
key: .init(request),
tlsConfiguration: tlsConfiguration,
clientConfiguration: .init(httpVersion: .automatic),
clientConfiguration: config,
sslContextCache: .init()
)

Expand Down Expand Up @@ -291,10 +293,12 @@ class TestConnectionCreator {

var tlsConfiguration = TLSConfiguration.makeClientConfiguration()
tlsConfiguration.certificateVerification = .none
var config = HTTPClient.Configuration()
config.httpVersion = .automatic
let factory = HTTPConnectionPool.ConnectionFactory(
key: .init(request),
tlsConfiguration: tlsConfiguration,
clientConfiguration: .init(httpVersion: .automatic),
clientConfiguration: config,
sslContextCache: .init()
)

Expand Down