Fixed regression in cache validity logic for file backed cache entries

This commit is contained in:
Oleg Kalnichevski 2017-10-13 12:16:08 +02:00
parent bb96781e5b
commit 6076f5542b
4 changed files with 23 additions and 35 deletions

View File

@ -31,8 +31,8 @@ import java.util.Iterator;
import org.apache.hc.client5.http.cache.HeaderConstants;
import org.apache.hc.client5.http.cache.HttpCacheEntry;
import org.apache.hc.client5.http.cache.Resource;
import org.apache.hc.client5.http.utils.DateUtils;
import org.apache.hc.core5.http.message.MessageSupport;
import org.apache.hc.core5.annotation.Contract;
import org.apache.hc.core5.annotation.ThreadingBehavior;
import org.apache.hc.core5.http.Header;
@ -40,6 +40,7 @@ import org.apache.hc.core5.http.HeaderElement;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.MessageHeaders;
import org.apache.hc.core5.http.message.MessageSupport;
/**
* @since 4.1
@ -180,23 +181,6 @@ class CacheValidityPolicy {
return DateUtils.parseDate(dateHdr.getValue());
}
protected long getContentLengthValue(final HttpCacheEntry entry) {
final Header cl = entry.getFirstHeader(HttpHeaders.CONTENT_LENGTH);
if (cl == null) {
return -1;
}
try {
return Long.parseLong(cl.getValue());
} catch (final NumberFormatException ex) {
return -1;
}
}
protected boolean hasContentLengthHeader(final HttpCacheEntry entry) {
return null != entry.getFirstHeader(HttpHeaders.CONTENT_LENGTH);
}
/**
* 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
@ -205,8 +189,21 @@ class CacheValidityPolicy {
* @return boolean indicating whether actual length matches Content-Length
*/
protected boolean contentLengthHeaderMatchesActualLength(final HttpCacheEntry entry) {
return !hasContentLengthHeader(entry) ||
(entry.getResource() != null && getContentLengthValue(entry) == entry.getResource().length());
final Header h = entry.getFirstHeader(HttpHeaders.CONTENT_LENGTH);
if (h != null) {
try {
final long responseLen = Long.parseLong(h.getValue());
final Resource resource = entry.getResource();
if (resource == null) {
return false;
}
final long resourceLen = resource.length();
return responseLen == resourceLen;
} catch (final NumberFormatException ex) {
return false;
}
}
return true;
}
protected long getApparentAgeSecs(final HttpCacheEntry entry) {

View File

@ -51,10 +51,13 @@ public class FileResource extends Resource {
private static final long serialVersionUID = 4132244415919043397L;
private final AtomicReference<File> fileRef;
private final long len;
public FileResource(final File file) {
super();
this.fileRef = new AtomicReference<>(Args.notNull(file, "File"));
Args.notNull(file, "File");
this.fileRef = new AtomicReference<>(file);
this.len = file.length();
}
File getFile() {
@ -96,12 +99,7 @@ public class FileResource extends Resource {
@Override
public long length() {
final File file = this.fileRef.get();
if (file != null) {
return file.length();
} else {
return -1;
}
return len;
}
@Override

View File

@ -361,13 +361,6 @@ public class TestCacheValidityPolicy {
assertFalse(impl.contentLengthHeaderMatchesActualLength(entry));
}
@Test
public void testMalformedContentLengthReturnsNegativeOne() {
final Header[] headers = new Header[] { new BasicHeader("Content-Length", "asdf") };
final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(headers);
assertEquals(-1, impl.getContentLengthValue(entry));
}
@Test
public void testNegativeAgeHeaderValueReturnsMaxAge() {
final Header[] headers = new Header[] { new BasicHeader("Age", "-100") };

View File

@ -135,7 +135,7 @@ public class TestHttpCacheJiraNumber1147 {
Assert.assertEquals(200, response2.getCode());
EntityUtils.consume(response2.getEntity());
verify(mockExecChain, Mockito.times(2)).proceed(
verify(mockExecChain, Mockito.times(1)).proceed(
isA(ClassicHttpRequest.class),
isA(ExecChain.Scope.class));
Assert.assertEquals(CacheResponseStatus.FAILURE, context.getCacheResponseStatus());