From c74831d62087e59f889497e6e7c4e6bb19196a1f Mon Sep 17 00:00:00 2001 From: jwilson Date: Tue, 10 Jan 2017 23:05:58 -0500 Subject: [PATCH] Drop cookies that contain ASCII NULL and other bad characters. Closes https://github.com/square/okhttp/issues/2939 --- .../src/test/java/okhttp3/CookieTest.java | 25 +++++++++++++++++++ okhttp/src/main/java/okhttp3/Cookie.java | 4 ++- okhttp/src/main/java/okhttp3/Headers.java | 2 +- .../src/main/java/okhttp3/internal/Util.java | 22 ++++++++++------ 4 files changed, 44 insertions(+), 9 deletions(-) diff --git a/okhttp-tests/src/test/java/okhttp3/CookieTest.java b/okhttp-tests/src/test/java/okhttp3/CookieTest.java index bee549dc7..e36fc0c82 100644 --- a/okhttp-tests/src/test/java/okhttp3/CookieTest.java +++ b/okhttp-tests/src/test/java/okhttp3/CookieTest.java @@ -50,6 +50,14 @@ public final class CookieTest { assertNull(Cookie.parse(url, "\r\t \n=b")); } + @Test public void spaceInName() throws Exception { + assertEquals("a b", Cookie.parse(url, "a b=cd").name()); + } + + @Test public void spaceInValue() throws Exception { + assertEquals("c d", Cookie.parse(url, "ab=c d").value()); + } + @Test public void trimLeadingAndTrailingWhitespaceFromName() throws Exception { assertEquals("a", Cookie.parse(url, " a=b").name()); assertEquals("a", Cookie.parse(url, "a =b").name()); @@ -69,6 +77,23 @@ public final class CookieTest { assertEquals("b", Cookie.parse(url, "a=\r\t \nb\n\t \n").value()); } + @Test public void invalidCharacters() throws Exception { + assertEquals(null, Cookie.parse(url, "a\u0000b=cd")); + assertEquals(null, Cookie.parse(url, "ab=c\u0000d")); + assertEquals(null, Cookie.parse(url, "a\u0001b=cd")); + assertEquals(null, Cookie.parse(url, "ab=c\u0001d")); + assertEquals(null, Cookie.parse(url, "a\u0009b=cd")); + assertEquals(null, Cookie.parse(url, "ab=c\u0009d")); + assertEquals(null, Cookie.parse(url, "a\u001fb=cd")); + assertEquals(null, Cookie.parse(url, "ab=c\u001fd")); + assertEquals(null, Cookie.parse(url, "a\u007fb=cd")); + assertEquals(null, Cookie.parse(url, "ab=c\u007fd")); + assertEquals(null, Cookie.parse(url, "a\u0080b=cd")); + assertEquals(null, Cookie.parse(url, "ab=c\u0080d")); + assertEquals(null, Cookie.parse(url, "a\u00ffb=cd")); + assertEquals(null, Cookie.parse(url, "ab=c\u00ffd")); + } + @Test public void maxAge() throws Exception { assertEquals(51000L, Cookie.parse(50000L, url, "a=b; Max-Age=1").expiresAt()); diff --git a/okhttp/src/main/java/okhttp3/Cookie.java b/okhttp/src/main/java/okhttp3/Cookie.java index 066c6e147..5b760b519 100644 --- a/okhttp/src/main/java/okhttp3/Cookie.java +++ b/okhttp/src/main/java/okhttp3/Cookie.java @@ -30,6 +30,7 @@ import okhttp3.internal.http.HttpDate; import static okhttp3.internal.Util.UTC; import static okhttp3.internal.Util.delimiterOffset; import static okhttp3.internal.Util.domainToAscii; +import static okhttp3.internal.Util.indexOfControlOrNonAscii; import static okhttp3.internal.Util.trimSubstring; import static okhttp3.internal.Util.verifyAsIpAddress; @@ -227,9 +228,10 @@ public final class Cookie { if (pairEqualsSign == cookiePairEnd) return null; String cookieName = trimSubstring(setCookie, pos, pairEqualsSign); - if (cookieName.isEmpty()) return null; + if (cookieName.isEmpty() || indexOfControlOrNonAscii(cookieName) != -1) return null; String cookieValue = trimSubstring(setCookie, pairEqualsSign + 1, cookiePairEnd); + if (indexOfControlOrNonAscii(cookieValue) != -1) return null; long expiresAt = HttpDate.MAX_DATE; long deltaSeconds = -1L; diff --git a/okhttp/src/main/java/okhttp3/Headers.java b/okhttp/src/main/java/okhttp3/Headers.java index 5bdacfbd4..fb007a0a1 100644 --- a/okhttp/src/main/java/okhttp3/Headers.java +++ b/okhttp/src/main/java/okhttp3/Headers.java @@ -316,7 +316,7 @@ public final class Headers { if (value == null) throw new NullPointerException("value == null"); for (int i = 0, length = value.length(); i < length; i++) { char c = value.charAt(i); - if ((c <= '\u001f' && c != '\u0009' /* htab */) || c >= '\u007f') { + if ((c <= '\u001f' && c != '\t') || c >= '\u007f') { throw new IllegalArgumentException(Util.format( "Unexpected char %#04x at %d in %s value: %s", (int) c, i, name, value)); } diff --git a/okhttp/src/main/java/okhttp3/internal/Util.java b/okhttp/src/main/java/okhttp3/internal/Util.java index 28c0b7938..5a88c8899 100644 --- a/okhttp/src/main/java/okhttp3/internal/Util.java +++ b/okhttp/src/main/java/okhttp3/internal/Util.java @@ -383,6 +383,21 @@ public final class Util { return false; } + /** + * Returns the index of the first character in {@code input} that is either a control character + * (like {@code \u0000 or \n}) or a non-ASCII character. Returns -1 if {@code input} has no such + * characters. + */ + public static int indexOfControlOrNonAscii(String input) { + for (int i = 0, length = input.length(); i < length; i++) { + char c = input.charAt(i); + if (c <= '\u001f' || c >= '\u007f') { + return i; + } + } + return -1; + } + /** Returns true if {@code host} is not a host name and might be an IP address. */ public static boolean verifyAsIpAddress(String host) { return VERIFY_AS_IP_ADDRESS.matcher(host).matches(); @@ -416,11 +431,4 @@ public final class Util { } return charset; } - - /** Re-throws {@code t} if it is a fatal exception which should not be handled. */ - public static void throwIfFatal(Throwable t) { - if (t instanceof VirtualMachineError) throw (VirtualMachineError) t; - if (t instanceof ThreadDeath) throw (ThreadDeath) t; - if (t instanceof LinkageError) throw (LinkageError) t; - } }