From 112f020c411c9d14f34e480b98325777b25a7434 Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Sat, 1 Nov 2014 11:53:05 -0400 Subject: [PATCH] Change the cache to have non-shared semantics. This means we'll cache responses that use an 'Authorization' header. This means OkHttp's cache shouldn't be used on middleboxes that sit between user agents and the origin server; in practice this is never a use case for OkHttp. Fixes https://github.com/square/okhttp/issues/1035 --- .../java/com/squareup/okhttp/CacheTest.java | 34 +------------------ .../okhttp/UrlConnectionCacheTest.java | 32 ++--------------- .../internal/huc/ResponseCacheTest.java | 32 ++--------------- .../okhttp/internal/http/CacheStrategy.java | 11 +----- 4 files changed, 6 insertions(+), 103 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 index b9fa9d036420..38a53ad9d3d4 100644 --- a/okhttp-tests/src/test/java/com/squareup/okhttp/CacheTest.java +++ b/okhttp-tests/src/test/java/com/squareup/okhttp/CacheTest.java @@ -1216,40 +1216,8 @@ private RecordedRequest assertClientSuppliedCondition(MockResponse seed, String assertEquals("", response.body().string()); } - @Test public void authorizationRequestHeaderPreventsCaching() throws Exception { + @Test public void authorizationRequestFullyCached() throws Exception { server.enqueue(new MockResponse() - .addHeader("Last-Modified: " + formatDate(-2, TimeUnit.MINUTES)) - .addHeader("Cache-Control: max-age=60") - .setBody("A")); - server.enqueue(new MockResponse() - .setBody("B")); - - URL url = server.getUrl("/"); - Request request = new Request.Builder() - .url(url) - .header("Authorization", "password") - .build(); - Response response = client.newCall(request).execute(); - assertEquals("A", response.body().string()); - assertEquals("B", get(url).body().string()); - } - - @Test public void authorizationResponseCachedWithSMaxAge() throws Exception { - assertAuthorizationRequestFullyCached( - new MockResponse().addHeader("Cache-Control: s-maxage=60")); - } - - @Test public void authorizationResponseCachedWithPublic() throws Exception { - assertAuthorizationRequestFullyCached(new MockResponse().addHeader("Cache-Control: public")); - } - - @Test public void authorizationResponseCachedWithMustRevalidate() throws Exception { - assertAuthorizationRequestFullyCached( - new MockResponse().addHeader("Cache-Control: must-revalidate")); - } - - public void assertAuthorizationRequestFullyCached(MockResponse mockResponse) throws Exception { - server.enqueue(mockResponse .addHeader("Cache-Control: max-age=60") .setBody("A")); server.enqueue(new MockResponse() diff --git a/okhttp-urlconnection/src/test/java/com/squareup/okhttp/UrlConnectionCacheTest.java b/okhttp-urlconnection/src/test/java/com/squareup/okhttp/UrlConnectionCacheTest.java index 2d6de3ebb884..e4c2f0181ad2 100644 --- a/okhttp-urlconnection/src/test/java/com/squareup/okhttp/UrlConnectionCacheTest.java +++ b/okhttp-urlconnection/src/test/java/com/squareup/okhttp/UrlConnectionCacheTest.java @@ -1098,36 +1098,8 @@ private RecordedRequest assertClientSuppliedCondition(MockResponse seed, String assertEquals("", readAscii(connection)); } - @Test public void authorizationRequestHeaderPreventsCaching() throws Exception { - server.enqueue( - new MockResponse().addHeader("Last-Modified: " + formatDate(-2, TimeUnit.MINUTES)) - .addHeader("Cache-Control: max-age=60") - .setBody("A")); - server.enqueue(new MockResponse().setBody("B")); - - URL url = server.getUrl("/"); - URLConnection connection = client.open(url); - connection.addRequestProperty("Authorization", "password"); - assertEquals("A", readAscii(connection)); - assertEquals("B", readAscii(client.open(url))); - } - - @Test public void authorizationResponseCachedWithSMaxAge() throws Exception { - assertAuthorizationRequestFullyCached( - new MockResponse().addHeader("Cache-Control: s-maxage=60")); - } - - @Test public void authorizationResponseCachedWithPublic() throws Exception { - assertAuthorizationRequestFullyCached(new MockResponse().addHeader("Cache-Control: public")); - } - - @Test public void authorizationResponseCachedWithMustRevalidate() throws Exception { - assertAuthorizationRequestFullyCached( - new MockResponse().addHeader("Cache-Control: must-revalidate")); - } - - public void assertAuthorizationRequestFullyCached(MockResponse response) throws Exception { - server.enqueue(response.addHeader("Cache-Control: max-age=60").setBody("A")); + @Test public void authorizationRequestFullyCached() throws Exception { + server.enqueue(new MockResponse().addHeader("Cache-Control: max-age=60").setBody("A")); server.enqueue(new MockResponse().setBody("B")); URL url = server.getUrl("/"); diff --git a/okhttp-urlconnection/src/test/java/com/squareup/okhttp/internal/huc/ResponseCacheTest.java b/okhttp-urlconnection/src/test/java/com/squareup/okhttp/internal/huc/ResponseCacheTest.java index 5fc68d4f3896..bf16b907ddfc 100644 --- a/okhttp-urlconnection/src/test/java/com/squareup/okhttp/internal/huc/ResponseCacheTest.java +++ b/okhttp-urlconnection/src/test/java/com/squareup/okhttp/internal/huc/ResponseCacheTest.java @@ -917,36 +917,8 @@ private RecordedRequest assertClientSuppliedCondition(MockResponse seed, String assertEquals("", readAscii(connection)); } - @Test public void authorizationRequestHeaderPreventsCaching() throws Exception { - server.enqueue( - new MockResponse().addHeader("Last-Modified: " + formatDate(-2, TimeUnit.MINUTES)) - .addHeader("Cache-Control: max-age=60") - .setBody("A")); - server.enqueue(new MockResponse().setBody("B")); - - URL url = server.getUrl("/"); - URLConnection connection = openConnection(url); - connection.addRequestProperty("Authorization", "password"); - assertEquals("A", readAscii(connection)); - assertEquals("B", readAscii(openConnection(url))); - } - - @Test public void authorizationResponseCachedWithSMaxAge() throws Exception { - assertAuthorizationRequestFullyCached( - new MockResponse().addHeader("Cache-Control: s-maxage=60")); - } - - @Test public void authorizationResponseCachedWithPublic() throws Exception { - assertAuthorizationRequestFullyCached(new MockResponse().addHeader("Cache-Control: public")); - } - - @Test public void authorizationResponseCachedWithMustRevalidate() throws Exception { - assertAuthorizationRequestFullyCached( - new MockResponse().addHeader("Cache-Control: must-revalidate")); - } - - public void assertAuthorizationRequestFullyCached(MockResponse response) throws Exception { - server.enqueue(response.addHeader("Cache-Control: max-age=60").setBody("A")); + @Test public void authorizationRequestFullyCached() throws Exception { + server.enqueue(new MockResponse().addHeader("Cache-Control: max-age=60").setBody("A")); server.enqueue(new MockResponse().setBody("B")); URL url = server.getUrl("/"); 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 index 44f745147034..70007dcb654e 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/CacheStrategy.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/CacheStrategy.java @@ -50,17 +50,8 @@ public static boolean isCacheable(Response response, Request request) { return false; } - // Responses to authorized requests aren't cacheable unless they include - // a 'public', 'must-revalidate' or 's-maxage' directive. - CacheControl responseCaching = response.cacheControl(); - if (request.header("Authorization") != null - && !responseCaching.isPublic() - && !responseCaching.mustRevalidate() - && responseCaching.sMaxAgeSeconds() == -1) { - return false; - } - // A 'no-store' directive on request or response prevents the response from being cached. + CacheControl responseCaching = response.cacheControl(); CacheControl requestCaching = request.cacheControl(); if (responseCaching.noStore() || requestCaching.noStore()) { return false;