You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by jo...@apache.org on 2013/06/06 23:25:04 UTC

svn commit: r1490448 - in /httpcomponents/httpclient/trunk/httpclient-cache/src: main/java/org/apache/http/impl/client/cache/ test/java/org/apache/http/impl/client/cache/

Author: jonm
Date: Thu Jun  6 21:25:04 2013
New Revision: 1490448

URL: http://svn.apache.org/r1490448
Log:
HTTPCLIENT-1353: 303 redirects should be cacheable
Commit of patch #1 to synchronous client from James Leigh
<james at 3roundstones dot com>

Modified:
    httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheConfig.java
    httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java
    httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java
    httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java
    httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestResponseCachingPolicy.java

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheConfig.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheConfig.java?rev=1490448&r1=1490447&r2=1490448&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheConfig.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheConfig.java Thu Jun  6 21:25:04 2013
@@ -56,6 +56,15 @@ import org.apache.http.util.Args;
  * browser cache), then you will want to {@link
  * CacheConfig#setSharedCache(boolean) turn off the shared cache setting}.</p>
  *
+ * <p><b>303 caching</b>. RFC2616 explicitly disallows caching 303 responses;
+ * however, the HTTPbis working group says they can be cached
+ * if explicitly indicated in the response headers and permitted by the request method.
+ * (They also indicate that disallowing 303 caching is actually an unintended
+ * spec error in RFC2616).
+ * This behavior is off by default, to err on the side of a conservative
+ * adherence to the existing standard, but you may want to
+ * {@link Builder#setAllow303Caching(boolean) enable it}.
+ *
  * <p><b>Heuristic caching</b>. Per RFC2616, a cache may cache certain cache
  * entries even if no explicit cache control headers are set by the origin.
  * This behavior is off by default, but you may want to turn this on if you
@@ -100,6 +109,10 @@ public class CacheConfig implements Clon
      */
     public final static int DEFAULT_MAX_UPDATE_RETRIES = 1;
 
+    /** Default setting for 303 caching
+     */
+    public final static boolean DEFAULT_303_CACHING_ENABLED = false;
+
     /** Default setting for heuristic caching
      */
     public final static boolean DEFAULT_HEURISTIC_CACHING_ENABLED = false;
@@ -139,6 +152,7 @@ public class CacheConfig implements Clon
     private long maxObjectSize;
     private int maxCacheEntries;
     private int maxUpdateRetries;
+    private boolean allow303Caching;
     private boolean heuristicCachingEnabled;
     private float heuristicCoefficient;
     private long heuristicDefaultLifetime;
@@ -158,6 +172,7 @@ public class CacheConfig implements Clon
         this.maxObjectSize = DEFAULT_MAX_OBJECT_SIZE_BYTES;
         this.maxCacheEntries = DEFAULT_MAX_CACHE_ENTRIES;
         this.maxUpdateRetries = DEFAULT_MAX_UPDATE_RETRIES;
+        this.allow303Caching = false;
         this.heuristicCachingEnabled = false;
         this.heuristicCoefficient = DEFAULT_HEURISTIC_COEFFICIENT;
         this.heuristicDefaultLifetime = DEFAULT_HEURISTIC_LIFETIME;
@@ -172,6 +187,7 @@ public class CacheConfig implements Clon
             final long maxObjectSize,
             final int maxCacheEntries,
             final int maxUpdateRetries,
+            final boolean allow303Caching,
             final boolean heuristicCachingEnabled,
             final float heuristicCoefficient,
             final long heuristicDefaultLifetime,
@@ -185,6 +201,7 @@ public class CacheConfig implements Clon
         this.maxObjectSize = maxObjectSize;
         this.maxCacheEntries = maxCacheEntries;
         this.maxUpdateRetries = maxUpdateRetries;
+        this.allow303Caching = allow303Caching;
         this.heuristicCachingEnabled = heuristicCachingEnabled;
         this.heuristicCoefficient = heuristicCoefficient;
         this.heuristicDefaultLifetime = heuristicDefaultLifetime;
@@ -288,6 +305,14 @@ public class CacheConfig implements Clon
     }
 
     /**
+     * Returns whether 303 caching is enabled.
+     * @return {@code true} if it is enabled.
+     */
+    public boolean is303CachingEnabled() {
+        return allow303Caching;
+    }
+
+    /**
      * Returns whether heuristic caching is enabled.
      * @return {@code true} if it is enabled.
      */
@@ -493,6 +518,7 @@ public class CacheConfig implements Clon
         private long maxObjectSize;
         private int maxCacheEntries;
         private int maxUpdateRetries;
+        private boolean allow303Caching;
         private boolean heuristicCachingEnabled;
         private float heuristicCoefficient;
         private long heuristicDefaultLifetime;
@@ -507,6 +533,7 @@ public class CacheConfig implements Clon
             this.maxObjectSize = DEFAULT_MAX_OBJECT_SIZE_BYTES;
             this.maxCacheEntries = DEFAULT_MAX_CACHE_ENTRIES;
             this.maxUpdateRetries = DEFAULT_MAX_UPDATE_RETRIES;
+            this.allow303Caching = false;
             this.heuristicCachingEnabled = false;
             this.heuristicCoefficient = DEFAULT_HEURISTIC_COEFFICIENT;
             this.heuristicDefaultLifetime = DEFAULT_HEURISTIC_LIFETIME;
@@ -543,6 +570,16 @@ public class CacheConfig implements Clon
         }
 
         /**
+         * Enables or disables 303 caching.
+         * @param allow303Caching should be {@code true} to
+         *   permit 303 caching, {@code false} to disable it.
+         */
+        public Builder setAllow303Caching(final boolean allow303Caching) {
+            this.allow303Caching = allow303Caching;
+            return this;
+        }
+
+        /**
          * Enables or disables heuristic caching.
          * @param heuristicCachingEnabled should be {@code true} to
          *   permit heuristic caching, {@code false} to enable it.
@@ -652,6 +689,7 @@ public class CacheConfig implements Clon
                     maxObjectSize,
                     maxCacheEntries,
                     maxUpdateRetries,
+                    allow303Caching,
                     heuristicCachingEnabled,
                     heuristicCoefficient,
                     heuristicDefaultLifetime,
@@ -670,7 +708,9 @@ public class CacheConfig implements Clon
         final StringBuilder builder = new StringBuilder();
         builder.append("[maxObjectSize=").append(this.maxObjectSize)
                 .append(", maxCacheEntries=").append(this.maxCacheEntries)
-                .append(", maxUpdateRetries=").append(this.heuristicCachingEnabled)
+                .append(", maxUpdateRetries=").append(this.maxUpdateRetries)
+                .append(", 303CachingEnabled=").append(this.allow303Caching)
+                .append(", heuristicCachingEnabled=").append(this.heuristicCachingEnabled)
                 .append(", heuristicCoefficient=").append(this.heuristicCoefficient)
                 .append(", heuristicDefaultLifetime=").append(this.heuristicDefaultLifetime)
                 .append(", isSharedCache=").append(this.isSharedCache)

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java?rev=1490448&r1=1490447&r2=1490448&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java Thu Jun  6 21:25:04 2013
@@ -139,7 +139,7 @@ public class CachingExec implements Clie
         this.requestCompliance = new RequestProtocolCompliance();
         this.responseCachingPolicy = new ResponseCachingPolicy(
                 this.cacheConfig.getMaxObjectSize(), this.cacheConfig.isSharedCache(),
-                this.cacheConfig.isNeverCacheHTTP10ResponsesWithQuery());
+                this.cacheConfig.isNeverCacheHTTP10ResponsesWithQuery(), config.is303CachingEnabled());
         this.asynchRevalidator = asynchRevalidator;
     }
 

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java?rev=1490448&r1=1490447&r2=1490448&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java Thu Jun  6 21:25:04 2013
@@ -175,7 +175,7 @@ public class CachingHttpClient implement
         this.responseCache = cache;
         this.validityPolicy = new CacheValidityPolicy();
         this.responseCachingPolicy = new ResponseCachingPolicy(maxObjectSizeBytes, sharedCache,
-                config.isNeverCacheHTTP10ResponsesWithQuery());
+                config.isNeverCacheHTTP10ResponsesWithQuery(), config.is303CachingEnabled());
         this.responseGenerator = new CachedHttpResponseGenerator(this.validityPolicy);
         this.cacheableRequestPolicy = new CacheableRequestPolicy();
         this.suitabilityChecker = new CachedResponseSuitabilityChecker(this.validityPolicy, config);

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java?rev=1490448&r1=1490447&r2=1490448&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java Thu Jun  6 21:25:04 2013
@@ -63,9 +63,7 @@ class ResponseCachingPolicy {
                 HttpStatus.SC_MULTIPLE_CHOICES,
                 HttpStatus.SC_MOVED_PERMANENTLY,
                 HttpStatus.SC_GONE));
-    private static final Set<Integer> uncacheableStatuses =
-            new HashSet<Integer>(Arrays.asList(HttpStatus.SC_PARTIAL_CONTENT,
-                    HttpStatus.SC_SEE_OTHER));
+    private final Set<Integer> uncacheableStatuses;
     /**
      * Define a cache policy that limits the size of things that should be stored
      * in the cache to a maximum of {@link HttpResponse} bytes in size.
@@ -75,13 +73,22 @@ class ResponseCachingPolicy {
      * non-shared/private cache (false)
      * @param neverCache1_0ResponsesWithQueryString true to never cache HTTP 1.0 responses with a query string, false
      * to cache if explicit cache headers are found.
+     * @param allow303Caching if this policy is permitted to cache 303 response
      */
