Fix request cache-control to match spec.

Fixes square#1081
huntervwang · Nov 1, 2014 · ddac008 · ddac008
1 parent a4c0d59
commit ddac008
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 9 deletions.
diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/CacheTest.java b/okhttp-tests/src/test/java/com/squareup/okhttp/CacheTest.java
@@ -825,6 +825,28 @@ private void testMethodInvalidates(String requestMethod) throws Exception {
     assertEquals("A", get(url).body().string());
   }
 
+  @Test public void clientSideNoStore() throws Exception {
+    server.enqueue(new MockResponse()
+        .addHeader("Cache-Control: max-age=60")
+        .setBody("A"));
+    server.enqueue(new MockResponse()
+        .addHeader("Cache-Control: max-age=60")
+        .setBody("B"));
+
+    Request request1 = new Request.Builder()
+        .url(server.getUrl("/"))
+        .cacheControl(new CacheControl.Builder().noStore().build())
+        .build();
+    Response response1 = client.newCall(request1).execute();
+    assertEquals("A", response1.body().string());
+
+    Request request2 = new Request.Builder()
+        .url(server.getUrl("/"))
+        .build();
+    Response response2 = client.newCall(request2).execute();
+    assertEquals("B", response2.body().string());
+  }
+
   @Test public void nonIdentityEncodingAndConditionalCache() throws Exception {
     assertNonIdentityEncodingCached(
         new MockResponse().addHeader("Last-Modified: " + formatDate(-2, TimeUnit.HOURS))
@@ -958,6 +980,27 @@ private void assertNonIdentityEncodingCached(MockResponse response) throws Excep
     assertEquals("110 HttpURLConnection \"Response is stale\"", response.header("Warning"));
   }
 
+  @Test public void requestMaxStaleDirectiveWithNoValue() throws IOException {
+    // Add a stale response to the cache.
+    server.enqueue(new MockResponse()
+        .setBody("A")
+        .addHeader("Cache-Control: max-age=120")
+        .addHeader("Date: " + formatDate(-4, TimeUnit.MINUTES)));
+    server.enqueue(new MockResponse()
+        .setBody("B"));
+
+    assertEquals("A", get(server.getUrl("/")).body().string());
+
+    // With max-stale, we'll return that stale response.
+    Request request = new Request.Builder()
+        .url(server.getUrl("/"))
+        .header("Cache-Control", "max-stale")
+        .build();
+    Response response = client.newCall(request).execute();
+    assertEquals("A", response.body().string());
+    assertEquals("110 HttpURLConnection \"Response is stale\"", response.header("Warning"));
+  }
+
   @Test public void requestMaxStaleNotHonoredWithMustRevalidate() throws IOException {
     server.enqueue(new MockResponse()
         .setBody("A")

diff --git a/okhttp/src/main/java/com/squareup/okhttp/CacheControl.java b/okhttp/src/main/java/com/squareup/okhttp/CacheControl.java
@@ -192,17 +192,17 @@ public static CacheControl parse(Headers headers) {
         } else if ("no-store".equalsIgnoreCase(directive)) {
           noStore = true;
         } else if ("max-age".equalsIgnoreCase(directive)) {
-          maxAgeSeconds = HeaderParser.parseSeconds(parameter);
+          maxAgeSeconds = HeaderParser.parseSeconds(parameter, -1);
         } else if ("s-maxage".equalsIgnoreCase(directive)) {
-          sMaxAgeSeconds = HeaderParser.parseSeconds(parameter);
+          sMaxAgeSeconds = HeaderParser.parseSeconds(parameter, -1);
         } else if ("public".equalsIgnoreCase(directive)) {
           isPublic = true;
         } else if ("must-revalidate".equalsIgnoreCase(directive)) {
           mustRevalidate = true;
         } else if ("max-stale".equalsIgnoreCase(directive)) {
-          maxStaleSeconds = HeaderParser.parseSeconds(parameter);
+          maxStaleSeconds = HeaderParser.parseSeconds(parameter, Integer.MAX_VALUE);
         } else if ("min-fresh".equalsIgnoreCase(directive)) {
-          minFreshSeconds = HeaderParser.parseSeconds(parameter);
+          minFreshSeconds = HeaderParser.parseSeconds(parameter, -1);
         } else if ("only-if-cached".equalsIgnoreCase(directive)) {
           onlyIfCached = true;
         } else if ("no-transform".equalsIgnoreCase(directive)) {

diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/CacheStrategy.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/CacheStrategy.java
@@ -60,7 +60,9 @@ public static boolean isCacheable(Response response, Request request) {
       return false;
     }
 
-    if (responseCaching.noStore()) {
+    // A 'no-store' directive on request or response prevents the response from being cached.
+    CacheControl requestCaching = request.cacheControl();
+    if (responseCaching.noStore() || requestCaching.noStore()) {
       return false;
     }
 
@@ -124,7 +126,7 @@ public Factory(long nowMillis, Request request, Response cacheResponse) {
           } else if ("ETag".equalsIgnoreCase(fieldName)) {
             etag = value;
           } else if ("Age".equalsIgnoreCase(fieldName)) {
-            ageSeconds = HeaderParser.parseSeconds(value);
+            ageSeconds = HeaderParser.parseSeconds(value, -1);
           } else if (OkHeaders.SENT_MILLIS.equalsIgnoreCase(fieldName)) {
             sentRequestMillis = Long.parseLong(value);
           } else if (OkHeaders.RECEIVED_MILLIS.equalsIgnoreCase(fieldName)) {

diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HeaderParser.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HeaderParser.java
@@ -47,9 +47,9 @@ public static int skipWhitespace(String input, int pos) {
 
   /**
    * Returns {@code value} as a positive integer, or 0 if it is negative, or
-   * -1 if it cannot be parsed.
+   * {@code defaultValue} if it cannot be parsed.
    */
-  public static int parseSeconds(String value) {
+  public static int parseSeconds(String value, int defaultValue) {
     try {
       long seconds = Long.parseLong(value);
       if (seconds > Integer.MAX_VALUE) {
@@ -60,7 +60,7 @@ public static int parseSeconds(String value) {
         return (int) seconds;
       }
     } catch (NumberFormatException e) {
-      return -1;
+      return defaultValue;
     }
   }