Skip to content

Commit

Permalink
Fix a crash in HttpUrl.toUri().
Browse files Browse the repository at this point in the history 
Closes: #5667
Closes: #5236
  • Loading branch information
@squarejesse
squarejesse committed Dec 31, 2019
1 parent b9e0422 commit 283418b
Show file tree
Hide file tree
Showing 3 changed files with 222 additions and 198 deletions.
4 changes: 4 additions & 0 deletions okhttp/src/main/java/okhttp3/HttpUrl.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1140,13 +1140,16 @@ class HttpUrl internal constructor(
* particularly strict for certain components.
*/
internal fun reencodeForUri() = apply {
host = host?.replace(Regex("[\"<>^`{|}]"), "")

for (i in 0 until encodedPathSegments.size) {
encodedPathSegments[i] = encodedPathSegments[i].canonicalize(
encodeSet = PATH_SEGMENT_ENCODE_SET_URI,
alreadyEncoded = true,
strict = true
)
}

val encodedQueryNamesAndValues = this.encodedQueryNamesAndValues
if (encodedQueryNamesAndValues != null) {
for (i in 0 until encodedQueryNamesAndValues.size) {
Expand All @@ -1158,6 +1161,7 @@ class HttpUrl internal constructor(
)
}
}

encodedFragment = encodedFragment?.canonicalize(
encodeSet = FRAGMENT_ENCODE_SET_URI,
alreadyEncoded = true,
Expand Down
65 changes: 52 additions & 13 deletions okhttp/src/test/java/okhttp3/HttpUrlTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,18 +365,18 @@ HttpUrl parse(String url) {
}

@Test public void usernameCharacters() throws Exception {
new UrlComponentEncodingTester()
UrlComponentEncodingTester.newInstance()
.override(Encoding.PERCENT, '[', ']', '{', '}', '|', '^', '\'', ';', '=', '@')
.override(Encoding.SKIP, ':', '/', '\\', '?', '#')
.skipForUri('%')
.escapeForUri('%')
.test(Component.USER);
}

@Test public void passwordCharacters() throws Exception {
new UrlComponentEncodingTester()
UrlComponentEncodingTester.newInstance()
.override(Encoding.PERCENT, '[', ']', '{', '}', '|', '^', '\'', ':', ';', '=', '@')
.override(Encoding.SKIP, '/', '\\', '?', '#')
.skipForUri('%')
.escapeForUri('%')
.test(Component.PASSWORD);
}

Expand Down Expand Up @@ -419,6 +419,27 @@ HttpUrl parse(String url) {
assertInvalid("http://\uDBFF\uDFFF", "Invalid URL host: \"\uDBFF\uDFFF\"");
}

@Test public void hostnameUri() throws Exception {
// Host names are special:
//
// * Several characters are forbidden and must throw exceptions if used.
// * They don't use percent escaping at all.
// * They use punycode for internationalization.
// * URI is much more strict that HttpUrl or URL on what's accepted.
//
// HttpUrl is quite lenient with what characters it accepts. In particular, characters like '{'
// and '"' are permitted but unlikely to occur in real-world URLs. Unfortunately we can't just
// lock it down due to URL templating: "http://{env}.{dc}.example.com".
UrlComponentEncodingTester.newInstance()
.nonPrintableAscii(Encoding.FORBIDDEN)
.nonAscii(Encoding.FORBIDDEN)
.override(Encoding.FORBIDDEN, '\t', '\n', '\f', '\r', ' ')
.override(Encoding.FORBIDDEN, '#', '%', '/', ':', '?', '@', '[', '\\', ']')
.override(Encoding.IDENTITY, '\"', '<', '>', '^', '`', '{', '|', '}')
.stripForUri('\"', '<', '>', '^', '`', '{', '|', '}')
.test(Component.HOST);
}

@Test public void hostIpv6() throws Exception {
// Square braces are absent from host()...
assertThat(parse("http://[::1]/").host()).isEqualTo("::1");
Expand Down Expand Up @@ -623,6 +644,24 @@ HttpUrl parse(String url) {
assertThat(parse("http://host./").host()).isEqualTo("host.");
}

/**
* Strip unexpected characters when converting to URI (which is more strict).
* https://github.com/square/okhttp/issues/5667
*/
@Test public void hostToUriStripsCharacters() throws Exception {
HttpUrl httpUrl = HttpUrl.get("http://example\".com/");
assertThat(httpUrl.uri().toString()).isEqualTo("http://example.com/");
}

/**
* Confirm that URI retains other characters.
* https://github.com/square/okhttp/issues/5236
*/
@Test public void hostToUriStripsCharacters2() throws Exception {
HttpUrl httpUrl = HttpUrl.get("http://${tracker}/");
assertThat(httpUrl.uri().toString()).isEqualTo("http://$tracker/");
}

@Test public void port() throws Exception {
assertThat(parse("http://host:80/")).isEqualTo(parse("http://host/"));
assertThat(parse("http://host:99/")).isEqualTo(parse("http://host:99/"));
Expand All @@ -636,36 +675,36 @@ HttpUrl parse(String url) {
}

@Test public void pathCharacters() throws Exception {
new UrlComponentEncodingTester()
UrlComponentEncodingTester.newInstance()
.override(Encoding.PERCENT, '^', '{', '}', '|')
.override(Encoding.SKIP, '\\', '?', '#')
.skipForUri('%', '[', ']')
.escapeForUri('%', '[', ']')
.test(Component.PATH);
}

@Test public void queryCharacters() throws Exception {
new UrlComponentEncodingTester()
UrlComponentEncodingTester.newInstance()
.override(Encoding.IDENTITY, '?', '`')
.override(Encoding.PERCENT, '\'')
.override(Encoding.SKIP, '#', '+')
.skipForUri('%', '\\', '^', '`', '{', '|', '}')
.escapeForUri('%', '\\', '^', '`', '{', '|', '}')
.test(Component.QUERY);
}

@Test public void queryValueCharacters() throws Exception {
new UrlComponentEncodingTester()
UrlComponentEncodingTester.newInstance()
.override(Encoding.IDENTITY, '?', '`')
.override(Encoding.PERCENT, '\'')
.override(Encoding.SKIP, '#', '+')
.skipForUri('%', '\\', '^', '`', '{', '|', '}')
.escapeForUri('%', '\\', '^', '`', '{', '|', '}')
.test(Component.QUERY_VALUE);
}

@Test public void fragmentCharacters() throws Exception {
new UrlComponentEncodingTester()
UrlComponentEncodingTester.newInstance()
.override(Encoding.IDENTITY, ' ', '"', '#', '<', '>', '?', '`')
.skipForUri('%', ' ', '"', '#', '<', '>', '\\', '^', '`', '{', '|', '}')
.identityForNonAscii()
.escapeForUri('%', ' ', '"', '#', '<', '>', '\\', '^', '`', '{', '|', '}')
.nonAscii(Encoding.IDENTITY)
.test(Component.FRAGMENT);
}

Expand Down
Loading

0 comments on commit 283418b

Please sign in to comment.