### Eclipse Workspace Patch 1.0 #P httpcomponents-client Index: httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRequirements.java =================================================================== --- httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRequirements.java (revision 996124) +++ httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRequirements.java (working copy) @@ -2878,7 +2878,7 @@ HttpRequest req2 = new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1); req2.setHeader("If-None-Match", "W/\"etag\""); - req2.setHeader("If-Unmodified-Since", DateUtils.formatDate(twentySecondsAgo)); + req2.setHeader("If-Modified-Since", DateUtils.formatDate(twentySecondsAgo)); // must hit the origin again for the second request EasyMock.expect( @@ -2893,6 +2893,36 @@ Assert.assertFalse(HttpStatus.SC_NOT_MODIFIED == result.getStatusLine().getStatusCode()); } + @Test + public void testConditionalRequestWhereAllValidatorsMatchMayBeServedFromCache() + throws Exception { + Date now = new Date(); + Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L); + + HttpRequest req1 = new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1); + HttpResponse resp1 = make200Response(); + resp1.setHeader("Date", DateUtils.formatDate(now)); + resp1.setHeader("Cache-Control", "max-age=3600"); + resp1.setHeader("Last-Modified", DateUtils.formatDate(tenSecondsAgo)); + resp1.setHeader("ETag", "W/\"etag\""); + + HttpRequest req2 = new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1); + req2.setHeader("If-None-Match", "W/\"etag\""); + req2.setHeader("If-Modified-Since", DateUtils.formatDate(tenSecondsAgo)); + + // may hit the origin again for the second request + EasyMock.expect( + mockBackend.execute(EasyMock.isA(HttpHost.class), + EasyMock.isA(HttpRequest.class), + (HttpContext) EasyMock.isNull())).andReturn(resp1).times(1,2); + + replayMocks(); + impl.execute(host, req1); + impl.execute(host, req2); + verifyMocks(); + } + + /* * "However, a cache that does not support the Range and Content-Range * headers MUST NOT cache 206 (Partial Content) responses." Index: httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheValidityPolicy.java =================================================================== --- httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheValidityPolicy.java (revision 996124) +++ httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheValidityPolicy.java (working copy) @@ -30,7 +30,6 @@ import org.apache.http.Header; import org.apache.http.HeaderElement; -import org.apache.http.HttpRequest; import org.apache.http.annotation.Immutable; import org.apache.http.client.cache.HeaderConstants; import org.apache.http.client.cache.HttpCacheEntry; @@ -79,28 +78,6 @@ || entry.getFirstHeader(HeaderConstants.LAST_MODIFIED) != null; } - public boolean modifiedSince(final HttpCacheEntry entry, final HttpRequest request) { - Header unmodHeader = request.getFirstHeader(HeaderConstants.IF_UNMODIFIED_SINCE); - - if (unmodHeader == null) { - return false; - } - - try { - Date unmodifiedSinceDate = DateUtils.parseDate(unmodHeader.getValue()); - Date lastModifiedDate = DateUtils.parseDate(entry.getFirstHeader( - HeaderConstants.LAST_MODIFIED).getValue()); - - if (unmodifiedSinceDate.before(lastModifiedDate)) { - return true; - } - } catch (DateParseException e) { - return false; - } - - return false; - } - public boolean mustRevalidate(final HttpCacheEntry entry) { return hasCacheControlDirective(entry, "must-revalidate"); } Index: httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachedResponseSuitabilityChecker.java =================================================================== --- httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachedResponseSuitabilityChecker.java (revision 996124) +++ httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachedResponseSuitabilityChecker.java (working copy) @@ -78,7 +78,6 @@ public void testSuitableIfContentLengthHeaderIsRight() { responseIsFresh(true); contentLengthMatchesActualLength(true); - modifiedSince(false, request); replayMocks(); boolean result = impl.canCachedResponseBeUsed(host, request, entry); @@ -92,7 +91,6 @@ public void testSuitableIfCacheEntryIsFresh() { responseIsFresh(true); contentLengthMatchesActualLength(true); - modifiedSince(false, request); replayMocks(); @@ -120,7 +118,6 @@ request.addHeader("Cache-Control", "no-cache"); responseIsFresh(true); contentLengthMatchesActualLength(true); - modifiedSince(false, request); replayMocks(); @@ -134,7 +131,6 @@ request.addHeader("Cache-Control", "max-age=10"); responseIsFresh(true); contentLengthMatchesActualLength(true); - modifiedSince(false, request); currentAge(20L); replayMocks(); @@ -149,7 +145,6 @@ request.addHeader("Cache-Control", "max-age=10"); responseIsFresh(true); contentLengthMatchesActualLength(true); - modifiedSince(false, request); currentAge(5L); replayMocks(); @@ -164,7 +159,6 @@ request.addHeader("Cache-Control", "min-fresh=10"); responseIsFresh(true); contentLengthMatchesActualLength(true); - modifiedSince(false, request); freshnessLifetime(15L); replayMocks(); @@ -179,7 +173,6 @@ request.addHeader("Cache-Control", "min-fresh=10"); responseIsFresh(true); contentLengthMatchesActualLength(true); - modifiedSince(false, request); freshnessLifetime(5L); replayMocks(); @@ -208,7 +201,6 @@ request.addHeader(new BasicHeader("Cache-Control", "max-age=foo")); responseIsFresh(true); contentLengthMatchesActualLength(true); - modifiedSince(false, request); replayMocks(); @@ -225,7 +217,6 @@ responseIsFresh(true); contentLengthMatchesActualLength(true); - modifiedSince(false, request); replayMocks(); @@ -251,11 +242,6 @@ mockValidityPolicy.isResponseFresh(entry)).andReturn(fresh); } - private void modifiedSince(boolean modified, HttpRequest request) { - EasyMock.expect( - mockValidityPolicy.modifiedSince(entry, request)).andReturn(modified); - } - private void contentLengthMatchesActualLength(boolean b) { EasyMock.expect( mockValidityPolicy.contentLengthHeaderMatchesActualLength(entry)).andReturn(b); Index: httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java =================================================================== --- httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java (revision 996124) +++ httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java (working copy) @@ -26,6 +26,8 @@ */ package org.apache.http.impl.client.cache; +import java.util.Date; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.http.Header; @@ -35,6 +37,8 @@ import org.apache.http.annotation.Immutable; import org.apache.http.client.cache.HeaderConstants; import org.apache.http.client.cache.HttpCacheEntry; +import org.apache.http.impl.cookie.DateParseException; +import org.apache.http.impl.cookie.DateUtils; /** * Determines whether a given {@link HttpCacheEntry} is suitable to be @@ -81,10 +85,15 @@ return false; } - if (validityStrategy.modifiedSince(entry, request)) { - log.debug("Cache entry modified times didn't line up. Cache Entry should not be used"); - return false; + if (hasUnsupportedConditionalHeaders(request)) { + log.debug("Request contained conditional headers we don't handle"); + return false; } + + if (containsEtagAndLastModifiedValidators(request) + && !allConditionalsMatch(request, entry)) { + return false; + } for (Header ccHdr : request.getHeaders(HeaderConstants.CACHE_CONTROL)) { for (HeaderElement elt : ccHdr.getElements()) { @@ -146,4 +155,93 @@ log.debug("Response from cache was suitable"); return true; } + + private boolean hasUnsupportedConditionalHeaders(HttpRequest request) { + return (request.getFirstHeader("If-Range") != null + || request.getFirstHeader("If-Match") != null + || hasValidDateField(request, "If-Unmodified-Since")); + } + + /** + * Should return false if some conditionals would allow a + * normal request but some would not. + * @param request + * @param entry + * @return + */ + private boolean allConditionalsMatch(HttpRequest request, + HttpCacheEntry entry) { + Header etagHeader = entry.getFirstHeader("ETag"); + String etag = (etagHeader != null) ? etagHeader.getValue() : null; + Header[] ifNoneMatch = request.getHeaders("If-None-Match"); + if (ifNoneMatch != null && ifNoneMatch.length > 0) { + boolean matched = false; + for(Header h : ifNoneMatch) { + for(HeaderElement elt : h.getElements()) { + String reqEtag = elt.toString(); + if (("*".equals(reqEtag) && etag != null) + || reqEtag.equals(etag)) { + matched = true; + break; + } + } + } + if (!matched) return false; + } + Header lmHeader = entry.getFirstHeader("Last-Modified"); + Date lastModified = null; + try { + if (lmHeader != null) { + lastModified = DateUtils.parseDate(lmHeader.getValue()); + } + } catch (DateParseException dpe) { + // nop + } + for(Header h : request.getHeaders("If-Modified-Since")) { + try { + Date cond = DateUtils.parseDate(h.getValue()); + if (lastModified == null + || lastModified.after(cond)) { + return false; + } + } catch (DateParseException dpe) { + } + } + return true; + } + + private boolean containsEtagAndLastModifiedValidators(HttpRequest request) { + boolean hasEtagValidators = (hasEtagIfRangeHeader(request) + || request.getFirstHeader("If-Match") != null + || request.getFirstHeader("If-None-Match") != null); + if (!hasEtagValidators) return false; + final boolean hasLastModifiedValidators = + hasValidDateField(request, "If-Modified-Since") + || hasValidDateField(request, "If-Unmodified-Since") + || hasValidDateField(request, "If-Range"); + return hasLastModifiedValidators; + } + + private boolean hasEtagIfRangeHeader(HttpRequest request) { + for(Header h : request.getHeaders("If-Range")) { + try { + DateUtils.parseDate(h.getValue()); + } catch (DateParseException dpe) { + return true; + } + } + return false; + } + + private boolean hasValidDateField(HttpRequest request, String headerName) { + for(Header h : request.getHeaders(headerName)) { + try { + DateUtils.parseDate(h.getValue()); + return true; + } catch (DateParseException dpe) { + // ignore malformed dates + } + } + return false; + } }