diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index 38b7853e6..b8437e2d0 100644 --- a/RELEASE_NOTES.txt +++ b/RELEASE_NOTES.txt @@ -1,6 +1,10 @@ Changes since 4.1 ALPHA2 ------------------- +* [HTTPCLIENT-964] no-cache directives with field names are no longer transmitted + downstream. + 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/RequestProtocolCompliance.java b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/RequestProtocolCompliance.java index 19a047df2..5de2085ed 100644 --- a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/RequestProtocolCompliance.java +++ b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/RequestProtocolCompliance.java @@ -76,6 +76,11 @@ public class RequestProtocolCompliance { theErrors.add(anError); } + anError = requestContainsNoCacheDirectiveWithFieldName(request); + if (anError != null) { + theErrors.add(anError); + } + return theErrors; } @@ -259,6 +264,11 @@ public class RequestProtocolCompliance { HttpStatus.SC_BAD_REQUEST, "Weak eTag not compatible with PUT or DELETE requests")); + case NO_CACHE_DIRECTIVE_WITH_FIELD_NAME: + return new BasicHttpResponse(new BasicStatusLine(CachingHttpClient.HTTP_1_1, + HttpStatus.SC_BAD_REQUEST, + "No-Cache directive MUST NOT include a field name")); + default: throw new IllegalStateException( "The request was compliant, therefore no error can be generated for it."); @@ -329,4 +339,16 @@ public class RequestProtocolCompliance { return RequestProtocolError.BODY_BUT_NO_LENGTH_ERROR; } + + private RequestProtocolError requestContainsNoCacheDirectiveWithFieldName(HttpRequest request) { + for(Header h : request.getHeaders("Cache-Control")) { + for(HeaderElement elt : h.getElements()) { + if ("no-cache".equalsIgnoreCase(elt.getName()) + && elt.getValue() != null) { + return RequestProtocolError.NO_CACHE_DIRECTIVE_WITH_FIELD_NAME; + } + } + } + return null; + } } diff --git a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/RequestProtocolError.java b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/RequestProtocolError.java index 5064e2d16..444a59610 100644 --- a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/RequestProtocolError.java +++ b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/RequestProtocolError.java @@ -34,6 +34,7 @@ public enum RequestProtocolError { UNKNOWN, BODY_BUT_NO_LENGTH_ERROR, WEAK_ETAG_ON_PUTDELETE_METHOD_ERROR, - WEAK_ETAG_AND_RANGE_ERROR + WEAK_ETAG_AND_RANGE_ERROR, + NO_CACHE_DIRECTIVE_WITH_FIELD_NAME } 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 a5afc81f8..7754160f0 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 @@ -4778,6 +4778,91 @@ public class TestProtocolRequirements { testSharedCacheMustUseNewRequestHeadersWhenRevalidatingAuthorizedResponse(resp1); } + /* "If a cache returns a stale response, either because of a max-stale + * directive on a request, or because the cache is configured to + * override the expiration time of a response, the cache MUST attach a + * Warning header to the stale response, using Warning 110 (Response + * is stale). + * + * http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9.3 + */ + @Test + public void testWarning110IsAddedToStaleResponses() + throws Exception { + Date now = new Date(); + Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L); + HttpRequest req1 = new BasicHttpRequest("GET","/",HTTP_1_1); + HttpResponse resp1 = make200Response(); + resp1.setHeader("Date", DateUtils.formatDate(tenSecondsAgo)); + resp1.setHeader("Cache-Control","max-age=5"); + resp1.setHeader("Etag","\"etag\""); + + backendExpectsAnyRequest().andReturn(resp1); + + HttpRequest req2 = new BasicHttpRequest("GET","/",HTTP_1_1); + req2.setHeader("Cache-Control","max-stale=60"); + HttpResponse resp2 = make200Response(); + + 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()) { + boolean found110Warning = false; + for(Header h : result.getHeaders("Warning")) { + for(HeaderElement elt : h.getElements()) { + String[] parts = elt.getName().split("\\s"); + if ("110".equals(parts[0])) { + found110Warning = true; + break; + } + } + } + Assert.assertTrue(found110Warning); + } + } + + /* "Field names MUST NOT be included with the no-cache directive in a + * request." + * + * http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9.4 + */ + @Test + public void testDoesNotTransmitNoCacheDirectivesWithFieldsDownstream() + throws Exception { + request.setHeader("Cache-Control","no-cache=\"X-Field\""); + Capture cap = new Capture(); + EasyMock.expect(mockBackend.execute(EasyMock.eq(host), + EasyMock.capture(cap), + (HttpContext)EasyMock.isNull())) + .andReturn(originResponse).times(0,1); + + replayMocks(); + try { + impl.execute(host, request); + } catch (ClientProtocolException acceptable) { + } + verifyMocks(); + + if (cap.hasCaptured()) { + HttpRequest captured = cap.getValue(); + for(Header h : captured.getHeaders("Cache-Control")) { + for(HeaderElement elt : h.getElements()) { + if ("no-cache".equals(elt.getName())) { + Assert.assertNull(elt.getValue()); + } + } + } + } + } + private class FakeHeaderGroup extends HeaderGroup{ public void addHeader(String name, String value){