Skip to content

Commit

Permalink
Reapply: Relax validation of HTTP header values to retain L/M behavior
Browse files Browse the repository at this point in the history
This reapplies AOSP commit 3c28a13
There is a chance that we may want to revert this change in future.

Previous commit message:

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: Id683ec13142f1d7d8792143066f1dd2e5f62cf86
  • Loading branch information
nfuller authored and 15characterlimi committed Jun 29, 2016
1 parent 637ebbf commit 86a4f0d
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 @@ -2839,11 +2839,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 86a4f0d

Please sign in to comment.