Merge pull request jetty#11034 from jetty/jetty-12.0.x-11021-websocke…

…t-upgradeListener Issue jetty#11021 - do not call UpgradeListener.onHandshakeResponse() in case of failures
kohlschuetter · Dec 11, 2023 · bd342ac · bd342ac
2 parents 80c24fc + 363ebb3
commit bd342ac
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 11 deletions.
diff --git a/...lient/src/main/java/org/eclipse/jetty/websocket/core/client/CoreClientUpgradeRequest.java b/...lient/src/main/java/org/eclipse/jetty/websocket/core/client/CoreClientUpgradeRequest.java
@@ -250,17 +250,6 @@ public void onComplete(Result result)
         int status = response.getStatus();
         String responseLine = status + " " + response.getReason();
 
-        if (!upgraded)
-        {
-            // We have failed to upgrade but have received a response, so notify the listener.
-            Throwable listenerError = notifyUpgradeListeners((listener) -> listener.onHandshakeResponse(request, response));
-            if (listenerError != null)
-            {
-                if (LOG.isDebugEnabled())
-                    LOG.debug("error from listener", listenerError);
-            }
-        }
-
         if (result.isFailed())
         {
             if (LOG.isDebugEnabled())
@@ -281,6 +270,17 @@ public void onComplete(Result result)
             return;
         }
 
+        if (!upgraded)
+        {
+            // We have failed to upgrade but have received a response, so notify the listener.
+            Throwable listenerError = notifyUpgradeListeners((listener) -> listener.onHandshakeResponse(request, response));
+            if (listenerError != null)
+            {
+                if (LOG.isDebugEnabled())
+                    LOG.debug("error from listener", listenerError);
+            }
+        }
+
         if (status != HttpStatus.SWITCHING_PROTOCOLS_101)
         {
             // Failed to upgrade (other reason)

diff --git a/...etty-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientResponseTest.java b/...etty-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientResponseTest.java
@@ -13,6 +13,7 @@
 
 package org.eclipse.jetty.websocket.tests.client;
 
+import java.io.EOFException;
 import java.net.URI;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
@@ -22,6 +23,7 @@
 import org.eclipse.jetty.client.Response;
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.io.Content;
+import org.eclipse.jetty.io.EofException;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
 import org.eclipse.jetty.server.handler.ContextHandler;
@@ -120,4 +122,44 @@ public void onHandshakeResponse(Request request, Response response)
         assertThat(response.getHeaders().get("specialHeader"), equalTo("value123"));
         assertThat(content, equalTo("failed by test"));
     }
+
+    @Test
+    public void testServerAbort() throws Exception
+    {
+        before((req, resp, cb) ->
+        {
+            req.getConnectionMetaData().getConnection().getEndPoint().close();
+            cb.failed(new EofException());
+            return null;
+        });
+
+        EchoSocket clientEndpoint = new EchoSocket();
+        URI uri = URI.create("ws://localhost:" + _connector.getLocalPort());
+        ClientUpgradeRequest upgradeRequest = new ClientUpgradeRequest();
+
+        CompletableFuture<Void> onHandShakeRequest = new CompletableFuture<>();
+        CompletableFuture<Void> onHandShakeResponse = new CompletableFuture<>();
+        JettyUpgradeListener upgradeListener = new JettyUpgradeListener()
+        {
+            @Override
+            public void onHandshakeRequest(Request request)
+            {
+                onHandShakeRequest.complete(null);
+            }
+
+            @Override
+            public void onHandshakeResponse(Request request, Response response)
+            {
+                onHandShakeResponse.complete(null);
+            }
+        };
+
+        Throwable t = assertThrows(Throwable.class, () ->
+            _client.connect(clientEndpoint, uri, upgradeRequest, upgradeListener).get(5, TimeUnit.SECONDS));
+        assertThat(t, instanceOf(ExecutionException.class));
+        assertThat(t.getCause(), instanceOf(EOFException.class));
+
+        assertDoesNotThrow(() -> onHandShakeRequest.get(5, TimeUnit.SECONDS));
+        assertThrows(Throwable.class, () -> onHandShakeResponse.get(1, TimeUnit.SECONDS));
+    }
 }