From ed43e07ffb3bb95708fe9840cf9fc2f523b2f1e5 Mon Sep 17 00:00:00 2001 From: jwilson Date: Wed, 13 Jan 2016 13:47:43 -0800 Subject: [PATCH] Parse the CookieHandler request cookies in JavaNetCookieJar. This brings the tests to all pass on Android. Closes: https://github.com/square/okhttp/issues/2202 --- .../okhttp3/internal/http/CookiesTest.java | 52 +++++++++++++++---- .../main/java/okhttp3/JavaNetCookieJar.java | 31 +++++++---- 2 files changed, 62 insertions(+), 21 deletions(-) diff --git a/okhttp-tests/src/test/java/okhttp3/internal/http/CookiesTest.java b/okhttp-tests/src/test/java/okhttp3/internal/http/CookiesTest.java index 7bfd773e1..f349df474 100644 --- a/okhttp-tests/src/test/java/okhttp3/internal/http/CookiesTest.java +++ b/okhttp-tests/src/test/java/okhttp3/internal/http/CookiesTest.java @@ -17,6 +17,7 @@ package okhttp3.internal.http; import java.io.IOException; +import java.net.CookieHandler; import java.net.CookieManager; import java.net.HttpCookie; import java.net.HttpURLConnection; @@ -28,9 +29,9 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import okhttp3.HttpUrl; +import okhttp3.JavaNetCookieJar; import okhttp3.OkHttpClient; import okhttp3.OkUrlFactory; -import okhttp3.JavaNetCookieJar; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; import okhttp3.mockwebserver.RecordedRequest; @@ -143,21 +144,50 @@ public class CookiesTest { MockWebServer server = new MockWebServer(); server.enqueue(new MockResponse()); server.start(); + HttpUrl serverUrl = urlWithIpAddress(server, "/"); CookieManager cookieManager = new CookieManager(null, ACCEPT_ORIGINAL_SERVER); HttpCookie cookieA = new HttpCookie("a", "android"); - cookieA.setDomain(server.getHostName()); + cookieA.setDomain(serverUrl.host()); cookieA.setPath("/"); - cookieManager.getCookieStore().add(server.url("/").uri(), cookieA); + cookieManager.getCookieStore().add(serverUrl.uri(), cookieA); HttpCookie cookieB = new HttpCookie("b", "banana"); - cookieB.setDomain(server.getHostName()); + cookieB.setDomain(serverUrl.host()); cookieB.setPath("/"); - cookieManager.getCookieStore().add(server.url("/").uri(), cookieB); + cookieManager.getCookieStore().add(serverUrl.uri(), cookieB); client = client.newBuilder() .cookieJar(new JavaNetCookieJar(cookieManager)) .build(); - get(server.url("/")); + get(serverUrl); + RecordedRequest request = server.takeRequest(); + + assertEquals("a=android; b=banana", request.getHeader("Cookie")); + } + + @Test public void cookieHandlerLikeAndroid() throws Exception { + final MockWebServer server = new MockWebServer(); + server.enqueue(new MockResponse()); + server.start(); + final HttpUrl serverUrl = urlWithIpAddress(server, "/"); + + CookieHandler androidCookieHandler = new CookieHandler() { + @Override public Map> get(URI uri, Map> map) + throws IOException { + return Collections.singletonMap("Cookie", Collections.singletonList("$Version=\"1\"; " + + "a=\"android\";$Path=\"/\";$Domain=\"" + serverUrl.host() + "\"; " + + "b=\"banana\";$Path=\"/\";$Domain=\"" + serverUrl.host() + "\"")); + } + + @Override public void put(URI uri, Map> map) throws IOException { + } + }; + + client = client.newBuilder() + .cookieJar(new JavaNetCookieJar(androidCookieHandler)) + .build(); + + get(serverUrl); RecordedRequest request = server.takeRequest(); assertEquals("a=android; b=banana", request.getHeader("Cookie")); @@ -167,25 +197,27 @@ public class CookiesTest { MockWebServer redirectTarget = new MockWebServer(); redirectTarget.enqueue(new MockResponse().setBody("A")); redirectTarget.start(); + HttpUrl redirectTargetUrl = urlWithIpAddress(redirectTarget, "/"); MockWebServer redirectSource = new MockWebServer(); redirectSource.enqueue(new MockResponse() .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP) - .addHeader("Location: " + redirectTarget.url("/"))); + .addHeader("Location: " + redirectTargetUrl)); redirectSource.start(); + HttpUrl redirectSourceUrl = urlWithIpAddress(redirectSource, "/"); CookieManager cookieManager = new CookieManager(null, ACCEPT_ORIGINAL_SERVER); HttpCookie cookie = new HttpCookie("c", "cookie"); - cookie.setDomain(redirectSource.getHostName()); + cookie.setDomain(redirectSourceUrl.host()); cookie.setPath("/"); String portList = Integer.toString(redirectSource.getPort()); cookie.setPortlist(portList); - cookieManager.getCookieStore().add(redirectSource.url("/").uri(), cookie); + cookieManager.getCookieStore().add(redirectSourceUrl.uri(), cookie); client = client.newBuilder() .cookieJar(new JavaNetCookieJar(cookieManager)) .build(); - get(redirectSource.url("/")); + get(redirectSourceUrl); RecordedRequest request = redirectSource.takeRequest(); assertEquals("c=cookie", request.getHeader("Cookie")); diff --git a/okhttp-urlconnection/src/main/java/okhttp3/JavaNetCookieJar.java b/okhttp-urlconnection/src/main/java/okhttp3/JavaNetCookieJar.java index 675e16051..3d9daf64b 100644 --- a/okhttp-urlconnection/src/main/java/okhttp3/JavaNetCookieJar.java +++ b/okhttp-urlconnection/src/main/java/okhttp3/JavaNetCookieJar.java @@ -25,6 +25,8 @@ import java.util.Map; import okhttp3.internal.Internal; import static java.util.logging.Level.WARNING; +import static okhttp3.internal.Util.delimiterOffset; +import static okhttp3.internal.Util.trimSubstring; /** A cookie jar that delegates to a {@link java.net.CookieHandler}. */ public final class JavaNetCookieJar implements CookieJar { @@ -82,19 +84,26 @@ public final class JavaNetCookieJar implements CookieJar { * multiple cookies in a single request header, which {@link Cookie#parse} doesn't support. */ private List decodeHeaderAsJavaNetCookies(HttpUrl url, String header) { - List javaNetCookies; - try { - javaNetCookies = HttpCookie.parse(header); - } catch (IllegalArgumentException e) { - // Unfortunately sometimes java.net gives a Cookie like "$Version=1" which it can't parse! - Internal.logger.log(WARNING, "Parsing request cookie failed for " + url.resolve("/..."), e); - return Collections.emptyList(); - } List result = new ArrayList<>(); - for (HttpCookie javaNetCookie : javaNetCookies) { + for (int pos = 0, limit = header.length(), pairEnd; pos < limit; pos = pairEnd + 1) { + pairEnd = delimiterOffset(header, pos, limit, ";,"); + int equalsSign = delimiterOffset(header, pos, pairEnd, '='); + String name = trimSubstring(header, pos, equalsSign); + if (name.startsWith("$")) continue; + + // We have either name=value or just a name. + String value = equalsSign < pairEnd + ? trimSubstring(header, equalsSign + 1, pairEnd) + : ""; + + // If the value is "quoted", drop the quotes. + if (value.startsWith("\"") && value.endsWith("\"")) { + value = value.substring(1, value.length() - 1); + } + result.add(new Cookie.Builder() - .name(javaNetCookie.getName()) - .value(javaNetCookie.getValue()) + .name(name) + .value(value) .domain(url.host()) .build()); }