From bb99db4963373742f67db1f70db15ee976bee791 Mon Sep 17 00:00:00 2001
From: "alessandro.gherardi" <alessandro.gherardi@schneider-electric.com>
Date: Mon, 8 Jan 2018 11:17:01 -0700
Subject: [PATCH] Properly close the Apache response so that connections can be
 reused

---
 .../apache/connector/ApacheConnector.java     | 23 ++++------
 .../apache/connector/StreamingTest.java       | 43 ++++++++++++++++++-
 2 files changed, 49 insertions(+), 17 deletions(-)

diff --git a/connectors/apache-connector/src/main/java/org/glassfish/jersey/apache/connector/ApacheConnector.java b/connectors/apache-connector/src/main/java/org/glassfish/jersey/apache/connector/ApacheConnector.java
index 4cfe1c8b4f..a4ebf8d712 100644
--- a/connectors/apache-connector/src/main/java/org/glassfish/jersey/apache/connector/ApacheConnector.java
+++ b/connectors/apache-connector/src/main/java/org/glassfish/jersey/apache/connector/ApacheConnector.java
@@ -486,7 +486,7 @@ public ClientResponse apply(final ClientRequest clientRequest) throws Processing
             }
 
             try {
-                responseContext.setEntityStream(new HttpClientResponseInputStream(getInputStream(response)));
+                responseContext.setEntityStream(getInputStream(response));
             } catch (final IOException e) {
                 LOGGER.log(Level.SEVERE, null, e);
             }
@@ -625,18 +625,6 @@ private static Map<String, String> writeOutBoundHeaders(final MultivaluedMap<Str
         return stringHeaders;
     }
 
-    private static final class HttpClientResponseInputStream extends FilterInputStream {
-
-        HttpClientResponseInputStream(final InputStream inputStream) throws IOException {
-            super(inputStream);
-        }
-
-        @Override
-        public void close() throws IOException {
-            super.close();
-        }
-    }
-
     private static InputStream getInputStream(final CloseableHttpResponse response) throws IOException {
 
         final InputStream inputStream;
@@ -655,8 +643,13 @@ private static InputStream getInputStream(final CloseableHttpResponse response)
         return new FilterInputStream(inputStream) {
             @Override
             public void close() throws IOException {
-                response.close();
-                super.close();
+                try {
+                    super.close();
+                } catch (IOException ex) {
+                    // Ignore
+                } finally {
+                    response.close();
+                }
             }
         };
     }
diff --git a/connectors/apache-connector/src/test/java/org/glassfish/jersey/apache/connector/StreamingTest.java b/connectors/apache-connector/src/test/java/org/glassfish/jersey/apache/connector/StreamingTest.java
index 0cb067577f..ca7aa5a7d1 100644
--- a/connectors/apache-connector/src/test/java/org/glassfish/jersey/apache/connector/StreamingTest.java
+++ b/connectors/apache-connector/src/test/java/org/glassfish/jersey/apache/connector/StreamingTest.java
@@ -49,10 +49,13 @@
 import javax.ws.rs.client.WebTarget;
 import javax.ws.rs.core.Application;
 import javax.ws.rs.core.MediaType;
-
+import javax.ws.rs.core.Response;
 import javax.inject.Singleton;
 
+import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
+
 import org.glassfish.jersey.client.ClientConfig;
+import org.glassfish.jersey.client.ClientProperties;
 import org.glassfish.jersey.server.ChunkedOutput;
 import org.glassfish.jersey.server.ResourceConfig;
 import org.glassfish.jersey.test.JerseyTest;
@@ -64,6 +67,7 @@
  * @author Petr Janouch (petr.janouch at oracle.com)
  */
 public class StreamingTest extends JerseyTest {
+    private PoolingHttpClientConnectionManager connectionManager;
 
     /**
      * Test that a data stream can be terminated from the client side.
@@ -71,7 +75,8 @@ public class StreamingTest extends JerseyTest {
     @Test
     public void clientCloseTest() throws IOException {
         // start streaming
-        InputStream inputStream = target().path("/streamingEndpoint").request().get(InputStream.class);
+        InputStream inputStream = target().path("/streamingEndpoint").request()
+                .property(ClientProperties.READ_TIMEOUT, 1_000).get(InputStream.class);
 
         WebTarget sendTarget = target().path("/streamingEndpoint/send");
         // trigger sending 'A' to the stream; OK is sent if everything on the server was OK
@@ -85,8 +90,35 @@ public void clientCloseTest() throws IOException {
         assertEquals("NOK", sendTarget.request().get().readEntity(String.class));
     }
 
+    /**
+     * Tests that closing a response after completely reading the entity reuses the connection
+     */
+    @Test
+    public void reuseConnectionTest() throws IOException {
+        Response response = target().path("/streamingEndpoint/get").request().get();
+        InputStream is = response.readEntity(InputStream.class);
+        byte[] buf = new byte[8192];
+        is.read(buf);
+        is.close();
+        response.close();
+
+        assertEquals(1, connectionManager.getTotalStats().getAvailable());
+        assertEquals(0, connectionManager.getTotalStats().getLeased());
+    }
+
+    /**
+     * Tests that closing a request without reading the entity does not throw an exception.
+     */
+    @Test
+    public void clientCloseThrowsNoExceptionTest() throws IOException {
+        Response response = target().path("/streamingEndpoint/get").request().get();
+        response.close();
+    }
+
     @Override
     protected void configureClient(ClientConfig config) {
+        connectionManager = new PoolingHttpClientConnectionManager();
+        config.property(ApacheClientProperties.CONNECTION_MANAGER, connectionManager);
         config.connectorProvider(new ApacheConnectorProvider());
     }
 
@@ -118,5 +150,12 @@ public String sendEvent() {
         public ChunkedOutput<String> get() {
             return output;
         }
+
+        @GET
+        @Path("get")
+        @Produces(MediaType.TEXT_PLAIN)
+        public String getString() {
+            return "OK";
+        }
     }
 }