484621 - Client hangs till timeout when Authentication.authenticate() throws exception.
Fixed by surrounding the call to Authentication.authenticate() with a try/catch and acting appropriately in case of exceptions.
This commit is contained in:
parent
b5db18378d
commit
0050ad5a99
|
@ -135,27 +135,36 @@ public abstract class AuthenticationProtocolHandler implements ProtocolHandler
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
final Authentication.Result authnResult = authentication.authenticate(request, response, headerInfo, conversation);
|
try
|
||||||
if (LOG.isDebugEnabled())
|
|
||||||
LOG.debug("Authentication result {}", authnResult);
|
|
||||||
if (authnResult == null)
|
|
||||||
{
|
{
|
||||||
forwardSuccessComplete(request, response);
|
final Authentication.Result authnResult = authentication.authenticate(request, response, headerInfo, conversation);
|
||||||
return;
|
if (LOG.isDebugEnabled())
|
||||||
}
|
LOG.debug("Authentication result {}", authnResult);
|
||||||
|
if (authnResult == null)
|
||||||
conversation.setAttribute(AUTHENTICATION_ATTRIBUTE, true);
|
|
||||||
|
|
||||||
Request newRequest = client.copyRequest(request, request.getURI());
|
|
||||||
authnResult.apply(newRequest);
|
|
||||||
newRequest.onResponseSuccess(new Response.SuccessListener()
|
|
||||||
{
|
|
||||||
@Override
|
|
||||||
public void onSuccess(Response response)
|
|
||||||
{
|
{
|
||||||
client.getAuthenticationStore().addAuthenticationResult(authnResult);
|
forwardSuccessComplete(request, response);
|
||||||
|
return;
|
||||||
}
|
}
|
||||||
}).send(null);
|
|
||||||
|
conversation.setAttribute(AUTHENTICATION_ATTRIBUTE, true);
|
||||||
|
|
||||||
|
Request newRequest = client.copyRequest(request, request.getURI());
|
||||||
|
authnResult.apply(newRequest);
|
||||||
|
newRequest.onResponseSuccess(new Response.SuccessListener()
|
||||||
|
{
|
||||||
|
@Override
|
||||||
|
public void onSuccess(Response response)
|
||||||
|
{
|
||||||
|
client.getAuthenticationStore().addAuthenticationResult(authnResult);
|
||||||
|
}
|
||||||
|
}).send(null);
|
||||||
|
}
|
||||||
|
catch (Throwable x)
|
||||||
|
{
|
||||||
|
if (LOG.isDebugEnabled())
|
||||||
|
LOG.debug("Authentication failed", x);
|
||||||
|
forwardFailureComplete(request, null, response, x);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private void forwardSuccessComplete(HttpRequest request, Response response)
|
private void forwardSuccessComplete(HttpRequest request, Response response)
|
||||||
|
|
|
@ -34,6 +34,8 @@ import org.eclipse.jetty.client.api.Authentication;
|
||||||
import org.eclipse.jetty.client.api.AuthenticationStore;
|
import org.eclipse.jetty.client.api.AuthenticationStore;
|
||||||
import org.eclipse.jetty.client.api.ContentResponse;
|
import org.eclipse.jetty.client.api.ContentResponse;
|
||||||
import org.eclipse.jetty.client.api.Request;
|
import org.eclipse.jetty.client.api.Request;
|
||||||
|
import org.eclipse.jetty.client.api.Response;
|
||||||
|
import org.eclipse.jetty.client.api.Result;
|
||||||
import org.eclipse.jetty.client.util.BasicAuthentication;
|
import org.eclipse.jetty.client.util.BasicAuthentication;
|
||||||
import org.eclipse.jetty.client.util.DigestAuthentication;
|
import org.eclipse.jetty.client.util.DigestAuthentication;
|
||||||
import org.eclipse.jetty.security.Authenticator;
|
import org.eclipse.jetty.security.Authenticator;
|
||||||
|
@ -47,6 +49,7 @@ import org.eclipse.jetty.server.Handler;
|
||||||
import org.eclipse.jetty.server.Server;
|
import org.eclipse.jetty.server.Server;
|
||||||
import org.eclipse.jetty.server.handler.AbstractHandler;
|
import org.eclipse.jetty.server.handler.AbstractHandler;
|
||||||
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
|
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
|
||||||
|
import org.eclipse.jetty.util.Attributes;
|
||||||
import org.eclipse.jetty.util.URIUtil;
|
import org.eclipse.jetty.util.URIUtil;
|
||||||
import org.eclipse.jetty.util.security.Constraint;
|
import org.eclipse.jetty.util.security.Constraint;
|
||||||
import org.eclipse.jetty.util.ssl.SslContextFactory;
|
import org.eclipse.jetty.util.ssl.SslContextFactory;
|
||||||
|
@ -320,4 +323,47 @@ public class HttpClientAuthenticationTest extends AbstractHttpClientServerTest
|
||||||
Assert.assertNotNull(response);
|
Assert.assertNotNull(response);
|
||||||
Assert.assertEquals(401, response.getStatus());
|
Assert.assertEquals(401, response.getStatus());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void test_Authentication_ThrowsException() throws Exception
|
||||||
|
{
|
||||||
|
startBasic(new EmptyServerHandler());
|
||||||
|
|
||||||
|
// Request without Authentication would cause a 401,
|
||||||
|
// but the client will throw an exception trying to
|
||||||
|
// send the credentials to the server.
|
||||||
|
final String cause = "thrown_explicitly_by_test";
|
||||||
|
client.getAuthenticationStore().addAuthentication(new Authentication()
|
||||||
|
{
|
||||||
|
@Override
|
||||||
|
public boolean matches(String type, URI uri, String realm)
|
||||||
|
{
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public Result authenticate(Request request, ContentResponse response, HeaderInfo headerInfo, Attributes context)
|
||||||
|
{
|
||||||
|
throw new RuntimeException(cause);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
final CountDownLatch latch = new CountDownLatch(1);
|
||||||
|
client.newRequest("localhost", connector.getLocalPort())
|
||||||
|
.scheme(scheme)
|
||||||
|
.path("/secure")
|
||||||
|
.timeout(5, TimeUnit.SECONDS)
|
||||||
|
.send(new Response.CompleteListener()
|
||||||
|
{
|
||||||
|
@Override
|
||||||
|
public void onComplete(Result result)
|
||||||
|
{
|
||||||
|
Assert.assertTrue(result.isFailed());
|
||||||
|
Assert.assertEquals(cause, result.getFailure().getMessage());
|
||||||
|
latch.countDown();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
Assert.assertTrue(latch.await(5, TimeUnit.SECONDS));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue