From 6aad9ccf99377c8c5b1dc5c4a1649fafde8a33c4 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Mon, 12 Jul 2010 21:36:47 +0000 Subject: [PATCH] HTTPCLIENT-963: Fixed handling of 'Cache-Control: no-store' on requests Contributed by Jonathan Moore git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@963495 13f79535-47bb-0310-9956-ffa450edef68 --- RELEASE_NOTES.txt | 5 +- .../client/cache/ResponseCachingPolicy.java | 11 +- .../cache/TestProtocolRequirements.java | 207 ++++++++++++++++++ .../cache/TestResponseCachingPolicy.java | 8 + 4 files changed, 227 insertions(+), 4 deletions(-) diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index e8b3887d4..1a65f0fb9 100644 --- a/RELEASE_NOTES.txt +++ b/RELEASE_NOTES.txt @@ -5,10 +5,13 @@ Changes since 4.1 ALPHA2 proxy-revalidate Cache-Control directives. Contributed by Jonathan Moore -* [HTTPCLIENT-964] no-cache directives with field names are no longer transmitted +* [HTTPCLIENT-964] 'no-cache' directives with field names are no longer transmitted downstream. Contributed by Jonathan Moore +* [HTTPCLIENT-963] Fixed handling of 'Cache-Control: no-store' on requests. + Contributed by Jonathan Moore + * [HTTPCLIENT-962] Fixed handling of Authorization headers in shared cache mode. Contributed by Jonathan Moore diff --git a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java index 6a646b41d..48488956c 100644 --- a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java +++ b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java @@ -30,6 +30,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.http.Header; import org.apache.http.HeaderElement; +import org.apache.http.HttpMessage; import org.apache.http.HttpRequest; import org.apache.http.HttpResponse; import org.apache.http.HttpStatus; @@ -153,12 +154,12 @@ public class ResponseCachingPolicy { return false; } - protected boolean hasCacheControlParameterFrom(HttpResponse response, String[] params) { - Header[] cacheControlHeaders = response.getHeaders(HeaderConstants.CACHE_CONTROL); + protected boolean hasCacheControlParameterFrom(HttpMessage msg, String[] params) { + Header[] cacheControlHeaders = msg.getHeaders(HeaderConstants.CACHE_CONTROL); for (Header header : cacheControlHeaders) { for (HeaderElement elem : header.getElements()) { for (String param : params) { - if (param.equals(elem.getName())) { + if (param.equalsIgnoreCase(elem.getName())) { return true; } } @@ -189,6 +190,10 @@ public class ResponseCachingPolicy { log.debug("Response was not cacheable."); return false; } + String[] uncacheableRequestDirectives = { "no-store" }; + if (hasCacheControlParameterFrom(request,uncacheableRequestDirectives)) { + return false; + } if (request.getRequestLine().getUri().contains("?") && !isExplicitlyCacheable(response)) { log.debug("Response was not cacheable."); diff --git a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRequirements.java b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRequirements.java index 2377f181c..beac5c5e0 100644 --- a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRequirements.java +++ b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRequirements.java @@ -5047,7 +5047,214 @@ public class TestProtocolRequirements { } } + /* "[The cache control directive] "private" Indicates that all or part of + * the response message is intended for a single user and MUST NOT be + * cached by a shared cache." + * + * http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9.1 + */ + @Test + public void testCacheControlPrivateIsNotCacheableBySharedCache() + throws Exception { + if (impl.isSharedCache()) { + HttpRequest req1 = new BasicHttpRequest("GET","/",HTTP_1_1); + HttpResponse resp1 = make200Response(); + resp1.setHeader("Cache-Control","private,max-age=3600"); + backendExpectsAnyRequest().andReturn(resp1); + + HttpRequest req2 = new BasicHttpRequest("GET","/",HTTP_1_1); + HttpResponse resp2 = make200Response(); + // this backend request MUST happen + backendExpectsAnyRequest().andReturn(resp2); + + replayMocks(); + impl.execute(host,req1); + impl.execute(host,req2); + verifyMocks(); + } + } + + @Test + public void testCacheControlPrivateOnFieldIsNotReturnedBySharedCache() + throws Exception { + if (impl.isSharedCache()) { + HttpRequest req1 = new BasicHttpRequest("GET","/",HTTP_1_1); + HttpResponse resp1 = make200Response(); + resp1.setHeader("X-Personal","stuff"); + resp1.setHeader("Cache-Control","private=\"X-Personal\",s-maxage=3600"); + + backendExpectsAnyRequest().andReturn(resp1); + + HttpRequest req2 = new BasicHttpRequest("GET","/",HTTP_1_1); + HttpResponse resp2 = make200Response(); + + // this backend request MAY happen + backendExpectsAnyRequest().andReturn(resp2).times(0,1); + + replayMocks(); + impl.execute(host,req1); + HttpResponse result = impl.execute(host,req2); + verifyMocks(); + Assert.assertNull(result.getFirstHeader("X-Personal")); + } + } + + /* "If the no-cache directive does not specify a field-name, then a + * cache MUST NOT use the response to satisfy a subsequent request + * without successful revalidation with the origin server. This allows + * an origin server to prevent caching even by caches that have been + * configured to return stale responses to client requests." + * + * http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9.1 + */ + @Test + public void testNoCacheCannotSatisfyASubsequentRequestWithoutRevalidation() + throws Exception { + HttpRequest req1 = new BasicHttpRequest("GET","/",HTTP_1_1); + HttpResponse resp1 = make200Response(); + resp1.setHeader("ETag","\"etag\""); + resp1.setHeader("Cache-Control","no-cache"); + + backendExpectsAnyRequest().andReturn(resp1); + + HttpRequest req2 = new BasicHttpRequest("GET","/",HTTP_1_1); + HttpResponse resp2 = make200Response(); + + // this MUST happen + backendExpectsAnyRequest().andReturn(resp2); + + replayMocks(); + impl.execute(host,req1); + impl.execute(host,req2); + verifyMocks(); + } + + @Test + public void testNoCacheCannotSatisfyASubsequentRequestWithoutRevalidationEvenWithContraryIndications() + throws Exception { + HttpRequest req1 = new BasicHttpRequest("GET","/",HTTP_1_1); + HttpResponse resp1 = make200Response(); + resp1.setHeader("ETag","\"etag\""); + resp1.setHeader("Cache-Control","no-cache,s-maxage=3600"); + + backendExpectsAnyRequest().andReturn(resp1); + + HttpRequest req2 = new BasicHttpRequest("GET","/",HTTP_1_1); + req2.setHeader("Cache-Control","max-stale=7200"); + HttpResponse resp2 = make200Response(); + + // this MUST happen + backendExpectsAnyRequest().andReturn(resp2); + + replayMocks(); + impl.execute(host,req1); + impl.execute(host,req2); + verifyMocks(); + } + + /* "If the no-cache directive does specify one or more field-names, then + * a cache MAY use the response to satisfy a subsequent request, subject + * to any other restrictions on caching. However, the specified + * field-name(s) MUST NOT be sent in the response to a subsequent request + * without successful revalidation with the origin server." + */ + @Test + public void testNoCacheOnFieldIsNotReturnedWithoutRevalidation() + throws Exception { + HttpRequest req1 = new BasicHttpRequest("GET","/",HTTP_1_1); + HttpResponse resp1 = make200Response(); + resp1.setHeader("ETag","\"etag\""); + resp1.setHeader("X-Stuff","things"); + resp1.setHeader("Cache-Control","no-cache=\"X-Stuff\", max-age=3600"); + + backendExpectsAnyRequest().andReturn(resp1); + + HttpRequest req2 = new BasicHttpRequest("GET","/",HTTP_1_1); + HttpResponse resp2 = make200Response(); + resp2.setHeader("ETag","\"etag\""); + resp2.setHeader("X-Stuff","things"); + resp2.setHeader("Cache-Control","no-cache=\"X-Stuff\",max-age=3600"); + + Capture cap = new Capture(); + EasyMock.expect(mockBackend.execute(EasyMock.eq(host), + EasyMock.capture(cap), + (HttpContext)EasyMock.isNull())) + .andReturn(resp2).times(0,1); + + replayMocks(); + impl.execute(host,req1); + HttpResponse result = impl.execute(host,req2); + verifyMocks(); + + if (!cap.hasCaptured()) { + Assert.assertNull(result.getFirstHeader("X-Stuff")); + } + } + + /* "The purpose of the no-store directive is to prevent the inadvertent + * release or retention of sensitive information (for example, on backup + * tapes). The no-store directive applies to the entire message, and MAY + * be sent either in a response or in a request. If sent in a request, a + * cache MUST NOT store any part of either this request or any response + * to it. If sent in a response, a cache MUST NOT store any part of + * either this response or the request that elicited it. This directive + * applies to both non- shared and shared caches. "MUST NOT store" in + * this context means that the cache MUST NOT intentionally store the + * information in non-volatile storage, and MUST make a best-effort + * attempt to remove the information from volatile storage as promptly + * as possible after forwarding it." + * + * http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9.2 + */ + @Test + public void testNoStoreOnRequestIsNotStoredInCache() + throws Exception { + emptyMockCacheExpectsNoPuts(); + request.setHeader("Cache-Control","no-store"); + backendExpectsAnyRequest().andReturn(originResponse); + + replayMocks(); + impl.execute(host,request); + verifyMocks(); + } + + @Test + public void testNoStoreOnRequestIsNotStoredInCacheEvenIfResponseMarkedCacheable() + throws Exception { + emptyMockCacheExpectsNoPuts(); + request.setHeader("Cache-Control","no-store"); + originResponse.setHeader("Cache-Control","max-age=3600"); + backendExpectsAnyRequest().andReturn(originResponse); + + replayMocks(); + impl.execute(host,request); + verifyMocks(); + } + + @Test + public void testNoStoreOnResponseIsNotStoredInCache() + throws Exception { + emptyMockCacheExpectsNoPuts(); + originResponse.setHeader("Cache-Control","no-store"); + backendExpectsAnyRequest().andReturn(originResponse); + + replayMocks(); + impl.execute(host,request); + verifyMocks(); + } + + @Test + public void testNoStoreOnResponseIsNotStoredInCacheEvenWithContraryIndicators() + throws Exception { + emptyMockCacheExpectsNoPuts(); + originResponse.setHeader("Cache-Control","no-store,max-age=3600"); + backendExpectsAnyRequest().andReturn(originResponse); + + replayMocks(); + impl.execute(host,request); + verifyMocks(); + } private class FakeHeaderGroup extends HeaderGroup{ diff --git a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestResponseCachingPolicy.java b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestResponseCachingPolicy.java index f0e2214a6..b07b47830 100644 --- a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestResponseCachingPolicy.java +++ b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestResponseCachingPolicy.java @@ -54,6 +54,7 @@ public class TestResponseCachingPolicy { @Before public void setUp() throws Exception { policy = new ResponseCachingPolicy(0); + request = new BasicHttpRequest("GET","/",HTTP_1_1); response = new BasicHttpResponse( new BasicStatusLine(HTTP_1_1, HttpStatus.SC_OK, "")); response.setHeader("Date", DateUtils.formatDate(new Date())); @@ -299,6 +300,13 @@ public class TestResponseCachingPolicy { Assert.assertFalse(policy.isResponseCacheable("get", response)); } + @Test + public void testResponsesToRequestsWithNoStoreAreNotCacheable() { + request.setHeader("Cache-Control","no-store"); + response.setHeader("Cache-Control","public"); + Assert.assertFalse(policy.isResponseCacheable(request,response)); + } + @Test public void testResponsesWithMultipleAgeHeadersAreNotCacheable() { response.addHeader("Age", "3");