HTTPCLIENT-1277: Caching client sends a 304 to an unconditional request.

Contributed by Francois-Xavier Bonnet <francois-xavier.bonnet at centraliens.net>

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

RELEASE_NOTES.txt

@@ -1,6 +1,9 @@
 Changes in trunk
 -------------------
 
+* [HTTPCLIENT-1277] Caching client sends a 304 to an unconditional request. 
+  Contributed by Francois-Xavier Bonnet <francois-xavier.bonnet at centraliens.net> 
+
 * [HTTPCLIENT-1278] Update NTLM documentation.
   Contributed by Karl Wright <DaddyWri at gmail.com>  
 

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

@@ -34,6 +34,7 @@ import org.apache.http.Header;
 import org.apache.http.HeaderElement;
 import org.apache.http.HttpHost;
 import org.apache.http.HttpRequest;
+import org.apache.http.HttpStatus;
 import org.apache.http.annotation.Immutable;
 import org.apache.http.client.cache.HeaderConstants;
 import org.apache.http.client.cache.HttpCacheEntry;
@@ -146,6 +147,10 @@ class CachedResponseSuitabilityChecker {
             return false;
         }
 
+        if (!isConditional(request) && entry.getStatusCode() == HttpStatus.SC_NOT_MODIFIED) {
+		return false;
+        }
+
         if (isConditional(request) && !allConditionalsMatch(request, entry, now)) {
             return false;
         }

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

@@ -283,7 +283,9 @@ public class CachingExec implements ClientExecChain {
         } else if (!mayCallBackend(request)) {
             log.debug("Cache entry not suitable but only-if-cached requested");
             out = Proxies.enhanceResponse(generateGatewayTimeout(context));
-        } else if (validityPolicy.isRevalidatable(entry)) {
+        } else if (validityPolicy.isRevalidatable(entry) 
+                && !(entry.getStatusCode() == HttpStatus.SC_NOT_MODIFIED 
+                && !suitabilityChecker.isConditional(request))) {
             log.debug("Revalidating cache entry");
             return revalidateCacheEntry(route, request, context, execAware, entry, now);
         } else {

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

@@ -1631,7 +1631,48 @@ public class TestCachingExec {
 		assertEquals("etag", result2.getFirstHeader("Etag").getValue());
 	}
 
-    private IExpectationSetters<CloseableHttpResponse> backendExpectsAnyRequestAndReturn(
+	@Test
+	public void testDoesNotSend304ForNonConditionalRequest() throws Exception {
+
+		Date now = new Date();
+		Date inOneMinute = new Date(System.currentTimeMillis()+60000);
+
+		impl = new CachingExec(mockBackend, new BasicHttpCache(),CacheConfig.DEFAULT);
+
+		HttpRequestWrapper req1 = HttpRequestWrapper.wrap(new HttpGet("http://foo.example.com/"));
+		req1.addHeader("If-None-Match", "etag");
+
+		HttpRequestWrapper req2 = HttpRequestWrapper.wrap(new HttpGet("http://foo.example.com/"));
+
+		HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_NOT_MODIFIED, "Not modified");
+		resp1.setHeader("Date", DateUtils.formatDate(now));
+		resp1.setHeader("Cache-Control","public, max-age=60");
+		resp1.setHeader("Expires",DateUtils.formatDate(inOneMinute));
+		resp1.setHeader("Etag", "etag");
+		resp1.setHeader("Vary", "Accept-Encoding");
+
+		HttpResponse resp2 = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_OK, "Ok");
+		resp2.setHeader("Date", DateUtils.formatDate(now));
+		resp2.setHeader("Cache-Control","public, max-age=60");
+		resp2.setHeader("Expires",DateUtils.formatDate(inOneMinute));
+		resp2.setHeader("Etag", "etag");
+		resp2.setHeader("Vary", "Accept-Encoding");
+		resp2.setEntity(HttpTestUtils.makeBody(128));
+
+		backendExpectsAnyRequestAndReturn(resp1);
+		backendExpectsAnyRequestAndReturn(resp2).anyTimes();
+		replayMocks();
+		HttpResponse result1 = impl.execute(route, req1);
+		HttpResponse result2 = impl.execute(route, req2);
+		verifyMocks();
+
+		assertEquals(HttpStatus.SC_NOT_MODIFIED, result1.getStatusLine().getStatusCode());
+		assertNull(result1.getEntity());
+		assertEquals(HttpStatus.SC_OK, result2.getStatusLine().getStatusCode());
+		Assert.assertNotNull(result2.getEntity());
+	}
+
+	private IExpectationSetters<CloseableHttpResponse> backendExpectsAnyRequestAndReturn(
             HttpResponse response) throws Exception {
         CloseableHttpResponse resp = mockBackend.execute(
                 EasyMock.isA(HttpRoute.class),