Fixes #12313 - Jetty 12 ee9/ee10 doesn't invoke callbacks when h2 cli…

…ent sends RST_STREAM. * Do not recycle EE9 HttpChannel for WebSocket requests. * Simplified sendError() error handling. Signed-off-by: Simone Bordet <[email protected]>
jetty · Oct 24, 2024 · 8824374 · 8824374
1 parent f48be0d
commit 8824374
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 17 deletions.
diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java
@@ -64,6 +64,7 @@
 import jakarta.servlet.http.HttpSessionListener;
 import org.eclipse.jetty.http.BadMessageException;
 import org.eclipse.jetty.http.HttpCookie;
+import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpMethod;
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.HttpURI;
@@ -2810,7 +2811,14 @@ else if (httpChannel.getContextHandler() == ContextHandler.this && !request.getC
             CoreContextRequest coreContextRequest = new CoreContextRequest(request, this.getContext(), httpChannel);
             httpChannel.onRequest(coreContextRequest);
             HttpChannel channel = httpChannel;
-            org.eclipse.jetty.server.Request.addCompletionListener(coreContextRequest, x -> channel.recycle());
+            org.eclipse.jetty.server.Request.addCompletionListener(coreContextRequest, x ->
+            {
+                // WebSocket needs a reference to the HttpServletRequest,
+                // so do not recycle the HttpChannel if it's a WebSocket
+                // request, no matter if the response is successful or not.
+                if (!request.getHeaders().contains(HttpHeader.SEC_WEBSOCKET_VERSION))
+                    channel.recycle();
+            });
             return coreContextRequest;
         }
 

diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpChannel.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpChannel.java
@@ -56,6 +56,7 @@
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.Callback;
+import org.eclipse.jetty.util.ExceptionUtil;
 import org.eclipse.jetty.util.HostPort;
 import org.eclipse.jetty.util.IO;
 import org.eclipse.jetty.util.SharedBlockingCallback.Blocker;
@@ -560,23 +561,15 @@ public boolean handle()
                         {
                             if (LOG.isDebugEnabled())
                                 LOG.debug("Could not perform ERROR dispatch, aborting", x);
-                            if (_state.isResponseCommitted())
+                            try
                             {
-                                abort(x);
+                                _response.resetContent();
+                                sendResponseAndComplete();
                             }
-                            else
+                            catch (Throwable t)
                             {
-                                try
-                                {
-                                    _response.resetContent();
-                                    sendResponseAndComplete();
-                                }
-                                catch (Throwable t)
-                                {
-                                    if (x != t)
-                                        x.addSuppressed(t);
-                                    abort(x);
-                                }
+                                ExceptionUtil.addSuppressedIfNotAssociated(x, t);
+                                throw x;
                             }
                         }
                         finally

diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpChannelState.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpChannelState.java
@@ -343,6 +343,15 @@ public boolean isResponseCompleted()
         }
     }
 
+    /**
+     * <p>Aborts the {@link HttpChannel}, eventually
+     * resulting in the completion of its state machine.</p>
+     *
+     * @param failure the cause of the abort
+     * @return {@code null} when no abort happened because it was already aborted;
+     * {@code false} when abort happened, but there is no need to call {@link HttpChannel#handle()};
+     * {@code true} when abort happened, and {@link HttpChannel#handle()} must be called.
+     */
     Boolean abort(Throwable failure)
     {
         boolean handle;
@@ -808,7 +817,7 @@ protected boolean onError(Throwable th)
             if (_sendError)
             {
                 LOG.warn("onError not handled due to prior sendError() {}", getStatusStringLocked(), th);
-                return false;
+                return true;
             }
 
             // Check async state to determine type of handling
@@ -841,7 +850,7 @@ protected boolean onError(Throwable th)
                 default:
                 {
                     LOG.warn("onError not handled due to invalid requestState {}", getStatusStringLocked(), th);
-                    return false;
+                    return true;
                 }
             }
         }