Index: httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheValidityPolicy.java =================================================================== --- httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheValidityPolicy.java (revision 1206582) +++ httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheValidityPolicy.java (revision ) @@ -139,7 +139,6 @@ public void testResidentTimeSecondsIsTimeSinceResponseTime() { HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(now, sixSecondsAgo); impl = new CacheValidityPolicy() { - @Override protected Date getCurrentDate() { return now; } Index: httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java =================================================================== --- httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java (revision 1206582) +++ httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java (revision ) @@ -84,26 +84,26 @@ } switch (response.getStatusLine().getStatusCode()) { - case HttpStatus.SC_OK: - case HttpStatus.SC_NON_AUTHORITATIVE_INFORMATION: - case HttpStatus.SC_MULTIPLE_CHOICES: - case HttpStatus.SC_MOVED_PERMANENTLY: - case HttpStatus.SC_GONE: - // these response codes MAY be cached - cacheable = true; - log.debug("Response was cacheable"); - break; - case HttpStatus.SC_PARTIAL_CONTENT: - // we don't implement Range requests and hence are not - // allowed to cache partial content - log.debug("Response was not cacheable (Partial Content)"); - return cacheable; + case HttpStatus.SC_OK: + case HttpStatus.SC_NON_AUTHORITATIVE_INFORMATION: + case HttpStatus.SC_MULTIPLE_CHOICES: + case HttpStatus.SC_MOVED_PERMANENTLY: + case HttpStatus.SC_GONE: + // these response codes MAY be cached + cacheable = true; + log.debug("Response was cacheable"); + break; + case HttpStatus.SC_PARTIAL_CONTENT: + // we don't implement Range requests and hence are not + // allowed to cache partial content + log.debug("Response was not cacheable (Partial Content)"); + return cacheable; - default: - // If the status code is not one of the recognized - // available codes in HttpStatus Don't Cache - log.debug("Response was not cacheable (Unknown Status code)"); - return cacheable; + default: + // If the status code is not one of the recognized + // available codes in HttpStatus Don't Cache + log.debug("Response was not cacheable (Unknown Status code)"); + return cacheable; } Header contentLength = response.getFirstHeader(HTTP.CONTENT_LEN); 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 1206582) +++ httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheValidityPolicy.java (revision ) @@ -81,10 +81,10 @@ * if last-modified and date are defined, freshness lifetime is coefficient*(date-lastModified), * else freshness lifetime is defaultLifetime * - * @param entry - * @param now - * @param coefficient - * @param defaultLifetime + * @param entry the cache entry + * @param now what time is it currently (When is right NOW) + * @param coefficient Part of the heuristic for cache entry freshness + * @param defaultLifetime How long can I assume a cache entry is default TTL * @return {@code true} if the response is fresh */ public boolean isResponseHeuristicallyFresh(final HttpCacheEntry entry, @@ -108,7 +108,10 @@ } public boolean isRevalidatable(final HttpCacheEntry entry) { + if (!entry.getResource().isValid()) + return false; + else - return entry.getFirstHeader(HeaderConstants.ETAG) != null + return entry.getFirstHeader(HeaderConstants.ETAG) != null || entry.getFirstHeader(HeaderConstants.LAST_MODIFIED) != null; } @@ -212,6 +215,7 @@ * This matters for deciding whether the cache entry is valid to serve as a * response. If these values do not match, we might have a partial response * + * @param entry The cache entry we are currently working with * @return boolean indicating whether actual length matches Content-Length */ protected boolean contentLengthHeaderMatchesActualLength(final HttpCacheEntry entry) { @@ -260,10 +264,6 @@ return getCorrectedReceivedAgeSecs(entry) + getResponseDelaySecs(entry); } - protected Date getCurrentDate() { - return new Date(); - } - protected long getResidentTimeSecs(HttpCacheEntry entry, Date now) { long diff = now.getTime() - entry.getResponseDate().getTime(); return (diff / 1000L); Index: httpclient-cache/src/test/java/org/apache/http/client/cache/TestHttpCacheJiraNumber1147.java =================================================================== --- httpclient-cache/src/test/java/org/apache/http/client/cache/TestHttpCacheJiraNumber1147.java (revision ) +++ httpclient-cache/src/test/java/org/apache/http/client/cache/TestHttpCacheJiraNumber1147.java (revision ) @@ -0,0 +1,84 @@ +package org.apache.http.client.cache; + +import org.apache.http.HttpResponse; +import org.apache.http.StatusLine; +import org.apache.http.client.HttpClient; +import org.apache.http.client.HttpResponseException; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.impl.client.DefaultHttpClient; +import org.apache.http.impl.client.cache.CacheConfig; +import org.apache.http.impl.client.cache.CachingHttpClient; +import org.apache.http.impl.client.cache.FileResourceFactory; +import org.apache.http.impl.client.cache.ManagedHttpCacheStorage; +import org.junit.Ignore; +import org.junit.Test; + +import java.io.File; + +public class TestHttpCacheJiraNumber1147 { + final String cacheDir = "/tmp/cachedir"; + HttpClient cachingHttpClient; + HttpClient client = new DefaultHttpClient(); + + @Test + public void testIssue1147() throws Exception { + final CacheConfig cacheConfig = new CacheConfig(); + cacheConfig.setSharedCache(true); + cacheConfig.setMaxObjectSizeBytes(262144); //256kb + + new File(cacheDir).mkdir(); + + if(! new File(cacheDir, "httpclient-cache").exists()){ + if(!new File(cacheDir, "httpclient-cache").mkdir()){ + throw new RuntimeException("failed to create httpclient cache directory: " + + new File(cacheDir, "httpclient-cache").getAbsolutePath()); + } + } + + final ResourceFactory resourceFactory = new FileResourceFactory(new File(cacheDir, "httpclient-cache")); + + final HttpCacheStorage httpCacheStorage = new ManagedHttpCacheStorage(cacheConfig); + + cachingHttpClient = new CachingHttpClient(client, resourceFactory, httpCacheStorage, cacheConfig); + + final HttpGet get = new HttpGet("http://www.apache.org/js/jquery.js"); + + System.out.println("Calling URL First time."); + executeCall(get); + + removeDirectory(cacheDir); + + System.out.println("Calling URL Second time."); + executeCall(get); + } + + private void removeDirectory(String cacheDir) { + File theDirectory = new File(cacheDir, "httpclient-cache"); + File[] files = theDirectory.listFiles(); + + for (File cacheFile : files) { + cacheFile.delete(); + } + + theDirectory.delete(); + + new File(cacheDir).delete(); + } + + private void executeCall(HttpGet get) throws Exception { + final HttpResponse response = cachingHttpClient.execute(get); + final StatusLine statusLine = response.getStatusLine(); + System.out.println("Status Code: " + statusLine.getStatusCode()); + + if (statusLine.getStatusCode() >= 300) { + if(statusLine.getStatusCode() == 404) + throw new NoResultException(); + + throw new HttpResponseException(statusLine.getStatusCode(), statusLine.getReasonPhrase()); + } + response.getEntity().getContent(); + } + + private class NoResultException extends Exception { + } +} 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 1206582) +++ httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java (revision ) @@ -125,9 +125,15 @@ * {@link HttpRequest} * @param entry * {@link HttpCacheEntry} + * @param now + * Right now in time * @return boolean yes/no answer */ public boolean canCachedResponseBeUsed(HttpHost host, HttpRequest request, HttpCacheEntry entry, Date now) { + if (!entry.getResource().isValid()) { + return false; + } + if (!isFreshEnough(entry, request, now)) { log.trace("Cache entry was not fresh enough"); return false; @@ -213,7 +219,7 @@ /** * Is this request the type of conditional request we support? - * @param request + * @param request The current httpRequest being made * @return {@code true} if the request is supported */ public boolean isConditional(HttpRequest request) { @@ -222,24 +228,26 @@ /** * Check that conditionals that are part of this request match - * @param request - * @param entry - * @param now + * @param request The current httpRequest being made + * @param entry the cache entry + * @param now right NOW in time * @return {@code true} if the request matches all conditionals */ public boolean allConditionalsMatch(HttpRequest request, HttpCacheEntry entry, Date now) { boolean hasEtagValidator = hasSupportedEtagValidator(request); boolean hasLastModifiedValidator = hasSupportedLastModifiedValidator(request); - boolean etagValidatorMatches = (hasEtagValidator) ? etagValidatorMatches(request, entry) : false; - boolean lastModifiedValidatorMatches = (hasLastModifiedValidator) ? lastModifiedValidatorMatches(request, entry, now) : false; + boolean etagValidatorMatches = (hasEtagValidator) && etagValidatorMatches(request, entry); + boolean lastModifiedValidatorMatches = (hasLastModifiedValidator) && lastModifiedValidatorMatches(request, entry, now); if ((hasEtagValidator && hasLastModifiedValidator) && !(etagValidatorMatches && lastModifiedValidatorMatches)) { return false; } else if (hasEtagValidator && !etagValidatorMatches) { return false; - } if (hasLastModifiedValidator && !lastModifiedValidatorMatches) { + } + + if (hasLastModifiedValidator && !lastModifiedValidatorMatches) { return false; } return true; @@ -261,9 +269,9 @@ /** * Check entry against If-None-Match - * @param request - * @param entry - * @return + * @param request The current httpRequest being made + * @param entry the cache entry + * @return boolean does the etag validator match */ private boolean etagValidatorMatches(HttpRequest request, HttpCacheEntry entry) { Header etagHeader = entry.getFirstHeader(HeaderConstants.ETAG); @@ -286,10 +294,10 @@ /** * Check entry against If-Modified-Since, if If-Modified-Since is in the future it is invalid as per * http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html - * @param request - * @param entry - * @param now - * @return + * @param request The current httpRequest being made + * @param entry the cache entry + * @param now right NOW in time + * @return boolean Does the last modified header match */ private boolean lastModifiedValidatorMatches(HttpRequest request, HttpCacheEntry entry, Date now) { Header lastModifiedHeader = entry.getFirstHeader(HeaderConstants.LAST_MODIFIED); Index: httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HeapResource.java =================================================================== --- httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HeapResource.java (revision 1206582) +++ httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HeapResource.java (revision ) @@ -64,4 +64,8 @@ public void dispose() { } + public boolean isValid() { + return true; -} + } + +} Index: httpclient-cache/src/main/java/org/apache/http/impl/client/cache/FileResource.java =================================================================== --- httpclient-cache/src/main/java/org/apache/http/impl/client/cache/FileResource.java (revision 1206582) +++ httpclient-cache/src/main/java/org/apache/http/impl/client/cache/FileResource.java (revision ) @@ -31,6 +31,8 @@ import java.io.IOException; import java.io.InputStream; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.apache.http.annotation.ThreadSafe; import org.apache.http.client.cache.Resource; @@ -48,30 +50,34 @@ private volatile boolean disposed; + private final Log log = LogFactory.getLog(getClass()); + public FileResource(final File file) { super(); this.file = file; this.disposed = false; } - private void ensureValid() { - if (this.disposed) { - throw new IllegalStateException("Resource has been deallocated"); + public boolean isValid() { + if (this.disposed || !file.exists()) { + log.warn("Resource has been deallocated"); + return false; } + return true; } synchronized File getFile() { - ensureValid(); + isValid(); return this.file; } public synchronized InputStream getInputStream() throws IOException { - ensureValid(); + isValid(); return new FileInputStream(this.file); } public synchronized long length() { - ensureValid(); + isValid(); return this.file.length(); } Index: httpclient-cache/src/main/java/org/apache/http/client/cache/Resource.java =================================================================== --- httpclient-cache/src/main/java/org/apache/http/client/cache/Resource.java (revision 1206582) +++ httpclient-cache/src/main/java/org/apache/http/client/cache/Resource.java (revision ) @@ -57,4 +57,9 @@ */ void dispose(); + /** + * Is this resource still valid to be used + */ + boolean isValid(); + }