From aad6a4243f0308699b05a0144dae16de25b38e71 Mon Sep 17 00:00:00 2001 From: Artem Redkin Date: Mon, 16 Sep 2019 16:00:47 +0100 Subject: [PATCH 1/6] preserve trailing slash in uri path --- Sources/AsyncHTTPClient/HTTPHandler.swift | 36 ++++++++++++++++--- .../HTTPClientInternalTests.swift | 35 ++++++++++++++++++ 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/Sources/AsyncHTTPClient/HTTPHandler.swift b/Sources/AsyncHTTPClient/HTTPHandler.swift index 088b7f562..df758863b 100644 --- a/Sources/AsyncHTTPClient/HTTPHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPHandler.swift @@ -350,14 +350,42 @@ extension HTTPClientResponseDelegate { public func didReceiveError(task: HTTPClient.Task, _: Error) {} } -internal extension URL { +extension URL { + var percentEncodedPath: String { + if self.path.isEmpty { + return "/" + } + return self.path.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) ?? self.path + } + + var pathHasTrailingSlash: Bool { + let url = self.absoluteString + + var pathEndIndex = url.index(before: url.endIndex) + if let queryIndex = url.firstIndex(of: "?") { + pathEndIndex = url.index(before: queryIndex) + } else if let fragmentIndex = url.firstIndex(of: "#") { + pathEndIndex = url.index(before: fragmentIndex) + } + + return url[pathEndIndex] == "/" + } + var uri: String { - let urlEncodedPath = path.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) ?? path - return path.isEmpty ? "/" : urlEncodedPath + (query.map { "?" + $0 } ?? "") + var uri = self.percentEncodedPath + if self.pathHasTrailingSlash, uri != "/" { + uri += "/" + } + + if let query = self.query { + uri += "?" + query + } + + return uri } func hasTheSameOrigin(as other: URL) -> Bool { - return host == other.host && scheme == other.scheme && port == other.port + return self.host == other.host && self.scheme == other.scheme && self.port == other.port } } diff --git a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift index 9ca4c26ed..68670f483 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift @@ -193,4 +193,39 @@ class HTTPClientInternalTests: XCTestCase { XCTAssertEqual(delegate.reads, 3) } + + func testRequestURITrailingSlash() throws { + let request1 = try Request(url: "https://someserver.com:8888/some/path?foo=bar#ref") + XCTAssertEqual(request1.url.uri, "/some/path?foo=bar") + + let request2 = try Request(url: "https://someserver.com:8888/some/path/?foo=bar#ref") + XCTAssertEqual(request2.url.uri, "/some/path/?foo=bar") + + let request3 = try Request(url: "https://someserver.com:8888?foo=bar#ref") + XCTAssertEqual(request3.url.uri, "/?foo=bar") + + let request4 = try Request(url: "https://someserver.com:8888/?foo=bar#ref") + XCTAssertEqual(request4.url.uri, "/?foo=bar") + + let request5 = try Request(url: "https://someserver.com:8888/some/path") + XCTAssertEqual(request5.url.uri, "/some/path") + + let request6 = try Request(url: "https://someserver.com:8888/some/path/") + XCTAssertEqual(request6.url.uri, "/some/path/") + + let request7 = try Request(url: "https://someserver.com:8888") + XCTAssertEqual(request7.url.uri, "/") + + let request8 = try Request(url: "https://someserver.com:8888/") + XCTAssertEqual(request8.url.uri, "/") + + let request9 = try Request(url: "https://someserver.com:8888#ref") + XCTAssertEqual(request9.url.uri, "/") + + let request10 = try Request(url: "https://someserver.com:8888/#ref") + XCTAssertEqual(request10.url.uri, "/") + + let request11 = try Request(url: "https://someserver.com/some%20path") + XCTAssertEqual(request11.url.uri, "/some%20path") + } } From 9b733f31baef9e977cfab1fd18183a861d90fb45 Mon Sep 17 00:00:00 2001 From: Artem Redkin Date: Mon, 16 Sep 2019 16:09:34 +0100 Subject: [PATCH 2/6] fix missing linux test --- Tests/AsyncHTTPClientTests/HTTPClientInternalTests+XCTest.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests+XCTest.swift index 6ad239e02..ad172d12b 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests+XCTest.swift @@ -30,6 +30,7 @@ extension HTTPClientInternalTests { ("testProxyStreaming", testProxyStreaming), ("testProxyStreamingFailure", testProxyStreamingFailure), ("testUploadStreamingBackpressure", testUploadStreamingBackpressure), + ("testRequestURITrailingSlash", testRequestURITrailingSlash), ] } } From 78b86a98663c8c62b2e2e746610bb80227b062bd Mon Sep 17 00:00:00 2001 From: Artem Redkin Date: Tue, 17 Sep 2019 10:28:07 +0100 Subject: [PATCH 3/6] use more robust way to detect trailing slash in path --- Sources/AsyncHTTPClient/HTTPHandler.swift | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/Sources/AsyncHTTPClient/HTTPHandler.swift b/Sources/AsyncHTTPClient/HTTPHandler.swift index c7d032ee4..0bc683257 100644 --- a/Sources/AsyncHTTPClient/HTTPHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPHandler.swift @@ -394,16 +394,20 @@ extension URL { } var pathHasTrailingSlash: Bool { - let url = self.absoluteString + if #available(OSX 10.11, iOS 9.0, tvOS 9.0, watchOS 2.0, *) { + return self.hasDirectoryPath + } else { + let url = self.absoluteString - var pathEndIndex = url.index(before: url.endIndex) - if let queryIndex = url.firstIndex(of: "?") { - pathEndIndex = url.index(before: queryIndex) - } else if let fragmentIndex = url.firstIndex(of: "#") { - pathEndIndex = url.index(before: fragmentIndex) - } + var pathEndIndex = url.index(before: url.endIndex) + if let queryIndex = url.firstIndex(of: "?") { + pathEndIndex = url.index(before: queryIndex) + } else if let fragmentIndex = url.suffix(from: url.firstIndex(of: "@") ?? url.startIndex).lastIndex(of: "#") { + pathEndIndex = url.index(before: fragmentIndex) + } - return url[pathEndIndex] == "/" + return url[pathEndIndex] == "/" + } } var uri: String { From 427fa0dc6869715202931859b6471f94f04b8431 Mon Sep 17 00:00:00 2001 From: Artem Redkin Date: Wed, 18 Sep 2019 14:52:53 +0100 Subject: [PATCH 4/6] review fix: limit to newer darwin platforms --- Package.swift | 1 + Sources/AsyncHTTPClient/HTTPHandler.swift | 19 +------------------ 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/Package.swift b/Package.swift index 21de3392d..0a1588e81 100644 --- a/Package.swift +++ b/Package.swift @@ -17,6 +17,7 @@ import PackageDescription let package = Package( name: "async-http-client", + platforms: [.macOS(.v10_11), .iOS(.v9), .tvOS(.v9), .watchOS(.v2)], products: [ .library(name: "AsyncHTTPClient", targets: ["AsyncHTTPClient"]), ], diff --git a/Sources/AsyncHTTPClient/HTTPHandler.swift b/Sources/AsyncHTTPClient/HTTPHandler.swift index 0bc683257..376db8d3c 100644 --- a/Sources/AsyncHTTPClient/HTTPHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPHandler.swift @@ -393,26 +393,9 @@ extension URL { return self.path.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) ?? self.path } - var pathHasTrailingSlash: Bool { - if #available(OSX 10.11, iOS 9.0, tvOS 9.0, watchOS 2.0, *) { - return self.hasDirectoryPath - } else { - let url = self.absoluteString - - var pathEndIndex = url.index(before: url.endIndex) - if let queryIndex = url.firstIndex(of: "?") { - pathEndIndex = url.index(before: queryIndex) - } else if let fragmentIndex = url.suffix(from: url.firstIndex(of: "@") ?? url.startIndex).lastIndex(of: "#") { - pathEndIndex = url.index(before: fragmentIndex) - } - - return url[pathEndIndex] == "/" - } - } - var uri: String { var uri = self.percentEncodedPath - if self.pathHasTrailingSlash, uri != "/" { + if self.hasDirectoryPath, uri != "/" { uri += "/" } From da15eb24ee7d6f41061700d70ccaeb85cc8a1006 Mon Sep 17 00:00:00 2001 From: Artem Redkin Date: Wed, 18 Sep 2019 17:55:51 +0100 Subject: [PATCH 5/6] add a comment --- Package.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Package.swift b/Package.swift index 0a1588e81..88d3b7479 100644 --- a/Package.swift +++ b/Package.swift @@ -17,6 +17,7 @@ import PackageDescription let package = Package( name: "async-http-client", + // this parameter is darwin-specific, linux is supported as well platforms: [.macOS(.v10_11), .iOS(.v9), .tvOS(.v9), .watchOS(.v2)], products: [ .library(name: "AsyncHTTPClient", targets: ["AsyncHTTPClient"]), From 6f75def8605077cc3da513b7497e259b3cd6a669 Mon Sep 17 00:00:00 2001 From: Artem Redkin Date: Thu, 19 Sep 2019 12:44:56 +0100 Subject: [PATCH 6/6] review fix: revert platform dependency --- Package.swift | 2 -- Sources/AsyncHTTPClient/HTTPHandler.swift | 21 ++++++++++++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/Package.swift b/Package.swift index 88d3b7479..21de3392d 100644 --- a/Package.swift +++ b/Package.swift @@ -17,8 +17,6 @@ import PackageDescription let package = Package( name: "async-http-client", - // this parameter is darwin-specific, linux is supported as well - platforms: [.macOS(.v10_11), .iOS(.v9), .tvOS(.v9), .watchOS(.v2)], products: [ .library(name: "AsyncHTTPClient", targets: ["AsyncHTTPClient"]), ], diff --git a/Sources/AsyncHTTPClient/HTTPHandler.swift b/Sources/AsyncHTTPClient/HTTPHandler.swift index 376db8d3c..678f5cc5a 100644 --- a/Sources/AsyncHTTPClient/HTTPHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPHandler.swift @@ -393,9 +393,28 @@ extension URL { return self.path.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) ?? self.path } + var pathHasTrailingSlash: Bool { + if #available(OSX 10.11, iOS 9.0, tvOS 9.0, watchOS 2.0, *) { + return self.hasDirectoryPath + } else { + // Most platforms should use `self.hasDirectoryPath`, but on older darwin platforms + // we have this approximation instead. + let url = self.absoluteString + + var pathEndIndex = url.index(before: url.endIndex) + if let queryIndex = url.firstIndex(of: "?") { + pathEndIndex = url.index(before: queryIndex) + } else if let fragmentIndex = url.suffix(from: url.firstIndex(of: "@") ?? url.startIndex).lastIndex(of: "#") { + pathEndIndex = url.index(before: fragmentIndex) + } + + return url[pathEndIndex] == "/" + } + } + var uri: String { var uri = self.percentEncodedPath - if self.hasDirectoryPath, uri != "/" { + if self.pathHasTrailingSlash, uri != "/" { uri += "/" }