HTTPCLIENT-1370: Response to non-GET requests should never be cached with the default

ResponseCachingPolicy
Contributed by James Leigh <james at 3roundstones dot com>


git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@1512552 13f79535-47bb-0310-9956-ffa450edef68

RELEASE_NOTES.txt

@@ -1,6 +1,11 @@
 
 Changes since release 4.3 BETA2
 -------------------
+
+* [HTTPCLIENT-1370] Response to non-GET requests should never be cached with the default
+  ResponseCachingPolicy
+  Contributed by James Leigh <james at 3roundstones dot com>
+
 * [HTTPCLIENT-1373] OPTIONS and TRACE should not invalidate cache
   Contributed by James Leigh <james at 3roundstones dot com>
 

httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java

@@ -53,6 +53,9 @@ import org.apache.http.protocol.HTTP;
 @Immutable
 class ResponseCachingPolicy {
 
+    private static final String[] AUTH_CACHEABLE_PARAMS = {
+            "s-maxage", HeaderConstants.CACHE_CONTROL_MUST_REVALIDATE, HeaderConstants.PUBLIC
+    };
     private final long maxObjectSizeBytes;
     private final boolean sharedCache;
     private final boolean neverCache1_0ResponsesWithQueryString;
@@ -258,11 +261,9 @@ class ResponseCachingPolicy {
 
         if (sharedCache) {
             final Header[] authNHeaders = request.getHeaders(HeaderConstants.AUTHORIZATION);
-            if (authNHeaders != null && authNHeaders.length > 0) {
+            if (authNHeaders != null && authNHeaders.length > 0
-                final String[] authCacheableParams = {
+                    && !hasCacheControlParameterFrom(response, AUTH_CACHEABLE_PARAMS)) {
-                        "s-maxage", HeaderConstants.CACHE_CONTROL_MUST_REVALIDATE, HeaderConstants.PUBLIC
+                return false;
-                };
-                return hasCacheControlParameterFrom(response, authCacheableParams);
             }
         }
 

httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExecChain.java

@@ -66,6 +66,7 @@ import org.apache.http.client.cache.HttpCacheStorage;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpExecutionAware;
 import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpOptions;
 import org.apache.http.client.methods.HttpRequestWrapper;
 import org.apache.http.client.methods.HttpUriRequest;
 import org.apache.http.client.protocol.HttpClientContext;
@@ -786,6 +787,41 @@ public abstract class TestCachingExecChain {
 
     }
 
+    @Test
+    public void testReturns200ForOptionsFollowedByGetIfAuthorizationHeaderAndSharedCache()
+            throws Exception {
+        impl = createCachingExecChain(mockBackend, new BasicHttpCache(), CacheConfig.custom().setSharedCache(true).build());
+        final Date now = new Date();
+        final HttpRequestWrapper req1 = HttpRequestWrapper.wrap(new HttpOptions("http://foo.example.com/"));
+        req1.setHeader("Authorization", "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==");
+        final HttpRequestWrapper req2 = HttpRequestWrapper.wrap(new HttpGet("http://foo.example.com/"));
+        req2.setHeader("Authorization", "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==");
+        final HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+                HttpStatus.SC_NO_CONTENT, "No Content");
+        resp1.setHeader("Content-Length", "0");
+        resp1.setHeader("ETag", "\"options-etag\"");
+        resp1.setHeader("Date", DateUtils.formatDate(now));
+        resp1.setHeader("Cache-Control", "public, max-age=3600");
+        resp1.setHeader("Last-Modified", DateUtils.formatDate(now));
+        final HttpResponse resp2 = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+                HttpStatus.SC_OK, "OK");
+        resp1.setEntity(HttpTestUtils.makeBody(128));
+        resp1.setHeader("Content-Length", "128");
+        resp1.setHeader("ETag", "\"get-etag\"");
+        resp1.setHeader("Date", DateUtils.formatDate(now));
+        resp1.setHeader("Cache-Control", "public, max-age=3600");
+        resp1.setHeader("Last-Modified", DateUtils.formatDate(now));
+
+        backendExpectsAnyRequestAndReturn(resp1);
+        backendExpectsAnyRequestAndReturn(resp2);
+
+        replayMocks();
+        impl.execute(route, req1, context, null);
+        final HttpResponse result = impl.execute(route, req2, context, null);
+        verifyMocks();
+        Assert.assertEquals(200, result.getStatusLine().getStatusCode());
+    }
+
     @Test
     public void testSetsValidatedContextIfRequestWasSuccessfullyValidated()
         throws Exception {

httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestResponseCachingPolicy.java

@@ -34,6 +34,7 @@ import org.apache.http.HttpResponse;
 import org.apache.http.HttpStatus;
 import org.apache.http.HttpVersion;
 import org.apache.http.ProtocolVersion;
+import org.apache.http.client.methods.HttpOptions;
 import org.apache.http.client.utils.DateUtils;
 import org.apache.http.message.BasicHttpRequest;
 import org.apache.http.message.BasicHttpResponse;
@@ -133,6 +134,16 @@ public class TestResponseCachingPolicy {
         Assert.assertFalse(policy.isResponseCacheable("GET", response));
     }
 
+    @Test
+    public void test206ResponseCodeIsNotCacheableUsingSharedPublicCache() {
+        policy = new ResponseCachingPolicy(0, true, false, false);
+
+        request.setHeader("Authorization", "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==");
+        response.setStatusCode(HttpStatus.SC_PARTIAL_CONTENT);
+        response.setHeader("Cache-Control", "public");
+        Assert.assertFalse(policy.isResponseCacheable(request, response));
+    }
+
     @Test
     public void test300ResponseCodeIsCacheable() {
         response.setStatusCode(HttpStatus.SC_MULTIPLE_CHOICES);
@@ -327,6 +338,16 @@ public class TestResponseCachingPolicy {
         Assert.assertFalse(policy.isResponseCacheable("GET", response));
     }
 
+    @Test
+    public void testVaryStarIsNotCacheableUsingSharedPublicCache() {
+        policy = new ResponseCachingPolicy(0, true, false, false);
+
+        request.setHeader("Authorization", "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==");
+        response.setHeader("Cache-Control", "public");
+        response.setHeader("Vary", "*");
+        Assert.assertFalse(policy.isResponseCacheable(request, response));
+    }
+
     @Test
     public void testIsGetWithVaryHeaderCacheable() {
         response.addHeader("Vary", "Accept-Encoding");
@@ -341,6 +362,18 @@ public class TestResponseCachingPolicy {
         Assert.assertFalse(policy.isResponseCacheable("get", response));
     }
 
+    @Test
+    public void testIsArbitraryMethodCacheableUsingSharedPublicCache() {
+        policy = new ResponseCachingPolicy(0, true, false, false);
+
+        request = new HttpOptions("http://foo.example.com/");
+        request.setHeader("Authorization", "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==");
+        response.setStatusCode(HttpStatus.SC_NO_CONTENT);
+        response.setHeader("Cache-Control", "public");
+
+        Assert.assertFalse(policy.isResponseCacheable(request, response));
+    }
+
     @Test
     public void testResponsesToRequestsWithNoStoreAreNotCacheable() {
         request.setHeader("Cache-Control","no-store");
@@ -355,6 +388,17 @@ public class TestResponseCachingPolicy {
         Assert.assertFalse(policy.isResponseCacheable("GET", response));
     }
 
+    @Test
+    public void testResponsesWithMultipleAgeHeadersAreNotCacheableUsingSharedPublicCache() {
+        policy = new ResponseCachingPolicy(0, true, false, false);
+
+        request.setHeader("Authorization", "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==");
+        response.setHeader("Cache-Control", "public");
+        response.addHeader("Age", "3");
+        response.addHeader("Age", "5");
+        Assert.assertFalse(policy.isResponseCacheable(request, response));
+    }
+
     @Test
     public void testResponsesWithMultipleDateHeadersAreNotCacheable() {
         response.addHeader("Date", DateUtils.formatDate(now));
@@ -362,12 +406,33 @@ public class TestResponseCachingPolicy {
         Assert.assertFalse(policy.isResponseCacheable("GET", response));
     }
 
+    @Test
+    public void testResponsesWithMultipleDateHeadersAreNotCacheableUsingSharedPublicCache() {
+        policy = new ResponseCachingPolicy(0, true, false, false);
+
+        request.setHeader("Authorization", "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==");
+        response.setHeader("Cache-Control", "public");
+        response.addHeader("Date", DateUtils.formatDate(now));
+        response.addHeader("Date", DateUtils.formatDate(sixSecondsAgo));
+        Assert.assertFalse(policy.isResponseCacheable(request, response));
+    }
+
     @Test
     public void testResponsesWithMalformedDateHeadersAreNotCacheable() {
         response.addHeader("Date", "garbage");
         Assert.assertFalse(policy.isResponseCacheable("GET", response));
     }
 
+    @Test
+    public void testResponsesWithMalformedDateHeadersAreNotCacheableUsingSharedPublicCache() {
+        policy = new ResponseCachingPolicy(0, true, false, false);
+
+        request.setHeader("Authorization", "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==");
+        response.setHeader("Cache-Control", "public");
+        response.addHeader("Date", "garbage");
+        Assert.assertFalse(policy.isResponseCacheable(request, response));
+    }
+
     @Test
     public void testResponsesWithMultipleExpiresHeadersAreNotCacheable() {
         final Date now = new Date();
@@ -377,6 +442,19 @@ public class TestResponseCachingPolicy {
         Assert.assertFalse(policy.isResponseCacheable("GET", response));
     }
 
+    @Test
+    public void testResponsesWithMultipleExpiresHeadersAreNotCacheableUsingSharedPublicCache() {
+        policy = new ResponseCachingPolicy(0, true, false, false);
+
+        request.setHeader("Authorization", "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==");
+        response.setHeader("Cache-Control", "public");
+        final Date now = new Date();
+        final Date sixSecondsAgo = new Date(now.getTime() - 6 * 1000L);
+        response.addHeader("Expires", DateUtils.formatDate(now));
+        response.addHeader("Expires", DateUtils.formatDate(sixSecondsAgo));
+        Assert.assertFalse(policy.isResponseCacheable(request, response));
+    }
+
     @Test
     public void testResponsesWithoutDateHeadersAreNotCacheable() {
         response.removeHeaders("Date");
@@ -389,6 +467,16 @@ public class TestResponseCachingPolicy {
         Assert.assertFalse(policy.isResponseCacheable("GET", response));
     }
 
+    @Test
+    public void testResponseThatHasTooMuchContentIsNotCacheableUsingSharedPublicCache() {
+        policy = new ResponseCachingPolicy(0, true, false, false);
+
+        request.setHeader("Authorization", "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==");
+        response.setHeader("Cache-Control", "public");
+        response.setHeader("Content-Length", "9000");
+        Assert.assertFalse(policy.isResponseCacheable(request, response));
+    }
+
     @Test
     public void testResponsesThatAreSmallEnoughAreCacheable() {
         response.setHeader("Content-Length", "0");