Fixed connection leak in basic and digest authenticators if 401 response has content (#3878)

Signed-off-by: agherardi <[email protected]>

Author: agherardi

connectors/apache-connector/src/test/java/org/glassfish/jersey/apache/connector/AuthTest.java

@@ -35,6 +35,7 @@
 
 import org.glassfish.jersey.client.ClientConfig;
 import org.glassfish.jersey.client.authentication.HttpAuthenticationFeature;
+import org.glassfish.jersey.client.authentication.ResponseAuthenticationException;
 import org.glassfish.jersey.server.ResourceConfig;
 import org.glassfish.jersey.test.JerseyTest;
 
@@ -47,6 +48,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 /**
  * @author Paul Sandoz
@@ -248,6 +250,40 @@ public String deleteFilterWithEntity(@Context HttpHeaders h, String e) {
             return e;
         }
 
+        @GET
+        @Path("content")
+        public String getWithContent(@Context HttpHeaders h) {
+            requestCount++;
+            String value = h.getRequestHeaders().getFirst("Authorization");
+            if (value == null) {
+                assertEquals(1, requestCount);
+                throw new WebApplicationException(
+                        Response.status(401).header("WWW-Authenticate", "Basic realm=\"WallyWorld\"")
+                            .entity("Forbidden").build());
+            } else {
+                assertTrue(requestCount > 1);
+            }
+
+            return "GET";
+        }
+
+        @GET
+        @Path("contentDigestAuth")
+        public String getWithContentDigestAuth(@Context HttpHeaders h) {
+            requestCount++;
+            String value = h.getRequestHeaders().getFirst("Authorization");
+            if (value == null) {
+                assertEquals(1, requestCount);
+                throw new WebApplicationException(
+                        Response.status(401).header("WWW-Authenticate", "Digest nonce=\"1234\"")
+                            .entity("Forbidden").build());
+            } else {
+                assertTrue(requestCount > 1);
+            }
+
+            return "GET";
+        }
+
         @GET
         @Path("queryParamsBasic")
         public String getQueryParamsBasic(@Context HttpHeaders h, @Context UriInfo uriDetails) {
@@ -455,6 +491,52 @@ public void testAuthInteractivePost() {
         assertEquals("POST", r.request().post(Entity.text("POST"), String.class));
     }
 
+    @Test
+    public void testAuthGetWithBasicFilterAndContent() {
+        ClientConfig cc = new ClientConfig();
+        PoolingHttpClientConnectionManager cm = new PoolingHttpClientConnectionManager();
+        cc.connectorProvider(new ApacheConnectorProvider());
+        cc.property(ApacheClientProperties.CONNECTION_MANAGER, cm);
+        Client client = ClientBuilder.newClient(cc);
+        client.register(HttpAuthenticationFeature.universalBuilder().build());
+        WebTarget r = client.target(getBaseUri()).path("test/content");
+
+        try {
+            assertEquals("GET", r.request().get(String.class));
+            fail();
+        } catch (ResponseAuthenticationException ex) {
+            // expected
+        }
+
+        // Verify the connection that was used for the request is available for reuse
+        // and no connections are leased
+        assertEquals(cm.getTotalStats().getAvailable(), 1);
+        assertEquals(cm.getTotalStats().getLeased(), 0);
+    }
+
+    @Test
+    public void testAuthGetWithDigestFilterAndContent() {
+        ClientConfig cc = new ClientConfig();
+        PoolingHttpClientConnectionManager cm = new PoolingHttpClientConnectionManager();
+        cc.connectorProvider(new ApacheConnectorProvider());
+        cc.property(ApacheClientProperties.CONNECTION_MANAGER, cm);
+        Client client = ClientBuilder.newClient(cc);
+        client.register(HttpAuthenticationFeature.universalBuilder().build());
+        WebTarget r = client.target(getBaseUri()).path("test/contentDigestAuth");
+
+        try {
+            assertEquals("GET", r.request().get(String.class));
+            fail();
+        } catch (ResponseAuthenticationException ex) {
+            // expected
+        }
+
+        // Verify the connection that was used for the request is available for reuse
+        // and no connections are leased
+        assertEquals(cm.getTotalStats().getAvailable(), 1);
+        assertEquals(cm.getTotalStats().getLeased(), 0);
+    }
+
     @Test
     public void testAuthGetQueryParamsBasic() {
         ClientConfig cc = new ClientConfig();

core-client/src/main/java/org/glassfish/jersey/client/authentication/BasicAuthenticator.java

@@ -101,6 +101,9 @@ public boolean filterResponseAndAuthenticate(ClientRequestContext request, Clien
                     .getCredentials(request, defaultCredentials, HttpAuthenticationFilter.Type.BASIC);
 
             if (credentials == null) {
+                if (response.hasEntity()) {
+                    AuthenticationUtil.discardInputAndClose(response.getEntityStream());
+                }
                 throw new ResponseAuthenticationException(null, LocalizationMessages.AUTHENTICATION_CREDENTIALS_MISSING_BASIC());
             }
 

core-client/src/main/java/org/glassfish/jersey/client/authentication/DigestAuthenticator.java

@@ -125,7 +125,9 @@ public boolean filterResponse(final ClientRequestContext request, final ClientRe
             final HttpAuthenticationFilter.Credentials cred = HttpAuthenticationFilter.getCredentials(request,
                     this.credentials, HttpAuthenticationFilter.Type.DIGEST);
             if (cred == null) {
-
+                if (response.hasEntity()) {
+                    AuthenticationUtil.discardInputAndClose(response.getEntityStream());
+                }
                 throw new ResponseAuthenticationException(null, LocalizationMessages.AUTHENTICATION_CREDENTIALS_MISSING_DIGEST());
             }