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 2013/01/08 13:16:24 UTC
svn commit: r1430247 - 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: Tue Jan 8 12:16:23 2013
New Revision: 1430247
URL: http://svn.apache.org/viewvc?rev=1430247&view=rev
Log:
HTTPCLIENT-1290: 304 cached response never reused with If-modified-since conditional requests
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/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=1430247&r1=1430246&r2=1430247&view=diff
==============================================================================
--- httpcomponents/httpclient/branches/4.2.x/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpclient/branches/4.2.x/RELEASE_NOTES.txt Tue Jan 8 12:16:23 2013
@@ -10,6 +10,10 @@ Users of HttpClient 4.x are advised to u
Changelog
-------------------
+* [HTTPCLIENT-1290] 304 cached response never reused with If-modified-since conditional
+ requests.
+ Contributed by Francois-Xavier Bonnet <francois-xavier.bonnet at centraliens.net>
+
* [HTTPCLIENT-1291] Absolute request URIs without an explicitly specified path are rewritten
to have "/" path).
Contributed by Oleg Kalnichevski <olegk at apache.org>
@@ -22,8 +26,8 @@ Changelog
route differs from the host name specified in the request URI.
Contributed by Oleg Kalnichevski <olegk at apache.org>
-* Kerberos and SPNego auth schemes use incorrect authorization header name when authenticating
- with a proxy.
+* [HTTPCLIENT-1293] Kerberos and SPNego auth schemes use incorrect authorization header name
+ when authenticating with a proxy.
Contributed by Oleg Kalnichevski <olegk at apache.org>
* [HTTPCLIENT-1283] NTLM needs to use Locale-independent form of
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=1430247&r1=1430246&r2=1430247&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 Tue Jan 8 12:16:23 2013
@@ -458,13 +458,13 @@ public class CachingHttpClient implement
return revalidateCacheEntry(target, request, context, entry, now);
} else {
log.debug("Cache entry not usable; calling backend");
- return callBackend(target, request, context);
+ return callBackend(target, request, context);
}
if (context != null) {
- context.setAttribute(ExecutionContext.HTTP_TARGET_HOST, target);
- context.setAttribute(ExecutionContext.HTTP_REQUEST, request);
- context.setAttribute(ExecutionContext.HTTP_RESPONSE, out);
- context.setAttribute(ExecutionContext.HTTP_REQ_SENT, true);
+ context.setAttribute(ExecutionContext.HTTP_TARGET_HOST, target);
+ context.setAttribute(ExecutionContext.HTTP_REQUEST, request);
+ context.setAttribute(ExecutionContext.HTTP_RESPONSE, out);
+ context.setAttribute(ExecutionContext.HTTP_REQ_SENT, true);
}
return out;
}
@@ -915,6 +915,7 @@ public class CachingHttpClient implement
if (cacheable &&
!alreadyHaveNewerCacheEntry(target, request, backendResponse)) {
try {
+ storeRequestIfModifiedSinceFor304Response(request, backendResponse);
return responseCache.cacheAndReturnResponse(target, request, backendResponse, requestDate,
responseDate);
} catch (IOException ioe) {
@@ -931,6 +932,24 @@ public class CachingHttpClient implement
return backendResponse;
}
+ /**
+ * For 304 Not modified responses, adds a "Last-Modified" header with the
+ * value of the "If-Modified-Since" header passed in the request. This
+ * header is required to be able to reuse match the cache entry for
+ * subsequent requests but as defined in http specifications it is not
+ * included in 304 responses by backend servers. This header will not be
+ * included in the resulting response.
+ */
+ private void storeRequestIfModifiedSinceFor304Response(
+ HttpRequest request, HttpResponse backendResponse) {
+ if (backendResponse.getStatusLine().getStatusCode() == HttpStatus.SC_NOT_MODIFIED) {
+ Header h = request.getFirstHeader("If-Modified-Since");
+ if (h != null) {
+ backendResponse.addHeader("Last-Modified", h.getValue());
+ }
+ }
+ }
+
private boolean alreadyHaveNewerCacheEntry(HttpHost target, HttpRequest request,
HttpResponse backendResponse) {
HttpCacheEntry existing = null;
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=1430247&r1=1430246&r2=1430247&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 Tue Jan 8 12:16:23 2013
@@ -34,6 +34,7 @@ import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import junit.framework.AssertionFailedError;
import org.apache.http.Header;
import org.apache.http.HttpEntity;
import org.apache.http.HttpHost;
@@ -1079,6 +1080,33 @@ public class TestCachingHttpClient {
}
+ @Test
+ public void testReturns304ForIfModifiedSinceHeaderIf304ResponseInCache() throws Exception {
+ Date now = new Date();
+ Date oneHourAgo = new Date(now.getTime() - 3600 * 1000L);
+ Date inTenMinutes = new Date(now.getTime() + 600 * 1000L);
+ impl = new CachingHttpClient(mockBackend);
+ HttpRequest req1 = new HttpGet("http://foo.example.com/");
+ req1.addHeader("If-Modified-Since", DateUtils.formatDate(oneHourAgo));
+ HttpRequest req2 = new HttpGet("http://foo.example.com/");
+ req2.addHeader("If-Modified-Since", DateUtils.formatDate(oneHourAgo));
+
+ HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_NOT_MODIFIED, "Not modified");
+ resp1.setHeader("Date", DateUtils.formatDate(now));
+ resp1.setHeader("Cache-control", "max-age=600");
+ resp1.setHeader("Expires", DateUtils.formatDate(inTenMinutes));
+
+ expect(mockBackend.execute(isA(HttpHost.class), isA(HttpRequest.class), (HttpContext) isNull())).andReturn(resp1).once();
+ expect(mockBackend.execute(isA(HttpHost.class), isA(HttpRequest.class), (HttpContext) isNull())).andThrow(new AssertionFailedError("Should have reused cached 304 response")).anyTimes();
+
+ replayMocks();
+ impl.execute(host, req1);
+ HttpResponse result = impl.execute(host, req2);
+ verifyMocks();
+ Assert.assertEquals(HttpStatus.SC_NOT_MODIFIED, result.getStatusLine().getStatusCode());
+ Assert.assertFalse(result.containsHeader("Last-Modified"));
+ }
+
@Test
public void testReturns200ForIfModifiedSinceDateIsLess() throws Exception {
Date now = new Date();