Fixed handling of HTTP upgrade in CoreClientUpgradeRequest.

For HTTP/2, the response may have arrived, but the exchange failed (due to the request being failed); in this case, we want to notify an UpgradeException, not a generic Exception. Signed-off-by: Simone Bordet <[email protected]>
jetty · sbordet · Nov 29, 2024 · Nov 29, 2024 · Nov 30, 2024 · Dec 1, 2024
commit 6dbeb960fad2cee1ecc7c2570d4a9a044b08d394
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
@@ -35,7  35,6 @@
 import org.eclipse.jetty.http.HttpFields;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpScheme;
-import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.HttpVersion;
 import org.eclipse.jetty.io.ByteBufferPool;
 import org.eclipse.jetty.io.EndPoint;
@@ -249,43  248,37 @@ public void onComplete(Result result)
         Response response = result.getResponse();
         int status = response.getStatus();
         String responseLine = status   " "   response.getReason();
         boolean receivedResponse = status > 0;
 
-        if (result.isFailed())
         if (receivedResponse)
         {
             if (upgraded)
                 return;
 
             // We have failed to upgrade but have received a response, notify the listeners.
             Throwable listenerError = notifyUpgradeListeners((listener) -> listener.onHandshakeResponse(request, response));
             if (listenerError != null)
             {
                 if (LOG.isDebugEnabled())
                     LOG.debug("Failure while notifying handshake response listeners", listenerError);
             }
             handleException(new UpgradeException(requestURI, status,
                 "Failed to upgrade to websocket: Unexpected HTTP Response Status Code: "   responseLine));
         }
         else
         {
             if (LOG.isDebugEnabled())
             {
-                if (result.getFailure() != null)
-                    LOG.debug("General Failure", result.getFailure());
                 if (result.getRequestFailure() != null)
-                    LOG.debug("Request Failure", result.getRequestFailure());
                     LOG.debug("Failed to upgrade to websocket: request failure", result.getRequestFailure());
                 if (result.getResponseFailure() != null)
-                    LOG.debug("Response Failure", result.getResponseFailure());
                     LOG.debug("Failed to upgrade to websocket: response failure", result.getResponseFailure());
             }
-
             Throwable failure = result.getFailure();
             boolean wrapFailure = !(failure instanceof IOException) && !(failure instanceof UpgradeException);
             if (wrapFailure)
                 failure = new UpgradeException(requestURI, status, responseLine, failure);
             handleException(failure);
-            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)
-            handleException(new UpgradeException(requestURI, status,
-                "Failed to upgrade to websocket: Unexpected HTTP Response Status Code: "   responseLine));
         }
     }
 

diff --git a/...ts/src/test/java/org/eclipse/jetty/ee10/test/client/transport/HttpClientContinueTest.java b/...ts/src/test/java/org/eclipse/jetty/ee10/test/client/transport/HttpClientContinueTest.java
@@ -338,25  338,17 @@ public void onComplete(Result result)
     @MethodSource("transportsNoFCGI")
     public void testRedirectWithExpect100ContinueWithContent(TransportType transportType) throws Exception
     {
-        // A request with Expect: 100-Continue cannot receive non-final responses like 3xx
         // A request with Expect: 100-Continue cannot receive non-final responses like 3xx.
 
-        String data = "success";
         start(transportType, new HttpServlet()
         {
             @Override
             protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException
             {
                 if (request.getRequestURI().endsWith("/done"))
-                {
-                    // Send 100-Continue and consume the content
-                    IO.copy(request.getInputStream(), new ByteArrayOutputStream());
-                    response.getOutputStream().print(data);
-                }
-                else
-                {
-                    // Send a redirect
-                    response.sendRedirect("/done");
-                }
                     throw new IOException("Redirect should not have been followed");
                 // Send a redirect.
                 response.sendRedirect("/done");
             }
         });
 
@@ -374,8  366,7 @@ public void onComplete(Result result)
                 {
                     assertTrue(result.isFailed());
                     assertNotNull(result.getRequestFailure());
-                    assertNull(result.getResponseFailure());
-                    assertEquals(302, result.getResponse().getStatus());
                     assertEquals(HttpStatus.MOVED_TEMPORARILY_302, result.getResponse().getStatus());
                     latch.countDown();
                 }
             });

diff --git a/...ts/src/test/java/org/eclipse/jetty/ee11/test/client/transport/HttpClientContinueTest.java b/...ts/src/test/java/org/eclipse/jetty/ee11/test/client/transport/HttpClientContinueTest.java
@@ -339,25  339,17 @@ public void onComplete(Result result)
     @MethodSource("transportsNoFCGI")
     public void testRedirectWithExpect100ContinueWithContent(TransportType transportType) throws Exception
     {
-        // A request with Expect: 100-Continue cannot receive non-final responses like 3xx
         // A request with Expect: 100-Continue cannot receive non-final responses like 3xx.
 
-        String data = "success";
         start(transportType, new HttpServlet()
         {
             @Override
             protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException
             {
                 if (request.getRequestURI().endsWith("/done"))
-                {
-                    // Send 100-Continue and consume the content
-                    IO.copy(request.getInputStream(), new ByteArrayOutputStream());
-                    response.getOutputStream().print(data);
-                }
-                else
-                {
-                    // Send a redirect
-                    response.sendRedirect("/done");
-                }
                     throw new IOException("Redirect should not have been followed");
                 // Send a redirect.
                 response.sendRedirect("/done");
             }
         });
 
@@ -375,8  367,7 @@ public void onComplete(Result result)
                 {
                     assertTrue(result.isFailed());
                     assertNotNull(result.getRequestFailure());
-                    assertNull(result.getResponseFailure());
-                    assertEquals(302, result.getResponse().getStatus());
                     assertEquals(HttpStatus.MOVED_TEMPORARILY_302, result.getResponse().getStatus());
                     latch.countDown();
                 }
             });

diff --git a/...ee9-websocket/jetty-ee9-websocket-jetty-tests/src/test/resources/jetty-logging.properties b/...ee9-websocket/jetty-ee9-websocket-jetty-tests/src/test/resources/jetty-logging.properties
@@ -1,4  1,3 @@
-# Jetty Logging using jetty-slf4j-impl
 # org.eclipse.jetty.LEVEL=DEBUG
 # org.eclipse.jetty.websocket.LEVEL=DEBUG
 # org.eclipse.jetty.websocket.test.LEVEL=DEBUG