From 5a49c2c17c60c87bf63801f58e4b7505f5001fdc Mon Sep 17 00:00:00 2001 From: David Evans Date: Wed, 16 Jun 2021 14:26:34 +0100 Subject: [PATCH 01/25] Basic socks functionality --- Package.swift | 4 ++-- Sources/AsyncHTTPClient/HTTPClient.swift | 8 ++++++++ .../HTTPClientProxyHandler.swift | 17 +++++++++++++++-- Sources/AsyncHTTPClient/Utils.swift | 11 ++++++++--- 4 files changed, 33 insertions(+), 7 deletions(-) diff --git a/Package.swift b/Package.swift index f2e606a93..8d4367d68 100644 --- a/Package.swift +++ b/Package.swift @@ -31,12 +31,12 @@ let package = Package( .target( name: "AsyncHTTPClient", dependencies: ["NIO", "NIOHTTP1", "NIOSSL", "NIOConcurrencyHelpers", "NIOHTTPCompression", - "NIOFoundationCompat", "NIOTransportServices", "Logging"] + "NIOFoundationCompat", "NIOTransportServices", "Logging", "NIOSOCKS"] ), .testTarget( name: "AsyncHTTPClientTests", dependencies: ["NIO", "NIOConcurrencyHelpers", "NIOSSL", "AsyncHTTPClient", "NIOFoundationCompat", - "NIOTestUtils", "Logging"] + "NIOTestUtils", "Logging", "NIOSOCKS"] ), ] ) diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index ec549d993..1d8ebcbf4 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -21,6 +21,7 @@ import NIOHTTPCompression import NIOSSL import NIOTLS import NIOTransportServices +import NIOSOCKS extension Logger { private func requestInfo(_ request: HTTPClient.Request) -> Logger.Metadata.Value { @@ -899,6 +900,13 @@ extension ChannelPipeline { try sync.addHandler(decoder) try sync.addHandler(handler) } + + func syncAddSOCKSProxyHandler() throws { + let address = try SocketAddress(ipAddress: "127.0.0.1", port: 12345) + let handler = SOCKSClientHandler(targetAddress: .address(address)) + let sync = self.syncOperations + try sync.addHandler(handler) + } func syncAddLateSSLHandlerIfNeeded(for key: ConnectionPool.Key, sslContext: NIOSSLContext, diff --git a/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift b/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift index ebdfbfa24..c711ecc8b 100644 --- a/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift @@ -14,6 +14,7 @@ import NIO import NIOHTTP1 +import NIOSOCKS public extension HTTPClient.Configuration { /// Proxy server configuration @@ -27,6 +28,12 @@ public extension HTTPClient.Configuration { /// TLS will be established _after_ successful proxy, between your client /// and the destination server. struct Proxy { + + enum ProxyType: Hashable { + case http + case socks + } + /// Specifies Proxy server host. public var host: String /// Specifies Proxy server port. @@ -34,13 +41,15 @@ public extension HTTPClient.Configuration { /// Specifies Proxy server authorization. public var authorization: HTTPClient.Authorization? + var type: ProxyType + /// Create proxy. /// /// - parameters: /// - host: proxy server host. /// - port: proxy server port. public static func server(host: String, port: Int) -> Proxy { - return .init(host: host, port: port, authorization: nil) + return .init(host: host, port: port, authorization: nil, type: .http) } /// Create proxy. @@ -50,7 +59,11 @@ public extension HTTPClient.Configuration { /// - port: proxy server port. /// - authorization: proxy server authorization. public static func server(host: String, port: Int, authorization: HTTPClient.Authorization? = nil) -> Proxy { - return .init(host: host, port: port, authorization: authorization) + return .init(host: host, port: port, authorization: authorization, type: .http) + } + + public static func socksServer(host: String, port: Int = 1080, httpAuthorization: HTTPClient.Authorization? = nil) -> Proxy { + return .init(host: host, port: port, authorization: httpAuthorization, type: .socks) } } } diff --git a/Sources/AsyncHTTPClient/Utils.swift b/Sources/AsyncHTTPClient/Utils.swift index 6069222b1..e048f9ec4 100644 --- a/Sources/AsyncHTTPClient/Utils.swift +++ b/Sources/AsyncHTTPClient/Utils.swift @@ -148,9 +148,14 @@ extension NIOClientTCPBootstrap { return bootstrap.channelInitializer { channel in do { if let proxy = configuration.proxy { - try channel.pipeline.syncAddProxyHandler(host: host, - port: port, - authorization: proxy.authorization) + switch proxy.type { + case .http: + try channel.pipeline.syncAddProxyHandler(host: host, + port: port, + authorization: proxy.authorization) + case .socks: + try channel.pipeline.syncAddSOCKSProxyHandler() + } } else if requiresTLS { // We only add the handshake verifier if we need TLS and we're not going through a proxy. // If we're going through a proxy we add it later (outside of this method). From b45f4d9a75c92a385c58a3b0de91038424ca3893 Mon Sep 17 00:00:00 2001 From: David Evans Date: Wed, 16 Jun 2021 15:23:29 +0100 Subject: [PATCH 02/25] Remove auth field --- Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift b/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift index c711ecc8b..e08253b8d 100644 --- a/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift @@ -62,8 +62,8 @@ public extension HTTPClient.Configuration { return .init(host: host, port: port, authorization: authorization, type: .http) } - public static func socksServer(host: String, port: Int = 1080, httpAuthorization: HTTPClient.Authorization? = nil) -> Proxy { - return .init(host: host, port: port, authorization: httpAuthorization, type: .socks) + public static func socksServer(host: String, port: Int = 1080) -> Proxy { + return .init(host: host, port: port, authorization: nil, type: .socks) } } } From 8e1232af8da639495b2ad1368cd5b421f4f268d9 Mon Sep 17 00:00:00 2001 From: David Evans Date: Wed, 16 Jun 2021 15:26:56 +0100 Subject: [PATCH 03/25] Remove test addresses --- Sources/AsyncHTTPClient/HTTPClient.swift | 4 ++-- Sources/AsyncHTTPClient/Utils.swift | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index 1d8ebcbf4..37c84c80a 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -901,8 +901,8 @@ extension ChannelPipeline { try sync.addHandler(handler) } - func syncAddSOCKSProxyHandler() throws { - let address = try SocketAddress(ipAddress: "127.0.0.1", port: 12345) + func syncAddSOCKSProxyHandler(host: String, port: Int) throws { + let address = try SocketAddress(ipAddress: host, port: port) let handler = SOCKSClientHandler(targetAddress: .address(address)) let sync = self.syncOperations try sync.addHandler(handler) diff --git a/Sources/AsyncHTTPClient/Utils.swift b/Sources/AsyncHTTPClient/Utils.swift index e048f9ec4..85e7c3721 100644 --- a/Sources/AsyncHTTPClient/Utils.swift +++ b/Sources/AsyncHTTPClient/Utils.swift @@ -154,7 +154,7 @@ extension NIOClientTCPBootstrap { port: port, authorization: proxy.authorization) case .socks: - try channel.pipeline.syncAddSOCKSProxyHandler() + try channel.pipeline.syncAddSOCKSProxyHandler(host: host, port: port) } } else if requiresTLS { // We only add the handshake verifier if we need TLS and we're not going through a proxy. From 974677fd949eb82bfc1d10e9af146db0acaf2a3a Mon Sep 17 00:00:00 2001 From: David Evans Date: Wed, 16 Jun 2021 19:56:57 +0100 Subject: [PATCH 04/25] Working tests --- .../HTTPClientTests.swift | 18 +++ .../AsyncHTTPClientTests/SOCKSTestUtils.swift | 111 ++++++++++++++++++ 2 files changed, 129 insertions(+) create mode 100644 Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 3e313088d..8a8f0674c 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -26,6 +26,7 @@ import NIOSSL import NIOTestUtils import NIOTransportServices import XCTest +import NIOSOCKS class HTTPClientTests: XCTestCase { typealias Request = HTTPClient.Request @@ -708,6 +709,23 @@ class HTTPClientTests: XCTestCase { } } } + + func testProxySOCKS() throws { + let socksBin = try MockSocksServer(expectedURL: "/socks/test", expectedResponse: "it works!") + let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), + configuration: .init(proxy: .socksServer(host: "127.0.0.1"))) + + do { + let response = try localClient.get(url: "http://127.0.0.1/socks/test").wait() + XCTAssertEqual(.ok, response.status) + XCTAssertEqual(ByteBuffer(string: "it works!"), response.body) + } catch { + XCTFail("\(error)") + } + + XCTAssertNoThrow(try localClient.syncShutdown()) + XCTAssertNoThrow(try socksBin.shutdown()) + } func testUploadStreaming() throws { let body: HTTPClient.Body = .stream(length: 8) { writer in diff --git a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift new file mode 100644 index 000000000..231499a9f --- /dev/null +++ b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift @@ -0,0 +1,111 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the AsyncHTTPClient open source project +// +// Copyright (c) 2020 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 +// +//===----------------------------------------------------------------------===// + +@testable import AsyncHTTPClient +import NIO +import NIOHTTP1 +import NIOSOCKS +import XCTest + +class MockSocksServer { + + let channel: Channel + + public init(expectedURL: String, expectedResponse: String, file: String = (#file), line: UInt = #line) throws { + let elg = MultiThreadedEventLoopGroup(numberOfThreads: 1) + let bootstrap = ServerBootstrap.init(group: elg) + .serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1) + .childChannelInitializer { channel in + let handshakeHandler = SOCKSServerHandshakeHandler() + return channel.pipeline.addHandlers([ + handshakeHandler, + SOCKSTestHandler(handshakeHandler: handshakeHandler), + SOCKSTestHTTPClient(expectedURL: expectedURL, expectedResponse: expectedResponse, file: file, line: line) + ]) + } + self.channel = try bootstrap.bind(host: "127.0.0.1", port: 1080).wait() + } + + func shutdown() throws { + try self.channel.close().wait() + } + +} + +class SOCKSTestHTTPClient: ChannelInboundHandler { + + typealias InboundIn = HTTPServerRequestPart + typealias OutboundOut = HTTPServerResponsePart + + let expectedURL: String + let expectedResponse: String + let file: String + let line: UInt + + init(expectedURL: String, expectedResponse: String, file: String, line: UInt) { + self.expectedURL = expectedURL + self.expectedResponse = expectedResponse + self.file = file + self.line = line + } + + func channelRead(context: ChannelHandlerContext, data: NIOAny) { + let message = self.unwrapInboundIn(data) + switch message { + case .head(let head): + XCTAssertEqual(head.uri, self.expectedURL) + case .body: + break + case .end: + context.write(self.wrapOutboundOut(.head(.init(version: .http1_1, status: .ok))), promise: nil) + context.write(self.wrapOutboundOut(.body(.byteBuffer(context.channel.allocator.buffer(string: self.expectedResponse)))), promise: nil) + context.writeAndFlush(self.wrapOutboundOut(.end(nil)), promise: nil) + } + } +} + +class SOCKSTestHandler: ChannelInboundHandler, RemovableChannelHandler { + + typealias InboundIn = ClientMessage + + let handshakeHandler: SOCKSServerHandshakeHandler + + init(handshakeHandler: SOCKSServerHandshakeHandler) { + self.handshakeHandler = handshakeHandler + } + + func channelRead(context: ChannelHandlerContext, data: NIOAny) { + let message = self.unwrapInboundIn(data) + switch message { + case .greeting: + context.writeAndFlush(.init( + ServerMessage.selectedAuthenticationMethod(.init(method: .noneRequired))), promise: nil) + context.writeAndFlush(.init( + ServerMessage.authenticationData(context.channel.allocator.buffer(capacity: 0), complete: true)), promise: nil) + case .authenticationData: + break + case .request(let request): + context.writeAndFlush(.init( + ServerMessage.response(.init(reply: .succeeded, boundAddress: request.addressType))), promise: nil) + context.channel.pipeline.addHandlers([ + ByteToMessageHandler(HTTPRequestDecoder()), + HTTPResponseEncoder(), + ], position: .after(self)).whenSuccess { + context.channel.pipeline.removeHandler(self, promise: nil) + context.channel.pipeline.removeHandler(self.handshakeHandler, promise: nil) + } + } + } + +} From 122f6504c8bc4cf5e6eb45b82835946b9e3632e4 Mon Sep 17 00:00:00 2001 From: David Evans Date: Wed, 16 Jun 2021 20:00:50 +0100 Subject: [PATCH 05/25] Update packages --- Package.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Package.swift b/Package.swift index 8d4367d68..af4fc7b0b 100644 --- a/Package.swift +++ b/Package.swift @@ -21,9 +21,9 @@ let package = Package( .library(name: "AsyncHTTPClient", targets: ["AsyncHTTPClient"]), ], dependencies: [ - .package(url: "https://github.com/apple/swift-nio.git", from: "2.27.0"), + .package(url: "https://github.com/apple/swift-nio.git", from: "2.29.0"), .package(url: "https://github.com/apple/swift-nio-ssl.git", from: "2.13.0"), - .package(url: "https://github.com/apple/swift-nio-extras.git", from: "1.3.0"), + .package(url: "https://github.com/apple/swift-nio-extras.git", from: "1.9.0"), .package(url: "https://github.com/apple/swift-nio-transport-services.git", from: "1.5.1"), .package(url: "https://github.com/apple/swift-log.git", from: "1.4.0"), ], From 481e1b11b49edfb4242d039c67ff8b68cdbf0a19 Mon Sep 17 00:00:00 2001 From: David Evans Date: Thu, 17 Jun 2021 11:21:56 +0100 Subject: [PATCH 06/25] Rename to MockSOCKSServer --- Tests/AsyncHTTPClientTests/HTTPClientTests.swift | 2 +- Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 9f9617496..00ba43c9c 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -711,7 +711,7 @@ class HTTPClientTests: XCTestCase { } func testProxySOCKS() throws { - let socksBin = try MockSocksServer(expectedURL: "/socks/test", expectedResponse: "it works!") + let socksBin = try MockSOCKSServer(expectedURL: "/socks/test", expectedResponse: "it works!") let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), configuration: .init(proxy: .socksServer(host: "127.0.0.1"))) diff --git a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift index 231499a9f..f9fa52b9f 100644 --- a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift +++ b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift @@ -18,7 +18,7 @@ import NIOHTTP1 import NIOSOCKS import XCTest -class MockSocksServer { +class MockSOCKSServer { let channel: Channel From 064edcb71a133628444140cd4f1e1375a9bf9d66 Mon Sep 17 00:00:00 2001 From: David Evans Date: Thu, 17 Jun 2021 11:42:51 +0100 Subject: [PATCH 07/25] Code review cleanup --- .../HTTPClientProxyHandler.swift | 6 +++++- .../AsyncHTTPClientTests/SOCKSTestUtils.swift | 18 +++++++++--------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift b/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift index e08253b8d..6589ab641 100644 --- a/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift @@ -38,7 +38,7 @@ public extension HTTPClient.Configuration { public var host: String /// Specifies Proxy server port. public var port: Int - /// Specifies Proxy server authorization. + /// Specifies Proxy server authorization. Ignored if the proxy is SOCKSv5. public var authorization: HTTPClient.Authorization? var type: ProxyType @@ -62,6 +62,10 @@ public extension HTTPClient.Configuration { return .init(host: host, port: port, authorization: authorization, type: .http) } + /// Create a SOCKSv5 proxy. + /// - parameter host: The SOCKSv5 proxy address. + /// - parameter port: The SOCKSv5 proxy port, defaults to 1080. + /// - returns: A new instance of `Proxy` configured to connect to a `SOCKSv5` server. public static func socksServer(host: String, port: Int = 1080) -> Proxy { return .init(host: host, port: port, authorization: nil, type: .socks) } diff --git a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift index f9fa52b9f..1453a9bef 100644 --- a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift +++ b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift @@ -2,7 +2,7 @@ // // This source file is part of the AsyncHTTPClient open source project // -// Copyright (c) 2020 Apple Inc. and the AsyncHTTPClient project authors +// Copyright (c) 2021 Apple Inc. and the AsyncHTTPClient project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -12,7 +12,7 @@ // //===----------------------------------------------------------------------===// -@testable import AsyncHTTPClient +import AsyncHTTPClient import NIO import NIOHTTP1 import NIOSOCKS @@ -27,13 +27,13 @@ class MockSOCKSServer { let bootstrap = ServerBootstrap.init(group: elg) .serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1) .childChannelInitializer { channel in - let handshakeHandler = SOCKSServerHandshakeHandler() - return channel.pipeline.addHandlers([ - handshakeHandler, - SOCKSTestHandler(handshakeHandler: handshakeHandler), - SOCKSTestHTTPClient(expectedURL: expectedURL, expectedResponse: expectedResponse, file: file, line: line) - ]) - } + let handshakeHandler = SOCKSServerHandshakeHandler() + return channel.pipeline.addHandlers([ + handshakeHandler, + SOCKSTestHandler(handshakeHandler: handshakeHandler), + SOCKSTestHTTPClient(expectedURL: expectedURL, expectedResponse: expectedResponse, file: file, line: line) + ]) + } self.channel = try bootstrap.bind(host: "127.0.0.1", port: 1080).wait() } From 210c4d70d6cfe6bc56232755cdb547453558a9e5 Mon Sep 17 00:00:00 2001 From: David Evans Date: Thu, 17 Jun 2021 11:47:57 +0100 Subject: [PATCH 08/25] Cleanup public authorization --- .../HTTPClientProxyHandler.swift | 30 ++++++++++++++----- Sources/AsyncHTTPClient/HTTPHandler.swift | 4 +-- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift b/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift index 6589ab641..30a77d067 100644 --- a/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift @@ -16,7 +16,7 @@ import NIO import NIOHTTP1 import NIOSOCKS -public extension HTTPClient.Configuration { +extension HTTPClient.Configuration { /// Proxy server configuration /// Specifies the remote address of an HTTP proxy. /// @@ -27,10 +27,10 @@ public extension HTTPClient.Configuration { /// If a `TLSConfiguration` is used in conjunction with `HTTPClient.Configuration.Proxy`, /// TLS will be established _after_ successful proxy, between your client /// and the destination server. - struct Proxy { + public struct Proxy { enum ProxyType: Hashable { - case http + case http(HTTPClient.Authorization?) case socks } @@ -38,8 +38,22 @@ public extension HTTPClient.Configuration { public var host: String /// Specifies Proxy server port. public var port: Int - /// Specifies Proxy server authorization. Ignored if the proxy is SOCKSv5. - public var authorization: HTTPClient.Authorization? + /// Specifies Proxy server authorization. + public var authorization: HTTPClient.Authorization? { + set { + precondition(self.type == .http(self.authorization), "Only HTTP proxies can have authorization set.") + self.type = .http(newValue) + } + + get { + switch self.type { + case .http(let authorization): + return authorization + case .socks: + return nil + } + } + } var type: ProxyType @@ -49,7 +63,7 @@ public extension HTTPClient.Configuration { /// - host: proxy server host. /// - port: proxy server port. public static func server(host: String, port: Int) -> Proxy { - return .init(host: host, port: port, authorization: nil, type: .http) + return .init(host: host, port: port, type: .http(nil)) } /// Create proxy. @@ -59,7 +73,7 @@ public extension HTTPClient.Configuration { /// - port: proxy server port. /// - authorization: proxy server authorization. public static func server(host: String, port: Int, authorization: HTTPClient.Authorization? = nil) -> Proxy { - return .init(host: host, port: port, authorization: authorization, type: .http) + return .init(host: host, port: port, type: .http(authorization)) } /// Create a SOCKSv5 proxy. @@ -67,7 +81,7 @@ public extension HTTPClient.Configuration { /// - parameter port: The SOCKSv5 proxy port, defaults to 1080. /// - returns: A new instance of `Proxy` configured to connect to a `SOCKSv5` server. public static func socksServer(host: String, port: Int = 1080) -> Proxy { - return .init(host: host, port: port, authorization: nil, type: .socks) + return .init(host: host, port: port, type: .socks) } } } diff --git a/Sources/AsyncHTTPClient/HTTPHandler.swift b/Sources/AsyncHTTPClient/HTTPHandler.swift index 4850c51d8..a9c1a9e22 100644 --- a/Sources/AsyncHTTPClient/HTTPHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPHandler.swift @@ -342,8 +342,8 @@ extension HTTPClient { } /// HTTP authentication - public struct Authorization { - private enum Scheme { + public struct Authorization: Hashable { + private enum Scheme: Hashable { case Basic(String) case Bearer(String) } From 18c7103e415cbe973380bfdd020d7a76920908a5 Mon Sep 17 00:00:00 2001 From: David Evans Date: Thu, 17 Jun 2021 11:51:59 +0100 Subject: [PATCH 09/25] Throw error on auth data --- Tests/AsyncHTTPClientTests/HTTPClientTests.swift | 8 +++++--- Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift | 8 ++++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 00ba43c9c..b3abac155 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -715,6 +715,11 @@ class HTTPClientTests: XCTestCase { let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), configuration: .init(proxy: .socksServer(host: "127.0.0.1"))) + defer { + XCTAssertNoThrow(try localClient.syncShutdown()) + XCTAssertNoThrow(try socksBin.shutdown()) + } + do { let response = try localClient.get(url: "http://127.0.0.1/socks/test").wait() XCTAssertEqual(.ok, response.status) @@ -722,9 +727,6 @@ class HTTPClientTests: XCTestCase { } catch { XCTFail("\(error)") } - - XCTAssertNoThrow(try localClient.syncShutdown()) - XCTAssertNoThrow(try socksBin.shutdown()) } func testUploadStreaming() throws { diff --git a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift index 1453a9bef..04865cd32 100644 --- a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift +++ b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift @@ -18,6 +18,10 @@ import NIOHTTP1 import NIOSOCKS import XCTest +struct MockSOCKSError: Error, Hashable { + var description: String +} + class MockSOCKSServer { let channel: Channel @@ -89,12 +93,12 @@ class SOCKSTestHandler: ChannelInboundHandler, RemovableChannelHandler { let message = self.unwrapInboundIn(data) switch message { case .greeting: - context.writeAndFlush(.init( + context.write(.init( ServerMessage.selectedAuthenticationMethod(.init(method: .noneRequired))), promise: nil) context.writeAndFlush(.init( ServerMessage.authenticationData(context.channel.allocator.buffer(capacity: 0), complete: true)), promise: nil) case .authenticationData: - break + context.fireErrorCaught(MockSOCKSError(description: "Received authentication data but didn't receive any.")) case .request(let request): context.writeAndFlush(.init( ServerMessage.response(.init(reply: .succeeded, boundAddress: request.addressType))), promise: nil) From 323eefdde935098182cbb5d58f396075cbf4fb3d Mon Sep 17 00:00:00 2001 From: David Evans Date: Thu, 17 Jun 2021 14:23:28 +0100 Subject: [PATCH 10/25] Add missing socks server test --- Tests/AsyncHTTPClientTests/HTTPClientTests.swift | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index b3abac155..93e79cab9 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -728,6 +728,16 @@ class HTTPClientTests: XCTestCase { XCTFail("\(error)") } } + + // there is no socks server, so we should fail + func testProxySOCKSFailureInvalidServer() throws { + let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), + configuration: .init(proxy: .socksServer(host: "127.0.0.1"))) + defer { + XCTAssertNoThrow(try localClient.syncShutdown()) + } + XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) + } func testUploadStreaming() throws { let body: HTTPClient.Body = .stream(length: 8) { writer in From c2738476cb8c44fb2413c2974f90c9bf8a7ce68d Mon Sep 17 00:00:00 2001 From: David Evans Date: Thu, 17 Jun 2021 14:24:45 +0100 Subject: [PATCH 11/25] Fabians test cleanup --- Tests/AsyncHTTPClientTests/HTTPClientTests.swift | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 93e79cab9..ce2dafc5d 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -720,13 +720,10 @@ class HTTPClientTests: XCTestCase { XCTAssertNoThrow(try socksBin.shutdown()) } - do { - let response = try localClient.get(url: "http://127.0.0.1/socks/test").wait() - XCTAssertEqual(.ok, response.status) - XCTAssertEqual(ByteBuffer(string: "it works!"), response.body) - } catch { - XCTFail("\(error)") - } + var response: HTTPClient.Response? = nil + XCTAssertNoThrow(response = try localClient.get(url: "http://127.0.0.1/socks/test").wait()) + XCTAssertEqual(.ok, response?.status) + XCTAssertEqual(ByteBuffer(string: "it works!"), response?.body) } // there is no socks server, so we should fail From 3c0e120c1c41f90c089a0f150471ebb6dd8d95a7 Mon Sep 17 00:00:00 2001 From: David Evans Date: Thu, 17 Jun 2021 14:25:19 +0100 Subject: [PATCH 12/25] Remove redundant auth complete write --- Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift index 04865cd32..32705c624 100644 --- a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift +++ b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift @@ -93,10 +93,8 @@ class SOCKSTestHandler: ChannelInboundHandler, RemovableChannelHandler { let message = self.unwrapInboundIn(data) switch message { case .greeting: - context.write(.init( - ServerMessage.selectedAuthenticationMethod(.init(method: .noneRequired))), promise: nil) context.writeAndFlush(.init( - ServerMessage.authenticationData(context.channel.allocator.buffer(capacity: 0), complete: true)), promise: nil) + ServerMessage.selectedAuthenticationMethod(.init(method: .noneRequired))), promise: nil) case .authenticationData: context.fireErrorCaught(MockSOCKSError(description: "Received authentication data but didn't receive any.")) case .request(let request): From ba629a43324c9d9b4663b11f25bbddb666ebb6e9 Mon Sep 17 00:00:00 2001 From: David Evans Date: Thu, 17 Jun 2021 14:27:57 +0100 Subject: [PATCH 13/25] Add test for speaking to wrong server type --- Tests/AsyncHTTPClientTests/HTTPClientTests.swift | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index ce2dafc5d..4d9abb7f6 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -727,6 +727,18 @@ class HTTPClientTests: XCTestCase { } // there is no socks server, so we should fail + func testProxySOCKSFailureNoServer() throws { + let localHTTPBin = HTTPBin() + let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), + configuration: .init(proxy: .socksServer(host: "127.0.0.1", port: localHTTPBin.port))) + defer { + XCTAssertNoThrow(try localClient.syncShutdown()) + XCTAssertNoThrow(try localHTTPBin.shutdown()) + } + XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) + } + + // speak to a server that doesn't speak SOCKS func testProxySOCKSFailureInvalidServer() throws { let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), configuration: .init(proxy: .socksServer(host: "127.0.0.1"))) From e4c1b52b7927fb4bb245355844c4dd69e2e1f0df Mon Sep 17 00:00:00 2001 From: David Evans Date: Thu, 17 Jun 2021 15:11:56 +0100 Subject: [PATCH 14/25] Add misbehaving server test --- .../HTTPClientTests.swift | 15 +++++++++++++ .../AsyncHTTPClientTests/SOCKSTestUtils.swift | 22 ++++++++++++++++--- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 4d9abb7f6..e69a0c300 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -747,6 +747,21 @@ class HTTPClientTests: XCTestCase { } XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) } + + // test a handshake failure with a misbehaving server + func testProxySOCKSMisbehavingServer() throws { + let socksBin = try MockSOCKSServer(expectedURL: "/socks/test", expectedResponse: "it works!", misbehave: true) + let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), + configuration: .init(proxy: .socksServer(host: "127.0.0.1"))) + + defer { + XCTAssertNoThrow(try localClient.syncShutdown()) + XCTAssertNoThrow(try socksBin.shutdown()) + } + + // the server will send a bogus message in response to the clients request + XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) + } func testUploadStreaming() throws { let body: HTTPClient.Body = .stream(length: 8) { writer in diff --git a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift index 32705c624..07d0d0633 100644 --- a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift +++ b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift @@ -26,7 +26,7 @@ class MockSOCKSServer { let channel: Channel - public init(expectedURL: String, expectedResponse: String, file: String = (#file), line: UInt = #line) throws { + public init(expectedURL: String, expectedResponse: String, misbehave: Bool = false, file: String = (#file), line: UInt = #line) throws { let elg = MultiThreadedEventLoopGroup(numberOfThreads: 1) let bootstrap = ServerBootstrap.init(group: elg) .serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1) @@ -34,7 +34,7 @@ class MockSOCKSServer { let handshakeHandler = SOCKSServerHandshakeHandler() return channel.pipeline.addHandlers([ handshakeHandler, - SOCKSTestHandler(handshakeHandler: handshakeHandler), + SOCKSTestHandler(handshakeHandler: handshakeHandler, misbehave: misbehave), SOCKSTestHTTPClient(expectedURL: expectedURL, expectedResponse: expectedResponse, file: file, line: line) ]) } @@ -77,6 +77,11 @@ class SOCKSTestHTTPClient: ChannelInboundHandler { context.writeAndFlush(self.wrapOutboundOut(.end(nil)), promise: nil) } } + + func errorCaught(context: ChannelHandlerContext, error: Error) { + context.fireErrorCaught(error) + context.close(promise: nil) + } } class SOCKSTestHandler: ChannelInboundHandler, RemovableChannelHandler { @@ -84,12 +89,18 @@ class SOCKSTestHandler: ChannelInboundHandler, RemovableChannelHandler { typealias InboundIn = ClientMessage let handshakeHandler: SOCKSServerHandshakeHandler + let misbehave: Bool - init(handshakeHandler: SOCKSServerHandshakeHandler) { + init(handshakeHandler: SOCKSServerHandshakeHandler, misbehave: Bool) { self.handshakeHandler = handshakeHandler + self.misbehave = misbehave } func channelRead(context: ChannelHandlerContext, data: NIOAny) { + guard context.channel.isActive else { + return + } + let message = self.unwrapInboundIn(data) switch message { case .greeting: @@ -98,6 +109,11 @@ class SOCKSTestHandler: ChannelInboundHandler, RemovableChannelHandler { case .authenticationData: context.fireErrorCaught(MockSOCKSError(description: "Received authentication data but didn't receive any.")) case .request(let request): + guard !misbehave else { + context.writeAndFlush( + .init(ServerMessage.authenticationData(context.channel.allocator.buffer(string: "bad server!"), complete: true)), promise: nil) + return + } context.writeAndFlush(.init( ServerMessage.response(.init(reply: .succeeded, boundAddress: request.addressType))), promise: nil) context.channel.pipeline.addHandlers([ From d71829957ec92e4797e973004781712d83b84235 Mon Sep 17 00:00:00 2001 From: David Evans Date: Thu, 17 Jun 2021 15:13:28 +0100 Subject: [PATCH 15/25] Guard against multiple requests --- Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift index 07d0d0633..1c1293c17 100644 --- a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift +++ b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift @@ -56,6 +56,7 @@ class SOCKSTestHTTPClient: ChannelInboundHandler { let expectedResponse: String let file: String let line: UInt + var requestCount = 0 init(expectedURL: String, expectedResponse: String, file: String, line: UInt) { self.expectedURL = expectedURL @@ -68,7 +69,11 @@ class SOCKSTestHTTPClient: ChannelInboundHandler { let message = self.unwrapInboundIn(data) switch message { case .head(let head): + guard self.requestCount == 0 else { + return + } XCTAssertEqual(head.uri, self.expectedURL) + self.requestCount += 1 case .body: break case .end: From eee59e326cde6fc7c9271af1b8d37bd31309a4a7 Mon Sep 17 00:00:00 2001 From: David Evans Date: Thu, 17 Jun 2021 15:48:37 +0100 Subject: [PATCH 16/25] Soundness --- Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift index 98bfb0b54..fd408f45f 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift @@ -58,6 +58,10 @@ extension HTTPClientTests { ("testProxyTLS", testProxyTLS), ("testProxyPlaintextWithCorrectlyAuthorization", testProxyPlaintextWithCorrectlyAuthorization), ("testProxyPlaintextWithIncorrectlyAuthorization", testProxyPlaintextWithIncorrectlyAuthorization), + ("testProxySOCKS", testProxySOCKS), + ("testProxySOCKSFailureNoServer", testProxySOCKSFailureNoServer), + ("testProxySOCKSFailureInvalidServer", testProxySOCKSFailureInvalidServer), + ("testProxySOCKSMisbehavingServer", testProxySOCKSMisbehavingServer), ("testUploadStreaming", testUploadStreaming), ("testNoContentLengthForSSLUncleanShutdown", testNoContentLengthForSSLUncleanShutdown), ("testNoContentLengthWithIgnoreErrorForSSLUncleanShutdown", testNoContentLengthWithIgnoreErrorForSSLUncleanShutdown), From 112d5b1cb476585f63e70891a1249a9f9879d3fa Mon Sep 17 00:00:00 2001 From: David Evans Date: Thu, 17 Jun 2021 16:03:13 +0100 Subject: [PATCH 17/25] Soundness --- Sources/AsyncHTTPClient/HTTPClient.swift | 4 +-- .../HTTPClientProxyHandler.swift | 9 +++-- .../HTTPClientTests.swift | 20 +++++------ .../AsyncHTTPClientTests/SOCKSTestUtils.swift | 36 +++++++++---------- 4 files changed, 32 insertions(+), 37 deletions(-) diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index 37c84c80a..be1f86ffd 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -18,10 +18,10 @@ import NIO import NIOConcurrencyHelpers import NIOHTTP1 import NIOHTTPCompression +import NIOSOCKS import NIOSSL import NIOTLS import NIOTransportServices -import NIOSOCKS extension Logger { private func requestInfo(_ request: HTTPClient.Request) -> Logger.Metadata.Value { @@ -900,7 +900,7 @@ extension ChannelPipeline { try sync.addHandler(decoder) try sync.addHandler(handler) } - + func syncAddSOCKSProxyHandler(host: String, port: Int) throws { let address = try SocketAddress(ipAddress: host, port: port) let handler = SOCKSClientHandler(targetAddress: .address(address)) diff --git a/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift b/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift index 30a77d067..a97fb3c70 100644 --- a/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift @@ -28,12 +28,11 @@ extension HTTPClient.Configuration { /// TLS will be established _after_ successful proxy, between your client /// and the destination server. public struct Proxy { - enum ProxyType: Hashable { case http(HTTPClient.Authorization?) case socks } - + /// Specifies Proxy server host. public var host: String /// Specifies Proxy server port. @@ -44,7 +43,7 @@ extension HTTPClient.Configuration { precondition(self.type == .http(self.authorization), "Only HTTP proxies can have authorization set.") self.type = .http(newValue) } - + get { switch self.type { case .http(let authorization): @@ -56,7 +55,7 @@ extension HTTPClient.Configuration { } var type: ProxyType - + /// Create proxy. /// /// - parameters: @@ -75,7 +74,7 @@ extension HTTPClient.Configuration { public static func server(host: String, port: Int, authorization: HTTPClient.Authorization? = nil) -> Proxy { return .init(host: host, port: port, type: .http(authorization)) } - + /// Create a SOCKSv5 proxy. /// - parameter host: The SOCKSv5 proxy address. /// - parameter port: The SOCKSv5 proxy port, defaults to 1080. diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index e69a0c300..4bc6ee55f 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -22,11 +22,11 @@ import NIOConcurrencyHelpers import NIOFoundationCompat import NIOHTTP1 import NIOHTTPCompression +import NIOSOCKS import NIOSSL import NIOTestUtils import NIOTransportServices import XCTest -import NIOSOCKS class HTTPClientTests: XCTestCase { typealias Request = HTTPClient.Request @@ -709,23 +709,23 @@ class HTTPClientTests: XCTestCase { } } } - + func testProxySOCKS() throws { let socksBin = try MockSOCKSServer(expectedURL: "/socks/test", expectedResponse: "it works!") let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), configuration: .init(proxy: .socksServer(host: "127.0.0.1"))) - + defer { XCTAssertNoThrow(try localClient.syncShutdown()) XCTAssertNoThrow(try socksBin.shutdown()) } - - var response: HTTPClient.Response? = nil + + var response: HTTPClient.Response? XCTAssertNoThrow(response = try localClient.get(url: "http://127.0.0.1/socks/test").wait()) XCTAssertEqual(.ok, response?.status) XCTAssertEqual(ByteBuffer(string: "it works!"), response?.body) } - + // there is no socks server, so we should fail func testProxySOCKSFailureNoServer() throws { let localHTTPBin = HTTPBin() @@ -737,7 +737,7 @@ class HTTPClientTests: XCTestCase { } XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) } - + // speak to a server that doesn't speak SOCKS func testProxySOCKSFailureInvalidServer() throws { let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), @@ -747,18 +747,18 @@ class HTTPClientTests: XCTestCase { } XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) } - + // test a handshake failure with a misbehaving server func testProxySOCKSMisbehavingServer() throws { let socksBin = try MockSOCKSServer(expectedURL: "/socks/test", expectedResponse: "it works!", misbehave: true) let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), configuration: .init(proxy: .socksServer(host: "127.0.0.1"))) - + defer { XCTAssertNoThrow(try localClient.syncShutdown()) XCTAssertNoThrow(try socksBin.shutdown()) } - + // the server will send a bogus message in response to the clients request XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) } diff --git a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift index 1c1293c17..ae169e113 100644 --- a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift +++ b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift @@ -23,48 +23,45 @@ struct MockSOCKSError: Error, Hashable { } class MockSOCKSServer { - let channel: Channel - - public init(expectedURL: String, expectedResponse: String, misbehave: Bool = false, file: String = (#file), line: UInt = #line) throws { + + public init(expectedURL: String, expectedResponse: String, misbehave: Bool = false, file: String = #file, line: UInt = #line) throws { let elg = MultiThreadedEventLoopGroup(numberOfThreads: 1) - let bootstrap = ServerBootstrap.init(group: elg) + let bootstrap = ServerBootstrap(group: elg) .serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1) .childChannelInitializer { channel in let handshakeHandler = SOCKSServerHandshakeHandler() return channel.pipeline.addHandlers([ handshakeHandler, SOCKSTestHandler(handshakeHandler: handshakeHandler, misbehave: misbehave), - SOCKSTestHTTPClient(expectedURL: expectedURL, expectedResponse: expectedResponse, file: file, line: line) + SOCKSTestHTTPClient(expectedURL: expectedURL, expectedResponse: expectedResponse, file: file, line: line), ]) } self.channel = try bootstrap.bind(host: "127.0.0.1", port: 1080).wait() } - + func shutdown() throws { try self.channel.close().wait() } - } class SOCKSTestHTTPClient: ChannelInboundHandler { - typealias InboundIn = HTTPServerRequestPart typealias OutboundOut = HTTPServerResponsePart - + let expectedURL: String let expectedResponse: String let file: String let line: UInt var requestCount = 0 - + init(expectedURL: String, expectedResponse: String, file: String, line: UInt) { self.expectedURL = expectedURL self.expectedResponse = expectedResponse self.file = file self.line = line } - + func channelRead(context: ChannelHandlerContext, data: NIOAny) { let message = self.unwrapInboundIn(data) switch message { @@ -82,7 +79,7 @@ class SOCKSTestHTTPClient: ChannelInboundHandler { context.writeAndFlush(self.wrapOutboundOut(.end(nil)), promise: nil) } } - + func errorCaught(context: ChannelHandlerContext, error: Error) { context.fireErrorCaught(error) context.close(promise: nil) @@ -90,22 +87,21 @@ class SOCKSTestHTTPClient: ChannelInboundHandler { } class SOCKSTestHandler: ChannelInboundHandler, RemovableChannelHandler { - typealias InboundIn = ClientMessage - + let handshakeHandler: SOCKSServerHandshakeHandler let misbehave: Bool - + init(handshakeHandler: SOCKSServerHandshakeHandler, misbehave: Bool) { self.handshakeHandler = handshakeHandler self.misbehave = misbehave } - + func channelRead(context: ChannelHandlerContext, data: NIOAny) { guard context.channel.isActive else { return } - + let message = self.unwrapInboundIn(data) switch message { case .greeting: @@ -114,9 +110,10 @@ class SOCKSTestHandler: ChannelInboundHandler, RemovableChannelHandler { case .authenticationData: context.fireErrorCaught(MockSOCKSError(description: "Received authentication data but didn't receive any.")) case .request(let request): - guard !misbehave else { + guard !self.misbehave else { context.writeAndFlush( - .init(ServerMessage.authenticationData(context.channel.allocator.buffer(string: "bad server!"), complete: true)), promise: nil) + .init(ServerMessage.authenticationData(context.channel.allocator.buffer(string: "bad server!"), complete: true)), promise: nil + ) return } context.writeAndFlush(.init( @@ -130,5 +127,4 @@ class SOCKSTestHandler: ChannelInboundHandler, RemovableChannelHandler { } } } - } From 976fe2e1c46b5fab4cca1ff9154e4acb136487f0 Mon Sep 17 00:00:00 2001 From: David Evans Date: Thu, 17 Jun 2021 17:04:10 +0100 Subject: [PATCH 18/25] Update Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift Co-authored-by: Cory Benfield --- Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift b/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift index a97fb3c70..80e13dcf4 100644 --- a/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift @@ -40,7 +40,7 @@ extension HTTPClient.Configuration { /// Specifies Proxy server authorization. public var authorization: HTTPClient.Authorization? { set { - precondition(self.type == .http(self.authorization), "Only HTTP proxies can have authorization set.") + precondition(self.type == .http(self.authorization), "SOCKS authorization support is not yet implemented.") self.type = .http(newValue) } From ea04cfab43df9c074b61492f541b50d7f1f6491e Mon Sep 17 00:00:00 2001 From: David Evans Date: Thu, 17 Jun 2021 17:09:06 +0100 Subject: [PATCH 19/25] Support bogus addresses --- Sources/AsyncHTTPClient/HTTPClient.swift | 2 +- Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift | 2 +- Tests/AsyncHTTPClientTests/HTTPClientTests.swift | 10 ++++++++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index be1f86ffd..299d167e9 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -902,7 +902,7 @@ extension ChannelPipeline { } func syncAddSOCKSProxyHandler(host: String, port: Int) throws { - let address = try SocketAddress(ipAddress: host, port: port) + let address = try SocketAddress.makeAddressResolvingHost(host, port: port) let handler = SOCKSClientHandler(targetAddress: .address(address)) let sync = self.syncOperations try sync.addHandler(handler) diff --git a/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift b/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift index a97fb3c70..80e13dcf4 100644 --- a/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift @@ -40,7 +40,7 @@ extension HTTPClient.Configuration { /// Specifies Proxy server authorization. public var authorization: HTTPClient.Authorization? { set { - precondition(self.type == .http(self.authorization), "Only HTTP proxies can have authorization set.") + precondition(self.type == .http(self.authorization), "SOCKS authorization support is not yet implemented.") self.type = .http(newValue) } diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 4bc6ee55f..0e619fcb4 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -725,6 +725,16 @@ class HTTPClientTests: XCTestCase { XCTAssertEqual(.ok, response?.status) XCTAssertEqual(ByteBuffer(string: "it works!"), response?.body) } + + func testProxySOCKSBogusAddress() throws { + let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), + configuration: .init(proxy: .socksServer(host: "127.0.."))) + + defer { + XCTAssertNoThrow(try localClient.syncShutdown()) + } + XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) + } // there is no socks server, so we should fail func testProxySOCKSFailureNoServer() throws { From 221420f4a385c5dc46bb844b94256b0d46e23c13 Mon Sep 17 00:00:00 2001 From: David Evans Date: Thu, 17 Jun 2021 17:12:32 +0100 Subject: [PATCH 20/25] Swift format --- Tests/AsyncHTTPClientTests/HTTPClientTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 0e619fcb4..7c407e323 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -725,7 +725,7 @@ class HTTPClientTests: XCTestCase { XCTAssertEqual(.ok, response?.status) XCTAssertEqual(ByteBuffer(string: "it works!"), response?.body) } - + func testProxySOCKSBogusAddress() throws { let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), configuration: .init(proxy: .socksServer(host: "127.0.."))) From 91a1636f21c5b4be443b399c5789c78f45423322 Mon Sep 17 00:00:00 2001 From: David Evans Date: Thu, 17 Jun 2021 17:14:54 +0100 Subject: [PATCH 21/25] Soundness --- Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift index fd408f45f..eda59910c 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift @@ -59,6 +59,7 @@ extension HTTPClientTests { ("testProxyPlaintextWithCorrectlyAuthorization", testProxyPlaintextWithCorrectlyAuthorization), ("testProxyPlaintextWithIncorrectlyAuthorization", testProxyPlaintextWithIncorrectlyAuthorization), ("testProxySOCKS", testProxySOCKS), + ("testProxySOCKSBogusAddress", testProxySOCKSBogusAddress), ("testProxySOCKSFailureNoServer", testProxySOCKSFailureNoServer), ("testProxySOCKSFailureInvalidServer", testProxySOCKSFailureInvalidServer), ("testProxySOCKSMisbehavingServer", testProxySOCKSMisbehavingServer), From 15e5334349ddfcc2fd332d31fec8ff4d279106f9 Mon Sep 17 00:00:00 2001 From: David Evans Date: Fri, 18 Jun 2021 12:05:05 +0100 Subject: [PATCH 22/25] Address PR comments --- Sources/AsyncHTTPClient/HTTPClient.swift | 6 +- .../HTTPClientProxyHandler.swift | 4 +- Sources/AsyncHTTPClient/Utils.swift | 2 +- .../HTTPClient+SOCKSTests+XCTest.swift | 35 +++++ .../HTTPClient+SOCKSTests.swift | 139 ++++++++++++++++++ .../HTTPClientTests+XCTest.swift | 5 - .../HTTPClientTests.swift | 63 -------- .../AsyncHTTPClientTests/SOCKSTestUtils.swift | 86 +++++------ Tests/LinuxMain.swift | 1 + 9 files changed, 224 insertions(+), 117 deletions(-) create mode 100644 Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests+XCTest.swift create mode 100644 Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests.swift diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index 299d167e9..0ca31a5be 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -884,7 +884,7 @@ extension HTTPClient.Configuration { } extension ChannelPipeline { - func syncAddProxyHandler(host: String, port: Int, authorization: HTTPClient.Authorization?) throws { + func syncAddHTTPProxyHandler(host: String, port: Int, authorization: HTTPClient.Authorization?) throws { let encoder = HTTPRequestEncoder() let decoder = ByteToMessageHandler(HTTPResponseDecoder(leftOverBytesStrategy: .forwardBytes)) let handler = HTTPClientProxyHandler(host: host, port: port, authorization: authorization) { channel in @@ -902,8 +902,8 @@ extension ChannelPipeline { } func syncAddSOCKSProxyHandler(host: String, port: Int) throws { - let address = try SocketAddress.makeAddressResolvingHost(host, port: port) - let handler = SOCKSClientHandler(targetAddress: .address(address)) +// let address = try SocketAddress.makeAddressResolvingHost(host, port: port) + let handler = SOCKSClientHandler(targetAddress: .domain(host, port: port)) let sync = self.syncOperations try sync.addHandler(handler) } diff --git a/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift b/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift index 80e13dcf4..e2b891c8b 100644 --- a/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPClientProxyHandler.swift @@ -56,7 +56,7 @@ extension HTTPClient.Configuration { var type: ProxyType - /// Create proxy. + /// Create a HTTP proxy. /// /// - parameters: /// - host: proxy server host. @@ -65,7 +65,7 @@ extension HTTPClient.Configuration { return .init(host: host, port: port, type: .http(nil)) } - /// Create proxy. + /// Create a HTTP proxy. /// /// - parameters: /// - host: proxy server host. diff --git a/Sources/AsyncHTTPClient/Utils.swift b/Sources/AsyncHTTPClient/Utils.swift index 85e7c3721..0f8236f76 100644 --- a/Sources/AsyncHTTPClient/Utils.swift +++ b/Sources/AsyncHTTPClient/Utils.swift @@ -150,7 +150,7 @@ extension NIOClientTCPBootstrap { if let proxy = configuration.proxy { switch proxy.type { case .http: - try channel.pipeline.syncAddProxyHandler(host: host, + try channel.pipeline.syncAddHTTPProxyHandler(host: host, port: port, authorization: proxy.authorization) case .socks: diff --git a/Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests+XCTest.swift new file mode 100644 index 000000000..40ef6e0ff --- /dev/null +++ b/Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests+XCTest.swift @@ -0,0 +1,35 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the AsyncHTTPClient open source project +// +// Copyright (c) 2018-2019 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 +// +//===----------------------------------------------------------------------===// +// +// HTTPClient+SOCKSTests+XCTest.swift +// +import XCTest + +/// +/// NOTE: This file was generated by generate_linux_tests.rb +/// +/// Do NOT edit this file directly as it will be regenerated automatically when needed. +/// + +extension HTTPClientSOCKSTests { + static var allTests: [(String, (HTTPClientSOCKSTests) -> () throws -> Void)] { + return [ + ("testProxySOCKS", testProxySOCKS), + ("testProxySOCKSBogusAddress", testProxySOCKSBogusAddress), + ("testProxySOCKSFailureNoServer", testProxySOCKSFailureNoServer), + ("testProxySOCKSFailureInvalidServer", testProxySOCKSFailureInvalidServer), + ("testProxySOCKSMisbehavingServer", testProxySOCKSMisbehavingServer), + ] + } +} diff --git a/Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests.swift b/Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests.swift new file mode 100644 index 000000000..82aea6396 --- /dev/null +++ b/Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests.swift @@ -0,0 +1,139 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the AsyncHTTPClient open source project +// +// Copyright (c) 2018-2019 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 +// +//===----------------------------------------------------------------------===// + +/* NOT @testable */ import AsyncHTTPClient // Tests that need @testable go into HTTPClientInternalTests.swift +import NIO +import NIOSOCKS +import XCTest +import Logging + +class HTTPClientSOCKSTests: XCTestCase { + + typealias Request = HTTPClient.Request + + var clientGroup: EventLoopGroup! + var serverGroup: EventLoopGroup! + var defaultHTTPBin: HTTPBin! + var defaultClient: HTTPClient! + var backgroundLogStore: CollectEverythingLogHandler.LogStore! + + var defaultHTTPBinURLPrefix: String { + return "http://localhost:\(self.defaultHTTPBin.port)/" + } + + override func setUp() { + XCTAssertNil(self.clientGroup) + XCTAssertNil(self.serverGroup) + XCTAssertNil(self.defaultHTTPBin) + XCTAssertNil(self.defaultClient) + XCTAssertNil(self.backgroundLogStore) + + self.clientGroup = getDefaultEventLoopGroup(numberOfThreads: 1) + self.serverGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1) + self.defaultHTTPBin = HTTPBin() + self.backgroundLogStore = CollectEverythingLogHandler.LogStore() + var backgroundLogger = Logger(label: "\(#function)", factory: { _ in + CollectEverythingLogHandler(logStore: self.backgroundLogStore!) + }) + backgroundLogger.logLevel = .trace + self.defaultClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), + backgroundActivityLogger: backgroundLogger) + } + + override func tearDown() { + if let defaultClient = self.defaultClient { + XCTAssertNoThrow(try defaultClient.syncShutdown()) + self.defaultClient = nil + } + + XCTAssertNotNil(self.defaultHTTPBin) + XCTAssertNoThrow(try self.defaultHTTPBin.shutdown()) + self.defaultHTTPBin = nil + + XCTAssertNotNil(self.clientGroup) + XCTAssertNoThrow(try self.clientGroup.syncShutdownGracefully()) + self.clientGroup = nil + + XCTAssertNotNil(self.serverGroup) + XCTAssertNoThrow(try self.serverGroup.syncShutdownGracefully()) + self.serverGroup = nil + + XCTAssertNotNil(self.backgroundLogStore) + self.backgroundLogStore = nil + } + + func testProxySOCKS() throws { + let socksBin = try MockSOCKSServer(expectedURL: "/socks/test", expectedResponse: "it works!") + let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), + configuration: .init(proxy: .socksServer(host: "127.0.0.1"))) + + defer { + XCTAssertNoThrow(try localClient.syncShutdown()) + XCTAssertNoThrow(try socksBin.shutdown()) + } + + var response: HTTPClient.Response? + XCTAssertNoThrow(response = try localClient.get(url: "http://127.0.0.1/socks/test").wait()) + XCTAssertEqual(.ok, response?.status) + XCTAssertEqual(ByteBuffer(string: "it works!"), response?.body) + } + + func testProxySOCKSBogusAddress() throws { + let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), + configuration: .init(proxy: .socksServer(host: "127.0.."))) + + defer { + XCTAssertNoThrow(try localClient.syncShutdown()) + } + XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) + } + + // there is no socks server, so we should fail + func testProxySOCKSFailureNoServer() throws { + let localHTTPBin = HTTPBin() + let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), + configuration: .init(proxy: .socksServer(host: "127.0.0.1", port: localHTTPBin.port))) + defer { + XCTAssertNoThrow(try localClient.syncShutdown()) + XCTAssertNoThrow(try localHTTPBin.shutdown()) + } + XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) + } + + // speak to a server that doesn't speak SOCKS + func testProxySOCKSFailureInvalidServer() throws { + let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), + configuration: .init(proxy: .socksServer(host: "127.0.0.1"))) + defer { + XCTAssertNoThrow(try localClient.syncShutdown()) + } + XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) + } + + // test a handshake failure with a misbehaving server + func testProxySOCKSMisbehavingServer() throws { + let socksBin = try MockSOCKSServer(expectedURL: "/socks/test", expectedResponse: "it works!", misbehave: true) + let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), + configuration: .init(proxy: .socksServer(host: "127.0.0.1"))) + + defer { + XCTAssertNoThrow(try localClient.syncShutdown()) + XCTAssertNoThrow(try socksBin.shutdown()) + } + + // the server will send a bogus message in response to the clients request + XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) + } + +} diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift index eda59910c..98bfb0b54 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift @@ -58,11 +58,6 @@ extension HTTPClientTests { ("testProxyTLS", testProxyTLS), ("testProxyPlaintextWithCorrectlyAuthorization", testProxyPlaintextWithCorrectlyAuthorization), ("testProxyPlaintextWithIncorrectlyAuthorization", testProxyPlaintextWithIncorrectlyAuthorization), - ("testProxySOCKS", testProxySOCKS), - ("testProxySOCKSBogusAddress", testProxySOCKSBogusAddress), - ("testProxySOCKSFailureNoServer", testProxySOCKSFailureNoServer), - ("testProxySOCKSFailureInvalidServer", testProxySOCKSFailureInvalidServer), - ("testProxySOCKSMisbehavingServer", testProxySOCKSMisbehavingServer), ("testUploadStreaming", testUploadStreaming), ("testNoContentLengthForSSLUncleanShutdown", testNoContentLengthForSSLUncleanShutdown), ("testNoContentLengthWithIgnoreErrorForSSLUncleanShutdown", testNoContentLengthWithIgnoreErrorForSSLUncleanShutdown), diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 7c407e323..ab2ad311f 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -710,69 +710,6 @@ class HTTPClientTests: XCTestCase { } } - func testProxySOCKS() throws { - let socksBin = try MockSOCKSServer(expectedURL: "/socks/test", expectedResponse: "it works!") - let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), - configuration: .init(proxy: .socksServer(host: "127.0.0.1"))) - - defer { - XCTAssertNoThrow(try localClient.syncShutdown()) - XCTAssertNoThrow(try socksBin.shutdown()) - } - - var response: HTTPClient.Response? - XCTAssertNoThrow(response = try localClient.get(url: "http://127.0.0.1/socks/test").wait()) - XCTAssertEqual(.ok, response?.status) - XCTAssertEqual(ByteBuffer(string: "it works!"), response?.body) - } - - func testProxySOCKSBogusAddress() throws { - let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), - configuration: .init(proxy: .socksServer(host: "127.0.."))) - - defer { - XCTAssertNoThrow(try localClient.syncShutdown()) - } - XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) - } - - // there is no socks server, so we should fail - func testProxySOCKSFailureNoServer() throws { - let localHTTPBin = HTTPBin() - let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), - configuration: .init(proxy: .socksServer(host: "127.0.0.1", port: localHTTPBin.port))) - defer { - XCTAssertNoThrow(try localClient.syncShutdown()) - XCTAssertNoThrow(try localHTTPBin.shutdown()) - } - XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) - } - - // speak to a server that doesn't speak SOCKS - func testProxySOCKSFailureInvalidServer() throws { - let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), - configuration: .init(proxy: .socksServer(host: "127.0.0.1"))) - defer { - XCTAssertNoThrow(try localClient.syncShutdown()) - } - XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) - } - - // test a handshake failure with a misbehaving server - func testProxySOCKSMisbehavingServer() throws { - let socksBin = try MockSOCKSServer(expectedURL: "/socks/test", expectedResponse: "it works!", misbehave: true) - let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), - configuration: .init(proxy: .socksServer(host: "127.0.0.1"))) - - defer { - XCTAssertNoThrow(try localClient.syncShutdown()) - XCTAssertNoThrow(try socksBin.shutdown()) - } - - // the server will send a bogus message in response to the clients request - XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) - } - func testUploadStreaming() throws { let body: HTTPClient.Body = .stream(length: 8) { writer in let buffer = ByteBuffer(string: "1234") diff --git a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift index ae169e113..1bf4c8281 100644 --- a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift +++ b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift @@ -25,7 +25,7 @@ struct MockSOCKSError: Error, Hashable { class MockSOCKSServer { let channel: Channel - public init(expectedURL: String, expectedResponse: String, misbehave: Bool = false, file: String = #file, line: UInt = #line) throws { + init(expectedURL: String, expectedResponse: String, misbehave: Bool = false, file: String = #file, line: UInt = #line) throws { let elg = MultiThreadedEventLoopGroup(numberOfThreads: 1) let bootstrap = ServerBootstrap(group: elg) .serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1) @@ -34,7 +34,7 @@ class MockSOCKSServer { return channel.pipeline.addHandlers([ handshakeHandler, SOCKSTestHandler(handshakeHandler: handshakeHandler, misbehave: misbehave), - SOCKSTestHTTPClient(expectedURL: expectedURL, expectedResponse: expectedResponse, file: file, line: line), + TestHTTPServer(expectedURL: expectedURL, expectedResponse: expectedResponse, file: file, line: line), ]) } self.channel = try bootstrap.bind(host: "127.0.0.1", port: 1080).wait() @@ -45,47 +45,6 @@ class MockSOCKSServer { } } -class SOCKSTestHTTPClient: ChannelInboundHandler { - typealias InboundIn = HTTPServerRequestPart - typealias OutboundOut = HTTPServerResponsePart - - let expectedURL: String - let expectedResponse: String - let file: String - let line: UInt - var requestCount = 0 - - init(expectedURL: String, expectedResponse: String, file: String, line: UInt) { - self.expectedURL = expectedURL - self.expectedResponse = expectedResponse - self.file = file - self.line = line - } - - func channelRead(context: ChannelHandlerContext, data: NIOAny) { - let message = self.unwrapInboundIn(data) - switch message { - case .head(let head): - guard self.requestCount == 0 else { - return - } - XCTAssertEqual(head.uri, self.expectedURL) - self.requestCount += 1 - case .body: - break - case .end: - context.write(self.wrapOutboundOut(.head(.init(version: .http1_1, status: .ok))), promise: nil) - context.write(self.wrapOutboundOut(.body(.byteBuffer(context.channel.allocator.buffer(string: self.expectedResponse)))), promise: nil) - context.writeAndFlush(self.wrapOutboundOut(.end(nil)), promise: nil) - } - } - - func errorCaught(context: ChannelHandlerContext, error: Error) { - context.fireErrorCaught(error) - context.close(promise: nil) - } -} - class SOCKSTestHandler: ChannelInboundHandler, RemovableChannelHandler { typealias InboundIn = ClientMessage @@ -128,3 +87,44 @@ class SOCKSTestHandler: ChannelInboundHandler, RemovableChannelHandler { } } } + +class TestHTTPServer: ChannelInboundHandler { + typealias InboundIn = HTTPServerRequestPart + typealias OutboundOut = HTTPServerResponsePart + + let expectedURL: String + let expectedResponse: String + let file: String + let line: UInt + var requestCount = 0 + + init(expectedURL: String, expectedResponse: String, file: String, line: UInt) { + self.expectedURL = expectedURL + self.expectedResponse = expectedResponse + self.file = file + self.line = line + } + + func channelRead(context: ChannelHandlerContext, data: NIOAny) { + let message = self.unwrapInboundIn(data) + switch message { + case .head(let head): + guard self.requestCount == 0 else { + return + } + XCTAssertEqual(head.uri, self.expectedURL) + self.requestCount += 1 + case .body: + break + case .end: + context.write(self.wrapOutboundOut(.head(.init(version: .http1_1, status: .ok))), promise: nil) + context.write(self.wrapOutboundOut(.body(.byteBuffer(context.channel.allocator.buffer(string: self.expectedResponse)))), promise: nil) + context.writeAndFlush(self.wrapOutboundOut(.end(nil)), promise: nil) + } + } + + func errorCaught(context: ChannelHandlerContext, error: Error) { + context.fireErrorCaught(error) + context.close(promise: nil) + } +} diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index 0db0dd9ce..094b0ee2a 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -31,6 +31,7 @@ import XCTest testCase(HTTPClientCookieTests.allTests), testCase(HTTPClientInternalTests.allTests), testCase(HTTPClientNIOTSTests.allTests), + testCase(HTTPClientSOCKSTests.allTests), testCase(HTTPClientTests.allTests), testCase(LRUCacheTests.allTests), testCase(RequestValidationTests.allTests), From c61184e621a29142da23cb220b4768d671f01e87 Mon Sep 17 00:00:00 2001 From: David Evans Date: Fri, 18 Jun 2021 12:13:44 +0100 Subject: [PATCH 23/25] Cory PR nits --- Sources/AsyncHTTPClient/HTTPClient.swift | 1 - .../HTTPClient+SOCKSTests.swift | 20 +++++++++---------- .../AsyncHTTPClientTests/SOCKSTestUtils.swift | 2 +- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index 0ca31a5be..51a1c8c4d 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -902,7 +902,6 @@ extension ChannelPipeline { } func syncAddSOCKSProxyHandler(host: String, port: Int) throws { -// let address = try SocketAddress.makeAddressResolvingHost(host, port: port) let handler = SOCKSClientHandler(targetAddress: .domain(host, port: port)) let sync = self.syncOperations try sync.addHandler(handler) diff --git a/Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests.swift b/Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests.swift index 82aea6396..0dd336c43 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests.swift @@ -2,7 +2,7 @@ // // This source file is part of the AsyncHTTPClient open source project // -// Copyright (c) 2018-2019 Apple Inc. and the AsyncHTTPClient project authors +// Copyright (c) 2021 Apple Inc. and the AsyncHTTPClient project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -76,7 +76,7 @@ class HTTPClientSOCKSTests: XCTestCase { func testProxySOCKS() throws { let socksBin = try MockSOCKSServer(expectedURL: "/socks/test", expectedResponse: "it works!") let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), - configuration: .init(proxy: .socksServer(host: "127.0.0.1"))) + configuration: .init(proxy: .socksServer(host: "localhost"))) defer { XCTAssertNoThrow(try localClient.syncShutdown()) @@ -84,7 +84,7 @@ class HTTPClientSOCKSTests: XCTestCase { } var response: HTTPClient.Response? - XCTAssertNoThrow(response = try localClient.get(url: "http://127.0.0.1/socks/test").wait()) + XCTAssertNoThrow(response = try localClient.get(url: "http://localhost/socks/test").wait()) XCTAssertEqual(.ok, response?.status) XCTAssertEqual(ByteBuffer(string: "it works!"), response?.body) } @@ -96,36 +96,36 @@ class HTTPClientSOCKSTests: XCTestCase { defer { XCTAssertNoThrow(try localClient.syncShutdown()) } - XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) + XCTAssertThrowsError(try localClient.get(url: "http://localhost/socks/test").wait()) } // there is no socks server, so we should fail func testProxySOCKSFailureNoServer() throws { let localHTTPBin = HTTPBin() let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), - configuration: .init(proxy: .socksServer(host: "127.0.0.1", port: localHTTPBin.port))) + configuration: .init(proxy: .socksServer(host: "localhost", port: localHTTPBin.port))) defer { XCTAssertNoThrow(try localClient.syncShutdown()) XCTAssertNoThrow(try localHTTPBin.shutdown()) } - XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) + XCTAssertThrowsError(try localClient.get(url: "http://localhost/socks/test").wait()) } // speak to a server that doesn't speak SOCKS func testProxySOCKSFailureInvalidServer() throws { let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), - configuration: .init(proxy: .socksServer(host: "127.0.0.1"))) + configuration: .init(proxy: .socksServer(host: "localhost"))) defer { XCTAssertNoThrow(try localClient.syncShutdown()) } - XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) + XCTAssertThrowsError(try localClient.get(url: "http://localhost/socks/test").wait()) } // test a handshake failure with a misbehaving server func testProxySOCKSMisbehavingServer() throws { let socksBin = try MockSOCKSServer(expectedURL: "/socks/test", expectedResponse: "it works!", misbehave: true) let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), - configuration: .init(proxy: .socksServer(host: "127.0.0.1"))) + configuration: .init(proxy: .socksServer(host: "localhost"))) defer { XCTAssertNoThrow(try localClient.syncShutdown()) @@ -133,7 +133,7 @@ class HTTPClientSOCKSTests: XCTestCase { } // the server will send a bogus message in response to the clients request - XCTAssertThrowsError(try localClient.get(url: "http://127.0.0.1/socks/test").wait()) + XCTAssertThrowsError(try localClient.get(url: "http://localhost/socks/test").wait()) } } diff --git a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift index 1bf4c8281..38fa706df 100644 --- a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift +++ b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift @@ -37,7 +37,7 @@ class MockSOCKSServer { TestHTTPServer(expectedURL: expectedURL, expectedResponse: expectedResponse, file: file, line: line), ]) } - self.channel = try bootstrap.bind(host: "127.0.0.1", port: 1080).wait() + self.channel = try bootstrap.bind(host: "localhost", port: 1080).wait() } func shutdown() throws { From 97c6b7f9e1a37d2033e618d1d41a7d27eb6837cd Mon Sep 17 00:00:00 2001 From: David Evans Date: Fri, 18 Jun 2021 12:54:29 +0100 Subject: [PATCH 24/25] Soundness --- Package.swift | 2 +- Sources/AsyncHTTPClient/Utils.swift | 4 ++-- Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests.swift | 6 ++---- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/Package.swift b/Package.swift index af4fc7b0b..b579b608a 100644 --- a/Package.swift +++ b/Package.swift @@ -23,7 +23,7 @@ let package = Package( dependencies: [ .package(url: "https://github.com/apple/swift-nio.git", from: "2.29.0"), .package(url: "https://github.com/apple/swift-nio-ssl.git", from: "2.13.0"), - .package(url: "https://github.com/apple/swift-nio-extras.git", from: "1.9.0"), + .package(url: "https://github.com/apple/swift-nio-extras.git", from: "1.9.1"), .package(url: "https://github.com/apple/swift-nio-transport-services.git", from: "1.5.1"), .package(url: "https://github.com/apple/swift-log.git", from: "1.4.0"), ], diff --git a/Sources/AsyncHTTPClient/Utils.swift b/Sources/AsyncHTTPClient/Utils.swift index 0f8236f76..174bc593e 100644 --- a/Sources/AsyncHTTPClient/Utils.swift +++ b/Sources/AsyncHTTPClient/Utils.swift @@ -151,8 +151,8 @@ extension NIOClientTCPBootstrap { switch proxy.type { case .http: try channel.pipeline.syncAddHTTPProxyHandler(host: host, - port: port, - authorization: proxy.authorization) + port: port, + authorization: proxy.authorization) case .socks: try channel.pipeline.syncAddSOCKSProxyHandler(host: host, port: port) } diff --git a/Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests.swift b/Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests.swift index 0dd336c43..3479d86b9 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests.swift @@ -13,13 +13,12 @@ //===----------------------------------------------------------------------===// /* NOT @testable */ import AsyncHTTPClient // Tests that need @testable go into HTTPClientInternalTests.swift +import Logging import NIO import NIOSOCKS import XCTest -import Logging class HTTPClientSOCKSTests: XCTestCase { - typealias Request = HTTPClient.Request var clientGroup: EventLoopGroup! @@ -72,7 +71,7 @@ class HTTPClientSOCKSTests: XCTestCase { XCTAssertNotNil(self.backgroundLogStore) self.backgroundLogStore = nil } - + func testProxySOCKS() throws { let socksBin = try MockSOCKSServer(expectedURL: "/socks/test", expectedResponse: "it works!") let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), @@ -135,5 +134,4 @@ class HTTPClientSOCKSTests: XCTestCase { // the server will send a bogus message in response to the clients request XCTAssertThrowsError(try localClient.get(url: "http://localhost/socks/test").wait()) } - } From dd405afa4ef0f88db210ba420e73b4f725ef8845 Mon Sep 17 00:00:00 2001 From: David Evans Date: Fri, 18 Jun 2021 12:58:50 +0100 Subject: [PATCH 25/25] Fabian nit --- Tests/AsyncHTTPClientTests/HTTPClientTests.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index ab2ad311f..eef14a78a 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -22,7 +22,6 @@ import NIOConcurrencyHelpers import NIOFoundationCompat import NIOHTTP1 import NIOHTTPCompression -import NIOSOCKS import NIOSSL import NIOTestUtils import NIOTransportServices