From e1ca6d2fb84726609e86269ad3e3e14aad2d0c5f Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Thu, 28 Mar 2013 12:36:03 +0000 Subject: [PATCH] HTTPCLIENT-1299: (regression) cache incorrectly disposes of the underlying cache resource when storing variant entry git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@1462069 13f79535-47bb-0310-9956-ffa450edef68 --- RELEASE_NOTES.txt | 4 ++ .../impl/client/cache/BasicHttpCache.java | 1 - .../client/cache/HeapResourceFactory.java | 8 ++- .../impl/client/cache/TestBasicHttpCache.java | 69 +++++++++++++++++++ 4 files changed, 79 insertions(+), 3 deletions(-) diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index 9fbebdda0..ca9fc0bc8 100644 --- a/RELEASE_NOTES.txt +++ b/RELEASE_NOTES.txt @@ -3,6 +3,10 @@ Changes since 4.3 ALPHA1 * [HTTPCLIENT-1317] InetAddressUtils should handle IPv6 Addresses with Embedded IPv4 Addresses Contributed Sebastian Bazley . +* [HTTPCLIENT-1299] (regression) cache incorrectly disposes of the underlying cache resource + when storing variant entry. + Contributed by Oleg Kalnichevski + * [HTTPCLIENT-1320] Leverage javax.net.ssl.SSLSocketFactory#getDefault() to initialize SSL context based on system defaults instead of using an internal custom routine. Contributed by Abe Backus and Oleg Kalnichevski diff --git a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java index 0192559c0..e6eb9e03e 100644 --- a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java +++ b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java @@ -203,7 +203,6 @@ class BasicHttpCache implements HttpCache { Resource resource = null; if (oldResource != null) { resource = resourceFactory.copy(requestId, entry.getResource()); - oldResource.dispose(); } final Map variantMap = new HashMap(src.getVariantMap()); variantMap.put(variantKey, variantCacheKey); diff --git a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HeapResourceFactory.java b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HeapResourceFactory.java index 1f1d97a80..f75bc2996 100644 --- a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HeapResourceFactory.java +++ b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HeapResourceFactory.java @@ -59,7 +59,7 @@ public class HeapResourceFactory implements ResourceFactory { break; } } - return new HeapResource(outstream.toByteArray()); + return createResource(outstream.toByteArray()); } public Resource copy( @@ -73,7 +73,11 @@ public class HeapResourceFactory implements ResourceFactory { IOUtils.copyAndClose(resource.getInputStream(), outstream); body = outstream.toByteArray(); } - return new HeapResource(body); + return createResource(body); + } + + Resource createResource(byte[] buf) { + return new HeapResource(buf); } } diff --git a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestBasicHttpCache.java b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestBasicHttpCache.java index 87378f87c..e68662243 100644 --- a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestBasicHttpCache.java +++ b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestBasicHttpCache.java @@ -34,6 +34,8 @@ import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.io.InputStream; import java.util.Date; import java.util.HashMap; @@ -407,4 +409,71 @@ public class TestBasicHttpCache { assertTrue(inputStream.wasClosed()); } + static class DisposableResource implements Resource { + + private static final long serialVersionUID = 1L; + + private final byte[] b; + private boolean dispoased; + + public DisposableResource(final byte[] b) { + super(); + this.b = b; + } + + byte[] getByteArray() { + return this.b; + } + + public InputStream getInputStream() throws IOException { + if (dispoased) { + throw new IOException("Already dispoased"); + } + return new ByteArrayInputStream(this.b); + } + + public long length() { + return this.b.length; + } + + public void dispose() { + this.dispoased = true; + } + + } + + @Test + public void testEntryUpdate() throws Exception { + + final HeapResourceFactory rf = new HeapResourceFactory() { + + @Override + Resource createResource(final byte[] buf) { + return new DisposableResource(buf); + } + + }; + + impl = new BasicHttpCache(rf, backing, CacheConfig.DEFAULT); + + final HttpHost host = new HttpHost("foo.example.com"); + + final HttpRequest origRequest = new HttpGet("http://foo.example.com/bar"); + origRequest.setHeader("Accept-Encoding","gzip"); + + final HttpResponse origResponse = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_OK, "OK"); + origResponse.setEntity(HttpTestUtils.makeBody(128)); + origResponse.setHeader("Date", DateUtils.formatDate(new Date())); + origResponse.setHeader("Cache-Control", "max-age=3600, public"); + origResponse.setHeader("ETag", "\"etag\""); + origResponse.setHeader("Vary", "Accept-Encoding"); + origResponse.setHeader("Content-Encoding","gzip"); + + final HttpResponse response = impl.cacheAndReturnResponse( + host, origRequest, origResponse, new Date(), new Date()); + final HttpEntity entity = response.getEntity(); + Assert.assertNotNull(entity); + IOUtils.copyAndClose(entity.getContent(), new ByteArrayOutputStream()); + } + }