Skip to content

Support UNIX Domain Sockets #151

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.

Sign up for GitHub

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 10 commits into from
Jan 27, 2020
Merged

Support UNIX Domain Sockets #151

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4932cb5
Support UNIX Domain Socket url
krzyzanowskim Jan 19, 2020
41ee04e
Use relative URL as socket url
krzyzanowskim Jan 19, 2020
ac69f69
Adapt unix scheme for sockets
krzyzanowskim Jan 20, 2020
d3bbf37
Use Request.Kind to distinguish between host and unix socket kind of …
krzyzanowskim Jan 26, 2020
bbd47f7
refactor: Move some request logic to enum helpers
krzyzanowskim Jan 27, 2020
cb19a2d
Merge branch 'master' into marcin/unix-socket-request
weissi Jan 27, 2020
45c443f
Update formatting
krzyzanowskim Jan 27, 2020
535303b
Revert host to let
krzyzanowskim Jan 27, 2020
5233d5c
Don't use Self to please Swift 5.0
krzyzanowskim Jan 27, 2020
b6cf0a4
Ups. Wrong property updated previously
krzyzanowskim Jan 27, 2020
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
15 changes: 11 additions & 4 deletions Sources/AsyncHTTPClient/HTTPClient.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,15 +281,22 @@ public class HTTPClient {
bootstrap = bootstrap.connectTimeout(timeout)
}

let address = self.resolveAddress(request: request, proxy: self.configuration.proxy)
bootstrap.connect(host: address.host, port: address.port)
.map { channel in
task.setChannel(channel)
let eventLoopChannel: EventLoopFuture<Channel>
if request.scheme == "file", let baseURL = request.url.baseURL {
eventLoopChannel = bootstrap.connect(unixDomainSocketPath: baseURL.path)
} else {
let address = self.resolveAddress(request: request, proxy: self.configuration.proxy)
eventLoopChannel = bootstrap.connect(host: address.host, port: address.port)
}

eventLoopChannel.map { channel in
task.setChannel(channel)
}
.flatMap { channel in
channel.writeAndFlush(request)
}
.cascadeFailure(to: task.promise)

return task
}

