You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2012/12/13 16:05:07 UTC
svn commit: r1421328 - in /httpcomponents/httpclient/branches/4.2.x: ./
httpclient-cache/src/main/java/org/apache/http/impl/client/cache/
httpclient-cache/src/test/java/org/apache/http/impl/client/cache/
Author: olegk
Date: Thu Dec 13 15:05:05 2012
New Revision: 1421328
URL: http://svn.apache.org/viewvc?rev=1421328&view=rev
Log:
HTTPCLIENT-1276: cache update on a 304 response causes NPE
Contributed by Francois-Xavier Bonnet <francois-xavier.bonnet at centraliens.net>
Modified:
httpcomponents/httpclient/branches/4.2.x/RELEASE_NOTES.txt
httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheEntity.java
httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheEntryUpdater.java
httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedHttpResponseGenerator.java
httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java
Modified: httpcomponents/httpclient/branches/4.2.x/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/branches/4.2.x/RELEASE_NOTES.txt?rev=1421328&r1=1421327&r2=1421328&view=diff
==============================================================================
--- httpcomponents/httpclient/branches/4.2.x/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpclient/branches/4.2.x/RELEASE_NOTES.txt Thu Dec 13 15:05:05 2012
@@ -1,6 +1,9 @@
Changes since 4.2.2
-------------------
+* [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.
Modified: httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheEntity.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheEntity.java?rev=1421328&r1=1421327&r2=1421328&view=diff
==============================================================================
--- httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheEntity.java (original)
+++ httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheEntity.java Thu Dec 13 15:05:05 2012
@@ -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,
}
public long getContentLength() {
- Resource resource = this.cacheEntry.getResource();
- return (resource != null) ? resource.length() : 0L;
+ return this.cacheEntry.getResource().length();
}
public InputStream getContent() throws IOException {
Modified: httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheEntryUpdater.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheEntryUpdater.java?rev=1421328&r1=1421327&r2=1421328&view=diff
==============================================================================
--- httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheEntryUpdater.java (original)
+++ httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheEntryUpdater.java Thu Dec 13 15:05:05 2012
@@ -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,
Modified: httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedHttpResponseGenerator.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedHttpResponseGenerator.java?rev=1421328&r1=1421327&r2=1421328&view=diff
==============================================================================
--- httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedHttpResponseGenerator.java (original)
+++ httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedHttpResponseGenerator.java Thu Dec 13 15:05:05 2012
@@ -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) {
Modified: httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java?rev=1421328&r1=1421327&r2=1421328&view=diff
==============================================================================
--- httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java (original)
+++ httpcomponents/httpclient/branches/4.2.x/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java Thu Dec 13 15:05:05 2012
@@ -2015,6 +2015,81 @@ public class TestCachingHttpClient {
impl.execute(host, request);
assertEquals(1, backend.getExecutions());
}
+
+ @Test
+ public void testNoEntityForIfNoneMatchRequestNotYetInCache() throws Exception {
+
+ Date now = new Date();
+ Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L);
+
+ impl = new CachingHttpClient(mockBackend);
+ HttpRequest req1 = 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");
+
+ expect(
+ mockBackend.execute(isA(HttpHost.class),
+ isA(HttpRequest.class), (HttpContext)
+ isNull())).andReturn(resp1);
+
+ replayMocks();
+ HttpResponse result = impl.execute(host, 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 CachingHttpClient(mockBackend);
+
+ HttpRequest req1 = new HttpGet("http://foo.example.com/");
+ req1.addHeader("If-None-Match", "etag");
+
+ HttpRequest req2 = 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");
+
+ expect(
+ mockBackend.execute(isA(HttpHost.class),
+ isA(HttpRequest.class), (HttpContext)
+ isNull())).andReturn(resp1);
+ expect(
+ mockBackend.execute(isA(HttpHost.class),
+ isA(HttpRequest.class), (HttpContext)
+ isNull())).andReturn(resp2);
+
+ replayMocks();
+ HttpResponse result1 = impl.execute(host, req1);
+ HttpResponse result2 = impl.execute(host, 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 void getCacheEntryReturns(HttpCacheEntry result) throws IOException {
expect(mockCache.getCacheEntry(host, request)).andReturn(result);