From 573983ed22f8022518a0795c188a6ec3060fab25 Mon Sep 17 00:00:00 2001 From: Johannes Weiss Date: Thu, 20 Feb 2020 18:16:58 +0000 Subject: [PATCH] fix UDS without a baseURL Previously, UNIX Domain Sockets would only work if the URL also had a "base URL". If it didn't have a base URL, it would try to connect to the empty string which would fail :). Now, we support both cases: - URLs with a baseURL (the path to the UDS) and a path (actual path) - URLs that just have an actual path (path to the UDS) where we'll just use "/" as the URL's path --- Sources/AsyncHTTPClient/HTTPClient.swift | 14 ++- Sources/AsyncHTTPClient/HTTPHandler.swift | 22 +++- .../HTTPClientInternalTests.swift | 13 ++- .../HTTPClientTestUtils.swift | 101 +++++++++++++++++- .../HTTPClientTests+XCTest.swift | 2 + .../HTTPClientTests.swift | 45 ++++++++ 6 files changed, 187 insertions(+), 10 deletions(-) diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index 6f15a2b52..2fcff89b5 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -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 - 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) } diff --git a/Sources/AsyncHTTPClient/HTTPHandler.swift b/Sources/AsyncHTTPClient/HTTPHandler.swift index 4bdee600c..eeb675f02 100644 --- a/Sources/AsyncHTTPClient/HTTPHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPHandler.swift @@ -559,12 +559,18 @@ internal class TaskHandler { var state: State = .idle var pendingRead = false var mayRead = true + let kind: HTTPClient.Request.Kind - init(task: HTTPClient.Task, delegate: Delegate, redirectHandler: RedirectHandler?, ignoreUncleanSSLShutdown: Bool) { + init(task: HTTPClient.Task, + kind: HTTPClient.Request.Kind, + delegate: Delegate, + redirectHandler: RedirectHandler?, + 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") { diff --git a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift index bc0d71784..917b3622f 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift @@ -28,7 +28,11 @@ class HTTPClientInternalTests: XCTestCase { let task = Task(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(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() diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift b/Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift index d2ac57fa6..81926f1e5 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift @@ -92,11 +92,87 @@ internal final class RecordingHandler: 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(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 = .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? = nil) { + init(ssl: Bool = false, + compress: Bool = false, + bindTarget: BindTarget = .localhostIPv4RandomPort, + simulateProxy: HTTPProxySimulator.Option? = nil, + channelPromise: EventLoopPromise? = 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 diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift index de11d548a..e35b9b679 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift @@ -70,6 +70,8 @@ extension HTTPClientTests { ("testSubsequentRequestsWorkWithServerAlternatingBetweenKeepAliveAndClose", testSubsequentRequestsWorkWithServerAlternatingBetweenKeepAliveAndClose), ("testManyConcurrentRequestsWork", testManyConcurrentRequestsWork), ("testRepeatedRequestsWorkWhenServerAlwaysCloses", testRepeatedRequestsWorkWhenServerAlwaysCloses), + ("testUDSBasic", testUDSBasic), + ("testUDSSocketAndPath", testUDSSocketAndPath), ] } } diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 4bf6daa93..96b3761c0 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -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"])) + }) + } }