Skip to content

Commit

Permalink
Relax validation of HTTP header values to retain L/M behavior
Browse files Browse the repository at this point in the history
This relaxes some validation logic newly introduced in the version
of OkHttp used in N.

This change leaves the character code validation stricter
than it was in M by still preventing control codes like \n, \r,
backspace and delete. It does allow developers to pass
Java characters > 7F to addRequestProperty() and also receive
headers from servers which contain characters > 7F.

Android's HttpURLConnection does not follow the HTTP spec as
it encodes request header values and interprets response
headers as UTF-8 and not ISO-8859-1.

If a server is expecting or sending ISO-8859-1 encoded characters
>7F in headers then these will still be corrupted or misinterpreted
by Android. However, this has been the behavior since L and is not
changed here.

The OkHttp change which caused characters >7F to generate an
IllegalArgumentException and partially reverted here:
square/okhttp#1785

See also: square/okhttp#1998
square/okhttp#2016
for recent upstream bugs.

Bug: 28867041
Bug: https://code.google.com/p/android/issues/detail?id=210205
(cherry picked from commit 75687ca5ae54f417afb4c02ba04767da6786d829)

Change-Id: Ib640b58addff4c0c4eac589c77eb74a6bd6b3ec2
  • Loading branch information
nfuller committed May 26, 2016
1 parent 02220ee commit 3c28a13
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,11 @@ public final class RequestTest {
assertForbiddenHeader("\t");
assertForbiddenHeader("\u001f");
assertForbiddenHeader("\u007f");
assertForbiddenHeader("\u0080");
assertForbiddenHeader("\ud83c\udf69");

// ANDROID-BEGIN Workaround for http://b/28867041
// assertForbiddenHeader("\u0080");
// assertForbiddenHeader("\ud83c\udf69");
// ANDROID-END
}

private void assertForbiddenHeader(String s) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2811,11 +2811,14 @@ private void reusedConnectionFailsWithPost(TransferKind transferKind, int reques
fail();
} catch (IllegalArgumentException expected) {
}
try {
connection.addRequestProperty("Name", "\u2615\ufe0f");
fail();
} catch (IllegalArgumentException expected) {
}

// ANDROID-BEGIN Disabled for http://b/28867041
// try {
// connection.addRequestProperty("Name", "\u2615\ufe0f");
// fail();
// } catch (IllegalArgumentException expected) {
// }
// ANDROID-END
}

@Test public void responseHeaderParsingIsLenient() throws Exception {
Expand Down
7 changes: 6 additions & 1 deletion okhttp/src/main/java/com/squareup/okhttp/Headers.java
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,12 @@ private void checkNameAndValue(String name, String value) {

for (int i = 0, length = value.length(); i < length; i++) {
char c = value.charAt(i);
if (c <= '\u001f' || c >= '\u007f') {
// ANDROID-BEGIN
// http://b/28867041 - keep things working for apps that rely on Android's (out of spec)
// UTF-8 header encoding behavior.
// if (c <= '\u001f' || c >= '\u007f') {
if (c <= '\u001f' || c == '\u007f') {
// ANDROID-END
throw new IllegalArgumentException(String.format(
"Unexpected char %#04x at %d in header value: %s", (int) c, i, value));
}
Expand Down

0 comments on commit 3c28a13

Please sign in to comment.