Response.InputStream not consumed in HttpAuthenticationFeature (Challenge-Response on authentication)

[glassfishrobot]

This is a really tricky issue.

Steps to Reproduce:

Sample Webservice:

import javax.ws.rs.GET;
import javax.ws.rs.Path;
import javax.ws.rs.Produces;
import javax.ws.rs.QueryParam;
import javax.ws.rs.core.MediaType;

import com.prostep.openpdm.bibo.jaxb.JaxbListWrapper;
import com.prostep.openpdm.opws.activity.ActivityId;
import com.prostep.openpdm.opws.service.vo.PropertyVO;

@Path("/")
public class TestRest  {

    @Path("test")
    @GET
    @Produces(MediaType.TEXT_PLAIN)
    public String testValue(@QueryParam("one") JaxbListWrapper<PropertyVO> value1, @QueryParam("other") String value2,
            @QueryParam("test") String test) {
        return "true";
    }

    @Path("test")
    @GET
    @Produces({ MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON })
    public ActivityId testValueJSON(@QueryParam("one") JaxbListWrapper<PropertyVO> value1,
            @QueryParam("other") String value2, @QueryParam("test") String test) {
        return new ActivityId("true");
    }

}

sample client:

import java.net.URISyntaxException;
import java.util.stream.LongStream;

import javax.ws.rs.client.Client;
import javax.ws.rs.client.ClientBuilder;
import javax.ws.rs.client.WebTarget;
import javax.ws.rs.client.Invocation.Builder;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;

import org.glassfish.jersey.client.ClientConfig;
import org.glassfish.jersey.client.ClientProperties;
import org.glassfish.jersey.client.authentication.HttpAuthenticationFeature;
import org.glassfish.jersey.jackson.JacksonFeature;
import org.glassfish.jersey.media.multipart.MultiPartFeature;
import org.glassfish.jersey.message.MessageProperties;
import org.junit.Test;

import com.fasterxml.jackson.jaxrs.json.JacksonJaxbJsonProvider;

public class Test401 {

    private TestClient c;

    public void setUp() throws Exception {
        c = TestClient.newInstance("some.url");
    }

    private static class TestClient {

        private Client client = null;

        private String url;

        public TestClient(String url) {
            this.url = url;
        }

        public String callTestPlain(String one, String other, String query) throws Exception {
            WebTarget resource = getResource().path("test").path("test");
            resource = resource.queryParam("one", one);
            resource = resource.queryParam("other", other);
            resource = resource.queryParam("test", query);

            Builder accept = resource.request(MediaType.TEXT_PLAIN_TYPE);
            Response response = accept.get();

            evaluateResponse(response);

            return response.readEntity(String.class);
        }

        private WebTarget getResource() throws URISyntaxException {
            return getClient().target(url);

        }

        public String callTestJSON(String one, String other, String query) throws Exception {
            WebTarget resource = getResource().path("test").path("test");
            resource = resource.queryParam("one", one);
            resource = resource.queryParam("other", other);
            resource = resource.queryParam("test", query);

            Builder accept = resource.request(MediaType.APPLICATION_JSON_TYPE);
            Response response = accept.get();

            evaluateResponse(response);

            return response.readEntity(String.class);
        }

        public String callTest(String one, String other, String query) throws Exception {
            WebTarget resource = getResource().path("test").path("test");
            resource = resource.queryParam("one", one);
            resource = resource.queryParam("other", other);
            resource = resource.queryParam("test", query);

            Builder accept = resource.request(MediaType.TEXT_PLAIN_TYPE);
            Response response = accept.get();

            return response.readEntity(String.class);
        }

        public static TestClient newInstance(final String url) throws Exception {
            return new TestClient(url);
        }

        private void evaluateResponse(final Response response) throws Exception {
            handleExceptions(response);
        }

