HTTPCLIENT-1003: Handle conditional requests in cache

Contributed by Michajlo Matijkiw <michajlo_matijkiw at comcast.com> and Mohammed Azeem Uddin <mohammedazeem_uddin at comcast.com>


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

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

@@ -38,21 +38,61 @@ import org.apache.http.HttpResponse;
  */
 public interface HttpCache {
 
+    /**
+     * Clear all matching {@link HttpCacheEntry}s.
+     * @param host
+     * @param request
+     * @throws IOException
+     */
     void flushCacheEntriesFor(HttpHost host, HttpRequest request)
         throws IOException;
 
+    /**
+     * Clear invalidated matching {@link HttpCacheEntry}s
+     * @param host
+     * @param request
+     * @throws IOException
+     */
     void flushInvalidatedCacheEntriesFor(HttpHost host, HttpRequest request)
         throws IOException;
 
+    /**
+     * Retrieve matching {@link HttpCacheEntry} from the cache if it exists
+     * @param host
+     * @param request
+     * @return
+     * @throws IOException
+     */
     HttpCacheEntry getCacheEntry(HttpHost host, HttpRequest request)
         throws IOException;
 
+    /**
+     * Store a {@link HttpResponse} in the cache if possible, and return
+     * @param host
+     * @param request
+     * @param originResponse
+     * @param requestSent
+     * @param responseReceived
+     * @return
+     * @throws IOException
+     */
     HttpResponse cacheAndReturnResponse(
             HttpHost host, HttpRequest request, HttpResponse originResponse,
             Date requestSent, Date responseReceived)
         throws IOException;
 
-    HttpResponse updateCacheEntry(
+    /**
+     * Update a {@link HttpCacheEntry} using a 304 {@link HttpResponse}.
+     * @param target
+     * @param request
+     * @param stale
+     * @param originResponse
+     * @param requestSent
+     * @param responseReceived
+     * @return
+     * @throws IOException
+     */
+    HttpCacheEntry updateCacheEntry(
             HttpHost target, HttpRequest request, HttpCacheEntry stale, HttpResponse originResponse,
             Date requestSent, Date responseReceived)
         throws IOException;

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

@@ -152,7 +152,7 @@ public class BasicHttpCache implements HttpCache {
                 variants);
     }
 
-    public HttpResponse updateCacheEntry(HttpHost target, HttpRequest request,
+    public HttpCacheEntry updateCacheEntry(HttpHost target, HttpRequest request,
             HttpCacheEntry stale, HttpResponse originResponse,
             Date requestSent, Date responseReceived) throws IOException {
         HttpCacheEntry updatedEntry = cacheEntryUpdater.updateCacheEntry(
@@ -162,7 +162,7 @@ public class BasicHttpCache implements HttpCache {
                 responseReceived,
                 originResponse);
         storeInCache(target, request, updatedEntry);
-        return responseGenerator.generateResponse(updatedEntry);
+        return updatedEntry;
     }
 
     public HttpResponse cacheAndReturnResponse(HttpHost host, HttpRequest request,

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

@@ -36,6 +36,7 @@ import org.apache.http.HttpVersion;
 import org.apache.http.annotation.Immutable;
 import org.apache.http.client.cache.HeaderConstants;
 import org.apache.http.client.cache.HttpCacheEntry;
+import org.apache.http.impl.cookie.DateUtils;
 import org.apache.http.message.BasicHeader;
 import org.apache.http.message.BasicHttpResponse;
 import org.apache.http.protocol.HTTP;
@@ -72,12 +73,11 @@ class CachedHttpResponseGenerator {
         HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, entry
                 .getStatusCode(), entry.getReasonPhrase());
 
-        if (entry.getStatusCode() != HttpStatus.SC_NOT_MODIFIED) {
-            HttpEntity entity = new CacheEntity(entry);
-            response.setHeaders(entry.getAllHeaders());
-            addMissingContentLengthHeader(response, entity);
-            response.setEntity(entity);
-        }
+        HttpEntity entity = new CacheEntity(entry);
+        response.setHeaders(entry.getAllHeaders());
+        addMissingContentLengthHeader(response, entity);
+        response.setEntity(entity);
+
 
         long age = this.validityStrategy.getCurrentAgeSecs(entry, now);
         if (age > 0) {
@@ -91,6 +91,61 @@ class CachedHttpResponseGenerator {
         return response;
     }
 
+    /**
+     * Generate a 304 - Not Modified response from a {@link CacheEntry}.  This should be
+     * used to respond to conditional requests, when the entry exists or has been revalidated.
+     *
+     * @param entry
+     * @return
+     */
+    HttpResponse generateNotModifiedResponse(HttpCacheEntry entry) {
+
+        HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+                HttpStatus.SC_NOT_MODIFIED, "Not Modified");
+
+        // The response MUST include the following headers
+        //  (http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html)
+
+        // - Date, unless its omission is required by section 14.8.1
+        Header dateHeader = entry.getFirstHeader("Date");
+        if (dateHeader == null) {
+             dateHeader = new BasicHeader("Date", DateUtils.formatDate(new Date()));
+        }
+        response.addHeader(dateHeader);
+
+        // - ETag and/or Content-Location, if the header would have been sent
+        //   in a 200 response to the same request
+        Header etagHeader = entry.getFirstHeader("ETag");
+        if (etagHeader != null) {
+            response.addHeader(etagHeader);
+        }
+
+        Header contentLocationHeader = entry.getFirstHeader("Content-Location");
+        if (contentLocationHeader != null) {
+            response.addHeader(contentLocationHeader);
+        }
+
+        // - Expires, Cache-Control, and/or Vary, if the field-value might
+        //   differ from that sent in any previous response for the same
+        //   variant
+        Header expiresHeader = entry.getFirstHeader("Expires");
+        if (expiresHeader != null) {
+            response.addHeader(expiresHeader);
+        }
+
+        Header cacheControlHeader = entry.getFirstHeader("Cache-Control");
+        if (cacheControlHeader != null) {
+            response.addHeader(cacheControlHeader);
+        }
+
+        Header varyHeader = entry.getFirstHeader("Vary");
+        if (varyHeader != null) {
+            response.addHeader(varyHeader);
+        }
+
+        return response;
+    }
+
     private void addMissingContentLengthHeader(HttpResponse response, HttpEntity entity) {
         if (transferEncodingIsPresent(response))
             return;

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

@@ -134,8 +134,7 @@ class CachedResponseSuitabilityChecker {
             return false;
         }
 
-        if (containsEtagAndLastModifiedValidators(request)
-            && !allConditionalsMatch(request, entry)) {
+        if (isConditional(request) && !allConditionalsMatch(request, entry, now)) {
             return false;
         }
 
@@ -203,83 +202,114 @@ class CachedResponseSuitabilityChecker {
         return true;
     }
 
+    /**
+     * Is this request the type of conditional request we support?
+     * @param request
+     * @return
+     */
+    public boolean isConditional(HttpRequest request) {
+        return hasSupportedEtagVadlidator(request) || hasSupportedLastModifiedValidator(request);
+    }
+
+    /**
+     * Check that conditionals that are part of this request match
+     * @param request
+     * @param entry
+     * @param now
+     * @return
+     */
+    public boolean allConditionalsMatch(HttpRequest request, HttpCacheEntry entry, Date now) {
+        boolean hasEtagValidator = hasSupportedEtagVadlidator(request);
+        boolean hasLastModifiedValidator = hasSupportedLastModifiedValidator(request);
+
+        boolean etagValidatorMatches = (hasEtagValidator) ? etagValidtorMatches(request, entry) : false;
+        boolean lastModifiedValidatorMatches = (hasLastModifiedValidator) ? lastModifiedValidatorMatches(request, entry, now) : false;
+
+        if ((hasEtagValidator && hasLastModifiedValidator)
+            && !(etagValidatorMatches && lastModifiedValidatorMatches)) {
+            return false;
+        } else if (hasEtagValidator && !etagValidatorMatches) {
+            return false;
+        } if (hasLastModifiedValidator && !lastModifiedValidatorMatches) {
+            return false;
+        }
+        return true;
+    }
+
     private boolean hasUnsupportedConditionalHeaders(HttpRequest request) {
         return (request.getFirstHeader("If-Range") != null
                 || request.getFirstHeader("If-Match") != null
                 || hasValidDateField(request, "If-Unmodified-Since"));
     }
 
+    private boolean hasSupportedEtagVadlidator(HttpRequest request) {
+        return request.containsHeader(HeaderConstants.IF_NONE_MATCH);
+    }
+
+    private boolean hasSupportedLastModifiedValidator(HttpRequest request) {
+        return hasValidDateField(request, HeaderConstants.IF_MODIFIED_SINCE);
+    }
+
     /**
-     * Should return false if some conditionals would allow a
-     * normal request but some would not.
+     * Check entry against If-None-Match
      * @param request
      * @param entry
      * @return
      */
-    private boolean allConditionalsMatch(HttpRequest request,
-            HttpCacheEntry entry) {
-        Header etagHeader = entry.getFirstHeader("ETag");
+    private boolean etagValidtorMatches(HttpRequest request, HttpCacheEntry entry) {
+        Header etagHeader = entry.getFirstHeader(HeaderConstants.ETAG);
         String etag = (etagHeader != null) ? etagHeader.getValue() : null;
-        Header[] ifNoneMatch = request.getHeaders("If-None-Match");
-        if (ifNoneMatch != null && ifNoneMatch.length > 0) {
-            boolean matched = false;
-            for(Header h : ifNoneMatch) {
-                for(HeaderElement elt : h.getElements()) {
+        Header[] ifNoneMatch = request.getHeaders(HeaderConstants.IF_NONE_MATCH);
+        if (ifNoneMatch != null) {
+            for (Header h : ifNoneMatch) {
+                for (HeaderElement elt : h.getElements()) {
                     String reqEtag = elt.toString();
                     if (("*".equals(reqEtag) && etag != null)
-                        || reqEtag.equals(etag)) {
-                        matched = true;
-                        break;
+                            || reqEtag.equals(etag)) {
+                        return true;
                     }
                 }
             }
-            if (!matched) return false;
         }
-        Header lmHeader = entry.getFirstHeader("Last-Modified");
+        return false;
+    }
+
+    /**
+     * Check entry against If-Modified-Since, if If-Modified-Since is in the future it is invalid as per
+     * http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html
+     * @param request
+     * @param entry
+     * @param now
+     * @return
+     */
+    private boolean lastModifiedValidatorMatches(HttpRequest request, HttpCacheEntry entry, Date now) {
+        Header lastModifiedHeader = entry.getFirstHeader(HeaderConstants.LAST_MODIFIED);
         Date lastModified = null;
         try {
-            if (lmHeader != null) {
-                lastModified = DateUtils.parseDate(lmHeader.getValue());
+            if(lastModifiedHeader != null) {
+                lastModified = DateUtils.parseDate(lastModifiedHeader.getValue());
             }
         } catch (DateParseException dpe) {
             // nop
         }
-        for(Header h : request.getHeaders("If-Modified-Since")) {
+
+        if (lastModified == null) {
+            return false;
+        }
+
+        for (Header h : request.getHeaders(HeaderConstants.IF_MODIFIED_SINCE)) {
             try {
-                Date cond = DateUtils.parseDate(h.getValue());
-                if (lastModified == null
-                    || lastModified.after(cond)) {
+                Date ifModifiedSince = DateUtils.parseDate(h.getValue());
+                if (ifModifiedSince.after(now) || lastModified.after(ifModifiedSince)) {
                     return false;
                 }
             } catch (DateParseException dpe) {
+                // nop
             }
         }
         return true;
     }
 
-    private boolean containsEtagAndLastModifiedValidators(HttpRequest request) {
-        boolean hasEtagValidators = (hasEtagIfRangeHeader(request)
-                || request.getFirstHeader("If-Match") != null
-                || request.getFirstHeader("If-None-Match") != null);
-        if (!hasEtagValidators) return false;
-        final boolean hasLastModifiedValidators =
-            hasValidDateField(request, "If-Modified-Since")
-            || hasValidDateField(request, "If-Unmodified-Since")
-            || hasValidDateField(request, "If-Range");
-        return hasLastModifiedValidators;
-    }
-
-    private boolean hasEtagIfRangeHeader(HttpRequest request) {
-        for(Header h : request.getHeaders("If-Range")) {
-            try {
-                DateUtils.parseDate(h.getValue());
-            } catch (DateParseException dpe) {
-                return true;
-            }
-        }
-        return false;
-    }
-
     private boolean hasValidDateField(HttpRequest request, String headerName) {
         for(Header h : request.getHeaders(headerName)) {
             try {

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

@@ -412,7 +412,13 @@ public class CachingHttpClient implements HttpClient {
 
         Date now = getCurrentDate();
         if (suitabilityChecker.canCachedResponseBeUsed(target, request, entry, now)) {
-            final HttpResponse cachedResponse = responseGenerator.generateResponse(entry);
+            final HttpResponse cachedResponse;
+            if (request.containsHeader(HeaderConstants.IF_NONE_MATCH)
+                    || request.containsHeader(HeaderConstants.IF_MODIFIED_SINCE)) {
+                cachedResponse = responseGenerator.generateNotModifiedResponse(entry);
+            } else {
+                cachedResponse = responseGenerator.generateResponse(entry);
+            }
             setResponseStatus(context, CacheResponseStatus.CACHE_HIT);
             if (validityPolicy.getStalenessSecs(entry, now) > 0L) {
                 cachedResponse.addHeader("Warning","110 localhost \"Response is stale\"");
@@ -564,10 +570,16 @@ public class CachingHttpClient implements HttpClient {
         if (statusCode == HttpStatus.SC_NOT_MODIFIED || statusCode == HttpStatus.SC_OK) {
             cacheUpdates.getAndIncrement();
             setResponseStatus(context, CacheResponseStatus.VALIDATED);
-            if (statusCode == HttpStatus.SC_NOT_MODIFIED) {
-                return responseCache.updateCacheEntry(target, request, cacheEntry,
-                        backendResponse, requestDate, responseDate);
+        }
+
+        if (statusCode == HttpStatus.SC_NOT_MODIFIED) {
+            HttpCacheEntry updatedEntry = responseCache.updateCacheEntry(target, request, cacheEntry,
+                    backendResponse, requestDate, responseDate);
+            if (suitabilityChecker.isConditional(request)
+                    && suitabilityChecker.allConditionalsMatch(request, updatedEntry, new Date())) {
+                return responseGenerator.generateNotModifiedResponse(updatedEntry);
             }
+            return responseGenerator.generateResponse(updatedEntry);
         }
 
         return handleBackendResponse(target, conditionalRequest, requestDate, responseDate,

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

@@ -399,7 +399,7 @@ public class TestCachingHttpClient {
         HttpResponse originResponse =
             new BasicHttpResponse(HttpVersion.HTTP_1_1,
                     HttpStatus.SC_NOT_MODIFIED, "Not Modified");
-        HttpResponse finalResponse = HttpTestUtils.make200Response();
+        HttpCacheEntry updatedEntry = HttpTestUtils.makeCacheEntry();
 
         conditionalRequestBuilderReturns(validate);
         getCurrentDateReturns(requestDate);
@@ -407,14 +407,13 @@ public class TestCachingHttpClient {
         getCurrentDateReturns(responseDate);
         EasyMock.expect(mockCache.updateCacheEntry(host, request,
                 entry, originResponse, requestDate, responseDate))
-            .andReturn(finalResponse);
+            .andReturn(updatedEntry);
+        EasyMock.expect(mockSuitabilityChecker.isConditional(request)).andReturn(false);
+        responseIsGeneratedFromCache();
 
         replayMocks();
-        HttpResponse result =
-            impl.revalidateCacheEntry(host, request, context, entry);
+        impl.revalidateCacheEntry(host, request, context, entry);
         verifyMocks();
-
-        Assert.assertSame(finalResponse, result);
     }
 
     @Test
@@ -1040,6 +1039,246 @@ public class TestCachingHttpClient {
         Assert.assertNotNull(result.getFirstHeader("Via"));
     }
 
+    @Test
+    public void testReturns304ForIfModifiedSinceHeaderIfRequestServedFromCache()
+            throws Exception {
+        Date now = new Date();
+        Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L);
+        impl = new CachingHttpClient(mockBackend);
+        HttpRequest req1 = new HttpGet("http://foo.example.com/");
+        HttpRequest req2 = new HttpGet("http://foo.example.com/");
+        req2.addHeader("If-Modified-Since", DateUtils.formatDate(now));
+        HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+                HttpStatus.SC_OK, "OK");
+        resp1.setEntity(HttpTestUtils.makeBody(128));
+        resp1.setHeader("Content-Length", "128");
+        resp1.setHeader("ETag", "\"etag\"");
+        resp1.setHeader("Date", DateUtils.formatDate(tenSecondsAgo));
+        resp1.setHeader("Cache-Control", "public, max-age=3600");
+        resp1.setHeader("Last-Modified", DateUtils.formatDate(tenSecondsAgo));
+
+        EasyMock.expect(
+                mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock
+                        .isA(HttpRequest.class), (HttpContext) EasyMock
+                        .isNull())).andReturn(resp1);
+
+        replayMocks();
+        impl.execute(host, req1);
+        HttpResponse result = impl.execute(host, req2);
+        verifyMocks();
+        Assert.assertEquals(304, result.getStatusLine().getStatusCode());
+
+    }
+
+    @Test
+    public void testReturns200ForIfModifiedSinceDateIsLess() throws Exception {
+        Date now = new Date();
+        Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L);
+        impl = new CachingHttpClient(mockBackend);
+        HttpRequest req1 = new HttpGet("http://foo.example.com/");
+        HttpRequest req2 = new HttpGet("http://foo.example.com/");
+
+        HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+                HttpStatus.SC_OK, "OK");
+        resp1.setEntity(HttpTestUtils.makeBody(128));
+        resp1.setHeader("Content-Length", "128");
+        resp1.setHeader("ETag", "\"etag\"");
+        resp1.setHeader("Date", DateUtils.formatDate(new Date()));
+        resp1.setHeader("Cache-Control", "public, max-age=3600");
+        resp1.setHeader("Last-Modified", DateUtils.formatDate(new Date()));
+
+        // The variant has been modified since this date
+        req2.addHeader("If-Modified-Since", DateUtils
+                        .formatDate(tenSecondsAgo));
+
+        HttpResponse resp2 = HttpTestUtils.make200Response();
+
+        EasyMock.expect(
+                mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock
+                        .isA(HttpRequest.class), (HttpContext) EasyMock
+                        .isNull())).andReturn(resp1);
+        EasyMock.expect(
+                mockBackend.execute(EasyMock.isA(HttpHost.class),
+                        EasyMock.isA(HttpRequest.class),
+                        (HttpContext) EasyMock.isNull()))
+                        .andReturn(resp2);
+
+        replayMocks();
+        impl.execute(host, req1);
+        HttpResponse result = impl.execute(host, req2);
+        verifyMocks();
+        Assert.assertEquals(200, result.getStatusLine().getStatusCode());
+
+    }
+
+    @Test
+    public void testReturns200ForIfModifiedSinceDateIsInvalid()
+            throws Exception {
+        Date now = new Date();
+        Date tenSecondsAfter = new Date(now.getTime() + 10 * 1000L);
+        impl = new CachingHttpClient(mockBackend);
+        HttpRequest req1 = new HttpGet("http://foo.example.com/");
+        HttpRequest req2 = new HttpGet("http://foo.example.com/");
+
+        HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+                HttpStatus.SC_OK, "OK");
+        resp1.setEntity(HttpTestUtils.makeBody(128));
+        resp1.setHeader("Content-Length", "128");
+        resp1.setHeader("ETag", "\"etag\"");
+        resp1.setHeader("Date", DateUtils.formatDate(new Date()));
+        resp1.setHeader("Cache-Control", "public, max-age=3600");
+        resp1.setHeader("Last-Modified", DateUtils.formatDate(new Date()));
+
+        // invalid date (date in the future)
+        req2.addHeader("If-Modified-Since", DateUtils
+                .formatDate(tenSecondsAfter));
+
+        EasyMock.expect(
+                mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock
+                        .isA(HttpRequest.class), (HttpContext) EasyMock
+                        .isNull())).andReturn(resp1).times(2);
+
+        replayMocks();
+        impl.execute(host, req1);
+        HttpResponse result = impl.execute(host, req2);
+        verifyMocks();
+        Assert.assertEquals(200, result.getStatusLine().getStatusCode());
+
+    }
+
+    @Test
+    public void testReturns304ForIfNoneMatchHeaderIfRequestServedFromCache()
+            throws Exception {
+        impl = new CachingHttpClient(mockBackend);
+        HttpRequest req1 = new HttpGet("http://foo.example.com/");
+        HttpRequest req2 = new HttpGet("http://foo.example.com/");
+        req2.addHeader("If-None-Match", "*");
+        HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+                HttpStatus.SC_OK, "OK");
+        resp1.setEntity(HttpTestUtils.makeBody(128));
+        resp1.setHeader("Content-Length", "128");
+        resp1.setHeader("ETag", "\"etag\"");
+        resp1.setHeader("Date", DateUtils.formatDate(new Date()));
+        resp1.setHeader("Cache-Control", "public, max-age=3600");
+
+        EasyMock.expect(
+                mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock
+                        .isA(HttpRequest.class), (HttpContext) EasyMock
+                        .isNull())).andReturn(resp1);
+
+        replayMocks();
+        impl.execute(host, req1);
+        HttpResponse result = impl.execute(host, req2);
+        verifyMocks();
+        Assert.assertEquals(304, result.getStatusLine().getStatusCode());
+
+    }
+
+    @Test
+    public void testReturns200ForIfNoneMatchHeaderFails() throws Exception {
+        impl = new CachingHttpClient(mockBackend);
+        HttpRequest req1 = new HttpGet("http://foo.example.com/");
+        HttpRequest req2 = new HttpGet("http://foo.example.com/");
+
+        HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+                HttpStatus.SC_OK, "OK");
+        resp1.setEntity(HttpTestUtils.makeBody(128));
+        resp1.setHeader("Content-Length", "128");
+        resp1.setHeader("ETag", "\"etag\"");
+        resp1.setHeader("Date", DateUtils.formatDate(new Date()));
+        resp1.setHeader("Cache-Control", "public, max-age=3600");
+
+        req2.addHeader("If-None-Match", "\"abc\"");
+
+        HttpResponse resp2 = HttpTestUtils.make200Response();
+
+        EasyMock.expect(
+                mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock
+                        .isA(HttpRequest.class), (HttpContext) EasyMock
+                        .isNull())).andReturn(resp1);
+        EasyMock.expect(
+                mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock
+                        .isA(HttpRequest.class), (HttpContext) EasyMock
+                        .isNull())).andReturn(resp2);
+
+        replayMocks();
+        impl.execute(host, req1);
+        HttpResponse result = impl.execute(host, req2);
+        verifyMocks();
+        Assert.assertEquals(200, result.getStatusLine().getStatusCode());
+
+    }
+
+    @Test
+    public void testReturns304ForIfNoneMatchHeaderAndIfModifiedSinceIfRequestServedFromCache()
+            throws Exception {
+        impl = new CachingHttpClient(mockBackend);
+        Date now = new Date();
+        Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L);
+        HttpRequest req1 = new HttpGet("http://foo.example.com/");
+        HttpRequest req2 = new HttpGet("http://foo.example.com/");
+
+        HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+                HttpStatus.SC_OK, "OK");
+        resp1.setEntity(HttpTestUtils.makeBody(128));
+        resp1.setHeader("Content-Length", "128");
+        resp1.setHeader("ETag", "\"etag\"");
+        resp1.setHeader("Date", DateUtils.formatDate(tenSecondsAgo));
+        resp1.setHeader("Cache-Control", "public, max-age=3600");
+        resp1.setHeader("Last-Modified", DateUtils.formatDate(new Date()));
+
+        req2.addHeader("If-None-Match", "*");
+        req2.addHeader("If-Modified-Since", DateUtils.formatDate(now));
+
+        EasyMock.expect(
+                mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock
+                        .isA(HttpRequest.class), (HttpContext) EasyMock
+                        .isNull())).andReturn(resp1);
+
+        replayMocks();
+        impl.execute(host, req1);
+        HttpResponse result = impl.execute(host, req2);
+        verifyMocks();
+        Assert.assertEquals(304, result.getStatusLine().getStatusCode());
+
+    }
+
+    @Test
+    public void testReturns200ForIfNoneMatchHeaderFailsIfModifiedSinceIgnored()
+            throws Exception {
+        impl = new CachingHttpClient(mockBackend);
+        Date now = new Date();
+        Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L);
+        HttpRequest req1 = new HttpGet("http://foo.example.com/");
+        HttpRequest req2 = new HttpGet("http://foo.example.com/");
+        req2.addHeader("If-None-Match", "\"abc\"");
+        req2.addHeader("If-Modified-Since", DateUtils.formatDate(now));
+        HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+                HttpStatus.SC_OK, "OK");
+        resp1.setEntity(HttpTestUtils.makeBody(128));
+        resp1.setHeader("Content-Length", "128");
+        resp1.setHeader("ETag", "\"etag\"");
+        resp1.setHeader("Date", DateUtils.formatDate(tenSecondsAgo));
+        resp1.setHeader("Cache-Control", "public, max-age=3600");
+        resp1.setHeader("Last-Modified", DateUtils.formatDate(tenSecondsAgo));
+
+        EasyMock.expect(
+                mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock
+                        .isA(HttpRequest.class), (HttpContext) EasyMock
+                        .isNull())).andReturn(resp1);
+        EasyMock.expect(
+                mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock
+                        .isA(HttpRequest.class), (HttpContext) EasyMock
+                        .isNull())).andReturn(resp1);
+
+        replayMocks();
+        impl.execute(host, req1);
+        HttpResponse result = impl.execute(host, req2);
+        verifyMocks();
+        Assert.assertEquals(200, result.getStatusLine().getStatusCode());
+
+    }
+
     @Test
     public void testSetsValidatedContextIfRequestWasSuccessfullyValidated()
         throws Exception {
@@ -1212,7 +1451,184 @@ public class TestCachingHttpClient {
         verifyMocks();
         Assert.assertNotNull(result.getFirstHeader("Via"));
     }
+    @Test
+    public void testReturns304ForIfNoneMatchPassesIfRequestServedFromOrigin()
+            throws Exception {
 
+        Date now = new Date();
+        Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L);
+
+        impl = new CachingHttpClient(mockBackend);
+        HttpRequest req1 = new HttpGet("http://foo.example.com/");
+        HttpRequest req2 = new HttpGet("http://foo.example.com/");
+
+        HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+                HttpStatus.SC_OK, "OK");
+        resp1.setEntity(HttpTestUtils.makeBody(128));
+        resp1.setHeader("Content-Length", "128");
+        resp1.setHeader("ETag", "\"etag\"");
+        resp1.setHeader("Date", DateUtils.formatDate(tenSecondsAgo));
+        resp1.setHeader("Cache-Control", "public, max-age=5");
+
+        req2.addHeader("If-None-Match", "\"etag\"");
+        HttpResponse resp2 = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+                HttpStatus.SC_NOT_MODIFIED, "Not Modified");
+        resp2.setEntity(HttpTestUtils.makeBody(128));
+        resp2.setHeader("Content-Length", "128");
+        resp2.setHeader("ETag", "\"etag\"");
+        resp2.setHeader("Date", DateUtils.formatDate(now));
+        resp2.setHeader("Cache-Control", "public, max-age=5");
+        EasyMock.expect(
+                mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock
+                        .isA(HttpRequest.class), (HttpContext) EasyMock
+                        .isNull())).andReturn(resp1);
+
+        EasyMock.expect(
+                mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock
+                        .isA(HttpRequest.class), (HttpContext) EasyMock
+                        .isNull())).andReturn(resp2);
+        replayMocks();
+        impl.execute(host, req1);
+        HttpResponse result = impl.execute(host, req2);
+        verifyMocks();
+
+        Assert.assertEquals(HttpStatus.SC_NOT_MODIFIED, result.getStatusLine().getStatusCode());
+    }
+
+    @Test
+    public void testReturns200ForIfNoneMatchFailsIfRequestServedFromOrigin()
+            throws Exception {
+
+        Date now = new Date();
+        Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L);
+
+        impl = new CachingHttpClient(mockBackend);
+        HttpRequest req1 = new HttpGet("http://foo.example.com/");
+        HttpRequest req2 = new HttpGet("http://foo.example.com/");
+
+        HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+                HttpStatus.SC_OK, "OK");
+        resp1.setEntity(HttpTestUtils.makeBody(128));
+        resp1.setHeader("Content-Length", "128");
+        resp1.setHeader("ETag", "\"etag\"");
+        resp1.setHeader("Date", DateUtils.formatDate(tenSecondsAgo));
+        resp1.setHeader("Cache-Control", "public, max-age=5");
+
+        req2.addHeader("If-None-Match", "\"etag\"");
+        HttpResponse resp2 = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+                HttpStatus.SC_OK, "OK");
+        resp2.setEntity(HttpTestUtils.makeBody(128));
+        resp2.setHeader("Content-Length", "128");
+        resp2.setHeader("ETag", "\"newetag\"");
+        resp2.setHeader("Date", DateUtils.formatDate(tenSecondsAgo));
+        resp2.setHeader("Cache-Control", "public, max-age=5");
+        EasyMock.expect(
+                mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock
+                        .isA(HttpRequest.class), (HttpContext) EasyMock
+                        .isNull())).andReturn(resp1);
+
+        EasyMock.expect(
+                mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock
+                        .isA(HttpRequest.class), (HttpContext) EasyMock
+                        .isNull())).andReturn(resp2);
+        replayMocks();
+        impl.execute(host, req1);
+        HttpResponse result = impl.execute(host, req2);
+        verifyMocks();
+
+        Assert.assertEquals(HttpStatus.SC_OK, result.getStatusLine().getStatusCode());
+    }
+
+    @Test
+    public void testReturns304ForIfModifiedSincePassesIfRequestServedFromOrigin()
+            throws Exception {
+        impl = new CachingHttpClient(mockBackend);
+
+        Date now = new Date();
+        Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L);
+
+        HttpRequest req1 = new HttpGet("http://foo.example.com/");
+        HttpRequest req2 = new HttpGet("http://foo.example.com/");
+
+        HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+                HttpStatus.SC_OK, "OK");
+        resp1.setEntity(HttpTestUtils.makeBody(128));
+        resp1.setHeader("Content-Length", "128");
+        resp1.setHeader("ETag", "\"etag\"");
+        resp1.setHeader("Date", DateUtils.formatDate(tenSecondsAgo));
+        resp1.setHeader("Last-Modified", DateUtils.formatDate(tenSecondsAgo));
+        resp1.setHeader("Cache-Control", "public, max-age=5");
+
+        req2.addHeader("If-Modified-Since", DateUtils.formatDate(tenSecondsAgo));
+        HttpResponse resp2 = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+                HttpStatus.SC_NOT_MODIFIED, "Not Modified");
+        resp2.setEntity(HttpTestUtils.makeBody(128));
+        resp2.setHeader("Content-Length", "128");
+        resp2.setHeader("ETag", "\"etag\"");
+        resp2.setHeader("Date", DateUtils.formatDate(tenSecondsAgo));
+        resp1.setHeader("Last-Modified", DateUtils.formatDate(tenSecondsAgo));
+        resp2.setHeader("Cache-Control", "public, max-age=5");
+        EasyMock.expect(
+                mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock
+                        .isA(HttpRequest.class), (HttpContext) EasyMock
+                        .isNull())).andReturn(resp1);
+
+        EasyMock.expect(
+                mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock
+                        .isA(HttpRequest.class), (HttpContext) EasyMock
+                        .isNull())).andReturn(resp2);
+        replayMocks();
+        impl.execute(host, req1);
+        HttpResponse result = impl.execute(host, req2);
+        verifyMocks();
+
+        Assert.assertEquals(HttpStatus.SC_NOT_MODIFIED, result.getStatusLine().getStatusCode());
+    }
+
+    @Test
+    public void testReturns200ForIfModifiedSinceFailsIfRequestServedFromOrigin()
+            throws Exception {
+        impl = new CachingHttpClient(mockBackend);
+        Date now = new Date();
+        Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L);
+
+        HttpRequest req1 = new HttpGet("http://foo.example.com/");
+        HttpRequest req2 = new HttpGet("http://foo.example.com/");
+
+        HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+                HttpStatus.SC_OK, "OK");
+        resp1.setEntity(HttpTestUtils.makeBody(128));
+        resp1.setHeader("Content-Length", "128");
+        resp1.setHeader("ETag", "\"etag\"");
+        resp1.setHeader("Date", DateUtils.formatDate(tenSecondsAgo));
+        resp1.setHeader("Last-Modified", DateUtils.formatDate(tenSecondsAgo));
+        resp1.setHeader("Cache-Control", "public, max-age=5");
+
+        req2.addHeader("If-Modified-Since", DateUtils.formatDate(tenSecondsAgo));
+        HttpResponse resp2 = new BasicHttpResponse(HttpVersion.HTTP_1_1,
+                HttpStatus.SC_OK, "OK");
+        resp2.setEntity(HttpTestUtils.makeBody(128));
+        resp2.setHeader("Content-Length", "128");
+        resp2.setHeader("ETag", "\"newetag\"");
+        resp2.setHeader("Date", DateUtils.formatDate(now));
+        resp1.setHeader("Last-Modified", DateUtils.formatDate(now));
+        resp2.setHeader("Cache-Control", "public, max-age=5");
+        EasyMock.expect(
+                mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock
+                        .isA(HttpRequest.class), (HttpContext) EasyMock
+                        .isNull())).andReturn(resp1);
+
+        EasyMock.expect(
+                mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock
+                        .isA(HttpRequest.class), (HttpContext) EasyMock
+                        .isNull())).andReturn(resp2);
+        replayMocks();
+        impl.execute(host, req1);
+        HttpResponse result = impl.execute(host, req2);
+        verifyMocks();
+
+        Assert.assertEquals(HttpStatus.SC_OK, result.getStatusLine().getStatusCode());
+    }
 
     @Test
     public void testIsSharedCache() {

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

@@ -2234,8 +2234,6 @@ public class TestProtocolRequirements extends AbstractProtocolTest {
         notModified.setHeader("Date", DateUtils.formatDate(now));
         notModified.setHeader("ETag", "\"etag\"");
 
-        HttpResponse reconstructed = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_OK, "OK");
-
         mockCache.flushInvalidatedCacheEntriesFor(host, request);
         EasyMock.expect(mockCache.getCacheEntry(host, request)).andReturn(entry);
         EasyMock.expect(
@@ -2244,13 +2242,11 @@ public class TestProtocolRequirements extends AbstractProtocolTest {
         EasyMock.expect(mockCache.updateCacheEntry(EasyMock.same(host), EasyMock.same(request),
                 EasyMock.same(entry), EasyMock.same(notModified), EasyMock.isA(Date.class),
                 EasyMock.isA(Date.class)))
-            .andReturn(reconstructed);
+            .andReturn(HttpTestUtils.makeCacheEntry());
 
         replayMocks();
-        HttpResponse result = impl.execute(host, request);
+        impl.execute(host, request);
         verifyMocks();
-
-        Assert.assertSame(reconstructed, result);
     }
 
     @Test