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 2013/10/22 12:10:17 UTC
svn commit: r1534585 - in /httpcomponents/httpclient/trunk: ./
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: Tue Oct 22 10:10:16 2013
New Revision: 1534585
URL: http://svn.apache.org/r1534585
Log:
HTTPCLIENT-1425: Fixed socket closed exception thrown by caching HttpClient when the origin server sends a long chunked response
Contributed by James Leigh <james at 3roundstones dot com>
Modified:
httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java
httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java
httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HttpCache.java
httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/AbstractProtocolTest.java
httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExec.java
httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExecChain.java
httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRequirements.java
Modified: httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt?rev=1534585&r1=1534584&r2=1534585&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpclient/trunk/RELEASE_NOTES.txt Tue Oct 22 10:10:16 2013
@@ -1,6 +1,10 @@
Changes since 4.3.1
-------------------
+* [HTTPCLIENT-1425] Fixed socket closed exception thrown by caching HttpClient when the origin
+ server sends a long chunked response.
+ Contributed by James Leigh <james at 3roundstones dot com>
+
* [HTTPCLIENT-1417] Fixed NPE in BrowserCompatSpec#formatCookies caused by version 1
cookies with null cookie value.
Contributed by Oleg Kalnichevski <olegk at apache.org>
Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java?rev=1534585&r1=1534584&r2=1534585&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java Tue Oct 22 10:10:16 2013
@@ -50,10 +50,10 @@ import org.apache.http.client.cache.Http
import org.apache.http.client.cache.HttpCacheUpdateException;
import org.apache.http.client.cache.Resource;
import org.apache.http.client.cache.ResourceFactory;
+import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.entity.ByteArrayEntity;
import org.apache.http.message.BasicHttpResponse;
import org.apache.http.protocol.HTTP;
-import org.apache.http.util.EntityUtils;
class BasicHttpCache implements HttpCache {
private static final Set<String> safeRequestMethods = new HashSet<String>(
@@ -276,12 +276,25 @@ class BasicHttpCache implements HttpCach
public HttpResponse cacheAndReturnResponse(final HttpHost host, final HttpRequest request,
final HttpResponse originResponse, final Date requestSent, final Date responseReceived)
throws IOException {
+ return cacheAndReturnResponse(host, request,
+ Proxies.enhanceResponse(originResponse), requestSent,
+ responseReceived);
+ }
+
+ public HttpResponse cacheAndReturnResponse(
+ final HttpHost host,
+ final HttpRequest request,
+ final CloseableHttpResponse originResponse,
+ final Date requestSent,
+ final Date responseReceived) throws IOException {
+ boolean closeOriginResponse = true;
final SizeLimitedResponseReader responseReader = getResponseReader(request, originResponse);
try {
responseReader.readResponse();
if (responseReader.isLimitReached()) {
+ closeOriginResponse = false;
return responseReader.getReconstructedResponse();
}
@@ -298,12 +311,10 @@ class BasicHttpCache implements HttpCach
resource);
storeInCache(host, request, entry);
return responseGenerator.generateResponse(entry);
- } catch (final IOException ex) {
- EntityUtils.consume(originResponse.getEntity());
- throw ex;
- } catch (final RuntimeException ex) {
- EntityUtils.consumeQuietly(originResponse.getEntity());
- throw ex;
+ } finally {
+ if (closeOriginResponse) {
+ originResponse.close();
+ }
}
}
Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java?rev=1534585&r1=1534584&r2=1534585&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java Tue Oct 22 10:10:16 2013
@@ -808,13 +808,9 @@ public class CachingExec implements Clie
final boolean cacheable = responseCachingPolicy.isResponseCacheable(request, backendResponse);
responseCache.flushInvalidatedCacheEntriesFor(target, request, backendResponse);
if (cacheable && !alreadyHaveNewerCacheEntry(target, request, backendResponse)) {
- try {
- storeRequestIfModifiedSinceFor304Response(request, backendResponse);
- return Proxies.enhanceResponse(responseCache.cacheAndReturnResponse(
- target, request, backendResponse, requestDate, responseDate));
- } finally {
- backendResponse.close();
- }
+ storeRequestIfModifiedSinceFor304Response(request, backendResponse);
+ return Proxies.enhanceResponse(responseCache.cacheAndReturnResponse(
+ target, request, backendResponse, requestDate, responseDate));
}
if (!cacheable) {
try {
Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HttpCache.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HttpCache.java?rev=1534585&r1=1534584&r2=1534585&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HttpCache.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HttpCache.java Tue Oct 22 10:10:16 2013
@@ -34,6 +34,7 @@ import org.apache.http.HttpHost;
import org.apache.http.HttpRequest;
import org.apache.http.HttpResponse;
import org.apache.http.client.cache.HttpCacheEntry;
+import org.apache.http.client.methods.CloseableHttpResponse;
/**
* @since 4.1
@@ -104,6 +105,21 @@ interface HttpCache {
throws IOException;
/**
+ * Store a {@link HttpResponse} in the cache if possible, and return
+ * @param host
+ * @param request
+ * @param originResponse
+ * @param requestSent
+ * @param responseReceived
+ * @return the {@link HttpResponse}
+ * @throws IOException
+ */
+ HttpResponse cacheAndReturnResponse(HttpHost host, HttpRequest request,
+ CloseableHttpResponse originResponse, Date requestSent,
+ Date responseReceived)
+ throws IOException;
+
+ /**
* Update a {@link HttpCacheEntry} using a 304 {@link HttpResponse}.
* @param target
* @param request
Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/AbstractProtocolTest.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/AbstractProtocolTest.java?rev=1534585&r1=1534584&r2=1534585&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/AbstractProtocolTest.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/AbstractProtocolTest.java Tue Oct 22 10:10:16 2013
@@ -72,6 +72,11 @@ public abstract class AbstractProtocolTe
return null;
}
+ public static CloseableHttpResponse eqCloseableResponse(final CloseableHttpResponse in) {
+ EasyMock.reportMatcher(new ResponseEquivalent(in));
+ return null;
+ }
+
@Before
public void setUp() {
host = new HttpHost("foo.example.com");
Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExec.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExec.java?rev=1534585&r1=1534584&r2=1534585&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExec.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExec.java Tue Oct 22 10:10:16 2013
@@ -36,11 +36,14 @@ import static org.easymock.classextensio
import static org.easymock.classextension.EasyMock.createNiceMock;
import static org.easymock.classextension.EasyMock.replay;
import static org.easymock.classextension.EasyMock.verify;
+import static org.junit.Assert.assertTrue;
import java.io.IOException;
+import java.io.InputStream;
import java.util.Date;
import java.util.HashMap;
import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
import org.apache.http.HttpHost;
import org.apache.http.HttpRequest;
@@ -52,11 +55,14 @@ import org.apache.http.client.methods.Cl
import org.apache.http.client.methods.HttpExecutionAware;
import org.apache.http.client.methods.HttpRequestWrapper;
import org.apache.http.client.protocol.HttpClientContext;
+import org.apache.http.client.utils.DateUtils;
import org.apache.http.conn.routing.HttpRoute;
+import org.apache.http.entity.InputStreamEntity;
import org.apache.http.impl.execchain.ClientExecChain;
import org.apache.http.message.BasicHttpRequest;
import org.apache.http.message.BasicHttpResponse;
import org.apache.http.message.BasicStatusLine;
+import org.apache.http.protocol.HTTP;
import org.easymock.IExpectationSetters;
import org.easymock.classextension.EasyMock;
import org.junit.Assert;
@@ -92,7 +98,7 @@ public class TestCachingExec extends Tes
}
@Override
- public ClientExecChain createCachingExecChain(final ClientExecChain mockBackend,
+ public CachingExec createCachingExecChain(final ClientExecChain mockBackend,
final HttpCache mockCache, final CacheValidityPolicy mockValidityPolicy,
final ResponseCachingPolicy mockResponsePolicy,
final CachedHttpResponseGenerator mockResponseGenerator,
@@ -118,7 +124,7 @@ public class TestCachingExec extends Tes
}
@Override
- public ClientExecChain createCachingExecChain(final ClientExecChain backend,
+ public CachingExec createCachingExecChain(final ClientExecChain backend,
final HttpCache cache, final CacheConfig config) {
return impl = new CachingExec(backend, cache, config);
}
@@ -301,6 +307,53 @@ public class TestCachingExec extends Tes
}
@Test
+ public void testEndlessResponsesArePassedThrough() throws Exception {
+ impl = createCachingExecChain(mockBackend, new BasicHttpCache(), CacheConfig.DEFAULT);
+
+ final HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_OK, "OK");
+ resp1.setHeader("Date", DateUtils.formatDate(new Date()));
+ resp1.setHeader("Server", "MockOrigin/1.0");
+ resp1.setHeader(HTTP.TRANSFER_ENCODING, HTTP.CHUNK_CODING);
+
+ final AtomicInteger size = new AtomicInteger();
+ final AtomicInteger maxlength = new AtomicInteger(Integer.MAX_VALUE);
+ resp1.setEntity(new InputStreamEntity(new InputStream() {
+ private Throwable closed;
+
+ public void close() throws IOException {
+ closed = new Throwable();
+ super.close();
+ }
+
+ public int read() throws IOException {
+ Thread.yield();
+ if (closed != null) {
+ throw new IOException("Response has been closed");
+
+ }
+ if (size.incrementAndGet() > maxlength.get())
+ return -1;
+ return 'y';
+ }
+ }));
+
+ final CloseableHttpResponse resp = mockBackend.execute(
+ EasyMock.isA(HttpRoute.class),
+ EasyMock.isA(HttpRequestWrapper.class),
+ EasyMock.isA(HttpClientContext.class),
+ EasyMock.<HttpExecutionAware>isNull());
+ EasyMock.expect(resp).andReturn(Proxies.enhanceResponse(resp1));
+
+ final HttpRequestWrapper req1 = HttpRequestWrapper.wrap(HttpTestUtils.makeDefaultRequest());
+
+ replayMocks();
+ final CloseableHttpResponse resp2 = impl.execute(route, req1, context, null);
+ maxlength.set(size.get() * 2);
+ verifyMocks();
+ assertTrue(HttpTestUtils.semanticallyTransparent(resp1, resp2));
+ }
+
+ @Test
public void testCallBackendMakesBackEndRequestAndHandlesResponse() throws Exception {
mockImplMethods(GET_CURRENT_DATE, HANDLE_BACKEND_RESPONSE);
final HttpResponse resp = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_OK, "OK");
Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExecChain.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExecChain.java?rev=1534585&r1=1534584&r2=1534585&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExecChain.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExecChain.java Tue Oct 22 10:10:16 2013
@@ -1319,7 +1319,7 @@ public abstract class TestCachingExecCha
isA(HttpRequest.class)))
.andThrow(new IOException()).anyTimes();
expect(mockCache.cacheAndReturnResponse(eq(host),
- isA(HttpRequest.class), isA(HttpResponse.class),
+ isA(HttpRequest.class), isA(CloseableHttpResponse.class),
isA(Date.class), isA(Date.class)))
.andReturn(resp).anyTimes();
expect(mockBackend.execute(
Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRequirements.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRequirements.java?rev=1534585&r1=1534584&r2=1534585&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRequirements.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRequirements.java Tue Oct 22 10:10:16 2013
@@ -2899,7 +2899,7 @@ public class TestProtocolRequirements ex
EasyMock.expect(mockCache.cacheAndReturnResponse(
EasyMock.isA(HttpHost.class),
EasyMock.isA(HttpRequestWrapper.class),
- eqResponse(validated),
+ eqCloseableResponse(validated),
EasyMock.isA(Date.class),
EasyMock.isA(Date.class))).andReturn(reconstructed).times(0, 1);