        private void handleExceptions(final Response response) throws Exception {
            final int status = response.getStatus();
            if (Status.Family.SUCCESSFUL != response.getStatusInfo().getFamily()) {
final String message = response.readEntity(String.class);
if (Status.NOT_IMPLEMENTED.getStatusCode() == status) {
    throw new Exception(message);
} else if (Status.UNAUTHORIZED.getStatusCode() == status) {
    throw new Exception(message);
} else if (Status.BAD_REQUEST.getStatusCode() == status) {
    throw new Exception(message);
} else if (Status.NOT_FOUND.getStatusCode() == status) {
    throw new Exception(message);
} else if (Status.FORBIDDEN.getStatusCode() == status) {
    throw new Exception(message);
} else {
    throw new Exception(message);
}
            }
        }

        public void setHttpAuthCredentials(String user, String password) {
            HttpAuthenticationFeature feature = HttpAuthenticationFeature.universal(user, password);
            getClient().register(feature);

        }

        protected ClientConfig configureClient() {

            ClientConfig config = new ClientConfig();
            config.register(JacksonFeature.class);
            config.register(MultiPartFeature.class);
            config.register(new JacksonJaxbJsonProvider());

            // allows put with null values
            config.property(ClientProperties.SUPPRESS_HTTP_COMPLIANCE_VALIDATION, true);
            config.property(ClientProperties.MOXY_JSON_FEATURE_DISABLE, true);
            config.property(MessageProperties.XML_FORMAT_OUTPUT, true);

            return config;
        }

        private Client getClient() {
            if (client == null) {
ClientConfig config = configureClient();
client = ClientBuilder.newClient(config);
            }
            return client;
        }

    }

    boolean toggle = true;
    private int amountParams = 1;

    @Test
    public void test401Error() throws Exception {

        c.setHttpAuthCredentials("openpdmadmin", "openpdmadmin");
        final String callee = "some.classpath.to.object.PropertyWrapper%23runTime%3D1%2Cparam0%3Dvalue0";
        LongStream.iterate(0, i -> i + 1).forEach(val -> call(callee, val));
    }

    private void call(String callee, long val) {
        try {
            if ((toggle = !toggle)) {
c.callTestPlain(callee, String.valueOf(val), callee);
            } else {
c.callTestJSON(callee, String.valueOf(val), callee);
            }
            if (val % 100l == 0) {
System.out.println("Called " + val + " times test.");
            }
        } catch (Exception e) {
            System.err.println("finally we called " + val + " times webservice before failed.");
            e.printStackTrace();
            System.exit(1);
        }
    }
}

If you run this client, with digest authentication, this authentication method takes most advantage of that bug, but it can appear even if a lot of clients (about 600 - 25000 - is most likely to appear) are instantiated in a short time period (about 3 seconds) and authenticate via a Http Digest/Basic (nonpreeemptive) authentication.

For each unauthenticated response (WWW-Authenticate Header is set) the inputstream isn't validated/emptied by the HttpAuthenticationFeature.

