Enable clients to call URLs that include %2F as an escaped backslash (#201)

* Enable clients to call URLs that include %2F as an escaped backslash

Previously `percentEncodedPath` was using `path.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed)`
which converts %2F to a literal '/'. This prevented users fetching URLs like https://api.travis-ci.org/repo/github/rails%2Frails
which use %2F as part of a path segment.

Migrating to `URLComponents(url: self, resolvingAgainstBaseURL: false)?.percentEncodedPath` has the desired behaviour
for the couple of test cases that exist.

Updated the test server to switch on the `percentEncodedPath` so it's easier
to understand the desired behaviour.

Author: iainsmith

Sources/AsyncHTTPClient/HTTPHandler.swift

@@ -464,33 +464,11 @@ extension URL {
         if self.path.isEmpty {
             return "/"
         }
-        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] == "/"
-        }
+        return URLComponents(url: self, resolvingAgainstBaseURL: false)?.percentEncodedPath ?? self.path
     }
 
     var uri: String {
         var uri = self.percentEncodedPath
-        if self.pathHasTrailingSlash, uri != "/" {
-            uri += "/"
-        }
 
         if let query = self.query {
             uri += "?" + query

Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift

@@ -286,6 +286,9 @@ class HTTPClientInternalTests: XCTestCase {
 
         let request11 = try Request(url: "https://someserver.com/some%20path")
         XCTAssertEqual(request11.url.uri, "/some%20path")
+
+        let request12 = try Request(url: "https://someserver.com/some%2Fpathsegment1/pathsegment2")
+        XCTAssertEqual(request12.url.uri, "/some%2Fpathsegment1/pathsegment2")
     }
 
     func testChannelAndDelegateOnDifferentEventLoops() throws {

Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift

@@ -394,8 +394,8 @@ internal final class HttpBinHandler: ChannelInboundHandler {
         switch self.unwrapInboundIn(data) {
         case .head(let req):
             self.parseAndSetOptions(from: req)
-            let url = URL(string: req.uri)!
-            switch url.path {
+            let urlComponents = URLComponents(string: req.uri)!
+            switch urlComponents.percentEncodedPath {
             case "/":
                 var headers = HTTPHeaders()
                 headers.add(name: "X-Is-This-Slash", value: "Yes")
@@ -429,13 +429,13 @@ internal final class HttpBinHandler: ChannelInboundHandler {
                 self.resps.append(HTTPResponseBuilder(status: .found, headers: headers))
                 return
             case "/redirect/https":
-                let port = self.value(for: "port", from: url.query!)
+                let port = self.value(for: "port", from: urlComponents.query!)
                 var headers = HTTPHeaders()
                 headers.add(name: "Location", value: "https://localhost:\(port)/ok")
                 self.resps.append(HTTPResponseBuilder(status: .found, headers: headers))
                 return
             case "/redirect/loopback":
-                let port = self.value(for: "port", from: url.query!)
+                let port = self.value(for: "port", from: urlComponents.query!)
                 var headers = HTTPHeaders()
                 headers.add(name: "Location", value: "http://127.0.0.1:\(port)/echohostheader")
                 self.resps.append(HTTPResponseBuilder(status: .found, headers: headers))
@@ -450,8 +450,14 @@ internal final class HttpBinHandler: ChannelInboundHandler {
                 headers.add(name: "Location", value: "/redirect/infinite1")
                 self.resps.append(HTTPResponseBuilder(status: .found, headers: headers))
                 return
-            // Since this String is taken from URL.path, the percent encoding has been removed
-            case "/percent encoded":
+            case "/percent%20encoded":
+                if req.method != .GET {
+                    self.resps.append(HTTPResponseBuilder(status: .methodNotAllowed))
+                    return
+                }
+                self.resps.append(HTTPResponseBuilder(status: .ok))
+                return
+            case "/percent%2Fencoded/hello":
                 if req.method != .GET {
                     self.resps.append(HTTPResponseBuilder(status: .methodNotAllowed))
                     return

Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift

@@ -37,6 +37,7 @@ extension HTTPClientTests {
             ("testHttpRedirect", testHttpRedirect),
             ("testHttpHostRedirect", testHttpHostRedirect),
             ("testPercentEncoded", testPercentEncoded),
+            ("testPercentEncodedBackslash", testPercentEncodedBackslash),
             ("testMultipleContentLengthHeaders", testMultipleContentLengthHeaders),
             ("testStreaming", testStreaming),
             ("testRemoteClose", testRemoteClose),

Tests/AsyncHTTPClientTests/HTTPClientTests.swift

@@ -218,6 +218,18 @@ class HTTPClientTests: XCTestCase {
         XCTAssertEqual(.ok, response.status)
     }
 
+    func testPercentEncodedBackslash() throws {
+        let httpBin = HTTPBin()
+        let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
+        defer {
+            XCTAssertNoThrow(try httpClient.syncShutdown(requiresCleanClose: true))
+            XCTAssertNoThrow(try httpBin.shutdown())
+        }
+
+        let response = try httpClient.get(url: "http://localhost:\(httpBin.port)/percent%2Fencoded/hello").wait()
+        XCTAssertEqual(.ok, response.status)
+    }
+
     func testMultipleContentLengthHeaders() throws {
         let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
         defer {