Merge pull request square#2684 from square/jwilson.0701.non_ascii_etag

Permit non-ASCII ETag headers.
rebaze · Jul 1, 2016 · 35e29d9 · 35e29d9
2 parents b337147 + 50ff1e1
commit 35e29d9
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 10 deletions.
diff --git a/mockwebserver/src/main/java/okhttp3/mockwebserver/MockWebServer.java b/mockwebserver/src/main/java/okhttp3/mockwebserver/MockWebServer.java
@@ -58,6 +58,7 @@
 import okhttp3.Protocol;
 import okhttp3.Request;
 import okhttp3.Response;
+import okhttp3.internal.Internal;
 import okhttp3.internal.NamedRunnable;
 import okhttp3.internal.Util;
 import okhttp3.internal.framed.ErrorCode;
@@ -99,6 +100,10 @@
  * in sequence.
  */
 public final class MockWebServer implements TestRule {
+  static {
+    Internal.initializeInstanceForTests();
+  }
+
   private static final X509TrustManager UNTRUSTED_TRUST_MANAGER = new X509TrustManager() {
     @Override public void checkClientTrusted(X509Certificate[] chain, String authType)
         throws CertificateException {
@@ -590,7 +595,7 @@ private RecordedRequest readRequest(Socket socket, BufferedSource source, Buffer
     boolean expectContinue = false;
     String header;
     while ((header = source.readUtf8LineStrict()).length() != 0) {
-      headers.add(header);
+      Internal.instance.addLenient(headers, header);
       String lowercaseHeader = header.toLowerCase(Locale.US);
       if (contentLength == -1 && lowercaseHeader.startsWith("content-length:")) {
         contentLength = Long.parseLong(header.substring(15).trim());

diff --git a/okhttp-tests/src/test/java/okhttp3/CacheTest.java b/okhttp-tests/src/test/java/okhttp3/CacheTest.java
@@ -2344,6 +2344,25 @@ public void assertCookies(HttpUrl url, String... expectedCookies) throws Excepti
     assertEquals("Delta", response2.header("Δ"));
     assertEquals("abcd", response2.body().string());
   }
+
+  @Test public void etagConditionCanBeNonAscii() throws Exception {
+    server.enqueue(new MockResponse()
+        .addHeaderLenient("Etag", "α")
+        .addHeader("Cache-Control: max-age=0")
+        .setBody("abcd"));
+    server.enqueue(new MockResponse()
+        .setResponseCode(HttpURLConnection.HTTP_NOT_MODIFIED));
+
+    Response response1 = get(server.url("/"));
+    assertEquals("abcd", response1.body().string());
+
+    Response response2 = get(server.url("/"));
+    assertEquals("abcd", response2.body().string());
+
+    assertEquals(null, server.takeRequest().getHeader("If-None-Match"));
+    assertEquals("α", server.takeRequest().getHeader("If-None-Match"));
+  }
+
   private Response get(HttpUrl url) throws IOException {
     Request request = new Request.Builder()
         .url(url)

diff --git a/okhttp/src/main/java/okhttp3/internal/cache/CacheStrategy.java b/okhttp/src/main/java/okhttp3/internal/cache/CacheStrategy.java
@@ -20,6 +20,7 @@
 import okhttp3.Headers;
 import okhttp3.Request;
 import okhttp3.Response;
+import okhttp3.internal.Internal;
 import okhttp3.internal.http.HttpDate;
 import okhttp3.internal.http.HttpHeaders;
 import okhttp3.internal.http.StatusLine;
@@ -232,20 +233,30 @@ private CacheStrategy getCandidate() {
         return new CacheStrategy(null, builder.build());
       }
 
-      Request.Builder conditionalRequestBuilder = request.newBuilder();
-
+      // Find a condition to add to the request. If the condition is satisfied, the response body
+      // will not be transmitted.
+      String conditionName;
+      String conditionValue;
       if (etag != null) {
-        conditionalRequestBuilder.header("If-None-Match", etag);
+        conditionName = "If-None-Match";
+        conditionValue = etag;
       } else if (lastModified != null) {
-        conditionalRequestBuilder.header("If-Modified-Since", lastModifiedString);
+        conditionName = "If-Modified-Since";
+        conditionValue = lastModifiedString;
       } else if (servedDate != null) {
-        conditionalRequestBuilder.header("If-Modified-Since", servedDateString);
+        conditionName = "If-Modified-Since";
+        conditionValue = servedDateString;
+      } else {
+        return new CacheStrategy(request, null); // No condition! Make a regular request.
       }
 
-      Request conditionalRequest = conditionalRequestBuilder.build();
-      return hasConditions(conditionalRequest)
-          ? new CacheStrategy(conditionalRequest, cacheResponse)
-          : new CacheStrategy(conditionalRequest, null);
+      Headers.Builder conditionalRequestHeaders = request.headers().newBuilder();
+      Internal.instance.addLenient(conditionalRequestHeaders, conditionName, conditionValue);
+
+      Request conditionalRequest = request.newBuilder()
+          .headers(conditionalRequestHeaders.build())
+          .build();
+      return new CacheStrategy(conditionalRequest, cacheResponse);
     }
 
     /**