With a network sniffing tool you can see a lot of open connections between client and server. In error case, an old TCP stream (SOCKET) is reused by java.net.HttpURLConnection (which shouldn't) and for some reason the request is sent twice by the HttpURLConnection.

In javadoc of HttpURLConnection and oracle documentation What can you do to help with Keep-Alive? it is highly recommended to consume input and error streams of HttpURLConnection to release this connection obejct for reuse in connection pooling.

so, a fix for this is:

From cc90e1131d730b32fc4982058080b90f6df29880 Mon Sep 17 00:00:00 2001
Date: Fri, 24 Mar 2017 14:06:09 +0100
Subject: [PATCH] test commit 401

---
 .../authentication/HttpAuthenticationFilter.java    | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/core-client/src/main/java/org/glassfish/jersey/client/authentication/HttpAuthenticationFilter.java b/core-client/src/main/java/org/glassfish/jersey/client/authentication/HttpAuthenticationFilter.java
index fc3e106..575d69e 100644
--- a/core-client/src/main/java/org/glassfish/jersey/client/authentication/HttpAuthenticationFilter.java
+++ b/core-client/src/main/java/org/glassfish/jersey/client/authentication/HttpAuthenticationFilter.java
@@ -50,6 +50,7 @@ import java.util.List;
 import java.util.Map;

 import javax.ws.rs.Priorities;
+import javax.ws.rs.ProcessingException;
 import javax.ws.rs.client.Client;
 import javax.ws.rs.client.ClientBuilder;
 import javax.ws.rs.client.ClientRequestContext;
@@ -316,7 +317,23 @@ class HttpAuthenticationFilter implements ClientRequestFilter, ClientResponseFil
         builder.headers(newHeaders);

         builder.property(REQUEST_PROPERTY_FILTER_REUSED, "true");
-
+        
+		/**
+         * Bugfix for some tricky issues, as they are:
+         * - Wrong 401, caused by a staled HttpConnection in ConnectionPool
+         * - Unexpected StreamClosedException, due to the same cause
+         * - Message Contend even if there is a call with no content issue: JERSEY-3145
+         */
+        {
+            InputStream suck = response.getEntityStream();
+            try {
+/** Empty Stream, as described in {@link HttpURLConnection#getInputStream()} **/
+while(suck.read() != -1); 
+            } catch (IOException e) {
+throw new ProcessingException(e);
+            }
+        }
+		
         Invocation invocation;
         if (request.getEntity() == null) {
             invocation = builder.build(method);
@@ -328,7 +345,7 @@ class HttpAuthenticationFilter implements ClientRequestFilter, ClientResponseFil

         if (nextResponse.hasEntity()) {
             response.setEntityStream(nextResponse.readEntity(InputStream.class));
-        }
+	}
         MultivaluedMap<String, String> headers = response.getHeaders();
         headers.clear();
         headers.putAll(nextResponse.getStringHeaders());
-- 
2.10.2.windows.1

Environment

jersey-client, java7, HttpAuthenticator

{filter}

Affected Versions

[2.5, 2.23.1, 2.25.1]

[glassfishrobot]

Reported by miiep

[glassfishrobot]

miiep said:
While debugging this, i've also fixed: #3417

[glassfishrobot]

miiep said:
This issue makes HttpAuthentication via Digest broken, because of the reauthentication of each called REST Url

[glassfishrobot]

girishp said:
Hi I am new to jersey don't have idea where to post my query, we are using jersey 1.19.1 ,jdk8 ,wildfly 10 and java security management in our product development.
I am getting following exception can you please guide where can I get proper solutions of this.

08:35:00,511 SEVERE [com.sun.jersey.core.spi.component.ProviderFactory] (ServerService Thread Pool – 61) The provider class, class com.sun.jersey.multipart.impl.MultiPartReaderServerSide, could not be instantiated. Processing will continue but the class will not be utilized: java.lang.SecurityException: Unable to create temporary file

[glassfishrobot]

This issue was imported from java.net JIRA JERSEY-3254

[agherardi]

I'm seeing a related issue, using the Apache connector. The connection that Jersey uses to send the original request - the one that the server rejects with 401 and WWW-Authenticate Digest - is NOT returned to the Apache connection pool and is therefore leaked.

Here's a tentative fix that works for me in jersey/core-client/src/main/java/org/glassfish/jersey/client/authentication/HttpAuthenticationFilter.java:

static boolean repeatRequest(ClientRequestContext request, ClientResponseContext response, String newAuthorizationHeader) throws IOException {
    // If the failed response has an entity stream, close it. We must do this to avoid leaking resources
    // when we replace the entity stream of the failed response with that of the repeated response (see below).
    // Notice that by closing the entity stream before sending the repeated request we allow resources allocated
    // to the failed request (e.g., the HTTP connection) to be reused, if possible, for the repeated request.
    if (response.hasEntity()) {
        response.getEntityStream().close();
        response.setEntityStream(null);
    }

   Client client = request.getClient();

Remove this line of code:
Client client = ClientBuilder.newClient(request.getConfiguration());

Jersey maintainers - if this looks like a viable solution, please either merge or reject https://github.com/jersey/jersey/pull/268 and I'll create a separate pull request for this issue.

[miiep]

We verified, this solution works for us and seems to be faster than our promoted solution.