HTTPCLIENT-992: cache should not generate stale responses to requests explicitly requesting first-hand or fresh ones

Contributed by Jonathan Moore <jonathan_moore at comcast.com>


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

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

@@ -34,6 +34,8 @@ import java.util.concurrent.atomic.AtomicLong;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.apache.http.Header;
+import org.apache.http.HeaderElement;
 import org.apache.http.HttpHost;
 import org.apache.http.HttpMessage;
 import org.apache.http.HttpRequest;
@@ -418,7 +420,8 @@ public class CachingHttpClient implements HttpClient {
                 return revalidateCacheEntry(target, request, context, entry);
             } catch (IOException ioex) {
                 if (validityPolicy.mustRevalidate(entry)
-                    || (isSharedCache() && validityPolicy.proxyRevalidate(entry))) {
+                    || (isSharedCache() && validityPolicy.proxyRevalidate(entry))
+                    || explicitFreshnessRequest(request, entry)) {
                     setResponseStatus(context, CacheResponseStatus.CACHE_MODULE_RESPONSE);
                     return new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_GATEWAY_TIMEOUT, "Gateway Timeout");
                 } else {
@@ -435,6 +438,27 @@ public class CachingHttpClient implements HttpClient {
         return callBackend(target, request, context);
     }
 
+    private boolean explicitFreshnessRequest(HttpRequest request, HttpCacheEntry entry) {
+        for(Header h : request.getHeaders("Cache-Control")) {
+            for(HeaderElement elt : h.getElements()) {
+                if ("max-stale".equals(elt.getName())) {
+                    try {
+                        int maxstale = Integer.parseInt(elt.getValue());
+                        long age = validityPolicy.getCurrentAgeSecs(entry);
+                        long lifetime = validityPolicy.getFreshnessLifetimeSecs(entry);
+                        if (age - lifetime > maxstale) return true;
+                    } catch (NumberFormatException nfe) {
+                        return true;
+                    }
+                } else if ("min-fresh".equals(elt.getName())
+                            || "max-age".equals(elt.getName())) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
     private String generateViaHeader(HttpMessage msg) {
         final VersionInfo vi = VersionInfo.loadVersionInfo("org.apache.http.client", getClass().getClassLoader());
         final String release = (vi != null) ? vi.getRelease() : VersionInfo.UNAVAILABLE;

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

@@ -74,6 +74,113 @@ public class TestProtocolRecommendations extends AbstractProtocolTest {
         assertFalse(foundIdentity);
     }
 
+    /*
+     * "For this reason, a cache SHOULD NOT return a stale response if the
+     * client explicitly requests a first-hand or fresh one, unless it is
+     * impossible to comply for technical or policy reasons."
+     */
+    private HttpRequest requestToPopulateStaleCacheEntry() throws Exception {
+        HttpRequest req1 = new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1);
+        HttpResponse resp1 = make200Response();
+        Date now = new Date();
+        Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L);
+        resp1.setHeader("Date", DateUtils.formatDate(tenSecondsAgo));
+        resp1.setHeader("Cache-Control","public,max-age=5");
+        resp1.setHeader("Etag","\"etag\"");
+
+        backendExpectsAnyRequest().andReturn(resp1);
+        return req1;
+    }
+
+    private void testDoesNotReturnStaleResponseOnError(HttpRequest req2)
+    throws Exception, IOException {
+        HttpRequest req1 = requestToPopulateStaleCacheEntry();
+
+        backendExpectsAnyRequest().andThrow(new IOException());
+
+        replayMocks();
+        impl.execute(host, req1);
+        HttpResponse result = null;
+        try {
+            result = impl.execute(host, req2);
+        } catch (IOException acceptable) {
+        }
+        verifyMocks();
+
+        if (result != null) {
+            assertFalse(result.getStatusLine().getStatusCode() == HttpStatus.SC_OK);
+        }
+    }
+
+    @Test
+    public void testDoesNotReturnStaleResponseIfClientExplicitlyRequestsFirstHandOneWithCacheControl()
+            throws Exception {
+        HttpRequest req = new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1);
+        req.setHeader("Cache-Control","no-cache");
+        testDoesNotReturnStaleResponseOnError(req);
+    }
+
+    @Test
+    public void testDoesNotReturnStaleResponseIfClientExplicitlyRequestsFirstHandOneWithPragma()
+            throws Exception {
+        HttpRequest req = new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1);
+        req.setHeader("Pragma","no-cache");
+        testDoesNotReturnStaleResponseOnError(req);
+    }
+
+    @Test
+    public void testDoesNotReturnStaleResponseIfClientExplicitlyRequestsFreshWithMaxAge()
+            throws Exception {
+        HttpRequest req = new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1);
+        req.setHeader("Cache-Control","max-age=0");
+        testDoesNotReturnStaleResponseOnError(req);
+    }
+
+    @Test
+    public void testDoesNotReturnStaleResponseIfClientExplicitlySpecifiesLargerMaxAge()
+            throws Exception {
+        HttpRequest req = new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1);
+        req.setHeader("Cache-Control","max-age=20");
+        testDoesNotReturnStaleResponseOnError(req);
+    }
+
+
+    @Test
+    public void testDoesNotReturnStaleResponseIfClientExplicitlyRequestsFreshWithMinFresh()
+    throws Exception {
+        HttpRequest req = new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1);
+        req.setHeader("Cache-Control","min-fresh=2");
+
+        testDoesNotReturnStaleResponseOnError(req);
+    }
+
+    @Test
+    public void testDoesNotReturnStaleResponseIfClientExplicitlyRequestsFreshWithMaxStale()
+    throws Exception {
+        HttpRequest req = new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1);
+        req.setHeader("Cache-Control","max-stale=2");
+
+        testDoesNotReturnStaleResponseOnError(req);
+    }
+
+    @Test
+    public void testMayReturnStaleResponseIfClientExplicitlySpecifiesAcceptableMaxStale()
+            throws Exception {
+        HttpRequest req1 = requestToPopulateStaleCacheEntry();
+        HttpRequest req2 = new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1);
+        req2.setHeader("Cache-Control","max-stale=20");
+
+        backendExpectsAnyRequest().andThrow(new IOException());
+
+        replayMocks();
+        impl.execute(host, req1);
+        HttpResponse result = impl.execute(host, req2);
+        verifyMocks();
+
+        assertEquals(HttpStatus.SC_OK, result.getStatusLine().getStatusCode());
+        assertNotNull(result.getFirstHeader("Warning"));
+    }
+
     /*
      * "A correct cache MUST respond to a request with the most up-to-date
      * response held by the cache that is appropriate to the request