hc-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ol...@apache.org
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 GMT
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);
 



Mime
View raw message