Skip to content

Commit

Permalink
Don't crash reading non-ASCII characters from the server or the cache.
Browse files Browse the repository at this point in the history
This doesn't completely support ISO-8859-1 headers; instead they will most likely
be mangled when they are decoded as UTF-8. If we decide we absolutely must support
ISO-8859-1 here we can do that in another change. (I'm not currently enthusiastic
about this idea.)

But this does prevent OkHttp from crashing when it encounters non-ASCII characters
in headers for HTTP/2, SPDY, and cached responses.

Closes: #1998
  • Loading branch information
squarejesse committed May 28, 2016
1 parent 16aed96 commit 3ec5227
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 4 deletions.
25 changes: 25 additions & 0 deletions okhttp-tests/src/test/java/okhttp3/CacheTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2320,6 +2320,31 @@ public void assertCookies(HttpUrl url, String... expectedCookies) throws Excepti
assertEquals("v2", server.takeRequest().getHeader("If-None-Match"));
}

@Test public void combinedCacheHeadersCanBeNonAscii() throws Exception {
server.enqueue(new MockResponse()
.addHeader("Last-Modified: " + formatDate(-1, TimeUnit.HOURS))
.addHeader("Cache-Control: max-age=0")
.addHeaderLenient("Alpha", "α")
.addHeaderLenient("β", "Beta")
.setBody("abcd"));
server.enqueue(new MockResponse()
.addHeader("Transfer-Encoding: none")
.addHeaderLenient("Gamma", "Γ")
.addHeaderLenient("Δ", "Delta")
.setResponseCode(HttpURLConnection.HTTP_NOT_MODIFIED));

Response response1 = get(server.url("/"));
assertEquals("α", response1.header("Alpha"));
assertEquals("Beta", response1.header("β"));
assertEquals("abcd", response1.body().string());

Response response2 = get(server.url("/"));
assertEquals("α", response2.header("Alpha"));
assertEquals("Beta", response2.header("β"));
assertEquals("Γ", response2.header("Gamma"));
assertEquals("Delta", response2.header("Δ"));
assertEquals("abcd", response2.body().string());
}
private Response get(HttpUrl url) throws IOException {
Request request = new Request.Builder()
.url(url)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,20 @@ private void noRecoveryFromErrorWithRetryDisabled(ErrorCode errorCode) throws Ex
}
}

@Test public void nonAsciiResponseHeader() throws Exception {
server.enqueue(new MockResponse()
.addHeaderLenient("Alpha", "α")
.addHeaderLenient("β", "Beta"));

Call call = client.newCall(new Request.Builder()
.url(server.url("/"))
.build());
Response response = call.execute();

assertEquals("α", response.header("Alpha"));
assertEquals("Beta", response.header("β"));
}

public Buffer gzip(String bytes) throws IOException {
Buffer bytesOut = new Buffer();
BufferedSink sink = Okio.buffer(new GzipSink(bytesOut));
Expand Down
5 changes: 3 additions & 2 deletions okhttp/src/main/java/okhttp3/internal/http/Http2xStream.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import okhttp3.Request;
import okhttp3.Response;
import okhttp3.ResponseBody;
import okhttp3.internal.Internal;
import okhttp3.internal.Util;
import okhttp3.internal.framed.ErrorCode;
import okhttp3.internal.framed.FramedConnection;
Expand Down Expand Up @@ -233,7 +234,7 @@ public static Response.Builder readSpdy3HeadersList(List<Header> headerBlock) th
} else if (name.equals(VERSION)) {
version = value;
} else if (!SPDY_3_SKIPPED_RESPONSE_HEADERS.contains(name)) {
headersBuilder.add(name.utf8(), value);
Internal.instance.addLenient(headersBuilder, name.utf8(), value);
}
start = end + 1;
}
Expand All @@ -260,7 +261,7 @@ public static Response.Builder readHttp2HeadersList(List<Header> headerBlock) th
if (name.equals(RESPONSE_STATUS)) {
status = value;
} else if (!HTTP_2_SKIPPED_RESPONSE_HEADERS.contains(name)) {
headersBuilder.add(name.utf8(), value);
Internal.instance.addLenient(headersBuilder, name.utf8(), value);
}
}
if (status == null) throw new ProtocolException("Expected ':status' header not present");
Expand Down
4 changes: 2 additions & 2 deletions okhttp/src/main/java/okhttp3/internal/http/HttpEngine.java
Original file line number Diff line number Diff line change
Expand Up @@ -892,7 +892,7 @@ private static Headers combine(Headers cachedHeaders, Headers networkHeaders) th
continue; // Drop 100-level freshness warnings.
}
if (!OkHeaders.isEndToEnd(fieldName) || networkHeaders.get(fieldName) == null) {
result.add(fieldName, value);
Internal.instance.addLenient(result, fieldName, value);
}
}

Expand All @@ -902,7 +902,7 @@ private static Headers combine(Headers cachedHeaders, Headers networkHeaders) th
continue; // Ignore content-length headers of validating responses.
}
if (OkHeaders.isEndToEnd(fieldName)) {
result.add(fieldName, networkHeaders.value(i));
Internal.instance.addLenient(result, fieldName, networkHeaders.value(i));
}
}

Expand Down

0 comments on commit 3ec5227

Please sign in to comment.