Enable clients to call URLs that include %2F as an escaped backslash#201
Conversation
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.
|
Can one of the admins verify this patch? |
3 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
@swift-server-bot test this please |
|
Flaky test: #175. |
|
@swift-server-bot test this please |
1 similar comment
|
@swift-server-bot test this please |
weissi
requested review from
Lukasa and
artemredkin
and removed request for
Lukasa
Lukasa
left a comment
Lukasa
left a comment
There was a problem hiding this comment.
This looks reasonable to me, assuming we can get the dang tests to pass.
|
@swift-server-bot test this please |
|
@Lukasa @artemredkin, I just rechecked this locally (disabling the flakey test) and I noticed that I'm hitting a few legitimate failures on let request6 = try Request(url: "https://someserver.com:8888/some/path/")
XCTAssertEqual(request6.url.uri, "/some/path/") // XCTAssertEqual failed: ("/some/path//") is not equal to ("/some/path/") |
|
|
||
| var uri: String { | ||
| var uri = self.percentEncodedPath | ||
| if self.pathHasTrailingSlash, uri != "/" { |
There was a problem hiding this comment.
So it seems this case is handled correctly through the URLComponenets.percentEncodedPath approach. This behaviour looks well tested in testRequestURITrailingSlash which should now be passing (at least on iOS and macOS).
|
I pushed another commit that removes some redundant code. Would one of you mind re-triggering CI? Also happy to squash the commits if that is preferred here. |
|
@swift-server-bot test this please |
5 participants
Hi 👋, thanks for all the great work on this project.
I have a package that calls the Travis API, which uses %2F as part of the URL path segment. This PR adds a test case & migrates to
URLComponents(url: self, resolvingAgainstBaseURL: false)?.percentEncodedPathwhich covers that use case.I also updated the test server to switch on the
percentEncodedPathso it's easierto understand the desired behaviour. I've tested this locally on 10.15.4 but not on Ubuntu.