Skip to content

Commit be517e3

Browse files
authored
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.
1 parent 65a55b6 commit be517e3

File tree

5 files changed

+29
-29
lines changed

5 files changed

+29
-29
lines changed

Diff for: Sources/AsyncHTTPClient/HTTPHandler.swift

+1-23
Original file line numberDiff line numberDiff line change
@@ -464,33 +464,11 @@ extension URL {
464464
if self.path.isEmpty {
465465
return "/"
466466
}
467-
return self.path.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) ?? self.path
468-
}
469-
470-
var pathHasTrailingSlash: Bool {
471-
if #available(OSX 10.11, iOS 9.0, tvOS 9.0, watchOS 2.0, *) {
472-
return self.hasDirectoryPath
473-
} else {
474-
// Most platforms should use `self.hasDirectoryPath`, but on older darwin platforms
475-
// we have this approximation instead.
476-
let url = self.absoluteString
477-
478-
var pathEndIndex = url.index(before: url.endIndex)
479-
if let queryIndex = url.firstIndex(of: "?") {
480-
pathEndIndex = url.index(before: queryIndex)
481-
} else if let fragmentIndex = url.suffix(from: url.firstIndex(of: "@") ?? url.startIndex).lastIndex(of: "#") {
482-
pathEndIndex = url.index(before: fragmentIndex)
483-
}
484-
485-
return url[pathEndIndex] == "/"
486-
}
467+
return URLComponents(url: self, resolvingAgainstBaseURL: false)?.percentEncodedPath ?? self.path
487468
}
488469

489470
var uri: String {
490471
var uri = self.percentEncodedPath
491-
if self.pathHasTrailingSlash, uri != "/" {
492-
uri += "/"
493-
}
494472

495473
if let query = self.query {
496474
uri += "?" + query

Diff for: Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift

+3
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,9 @@ class HTTPClientInternalTests: XCTestCase {
286286

287287
let request11 = try Request(url: "https://someserver.com/some%20path")
288288
XCTAssertEqual(request11.url.uri, "/some%20path")
289+
290+
let request12 = try Request(url: "https://someserver.com/some%2Fpathsegment1/pathsegment2")
291+
XCTAssertEqual(request12.url.uri, "/some%2Fpathsegment1/pathsegment2")
289292
}
290293

291294
func testChannelAndDelegateOnDifferentEventLoops() throws {

Diff for: Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift

+12-6
Original file line numberDiff line numberDiff line change
@@ -394,8 +394,8 @@ internal final class HttpBinHandler: ChannelInboundHandler {
394394
switch self.unwrapInboundIn(data) {
395395
case .head(let req):
396396
self.parseAndSetOptions(from: req)
397-
let url = URL(string: req.uri)!
398-
switch url.path {
397+
let urlComponents = URLComponents(string: req.uri)!
398+
switch urlComponents.percentEncodedPath {
399399
case "/":
400400
var headers = HTTPHeaders()
401401
headers.add(name: "X-Is-This-Slash", value: "Yes")
@@ -429,13 +429,13 @@ internal final class HttpBinHandler: ChannelInboundHandler {
429429
self.resps.append(HTTPResponseBuilder(status: .found, headers: headers))
430430
return
431431
case "/redirect/https":
432-
let port = self.value(for: "port", from: url.query!)
432+
let port = self.value(for: "port", from: urlComponents.query!)
433433
var headers = HTTPHeaders()
434434
headers.add(name: "Location", value: "https://localhost:\(port)/ok")
435435
self.resps.append(HTTPResponseBuilder(status: .found, headers: headers))
436436
return
437437
case "/redirect/loopback":
438-
let port = self.value(for: "port", from: url.query!)
438+
let port = self.value(for: "port", from: urlComponents.query!)
439439
var headers = HTTPHeaders()
440440
headers.add(name: "Location", value: "http://127.0.0.1:\(port)/echohostheader")
441441
self.resps.append(HTTPResponseBuilder(status: .found, headers: headers))
@@ -450,8 +450,14 @@ internal final class HttpBinHandler: ChannelInboundHandler {
450450
headers.add(name: "Location", value: "/redirect/infinite1")
451451
self.resps.append(HTTPResponseBuilder(status: .found, headers: headers))
452452
return
453-
// Since this String is taken from URL.path, the percent encoding has been removed
454-
case "/percent encoded":
453+
case "/percent%20encoded":
454+
if req.method != .GET {
455+
self.resps.append(HTTPResponseBuilder(status: .methodNotAllowed))
456+
return
457+
}
458+
self.resps.append(HTTPResponseBuilder(status: .ok))
459+
return
460+
case "/percent%2Fencoded/hello":
455461
if req.method != .GET {
456462
self.resps.append(HTTPResponseBuilder(status: .methodNotAllowed))
457463
return

Diff for: Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift

+1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ extension HTTPClientTests {
3737
("testHttpRedirect", testHttpRedirect),
3838
("testHttpHostRedirect", testHttpHostRedirect),
3939
("testPercentEncoded", testPercentEncoded),
40+
("testPercentEncodedBackslash", testPercentEncodedBackslash),
4041
("testMultipleContentLengthHeaders", testMultipleContentLengthHeaders),
4142
("testStreaming", testStreaming),
4243
("testRemoteClose", testRemoteClose),

Diff for: Tests/AsyncHTTPClientTests/HTTPClientTests.swift

+12
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,18 @@ class HTTPClientTests: XCTestCase {
218218
XCTAssertEqual(.ok, response.status)
219219
}
220220

221+
func testPercentEncodedBackslash() throws {
222+
let httpBin = HTTPBin()
223+
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
224+
defer {
225+
XCTAssertNoThrow(try httpClient.syncShutdown(requiresCleanClose: true))
226+
XCTAssertNoThrow(try httpBin.shutdown())
227+
}
228+
229+
let response = try httpClient.get(url: "http://localhost:\(httpBin.port)/percent%2Fencoded/hello").wait()
230+
XCTAssertEqual(.ok, response.status)
231+
}
232+
221233
func testMultipleContentLengthHeaders() throws {
222234
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
223235
defer {

0 commit comments

Comments
 (0)