Skip to content
This repository has been archived by the owner on May 28, 2018. It is now read-only.

Patch for client connection leak when using digest authentication #3751

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
import org.apache.http.auth.AuthScope;
import org.apache.http.auth.UsernamePasswordCredentials;
import org.apache.http.client.CredentialsProvider;
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
import org.junit.Ignore;
import org.junit.Test;
import static org.junit.Assert.assertEquals;
Expand Down Expand Up @@ -168,6 +169,19 @@ public String getFilter(@Context HttpHeaders h) {
return "GET";
}

@GET
@Path("digest")
public String getDigest(@Context HttpHeaders h) {
String value = h.getRequestHeaders().getFirst("Authorization");
if (value == null) {
throw new WebApplicationException(
Response.status(401).header("WWW-Authenticate", "Digest realm=\"WallyWorld\"")
.entity("Forbidden").build());
}

return "GET";
}

@POST
public String post(@Context HttpHeaders h, String e) {
requestCount++;
Expand Down Expand Up @@ -278,6 +292,24 @@ public void testAuthGetWithClientFilter() {
assertEquals("GET", r.request().get(String.class));
}

@Test
public void testAuthGetWithDigestFilter() {
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.universal("name", "password"));
WebTarget r = client.target(getBaseUri()).path("test/digest");

assertEquals("GET", r.request().get(String.class));

// 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
@Ignore("JERSEY-1750: Cannot retry request with a non-repeatable request entity. How to buffer the entity?"
+ " Allow repeatable write in jersey?")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@
import java.util.List;
import java.util.Map;

import javax.annotation.Priority;
import javax.ws.rs.Priorities;
import javax.ws.rs.client.Client;
import javax.ws.rs.client.ClientBuilder;
import javax.ws.rs.client.ClientRequestContext;
import javax.ws.rs.client.ClientRequestFilter;
import javax.ws.rs.client.ClientResponseContext;
Expand All @@ -66,8 +66,6 @@
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.Response;

import javax.annotation.Priority;

import org.glassfish.jersey.client.ClientProperties;
import org.glassfish.jersey.client.internal.LocalizationMessages;

Expand Down Expand Up @@ -294,8 +292,16 @@ private void updateCache(ClientRequestContext request, boolean success, Type ope
* {@code false} otherwise).
*/
static boolean repeatRequest(ClientRequestContext request, ClientResponseContext response, String newAuthorizationHeader) {
Client client = request.getClient();
// If the failed response has an entity stream, close it. We must do this to avoid leaking a connection
// 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 the connection allocated
// to the failed request to be reused, if possible, for the repeated request.
if (response.hasEntity()) {
discardInputAndClose(response.getEntityStream());
response.setEntityStream(null);
}

Client client = request.getClient();
String method = request.getMethod();
MediaType mediaType = request.getMediaType();
URI lUri = request.getUri();
Expand All @@ -318,6 +324,12 @@ static boolean repeatRequest(ClientRequestContext request, ClientResponseContext

builder.property(REQUEST_PROPERTY_FILTER_REUSED, "true");

// Copy other properties, if any, from the original request
for (String propertyName : request.getPropertyNames()) {
Object propertyValue = request.getProperty(propertyName);
builder.property(propertyName, propertyValue);
}

Invocation invocation;
if (request.getEntity() == null) {
invocation = builder.build(method);
Expand Down Expand Up @@ -443,4 +455,23 @@ static Credentials getCredentials(ClientRequestContext request, Credentials defa
return specificCredentials != null ? specificCredentials : defaultCredentials;
}
}

private static void discardInputAndClose(InputStream is) {
byte[] buf = new byte[4096];
try {
while (true) {
if (is.read(buf) <= 0) {
break;
}
}
} catch (IOException ex) {
// ignore
} finally {
try {
is.close();
} catch (IOException ex) {
// ignore
}
}
}
}