You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2012/12/14 17:46:17 UTC

svn commit: r1421973 - in /httpcomponents/httpclient/branches/4.2.x: ./ httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ httpclient-cache/src/test/java/org/apache/http/impl/client/cache/

Author: olegk
Date: Fri Dec 14 16:46:14 2012
New Revision: 1421973

URL: http://svn.apache.org/viewvc?rev=1421973&view=rev
Log:
HTTPCLIENT-1277: Caching client sends a 304 to an unconditional request.
Contributed by Francois-Xavier Bonnet <francois-xavier.bonnet at centraliens.net>

Modified:
    httpcomponents/httpclient/branches/4.2.x/RELEASE_NOTES.txt
    httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java
    httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java
    httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java

Modified: httpcomponents/httpclient/branches/4.2.x/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/branches/4.2.x/RELEASE_NOTES.txt?rev=1421973&r1=1421972&r2=1421973&view=diff
==============================================================================
--- httpcomponents/httpclient/branches/4.2.x/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpclient/branches/4.2.x/RELEASE_NOTES.txt Fri Dec 14 16:46:14 2012
@@ -1,6 +1,9 @@
 Changes since 4.2.2
 -------------------
 
+* [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>  
 

Modified: httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java?rev=1421973&r1=1421972&r2=1421973&view=diff
==============================================================================
--- httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java (original)
+++ httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java Fri Dec 14 16:46:14 2012
@@ -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;
         }

Modified: httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java?rev=1421973&r1=1421972&r2=1421973&view=diff
==============================================================================
--- httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java (original)
+++ httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java Fri Dec 14 16:46:14 2012
@@ -451,7 +451,9 @@ public class CachingHttpClient implement
         } else if (!mayCallBackend(request)) {
             log.debug("Cache entry not suitable but only-if-cached requested");
             out = 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(target, request, context, entry, now);
         } else {

Modified: httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java?rev=1421973&r1=1421972&r2=1421973&view=diff
==============================================================================
--- httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java (original)
+++ httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java Fri Dec 14 16:46:14 2012
@@ -2136,6 +2136,54 @@ public class TestCachingHttpClient {
         assertEquals("etag", result2.getFirstHeader("Etag").getValue());
     }
 
+    @Test
+    public void testDoesNotSend304ForNonConditionalRequest() throws Exception {
+
+        Date now = new Date();
+        Date inOneMinute = new Date(System.currentTimeMillis()+60000);
+
+        impl = new CachingHttpClient(mockBackend);
+
+        HttpRequest req1 = new HttpGet("http://foo.example.com/");
+        req1.addHeader("If-None-Match", "etag");
+
+        HttpRequest req2 = 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));
+
+        expect(
+                mockBackend.execute(isA(HttpHost.class),
+                        isA(HttpRequest.class), (HttpContext)
+                        isNull())).andReturn(resp1);
+        expect(
+                mockBackend.execute(isA(HttpHost.class),
+                        isA(HttpRequest.class), (HttpContext)
+                        isNull())).andReturn(resp2);
+
+        replayMocks();
+        HttpResponse result1 = impl.execute(host, req1);
+        HttpResponse result2 = impl.execute(host, 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 void getCacheEntryReturns(HttpCacheEntry result) throws IOException {
         expect(mockCache.getCacheEntry(host, request)).andReturn(result);
     }