Skip to content

Refactor deconstructURL and scheme parsing #504

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

Merged
merged 11 commits into from
Dec 1, 2021
Merged
Show file tree
Hide file tree
Changes from 9 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
43 changes: 2 additions & 41 deletions Sources/AsyncHTTPClient/ConnectionPool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,52 +24,13 @@ enum ConnectionPool {
private var tlsConfiguration: BestEffortHashableTLSConfiguration?

init(_ request: HTTPClient.Request) {
self.connectionTarget = request.connectionTarget
switch request.scheme {
case "http":
self.scheme = .http
case "https":
self.scheme = .https
case "unix":
self.scheme = .unix
case "http+unix":
self.scheme = .http_unix
case "https+unix":
self.scheme = .https_unix
default:
fatalError("HTTPClient.Request scheme should already be a valid one")
}
self.scheme = request.deconstructedURL.scheme
self.connectionTarget = request.deconstructedURL.connectionTarget
if let tls = request.tlsConfiguration {
self.tlsConfiguration = BestEffortHashableTLSConfiguration(wrapping: tls)
}
}

enum Scheme: Hashable {
case http
case https
case unix
case http_unix
case https_unix

var requiresTLS: Bool {
switch self {
case .https, .https_unix:
return true
default:
return false
}
}
}

/// Returns a key-specific `HTTPClient.Configuration` by overriding the properties of `base`
func config(overriding base: HTTPClient.Configuration) -> HTTPClient.Configuration {
var config = base
if let tlsConfiguration = self.tlsConfiguration {
config.tlsConfiguration = tlsConfiguration.base
}
return config
}

var description: String {
var hasher = Hasher()
self.tlsConfiguration?.hash(into: &hasher)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,9 @@ extension HTTPConnectionPool.ConnectionFactory {
logger: Logger
) -> EventLoopFuture<NegotiatedProtocol> {
switch self.key.scheme {
case .http, .http_unix, .unix:
case .http, .httpUnix, .unix:
return self.makePlainChannel(deadline: deadline, eventLoop: eventLoop).map { .http1_1($0) }
case .https, .https_unix:
case .https, .httpsUnix:
return self.makeTLSChannel(deadline: deadline, eventLoop: eventLoop, logger: logger).flatMapThrowing {
channel, negotiated in

Expand All @@ -197,7 +197,7 @@ extension HTTPConnectionPool.ConnectionFactory {
}

private func makePlainChannel(deadline: NIODeadline, eventLoop: EventLoop) -> EventLoopFuture<Channel> {
precondition(!self.key.scheme.requiresTLS, "Unexpected scheme")
precondition(!self.key.scheme.usesTLS, "Unexpected scheme")
return self.makePlainBootstrap(deadline: deadline, eventLoop: eventLoop).connect(target: self.key.connectionTarget)
}

Expand Down Expand Up @@ -283,7 +283,7 @@ extension HTTPConnectionPool.ConnectionFactory {
logger: Logger
) -> EventLoopFuture<NegotiatedProtocol> {
switch self.key.scheme {
case .unix, .http_unix, .https_unix:
case .unix, .httpUnix, .httpsUnix:
preconditionFailure("Unexpected scheme. Not supported for proxy!")
case .http:
return channel.eventLoop.makeSucceededFuture(.http1_1(channel))
Expand Down Expand Up @@ -356,7 +356,7 @@ extension HTTPConnectionPool.ConnectionFactory {
}

private func makeTLSChannel(deadline: NIODeadline, eventLoop: EventLoop, logger: Logger) -> EventLoopFuture<(Channel, String?)> {
precondition(self.key.scheme.requiresTLS, "Unexpected scheme")
precondition(self.key.scheme.usesTLS, "Unexpected scheme")
let bootstrapFuture = self.makeTLSBootstrap(
deadline: deadline,
eventLoop: eventLoop,
Expand Down Expand Up @@ -470,12 +470,12 @@ extension HTTPConnectionPool.ConnectionFactory {
}
}

extension ConnectionPool.Key.Scheme {
extension Scheme {
var isProxyable: Bool {
switch self {
case .http, .https:
return true
case .unix, .http_unix, .https_unix:
case .unix, .httpUnix, .httpsUnix:
return false
}
}
Expand Down
76 changes: 76 additions & 0 deletions Sources/AsyncHTTPClient/DeconstructedURL.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the AsyncHTTPClient open source project
//
// Copyright (c) 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 struct Foundation.URL

struct DeconstructedURL {
var scheme: Scheme
var connectionTarget: ConnectionTarget
var uri: String

init(
scheme: Scheme,
connectionTarget: ConnectionTarget,
uri: String
) {
self.scheme = scheme
self.connectionTarget = connectionTarget
self.uri = uri
}
}

extension DeconstructedURL {
init(url: URL) throws {
guard let schemeString = url.scheme else {
throw HTTPClientError.emptyScheme
}
guard let scheme = Scheme(rawValue: schemeString.lowercased()) else {
throw HTTPClientError.unsupportedScheme(schemeString)
}

switch scheme {
case .http, .https:
guard let host = url.host, !host.isEmpty else {
throw HTTPClientError.emptyHost
}
self.init(
scheme: scheme,
connectionTarget: .init(remoteHost: host, port: url.port ?? scheme.defaultPort),
uri: url.uri
)

case .httpUnix, .httpsUnix:
guard let socketPath = url.host, !socketPath.isEmpty else {
throw HTTPClientError.missingSocketPath
}
self.init(
scheme: scheme,
connectionTarget: .unixSocket(path: socketPath),
uri: 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
}
self.init(
scheme: scheme,
connectionTarget: .unixSocket(path: socketPath),
uri: uri
)
}
}
}
129 changes: 45 additions & 84 deletions Sources/AsyncHTTPClient/HTTPHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -92,82 +92,19 @@ extension HTTPClient {

/// Represent HTTP request.
public struct Request {
/// Represent kind of Request
enum Kind: Equatable {
enum UnixScheme: Equatable {
case baseURL
case http_unix
case https_unix
}

/// Remote host request.
case host
/// UNIX Domain Socket HTTP request.
case unixSocket(_ scheme: UnixScheme)

private static var hostRestrictedSchemes: Set = ["http", "https"]
private static var allSupportedSchemes: Set = ["http", "https", "unix", "http+unix", "https+unix"]

func supportsRedirects(to scheme: String?) -> Bool {
guard let scheme = scheme?.lowercased() else { return false }

switch self {
case .host:
return Kind.hostRestrictedSchemes.contains(scheme)
case .unixSocket:
return Kind.allSupportedSchemes.contains(scheme)
}
}
}

static func useTLS(_ scheme: String) -> Bool {
return scheme == "https" || scheme == "https+unix"
}

static func deconstructURL(
_ url: URL
) throws -> (kind: Kind, scheme: String, connectionTarget: ConnectionTarget, uri: String) {
guard let scheme = url.scheme?.lowercased() else {
throw HTTPClientError.emptyScheme
}
switch scheme {
case "http", "https":
guard let host = url.host, !host.isEmpty else {
throw HTTPClientError.emptyHost
}
let defaultPort = self.useTLS(scheme) ? 443 : 80
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 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
}
let socketTarget = ConnectionTarget.unixSocket(path: socketPath)
return (.unixSocket(.baseURL), scheme, socketTarget, uri)
default:
throw HTTPClientError.unsupportedScheme(url.scheme!)
}
}

/// Request HTTP method, defaults to `GET`.
public let method: HTTPMethod
/// Remote URL.
public let url: URL

/// Parsed, validated and deconstructed URL.
internal let deconstructedURL: DeconstructedURL

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: Move down to other internal var like redirect state. Mixing internal and public makes this hard to read.

/// Remote HTTP scheme, resolved from `URL`.
public let scheme: String
/// The connection target, resolved from `URL`.
let connectionTarget: ConnectionTarget
/// URI composed of the path and query, resolved from `URL`.
let uri: String
public var scheme: String {
self.deconstructedURL.scheme.rawValue
}

/// Request custom HTTP Headers, defaults to no headers.
public var headers: HTTPHeaders
/// Request body, defaults to no body.
Expand All @@ -181,7 +118,6 @@ extension HTTPClient {
}

var redirectState: RedirectState?
let kind: Kind

/// Create HTTP request.
///
Expand Down Expand Up @@ -252,7 +188,8 @@ 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.connectionTarget, self.uri) = try Request.deconstructURL(url)
self.deconstructedURL = try DeconstructedURL(url: url)

self.redirectState = nil
self.url = url
self.method = method
Expand All @@ -261,14 +198,9 @@ extension HTTPClient {
self.tlsConfiguration = tlsConfiguration
}

/// Whether request will be executed using secure socket.
public var useTLS: Bool {
return Request.useTLS(self.scheme)
}

/// Remote host, resolved from `URL`.
public var host: String {
switch self.connectionTarget {
switch self.deconstructedURL.connectionTarget {
case .ipAddress(let serialization, _): return serialization
case .domain(let name, _): return name
case .unixSocket: return ""
Expand All @@ -277,25 +209,28 @@ extension HTTPClient {

/// Resolved port.
public var port: Int {
switch self.connectionTarget {
switch self.deconstructedURL.connectionTarget {
case .ipAddress(_, let address): return address.port!
case .domain(_, let port): return port
case .unixSocket: return Request.useTLS(self.scheme) ? 443 : 80
case .unixSocket: return self.deconstructedURL.scheme.defaultPort
}
}

/// Whether request will be executed using secure socket.
public var useTLS: Bool { self.deconstructedURL.scheme.usesTLS }

func createRequestHead() throws -> (HTTPRequestHead, RequestFramingMetadata) {
var head = HTTPRequestHead(
version: .http1_1,
method: self.method,
uri: self.uri,
uri: self.deconstructedURL.uri,
headers: self.headers
)

if !head.headers.contains(name: "host") {
let port = self.port
var host = self.host
if !(port == 80 && self.scheme == "http"), !(port == 443 && self.scheme == "https") {
if !(port == 80 && self.deconstructedURL.scheme == .http), !(port == 443 && self.deconstructedURL.scheme == .https) {
host += ":\(port)"
}
head.headers.add(name: "host", value: host)
Expand Down Expand Up @@ -740,7 +675,7 @@ internal struct RedirectHandler<ResponseType> {
return nil
}

guard self.request.kind.supportsRedirects(to: url.scheme) else {
guard self.request.deconstructedURL.scheme.supportsRedirects(to: url.scheme) else {
return nil
}

Expand Down Expand Up @@ -813,6 +748,32 @@ internal struct RedirectHandler<ResponseType> {
}
}

extension Scheme {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should live in the Scheme file.

func supportsRedirects(to destinationScheme: String?) -> Bool {
guard
let destinationSchemeString = destinationScheme?.lowercased(),
let destinationScheme = Self(rawValue: destinationSchemeString)
else {
return false
}
return self.supportsRedirects(to: destinationScheme)
}

func supportsRedirects(to destinationScheme: Self) -> Bool {
switch self {
case .http, .https:
switch destinationScheme {
case .http, .https:
return true
case .unix, .httpUnix, .httpsUnix:
return false
}
case .unix, .httpUnix, .httpsUnix:
return true
}
}
}

extension RequestBodyLength {
init(_ body: HTTPClient.Body?) {
guard let body = body else {
Expand Down
Loading