Expand Down
146 changes: 129 additions & 17 deletions Sources/AsyncHTTPClient/HTTPHandler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,24 +89,47 @@ extension HTTPClient {
/// Represent HTTP request.
public struct Request {
/// Request HTTP method, defaults to `GET`.
public let method: HTTPMethod
public var method: HTTPMethod {
return value.method
}
/// Remote URL.
public let url: URL
public var url: URL {
return value.url
}
/// Remote HTTP scheme, resolved from `URL`.
public let scheme: String
public var scheme: String {
return value.scheme
}
/// Remote host, resolved from `URL`.
public let host: String
public var host: String {
return value.host
}
/// Request custom HTTP Headers, defaults to no headers.
public var headers: HTTPHeaders
public var headers: HTTPHeaders {
get {
return value.headers
}
set {
value.headers = newValue
}
}
/// Request body, defaults to no body.
public var body: Body?
public var body: Body? {
get {
return value.body
}
set {
value.body = newValue
}
}

struct RedirectState {
var count: Int
var visited: Set<URL>?
}

var redirectState: RedirectState?
private var value: HTTPRequest
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't love using a protocol existential here. I think the splitting of basic request types is a good idea, but I'd rather use an enumeration that we can switch over than the protocol existential.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems the only difference between implementations is how they handle scheme? Why not just add a unix:// to a list of supported schemes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not just add a unix:// to a list of supported schemes?

slightly different asserts in Request constructor

Copy link
Contributor Author

@krzyzanowskim krzyzanowskim Jan 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I'd rather use an enumeration that we can switch over than the protocol existential.

I'm not a fan. Do you think about if'ing in every Request property?

I've rework it with enum: d3bbf37 let me know if that's something you love more @Lukasa 😅


/// Create HTTP request.
///
Expand All @@ -129,6 +152,57 @@ extension HTTPClient {
try self.init(url: url, method: method, headers: headers, body: body)
}

/// Create an HTTP `Request`.
///
/// - parameters:
/// - url: Remote `URL`.
/// - version: HTTP version.
/// - method: HTTP method.
/// - headers: Custom HTTP headers.
/// - body: Request body.
/// - throws:
/// - `emptyScheme` if URL does not contain HTTP scheme.
/// - `unsupportedScheme` if URL does contains unsupported HTTP scheme.
/// - `emptyHost` if URL does not contains a host.
public init(url: URL, method: HTTPMethod = .GET, headers: HTTPHeaders = HTTPHeaders(), body: Body? = nil) throws {
if url.isFileURL {
self.value = try UnixDomainRequest(url: url, method: method, headers: headers, body: body)
} else {
self.value = try HostRequest(url: url, method: method, headers: headers, body: body)
}
self.redirectState = nil
}

/// Whether request will be executed using secure socket.
public var useTLS: Bool {
return self.scheme == "https"
}

/// Resolved port.
public var port: Int {
return self.url.port ?? (self.useTLS ? 443 : 80)
}

func isSchemeSupported(scheme: String) -> Bool {
return type(of: self.value).isSchemeSupported(scheme: scheme)
}
}

/// Represent HTTP request.
public struct HostRequest: HTTPRequest {
/// Request HTTP method, defaults to `GET`.
public let method: HTTPMethod
/// Remote URL.
public let url: URL
/// Remote HTTP scheme, resolved from `URL`.
public let scheme: String
/// Remote host, resolved from `URL`.
public let host: String
/// Request custom HTTP Headers, defaults to no headers.
public var headers: HTTPHeaders
/// Request body, defaults to no body.
public var body: Body?

/// Create an HTTP `Request`.
///
/// - parameters:
Expand All @@ -146,7 +220,7 @@ extension HTTPClient {
throw HTTPClientError.emptyScheme
}

guard Request.isSchemeSupported(scheme: scheme) else {
guard Self.isSchemeSupported(scheme: scheme) else {
throw HTTPClientError.unsupportedScheme(scheme)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This block can be rewritten to be substantially cleaner by defining some helpers:

extension Kind {
    private static var hostSchemes = ["http", "https"]
    private static var unixSchemes = ["unix"]

    init(forScheme scheme: String) throws {
        if Kind.hostSchemes.contains(scheme) {
            self = .host
        } else if Kind.unixSchemes.contains(scheme) {
            self = .unixSocket
        } else {
            throw HTTPClientError.unsupportedScheme
        }
    }

    func hostFromURL(_ url: URL) throws -> String {
        switch self {
        case .host:
            guard let host = url.host else {
                throw HTTPClientError.emptyHost
            }
            return host
        case .unixSocket:
            return ""
        }
    }
}

This code then becomes:

self.kind = try Kind(scheme: scheme)
self.host = try self.kind.hostFromURL(url)

We can then also rewrite kind.isSchemeSupported in terms of our statics. This should put most of the complexity into the enum definition, which helps keep the code elsewhere a lot nicer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the feedback. Applied as requested.


Expand All @@ -160,23 +234,61 @@ extension HTTPClient {
self.host = host
self.headers = headers
self.body = body

self.redirectState = nil
}

/// Whether request will be executed using secure socket.
public var useTLS: Bool {
return self.scheme == "https"
static func isSchemeSupported(scheme: String) -> Bool {
return scheme == "http" || scheme == "https"
}
}

/// Resolved port.
public var port: Int {
return self.url.port ?? (self.useTLS ? 443 : 80)
/// Represent UNIX Domain Socket HTTP request.
public struct UnixDomainRequest: HTTPRequest {
/// Request HTTP method, defaults to `GET`.
public let method: HTTPMethod
/// UNIX Domain Socket file URL.
public let url: URL
/// Remote HTTP scheme, resolved from `URL`. Unused.
public let scheme: String
/// Remote host, resolved from `URL`. Unused.
public let host: String
/// Request custom HTTP Headers, defaults to no headers.
public var headers: HTTPHeaders
/// Request body, defaults to no body.
public var body: Body?

/// Create an HTTP `Request`.
///
/// - parameters:
/// - url: UNIX Domain Socket `URL`.
/// - version: HTTP version.
/// - method: HTTP method.
/// - headers: Custom HTTP headers.
/// - body: Request body.
/// - throws:
/// - `emptyScheme` if URL does not contain HTTP scheme.
/// - `unsupportedScheme` if URL does contains unsupported HTTP scheme.
/// - `emptyHost` if URL does not contains a host.
public init(url: URL, method: HTTPMethod = .GET, headers: HTTPHeaders = HTTPHeaders(), body: Body? = nil) throws {
guard url.isFileURL else {
throw HTTPClientError.invalidURL
}

guard let scheme = url.scheme?.lowercased() else {
throw HTTPClientError.emptyScheme
}

self.method = method
self.url = url
self.scheme = scheme
self.host = ""
self.headers = headers
self.body = body
}

static func isSchemeSupported(scheme: String) -> Bool {
return scheme == "http" || scheme == "https"
return scheme == "file"
}

}

/// Represent HTTP response.
Expand Down Expand Up @@ -812,7 +924,7 @@ internal struct RedirectHandler<ResponseType> {
return nil
}

guard HTTPClient.Request.isSchemeSupported(scheme: self.request.scheme) else {
guard request.isSchemeSupported(scheme: self.request.scheme) else {
return nil
}

Expand Down
26 changes: 26 additions & 0 deletions Sources/AsyncHTTPClient/HTTPRequest.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//
// File.swift
//
//
// Created by Marcin Krzyzanowski on 19/01/2020.
//

import Foundation
import NIOHTTP1

protocol HTTPRequest {
/// Request HTTP method
var method: HTTPMethod { get }
/// Remote URL.
var url: URL { get }
/// Remote HTTP scheme, resolved from `URL`.
var scheme: String { get }
/// Remote host, resolved from `URL`.
var host: String { get }
/// Request custom HTTP Headers, defaults to no headers.
var headers: HTTPHeaders { get set }
/// Request body, defaults to no body.
var body: HTTPClient.Body? { get set }

static func isSchemeSupported(scheme: String) -> Bool
}
12 changes: 10 additions & 2 deletions Tests/AsyncHTTPClientTests/HTTPClientTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,21 @@ class HTTPClientTests: XCTestCase {

let request2 = try Request(url: "https://someserver.com")
XCTAssertEqual(request2.url.path, "")

let request3 = try Request(url: "file:///tmp/file")
XCTAssertNil(request3.url.host)
XCTAssertEqual(request3.host, "")
XCTAssertEqual(request3.url.path, "/tmp/file")
XCTAssertEqual(request3.port, 80)
XCTAssertFalse(request3.useTLS)
}

func testBadRequestURI() throws {
XCTAssertThrowsError(try Request(url: "some/path"), "should throw") { error in
XCTAssertEqual(error as! HTTPClientError, HTTPClientError.emptyScheme)
}
XCTAssertThrowsError(try Request(url: "file://somewhere/some/path?foo=bar"), "should throw") { error in
XCTAssertEqual(error as! HTTPClientError, HTTPClientError.unsupportedScheme("file"))
XCTAssertThrowsError(try Request(url: "app://somewhere/some/path?foo=bar"), "should throw") { error in
XCTAssertEqual(error as! HTTPClientError, HTTPClientError.unsupportedScheme("app"))
}
XCTAssertThrowsError(try Request(url: "https:/foo"), "should throw") { error in
XCTAssertEqual(error as! HTTPClientError, HTTPClientError.emptyHost)
Expand All @@ -63,6 +70,7 @@ class HTTPClientTests: XCTestCase {

func testSchemaCasing() throws {
XCTAssertNoThrow(try Request(url: "hTTpS://someserver.com:8888/some/path?foo=bar"))
XCTAssertNoThrow(try Request(url: "File://someserver.com:8888/some/path?foo=bar"))
}

func testGet() throws {
Expand Down