From 0050ad5a99a023a94fd39a5677f352e2c3991a88 Mon Sep 17 00:00:00 2001
From: Simone Bordet
Date: Tue, 5 Jan 2016 11:02:52 +0100
Subject: 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.
---
 .../client/AuthenticationProtocolHandler.java      | 43 ++++++++++++--------
 .../jetty/client/HttpClientAuthenticationTest.java | 46 ++++++++++++++++++++++
 2 files changed, 72 insertions(+), 17 deletions(-)

diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/AuthenticationProtocolHandler.java b/jetty-client/src/main/java/org/eclipse/jetty/client/AuthenticationProtocolHandler.java
index 7bc6cda592..72e2aa0e8c 100644
--- a/jetty-client/src/main/java/org/eclipse/jetty/client/AuthenticationProtocolHandler.java
+++ b/jetty-client/src/main/java/org/eclipse/jetty/client/AuthenticationProtocolHandler.java
@@ -135,27 +135,36 @@ public abstract class AuthenticationProtocolHandler implements ProtocolHandler
                 return;
             }
 
-            final Authentication.Result authnResult = authentication.authenticate(request, response, headerInfo, conversation);
-            if (LOG.isDebugEnabled())
-                LOG.debug("Authentication result {}", authnResult);
-            if (authnResult == null)
+            try
             {
-                forwardSuccessComplete(request, response);
-                return;
-            }
+                final Authentication.Result authnResult = authentication.authenticate(request, response, headerInfo, conversation);
+                if (LOG.isDebugEnabled())
+                    LOG.debug("Authentication result {}", authnResult);
+                if (authnResult == null)
+                {
+                    forwardSuccessComplete(request, response);
+                    return;
+                }
 
-            conversation.setAttribute(AUTHENTICATION_ATTRIBUTE, true);
+                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)
+                Request newRequest = client.copyRequest(request, request.getURI());
+                authnResult.apply(newRequest);
+                newRequest.onResponseSuccess(new Response.SuccessListener()
                 {
-                    client.getAuthenticationStore().addAuthenticationResult(authnResult);
-                }
-            }).send(null);
+                    @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)
diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java
index aa897174e3..f14f0562ed 100644
--- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java
+++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java
@@ -34,6 +34,8 @@ import org.eclipse.jetty.client.api.Authentication;
 import org.eclipse.jetty.client.api.AuthenticationStore;
 import org.eclipse.jetty.client.api.ContentResponse;
 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.DigestAuthentication;
 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.handler.AbstractHandler;
 import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
+import org.eclipse.jetty.util.Attributes;
 import org.eclipse.jetty.util.URIUtil;
 import org.eclipse.jetty.util.security.Constraint;
 import org.eclipse.jetty.util.ssl.SslContextFactory;
@@ -320,4 +323,47 @@ public class HttpClientAuthenticationTest extends AbstractHttpClientServerTest
         Assert.assertNotNull(response);
         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));
+    }
 }
-- 
cgit v1.2.3