Skip to content

fix UDS without a baseURL #165

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 1 commit into from
Feb 24, 2020
Merged
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
14 changes: 10 additions & 4 deletions Sources/AsyncHTTPClient/HTTPClient.swift
Original file line number Diff line number Diff line change
@@ -272,7 +272,11 @@ public class HTTPClient {
return channel.eventLoop.makeSucceededFuture(())
}
}.flatMap {
let taskHandler = TaskHandler(task: task, delegate: delegate, redirectHandler: redirectHandler, ignoreUncleanSSLShutdown: self.configuration.ignoreUncleanSSLShutdown)
let taskHandler = TaskHandler(task: task,
kind: request.kind,
delegate: delegate,
redirectHandler: redirectHandler,
ignoreUncleanSSLShutdown: self.configuration.ignoreUncleanSSLShutdown)
return channel.pipeline.addHandler(taskHandler)
}
}
@@ -282,9 +286,11 @@ public class HTTPClient {
}

let eventLoopChannel: EventLoopFuture<Channel>
if request.kind == .unixSocket, let baseURL = request.url.baseURL {
eventLoopChannel = bootstrap.connect(unixDomainSocketPath: baseURL.path)
} else {
switch request.kind {
case .unixSocket:
let socketPath = request.url.baseURL?.path ?? request.url.path
eventLoopChannel = bootstrap.connect(unixDomainSocketPath: socketPath)
case .host:
let address = self.resolveAddress(request: request, proxy: self.configuration.proxy)
eventLoopChannel = bootstrap.connect(host: address.host, port: address.port)
}
22 changes: 20 additions & 2 deletions Sources/AsyncHTTPClient/HTTPHandler.swift
Original file line number Diff line number Diff line change
@@ -559,12 +559,18 @@ internal class TaskHandler<Delegate: HTTPClientResponseDelegate> {
var state: State = .idle
var pendingRead = false
var mayRead = true
let kind: HTTPClient.Request.Kind

init(task: HTTPClient.Task<Delegate.Response>, delegate: Delegate, redirectHandler: RedirectHandler<Delegate.Response>?, ignoreUncleanSSLShutdown: Bool) {
init(task: HTTPClient.Task<Delegate.Response>,
kind: HTTPClient.Request.Kind,
delegate: Delegate,
redirectHandler: RedirectHandler<Delegate.Response>?,
ignoreUncleanSSLShutdown: Bool) {
self.task = task
self.delegate = delegate
self.redirectHandler = redirectHandler
self.ignoreUncleanSSLShutdown = ignoreUncleanSSLShutdown
self.kind = kind
}
}

@@ -653,7 +659,19 @@ extension TaskHandler: ChannelDuplexHandler {
self.state = .idle
let request = unwrapOutboundIn(data)

var head = HTTPRequestHead(version: HTTPVersion(major: 1, minor: 1), method: request.method, uri: request.url.uri)
let uri: String
switch (self.kind, request.url.baseURL) {
case (.host, _):
uri = request.url.uri
case (.unixSocket, .none):
uri = "/" // we don't have a real path, the path we have is the path of the UNIX Domain Socket.
case (.unixSocket, .some(_)):
uri = request.url.uri
}

var head = HTTPRequestHead(version: HTTPVersion(major: 1, minor: 1),
method: request.method,
uri: uri)
var headers = request.headers

if !request.headers.contains(name: "Host") {
13 changes: 11 additions & 2 deletions Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift
Original file line number Diff line number Diff line change
@@ -28,7 +28,11 @@ class HTTPClientInternalTests: XCTestCase {
let task = Task<Void>(eventLoop: channel.eventLoop)

try channel.pipeline.addHandler(recorder).wait()
try channel.pipeline.addHandler(TaskHandler(task: task, delegate: TestHTTPDelegate(), redirectHandler: nil, ignoreUncleanSSLShutdown: false)).wait()
try channel.pipeline.addHandler(TaskHandler(task: task,
kind: .host,
delegate: TestHTTPDelegate(),
redirectHandler: nil,
ignoreUncleanSSLShutdown: false)).wait()

var request = try Request(url: "http://localhost/get")
request.headers.add(name: "X-Test-Header", value: "X-Test-Value")
@@ -56,6 +60,7 @@ class HTTPClientInternalTests: XCTestCase {

XCTAssertNoThrow(try channel.pipeline.addHandler(recorder).wait())
XCTAssertNoThrow(try channel.pipeline.addHandler(TaskHandler(task: task,
kind: .host,
delegate: TestHTTPDelegate(),
redirectHandler: nil,
ignoreUncleanSSLShutdown: false)).wait())
@@ -74,7 +79,11 @@ class HTTPClientInternalTests: XCTestCase {
let channel = EmbeddedChannel()
let delegate = TestHTTPDelegate()
let task = Task<Void>(eventLoop: channel.eventLoop)
let handler = TaskHandler(task: task, delegate: delegate, redirectHandler: nil, ignoreUncleanSSLShutdown: false)
let handler = TaskHandler(task: task,
kind: .host,
delegate: delegate,
redirectHandler: nil,
ignoreUncleanSSLShutdown: false)

try channel.pipeline.addHandler(handler).wait()

101 changes: 99 additions & 2 deletions Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift
Original file line number Diff line number Diff line change
@@ -92,11 +92,87 @@ internal final class RecordingHandler<Input, Output>: ChannelDuplexHandler {
}
}

enum TemporaryFileHelpers {
private static var temporaryDirectory: String {
#if targetEnvironment(simulator)
// Simulator temp directories are so long (and contain the user name) that they're not usable
// for UNIX Domain Socket paths (which are limited to 103 bytes).
return "/tmp"
#else
#if os(Android)
return "/data/local/tmp"
#elseif os(Linux)
return "/tmp"
#else
if #available(macOS 10.12, iOS 10, tvOS 10, watchOS 3, *) {
return FileManager.default.temporaryDirectory.path
} else {
return "/tmp"
}
#endif // os
#endif // targetEnvironment
}

private static func openTemporaryFile() -> (CInt, String) {
let template = "\(temporaryDirectory)/ahc_XXXXXX"
var templateBytes = template.utf8 + [0]
let templateBytesCount = templateBytes.count
let fd = templateBytes.withUnsafeMutableBufferPointer { ptr in
ptr.baseAddress!.withMemoryRebound(to: Int8.self, capacity: templateBytesCount) { ptr in
mkstemp(ptr)
}
}
templateBytes.removeLast()
return (fd, String(decoding: templateBytes, as: Unicode.UTF8.self))
}

/// This function creates a filename that can be used for a temporary UNIX domain socket path.
///
/// If the temporary directory is too long to store a UNIX domain socket path, it will `chdir` into the temporary
/// directory and return a short-enough path. The iOS simulator is known to have too long paths.
internal static func withTemporaryUnixDomainSocketPathName<T>(directory: String = temporaryDirectory,
_ body: (String) throws -> T) throws -> T {
// this is racy but we're trying to create the shortest possible path so we can't add a directory...
let (fd, path) = self.openTemporaryFile()
close(fd)
try! FileManager.default.removeItem(atPath: path)

let saveCurrentDirectory = FileManager.default.currentDirectoryPath
let restoreSavedCWD: Bool
let shortEnoughPath: String
do {
_ = try SocketAddress(unixDomainSocketPath: path)
// this seems to be short enough for a UDS
shortEnoughPath = path
restoreSavedCWD = false
} catch SocketAddressError.unixDomainSocketPathTooLong {
FileManager.default.changeCurrentDirectoryPath(URL(fileURLWithPath: path).deletingLastPathComponent().absoluteString)
shortEnoughPath = URL(fileURLWithPath: path).lastPathComponent
restoreSavedCWD = true
print("WARNING: Path '\(path)' could not be used as UNIX domain socket path, using chdir & '\(shortEnoughPath)'")
}
defer {
if FileManager.default.fileExists(atPath: path) {
try? FileManager.default.removeItem(atPath: path)
}
if restoreSavedCWD {
FileManager.default.changeCurrentDirectoryPath(saveCurrentDirectory)
}
}
return try body(shortEnoughPath)
}
}

internal final class HTTPBin {
let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
let serverChannel: Channel
let isShutdown: NIOAtomic<Bool> = .makeAtomic(value: false)

enum BindTarget {
case unixDomainSocket(String)
case localhostIPv4RandomPort
}

var port: Int {
return Int(self.serverChannel.localAddress!.port!)
}
@@ -112,7 +188,18 @@ internal final class HTTPBin {
return channel.pipeline.addHandler(try! NIOSSLServerHandler(context: context), position: .first)
}

init(ssl: Bool = false, compress: Bool = false, simulateProxy: HTTPProxySimulator.Option? = nil, channelPromise: EventLoopPromise<Channel>? = nil) {
init(ssl: Bool = false,
compress: Bool = false,
bindTarget: BindTarget = .localhostIPv4RandomPort,
simulateProxy: HTTPProxySimulator.Option? = nil,
channelPromise: EventLoopPromise<Channel>? = nil) {
let socketAddress: SocketAddress
switch bindTarget {
case .localhostIPv4RandomPort:
socketAddress = try! SocketAddress(ipAddress: "127.0.0.1", port: 0)
case .unixDomainSocket(let path):
socketAddress = try! SocketAddress(unixDomainSocketPath: path)
}
self.serverChannel = try! ServerBootstrap(group: self.group)
.serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
.childChannelOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1)
@@ -145,7 +232,7 @@ internal final class HTTPBin {
}
}
}
.bind(host: "127.0.0.1", port: 0).wait()
.bind(to: socketAddress).wait()
}

func shutdown() throws {
@@ -250,6 +337,16 @@ internal final class HttpBinHandler: ChannelInboundHandler {
case .head(let req):
let url = URL(string: req.uri)!
switch url.path {
case "/":
var headers = HTTPHeaders()
headers.add(name: "X-Is-This-Slash", value: "Yes")
self.resps.append(HTTPResponseBuilder(status: .ok, headers: headers))
return
case "/echo-uri":
var headers = HTTPHeaders()
headers.add(name: "X-Calling-URI", value: req.uri)
self.resps.append(HTTPResponseBuilder(status: .ok, headers: headers))
return
case "/ok":
self.resps.append(HTTPResponseBuilder(status: .ok))
return
2 changes: 2 additions & 0 deletions Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift
Original file line number Diff line number Diff line change
@@ -70,6 +70,8 @@ extension HTTPClientTests {
("testSubsequentRequestsWorkWithServerAlternatingBetweenKeepAliveAndClose", testSubsequentRequestsWorkWithServerAlternatingBetweenKeepAliveAndClose),
("testManyConcurrentRequestsWork", testManyConcurrentRequestsWork),
("testRepeatedRequestsWorkWhenServerAlwaysCloses", testRepeatedRequestsWorkWhenServerAlwaysCloses),
("testUDSBasic", testUDSBasic),
("testUDSSocketAndPath", testUDSSocketAndPath),
]
}
}
45 changes: 45 additions & 0 deletions Tests/AsyncHTTPClientTests/HTTPClientTests.swift
Original file line number Diff line number Diff line change
@@ -1003,4 +1003,49 @@ class HTTPClientTests: XCTestCase {
XCTAssertNil(response?.body)
}
}

func testUDSBasic() {
// This tests just connecting to a URL where the whole URL is the UNIX domain socket path like
// unix:///this/is/my/socket.sock
// We don't really have a path component, so we'll have to use "/"
XCTAssertNoThrow(try TemporaryFileHelpers.withTemporaryUnixDomainSocketPathName { path in
let httpBin = HTTPBin(bindTarget: .unixDomainSocket(path))
defer {
XCTAssertNoThrow(try httpBin.shutdown())
}
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
defer {
XCTAssertNoThrow(try httpClient.syncShutdown())
}

let target = "unix://\(path)"
XCTAssertNoThrow(XCTAssertEqual(["Yes"[...]],
try httpClient.get(url: target).wait().headers[canonicalForm: "X-Is-This-Slash"]))
})
}

func testUDSSocketAndPath() {
// Here, we're testing a URL that's encoding two different paths:
//
// 1. a "base path" which is the path to the UNIX domain socket
// 2. an actual path which is the normal path in a regular URL like https://example.com/this/is/the/path
XCTAssertNoThrow(try TemporaryFileHelpers.withTemporaryUnixDomainSocketPathName { path in
let httpBin = HTTPBin(bindTarget: .unixDomainSocket(path))
defer {
XCTAssertNoThrow(try httpBin.shutdown())
}
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
defer {
XCTAssertNoThrow(try httpClient.syncShutdown())
}

guard let target = URL(string: "/echo-uri", relativeTo: URL(string: "unix://\(path)")),
let request = try? Request(url: target) else {
XCTFail("couldn't build URL for request")
return
}
XCTAssertNoThrow(XCTAssertEqual(["/echo-uri"[...]],
try httpClient.execute(request: request).wait().headers[canonicalForm: "X-Calling-URI"]))
})
}
}