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 18:02:49 UTC
svn commit: r1421977 - in /httpcomponents/httpclient/trunk: ./
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 17:02:46 2012
New Revision: 1421977
URL: http://svn.apache.org/viewvc?rev=1421977&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/trunk/RELEASE_NOTES.txt
httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java
httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java
httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExec.java
Modified: httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt?rev=1421977&r1=1421976&r2=1421977&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpclient/trunk/RELEASE_NOTES.txt Fri Dec 14 17:02:46 2012
@@ -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>
Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java?rev=1421977&r1=1421976&r2=1421977&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java Fri Dec 14 17:02:46 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/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=1421977&r1=1421976&r2=1421977&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 Fri Dec 14 17:02:46 2012
@@ -283,7 +283,9 @@ public class CachingExec implements Clie
} 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 {
Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExec.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExec.java?rev=1421977&r1=1421976&r2=1421977&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExec.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExec.java Fri Dec 14 17:02:46 2012
@@ -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),