-    public ResponseCachingPolicy(final long maxObjectSizeBytes, final boolean sharedCache,
-                                 final boolean neverCache1_0ResponsesWithQueryString
-    ) {
+    public ResponseCachingPolicy(final long maxObjectSizeBytes,
+            final boolean sharedCache,
+            final boolean neverCache1_0ResponsesWithQueryString,
+            boolean allow303Caching) {
         this.maxObjectSizeBytes = maxObjectSizeBytes;
         this.sharedCache = sharedCache;
         this.neverCache1_0ResponsesWithQueryString = neverCache1_0ResponsesWithQueryString;
+        if (allow303Caching) {
+            uncacheableStatuses = new HashSet<Integer>(
+                    Arrays.asList(HttpStatus.SC_PARTIAL_CONTENT));
+        } else {
+            uncacheableStatuses = new HashSet<Integer>(Arrays.asList(
+                    HttpStatus.SC_PARTIAL_CONTENT, HttpStatus.SC_SEE_OTHER));
+        }
     }
 
     /**

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestResponseCachingPolicy.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestResponseCachingPolicy.java?rev=1490448&r1=1490447&r2=1490448&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestResponseCachingPolicy.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestResponseCachingPolicy.java Thu Jun  6 21:25:04 2013
@@ -61,7 +61,7 @@ public class TestResponseCachingPolicy {
         sixSecondsAgo = new Date(now.getTime() - 6 * 1000L);
         tenSecondsFromNow = new Date(now.getTime() + 10 * 1000L);
 
-        policy = new ResponseCachingPolicy(0, true, false);
+        policy = new ResponseCachingPolicy(0, true, false, false);
         request = new BasicHttpRequest("GET","/",HTTP_1_1);
         response = new BasicHttpResponse(
                 new BasicStatusLine(HTTP_1_1, HttpStatus.SC_OK, ""));
@@ -83,7 +83,7 @@ public class TestResponseCachingPolicy {
 
     @Test
     public void testResponsesToRequestsWithAuthorizationHeadersAreCacheableByNonSharedCache() {
-        policy = new ResponseCachingPolicy(0, false, false);
+        policy = new ResponseCachingPolicy(0, false, false, false);
         request = new BasicHttpRequest("GET","/",HTTP_1_1);
         request.setHeader("Authorization","Basic dXNlcjpwYXNzd2Q=");
         Assert.assertTrue(policy.isResponseCacheable(request,response));
@@ -160,6 +160,24 @@ public class TestResponseCachingPolicy {
     }
 
     @Test
+    public void testPlain303ResponseCodeIsNotCacheableUnderDefaultBehavior() {
+        response.setStatusCode(HttpStatus.SC_SEE_OTHER);
+        response.removeHeaders("Expires");
+        response.removeHeaders("Cache-Control");
+        Assert.assertFalse(policy.isResponseCacheable("GET", response));
+    }
+
+    @Test
+    public void testPlain303ResponseCodeIsNotCacheableEvenIf303CachingEnabled() {
+        policy = new ResponseCachingPolicy(0, true, false, true);
+        response.setStatusCode(HttpStatus.SC_SEE_OTHER);
+        response.removeHeaders("Expires");
+        response.removeHeaders("Cache-Control");
+        Assert.assertFalse(policy.isResponseCacheable("GET", response));
+    }
+
+
+    @Test
     public void testPlain307ResponseCodeIsNotCacheable() {
         response.setStatusCode(HttpStatus.SC_TEMPORARY_REDIRECT);
         response.removeHeaders("Expires");
@@ -225,7 +243,7 @@ public class TestResponseCachingPolicy {
 
     @Test
     public void test200ResponseWithPrivateCacheControlIsCacheableByNonSharedCache() {
-        policy = new ResponseCachingPolicy(0, false, false);
+        policy = new ResponseCachingPolicy(0, false, false, false);
         response.setStatusCode(HttpStatus.SC_OK);
         response.setHeader("Cache-Control", "private");
         Assert.assertTrue(policy.isResponseCacheable("GET", response));
@@ -385,7 +403,7 @@ public class TestResponseCachingPolicy {
 
     @Test
     public void testResponsesToGETWithQueryParamsButNoExplicitCachingAreNotCacheableEvenWhen1_0QueryCachingDisabled() {
-        policy = new ResponseCachingPolicy(0, true, true);
+        policy = new ResponseCachingPolicy(0, true, true, false);
         request = new BasicHttpRequest("GET", "/foo?s=bar");
         Assert.assertFalse(policy.isResponseCacheable(request, response));
     }
@@ -400,7 +418,7 @@ public class TestResponseCachingPolicy {
 
     @Test
     public void testResponsesToGETWithQueryParamsAndExplicitCachingAreCacheableEvenWhen1_0QueryCachingDisabled() {
-        policy = new ResponseCachingPolicy(0, true, true);
+        policy = new ResponseCachingPolicy(0, true, true, false);
         request = new BasicHttpRequest("GET", "/foo?s=bar");
         response.setHeader("Date", DateUtils.formatDate(now));
         response.setHeader("Expires", DateUtils.formatDate(tenSecondsFromNow));
@@ -416,7 +434,7 @@ public class TestResponseCachingPolicy {
 
     @Test
     public void getsWithQueryParametersDirectlyFrom1_0OriginsAreNotCacheableEvenWithSetting() {
-        policy = new ResponseCachingPolicy(0, true, true);
+        policy = new ResponseCachingPolicy(0, true, true, false);
         request = new BasicHttpRequest("GET", "/foo?s=bar");
         response = new BasicHttpResponse(HttpVersion.HTTP_1_0, HttpStatus.SC_OK, "OK");
         Assert.assertFalse(policy.isResponseCacheable(request, response));
@@ -433,7 +451,7 @@ public class TestResponseCachingPolicy {
 
     @Test
     public void getsWithQueryParametersDirectlyFrom1_0OriginsCanBeNotCacheableEvenWithExpires() {
-        policy = new ResponseCachingPolicy(0, true, true);
+        policy = new ResponseCachingPolicy(0, true, true, false);
         request = new BasicHttpRequest("GET", "/foo?s=bar");
         response = new BasicHttpResponse(HttpVersion.HTTP_1_0, HttpStatus.SC_OK, "OK");
         response.setHeader("Date", DateUtils.formatDate(now));
@@ -461,7 +479,7 @@ public class TestResponseCachingPolicy {
 
     @Test
     public void getsWithQueryParametersFrom1_0OriginsViaProxiesCanNotBeCacheableEvenWithExpires() {
-        policy = new ResponseCachingPolicy(0, true, true);
+        policy = new ResponseCachingPolicy(0, true, true, true);
         request = new BasicHttpRequest("GET", "/foo?s=bar");
         final Date now = new Date();
         final Date tenSecondsFromNow = new Date(now.getTime() + 10 * 1000L);
@@ -482,7 +500,7 @@ public class TestResponseCachingPolicy {
 
     @Test
     public void getsWithQueryParametersFrom1_0OriginsViaExplicitProxiesCanNotBeCacheableEvenWithExpires() {
-        policy = new ResponseCachingPolicy(0, true, true);
+        policy = new ResponseCachingPolicy(0, true, true, true);
         request = new BasicHttpRequest("GET", "/foo?s=bar");
         response.setHeader("Date", DateUtils.formatDate(now));
         response.setHeader("Expires", DateUtils.formatDate(tenSecondsFromNow));
@@ -517,6 +535,42 @@ public class TestResponseCachingPolicy {
     }
 
     @Test
+    public void test302WithExplicitCachingHeaders() {
+        response.setStatusCode(HttpStatus.SC_MOVED_TEMPORARILY);
+        response.setHeader("Date", DateUtils.formatDate(now));
+        response.setHeader("Cache-Control","max-age=300");
+        Assert.assertTrue(policy.isResponseCacheable(request, response));
+    }
+
+    @Test
+    public void test303WithExplicitCachingHeadersUnderDefaultBehavior() {
+        // RFC 2616 says: 303 should not be cached
+        response.setStatusCode(HttpStatus.SC_SEE_OTHER);
+        response.setHeader("Date", DateUtils.formatDate(now));
+        response.setHeader("Cache-Control","max-age=300");
+        Assert.assertFalse(policy.isResponseCacheable(request, response));
+    }
+
+    @Test
+    public void test303WithExplicitCachingHeadersWhenPermittedByConfig() {
+        // HTTPbis working group says ok if explicitly indicated by
+        // response headers
+        policy = new ResponseCachingPolicy(0, true, false, true);
+        response.setStatusCode(HttpStatus.SC_SEE_OTHER);
+        response.setHeader("Date", DateUtils.formatDate(now));
+        response.setHeader("Cache-Control","max-age=300");
+        Assert.assertTrue(policy.isResponseCacheable(request, response));
+    }
+
+    @Test
+    public void test307WithExplicitCachingHeaders() {
+        response.setStatusCode(HttpStatus.SC_TEMPORARY_REDIRECT);
+        response.setHeader("Date", DateUtils.formatDate(now));
+        response.setHeader("Cache-Control","max-age=300");
+        Assert.assertTrue(policy.isResponseCacheable(request, response));
+    }
+
+    @Test
     public void otherStatusCodesAreCacheableWithExplicitCachingHeaders() {
         response.setStatusCode(HttpStatus.SC_NOT_FOUND);
         response.setHeader("Date", DateUtils.formatDate(now));