HTTPCLIENT-1276: cache update on a 304 response causes NPE
Contributed by Francois-Xavier Bonnet <francois-xavier.bonnet at centraliens.net> git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@1421330 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
parent
bfbc573cbd
commit
1779158a4f
|
@ -1,6 +1,9 @@
|
|||
Changes in trunk
|
||||
-------------------
|
||||
|
||||
* [HTTPCLIENT-1276] Cache update on a 304 response causes NPE.
|
||||
Contributed by Francois-Xavier Bonnet <francois-xavier.bonnet at centraliens.net>
|
||||
|
||||
* [HTTPCLIENT-1273] DecompressingHttpClient does not automatically consume response
|
||||
content in case of an i/o, HTTP or runtime exception thrown by the decompressing
|
||||
protocol interceptor leading to a potential connection leak.
|
||||
|
|
|
@ -35,7 +35,6 @@ import org.apache.http.Header;
|
|||
import org.apache.http.HttpEntity;
|
||||
import org.apache.http.annotation.Immutable;
|
||||
import org.apache.http.client.cache.HttpCacheEntry;
|
||||
import org.apache.http.client.cache.Resource;
|
||||
import org.apache.http.protocol.HTTP;
|
||||
|
||||
@Immutable
|
||||
|
@ -67,8 +66,7 @@ class CacheEntity implements HttpEntity, Serializable {
|
|||
}
|
||||
|
||||
public long getContentLength() {
|
||||
Resource resource = this.cacheEntry.getResource();
|
||||
return (resource != null) ? resource.length() : 0L;
|
||||
return this.cacheEntry.getResource().length();
|
||||
}
|
||||
|
||||
public InputStream getContent() throws IOException {
|
||||
|
|
|
@ -87,7 +87,12 @@ class CacheEntryUpdater {
|
|||
if (response.getStatusLine().getStatusCode() != HttpStatus.SC_NOT_MODIFIED)
|
||||
throw new IllegalArgumentException("Response must have 304 status code");
|
||||
Header[] mergedHeaders = mergeHeaders(entry, response);
|
||||
Resource resource = resourceFactory.copy(requestId, entry.getResource());
|
||||
Resource oldResource = entry.getResource();
|
||||
Resource resource = null;
|
||||
if (oldResource != null) {
|
||||
resource = resourceFactory.copy(requestId, entry.getResource());
|
||||
oldResource.dispose();
|
||||
}
|
||||
return new HttpCacheEntry(
|
||||
requestDate,
|
||||
responseDate,
|
||||
|
|
|
@ -73,11 +73,13 @@ class CachedHttpResponseGenerator {
|
|||
HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, entry
|
||||
.getStatusCode(), entry.getReasonPhrase());
|
||||
|
||||
HttpEntity entity = new CacheEntity(entry);
|
||||
response.setHeaders(entry.getAllHeaders());
|
||||
addMissingContentLengthHeader(response, entity);
|
||||
response.setEntity(entity);
|
||||
|
||||
if (entry.getResource() != null) {
|
||||
HttpEntity entity = new CacheEntity(entry);
|
||||
addMissingContentLengthHeader(response, entity);
|
||||
response.setEntity(entity);
|
||||
}
|
||||
|
||||
long age = this.validityStrategy.getCurrentAgeSecs(entry, now);
|
||||
if (age > 0) {
|
||||
|
|
|
@ -27,12 +27,12 @@
|
|||
package org.apache.http.impl.client.cache;
|
||||
|
||||
import static org.easymock.EasyMock.anyObject;
|
||||
import static org.easymock.EasyMock.eq;
|
||||
import static org.easymock.EasyMock.expect;
|
||||
import static org.easymock.EasyMock.expectLastCall;
|
||||
import static org.easymock.EasyMock.isA;
|
||||
import static org.easymock.EasyMock.isNull;
|
||||
import static org.easymock.EasyMock.same;
|
||||
import static org.easymock.EasyMock.eq;
|
||||
import static org.easymock.classextension.EasyMock.createMockBuilder;
|
||||
import static org.easymock.classextension.EasyMock.createNiceMock;
|
||||
import static org.easymock.classextension.EasyMock.replay;
|
||||
|
@ -1527,6 +1527,72 @@ public class TestCachingExec {
|
|||
assertEquals(1, backend.getExecutions());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testNoEntityForIfNoneMatchRequestNotYetInCache() throws Exception {
|
||||
|
||||
Date now = new Date();
|
||||
Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L);
|
||||
|
||||
impl = new CachingExec(mockBackend, new BasicHttpCache(),
|
||||
CacheConfig.DEFAULT);
|
||||
HttpRequestWrapper req1 = HttpRequestWrapper.wrap(new HttpGet(
|
||||
"http://foo.example.com/"));
|
||||
req1.addHeader("If-None-Match", "\"etag\"");
|
||||
|
||||
HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1,
|
||||
HttpStatus.SC_NOT_MODIFIED, "Not modified");
|
||||
resp1.setHeader("Content-Length", "128");
|
||||
resp1.setHeader("ETag", "\"etag\"");
|
||||
resp1.setHeader("Date", DateUtils.formatDate(tenSecondsAgo));
|
||||
resp1.setHeader("Cache-Control", "public, max-age=5");
|
||||
|
||||
backendExpectsAnyRequestAndReturn(resp1);
|
||||
replayMocks();
|
||||
HttpResponse result = impl.execute(route, req1);
|
||||
verifyMocks();
|
||||
|
||||
assertEquals(HttpStatus.SC_NOT_MODIFIED, result.getStatusLine()
|
||||
.getStatusCode());
|
||||
assertNull("The 304 response messages MUST NOT contain a message-body",
|
||||
result.getEntity());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testNotModifiedResponseUpdatesCacheEntryWhenNoEntity() throws Exception {
|
||||
|
||||
Date now = new Date();
|
||||
|
||||
impl = new CachingExec(mockBackend, new BasicHttpCache(),CacheConfig.DEFAULT);
|
||||
|
||||
HttpRequestWrapper req1 = HttpRequestWrapper.wrap(new HttpGet("http://foo.example.com/"));
|
||||
req1.addHeader("If-None-Match", "etag");
|
||||
|
||||
HttpRequestWrapper req2 = HttpRequestWrapper.wrap(new HttpGet("http://foo.example.com/"));
|
||||
req2.addHeader("If-None-Match", "etag");
|
||||
|
||||
HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_NOT_MODIFIED, "Not modified");
|
||||
resp1.setHeader("Date", DateUtils.formatDate(now));
|
||||
resp1.setHeader("Cache-Control","max-age=0");
|
||||
resp1.setHeader("Etag", "etag");
|
||||
|
||||
HttpResponse resp2 = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_NOT_MODIFIED, "Not modified");
|
||||
resp2.setHeader("Date", DateUtils.formatDate(now));
|
||||
resp2.setHeader("Cache-Control","max-age=0");
|
||||
resp1.setHeader("Etag", "etag");
|
||||
|
||||
backendExpectsAnyRequestAndReturn(resp1);
|
||||
backendExpectsAnyRequestAndReturn(resp2);
|
||||
replayMocks();
|
||||
HttpResponse result1 = impl.execute(route, req1);
|
||||
HttpResponse result2 = impl.execute(route, req2);
|
||||
verifyMocks();
|
||||
|
||||
assertEquals(HttpStatus.SC_NOT_MODIFIED, result1.getStatusLine().getStatusCode());
|
||||
assertEquals("etag", result1.getFirstHeader("Etag").getValue());
|
||||
assertEquals(HttpStatus.SC_NOT_MODIFIED, result2.getStatusLine().getStatusCode());
|
||||
assertEquals("etag", result2.getFirstHeader("Etag").getValue());
|
||||
}
|
||||
|
||||
private IExpectationSetters<CloseableHttpResponse> backendExpectsAnyRequestAndReturn(
|
||||
HttpResponse response) throws Exception {
|
||||
CloseableHttpResponse resp = mockBackend.execute(
|
||||
|
|
Loading…
Reference in New Issue