Skip to content

Patch for client connection leak when using digest authentication #3860

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 25, 2018
Merged
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 @@ -41,6 +41,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 @@ -147,6 +148,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 @@ -281,6 +295,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 @@ -16,6 +16,8 @@

package org.glassfish.jersey.client.authentication;

import java.io.IOException;
import java.io.InputStream;
import java.net.URI;
import java.net.URISyntaxException;

Expand All @@ -25,6 +27,25 @@
* Common authentication utilities
*/
class AuthenticationUtil {
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
}
}
}

static URI getCacheKey(ClientRequestContext request) {
URI requestUri = request.getUri();
if (requestUri.getRawQuery() != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,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()) {
AuthenticationUtil.discardInputAndClose(response.getEntityStream());
response.setEntityStream(null);
}

Client client = request.getClient();
String method = request.getMethod();
MediaType mediaType = request.getMediaType();
URI lUri = request.getUri();
Expand All @@ -292,6 +300,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