From 75687ca5ae54f417afb4c02ba04767da6786d829 Mon Sep 17 00:00:00 2001 From: Neil Fuller Date: Wed, 25 May 2016 12:01:29 +0100 Subject: [PATCH] Relax validation of HTTP header values to retain L/M behavior 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: https://github.com/square/okhttp/pull/1785 See also: https://github.com/square/okhttp/issues/1998 https://github.com/square/okhttp/issues/2016 for recent upstream bugs. Bug: 28867041 Bug: https://code.google.com/p/android/issues/detail?id=210205 Change-Id: Ibdf14d819411a12fcc78d012bfca97db048b7e6e --- .../test/java/com/squareup/okhttp/RequestTest.java | 7 +++++-- .../java/com/squareup/okhttp/URLConnectionTest.java | 13 ++++++++----- .../src/main/java/com/squareup/okhttp/Headers.java | 7 ++++++- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/RequestTest.java b/okhttp-tests/src/test/java/com/squareup/okhttp/RequestTest.java index 40f4f0f..47c6010 100644 --- a/okhttp-tests/src/test/java/com/squareup/okhttp/RequestTest.java +++ b/okhttp-tests/src/test/java/com/squareup/okhttp/RequestTest.java @@ -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) { diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/URLConnectionTest.java b/okhttp-tests/src/test/java/com/squareup/okhttp/URLConnectionTest.java index 59cbc54..3598da0 100644 --- a/okhttp-tests/src/test/java/com/squareup/okhttp/URLConnectionTest.java +++ b/okhttp-tests/src/test/java/com/squareup/okhttp/URLConnectionTest.java @@ -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 { diff --git a/okhttp/src/main/java/com/squareup/okhttp/Headers.java b/okhttp/src/main/java/com/squareup/okhttp/Headers.java index dad91bf..d5b4cef 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Headers.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Headers.java @@ -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)); }