Return-Path: X-Original-To: apmail-hc-commits-archive@www.apache.org Delivered-To: apmail-hc-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id AAC4310C85 for ; Tue, 22 Oct 2013 10:10:54 +0000 (UTC) Received: (qmail 16491 invoked by uid 500); 22 Oct 2013 10:10:51 -0000 Delivered-To: apmail-hc-commits-archive@hc.apache.org Received: (qmail 16429 invoked by uid 500); 22 Oct 2013 10:10:45 -0000 Mailing-List: contact commits-help@hc.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "HttpComponents Project" Delivered-To: mailing list commits@hc.apache.org Received: (qmail 16420 invoked by uid 99); 22 Oct 2013 10:10:43 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 22 Oct 2013 10:10:43 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 22 Oct 2013 10:10:39 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 6DB1D2388831 for ; Tue, 22 Oct 2013 10:10:17 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: 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/ Date: Tue, 22 Oct 2013 10:10:17 -0000 To: commits@hc.apache.org From: olegk@apache.org X-Mailer: svnmailer-1.0.9 Message-Id: <20131022101017.6DB1D2388831@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org 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 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 + * [HTTPCLIENT-1417] Fixed NPE in BrowserCompatSpec#formatCookies caused by version 1 cookies with null cookie value. Contributed by Oleg Kalnichevski 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 safeRequestMethods = new HashSet( @@ -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.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);