sendError(-1) is an abort (#12206)

restore behaviour from jetty <= 11 where a sendError(-1) is a true abort, without an attempt to send an error response.

jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java

@@ -47,6 +47,7 @@ import org.eclipse.jetty.http.MultiPartCompliance;
 import org.eclipse.jetty.http.MultiPartConfig;
 import org.eclipse.jetty.http.Trailers;
 import org.eclipse.jetty.io.Content;
+import org.eclipse.jetty.io.EndPoint;
 import org.eclipse.jetty.server.internal.CompletionStreamWrapper;
 import org.eclipse.jetty.server.internal.HttpChannelState;
 import org.eclipse.jetty.util.Attributes;
@@ -716,6 +717,7 @@ public interface Request extends Attributes, Content.Source
          *         is returned, then this method must not generate a response, nor complete the callback.
          * @throws Exception if there is a failure during the handling. Catchers cannot assume that the callback will be
          *                   called and thus should attempt to complete the request as if a false had been returned.
+         * @see AbortException
          */
         boolean handle(Request request, Response response, Callback callback) throws Exception;
 
@@ -725,6 +727,34 @@ public interface Request extends Attributes, Content.Source
         {
             return InvocationType.BLOCKING;
         }
+
+        /**
+         * A marker {@link Exception} that can be passed the {@link Callback#failed(Throwable)} of the {@link Callback}
+         * passed in {@link #handle(Request, Response, Callback)}, to cause request handling to be aborted.  For HTTP/1
+         * an abort is handled with a {@link EndPoint#close()}, for later versions of HTTP, a reset message will be sent.
+         */
+        class AbortException extends Exception
+        {
+            public AbortException()
+            {
+                super();
+            }
+
+            public AbortException(String message)
+            {
+                super(message);
+            }
+
+            public AbortException(String message, Throwable cause)
+            {
+                super(message, cause);
+            }
+
+            public AbortException(Throwable cause)
+            {
+                super(cause);
+            }
+        }
     }
 
     /**

jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java

@@ -1590,6 +1590,8 @@ public class HttpChannelState implements HttpChannel, Components
 
                     httpChannelState._callbackFailure = failure;
 
+                    if (!stream.isCommitted() && !(failure instanceof Request.Handler.AbortException))
+                    {
                         // Consume any input.
                         Throwable unconsumed = stream.consumeAvailable();
                         ExceptionUtil.addSuppressedIfNotAssociated(failure, unconsumed);
@@ -1598,11 +1600,9 @@ public class HttpChannelState implements HttpChannel, Components
                         if (LOG.isDebugEnabled())
                             LOG.debug("failed stream.isCommitted={}, response.isCommitted={} {}", stream.isCommitted(), response.isCommitted(), this);
 
-                    // There may have been an attempt to write an error response that failed.
-                    // Do not try to write again an error response if already committed.
-                    if (!stream.isCommitted())
                         errorResponse = new ErrorResponse(request);
                     }
+                }
 
                 if (errorResponse != null)
                     Response.writeError(request, errorResponse, new ErrorCallback(request, errorResponse, stream, failure), failure);

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java

@@ -138,7 +138,7 @@ public class ServletApiResponse implements HttpServletResponse
     {
         switch (sc)
         {
-            case -1 -> getServletChannel().abort(new IOException(msg));
+            case -1 -> getServletChannel().abort(new Request.Handler.AbortException(msg));
             case HttpStatus.PROCESSING_102, HttpStatus.EARLY_HINTS_103 ->
             {
                 if (!isCommitted())

jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ErrorPageTest.java

@@ -13,8 +13,12 @@
 
 package org.eclipse.jetty.ee10.servlet;
 
+import java.io.BufferedReader;
 import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.OutputStream;
 import java.io.PrintWriter;
+import java.net.Socket;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -50,6 +54,7 @@ import org.eclipse.jetty.server.LocalConnector;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.Response;
 import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
 import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
 import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
 import org.eclipse.jetty.util.Callback;
@@ -70,6 +75,7 @@ import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -780,6 +786,220 @@ public class ErrorPageTest
         assertThat(responseBody, Matchers.containsString("ERROR_REQUEST_URI: /fail/599"));
     }
 
+    @Test
+    public void testAbortWithSendError() throws Exception
+    {
+        ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.NO_SECURITY | ServletContextHandler.NO_SESSIONS);
+        contextHandler.setContextPath("/");
+
+        HttpServlet failServlet = new HttpServlet()
+        {
+            @Override
+            protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException
+            {
+                response.sendError(-1);
+            }
+        };
+
+        contextHandler.addServlet(failServlet, "/abort");
+        startServer(contextHandler);
+
+        ServerConnector connector = new ServerConnector(_server);
+        connector.setPort(0);
+        _server.addConnector(connector);
+        connector.start();
+        try (Socket socket = new Socket("localhost", connector.getLocalPort()))
+        {
+            OutputStream output = socket.getOutputStream();
+
+            String request = """
+            GET /abort HTTP/1.1\r
+            Host: test\r
+            \r
+            """;
+            output.write(request.getBytes(StandardCharsets.UTF_8));
+            output.flush();
+
+            BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
+            String line = in.readLine();
+            assertNull(line);
+        }
+    }
+
+    @Test
+    public void testAbortWithSendErrorChunked() throws Exception
+    {
+        ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.NO_SECURITY | ServletContextHandler.NO_SESSIONS);
+        contextHandler.setContextPath("/");
+
+        HttpServlet failServlet = new HttpServlet()
+        {
+            @Override
+            protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException
+            {
+                response.getOutputStream().write("test".getBytes(StandardCharsets.UTF_8));
+                response.flushBuffer();
+                response.sendError(-1);
+            }
+        };
+
+        contextHandler.addServlet(failServlet, "/abort");
+        startServer(contextHandler);
+
+        ServerConnector connector = new ServerConnector(_server);
+        connector.setPort(0);
+        _server.addConnector(connector);
+        connector.start();
+        try (Socket socket = new Socket("localhost", connector.getLocalPort()))
+        {
+            OutputStream output = socket.getOutputStream();
+
+            String request = """
+            GET /abort HTTP/1.1\r
+            Host: test\r
+            \r
+            """;
+            output.write(request.getBytes(StandardCharsets.UTF_8));
+            output.flush();
+
+            BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
+            String line = in.readLine();
+            assertThat(line, is("HTTP/1.1 200 OK"));
+
+            boolean chunked = false;
+            while (!line.isEmpty())
+            {
+                line = in.readLine();
+                assertNotNull(line);
+                chunked |= line.equals("Transfer-Encoding: chunked");
+            }
+            assertTrue(chunked);
+
+            line = in.readLine();
+            assertThat(line, is("4"));
+            line = in.readLine();
+            assertThat(line, is("test"));
+
+            line = in.readLine();
+            assertNull(line);
+        }
+    }
+
+    @Test
+    public void testAbortWithSendErrorContent() throws Exception
+    {
+        ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.NO_SECURITY | ServletContextHandler.NO_SESSIONS);
+        contextHandler.setContextPath("/");
+
+        HttpServlet failServlet = new HttpServlet()
+        {
+            @Override
+            protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException
+            {
+                response.setContentLength(10);
+                response.getOutputStream().write("test\r\n".getBytes(StandardCharsets.UTF_8));
+                response.flushBuffer();
+                response.sendError(-1);
+            }
+        };
+
+        contextHandler.addServlet(failServlet, "/abort");
+        startServer(contextHandler);
+
+        ServerConnector connector = new ServerConnector(_server);
+        connector.setPort(0);
+        _server.addConnector(connector);
+        connector.start();
+        try (Socket socket = new Socket("localhost", connector.getLocalPort()))
+        {
+            OutputStream output = socket.getOutputStream();
+
+            String request = """
+            GET /abort HTTP/1.1\r
+            Host: test\r
+            \r
+            """;
+            output.write(request.getBytes(StandardCharsets.UTF_8));
+            output.flush();
+
+            BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
+            String line = in.readLine();
+            assertThat(line, is("HTTP/1.1 200 OK"));
+
+            boolean chunked = false;
+            while (!line.isEmpty())
+            {
+                line = in.readLine();
+                assertNotNull(line);
+                chunked |= line.equals("Transfer-Encoding: chunked");
+            }
+            assertFalse(chunked);
+
+            line = in.readLine();
+            assertThat(line, is("test"));
+
+            line = in.readLine();
+            assertNull(line);
+        }
+    }
+
+    @Test
+    public void testAbortWithSendErrorComplete() throws Exception
+    {
+        ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.NO_SECURITY | ServletContextHandler.NO_SESSIONS);
+        contextHandler.setContextPath("/");
+
+        HttpServlet failServlet = new HttpServlet()
+        {
+            @Override
+            protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException
+            {
+                response.setContentLength(6);
+                response.getOutputStream().write("test\r\n".getBytes(StandardCharsets.UTF_8));
+                response.sendError(-1);
+            }
+        };
+
+        contextHandler.addServlet(failServlet, "/abort");
+        startServer(contextHandler);
+
+        ServerConnector connector = new ServerConnector(_server);
+        connector.setPort(0);
+        _server.addConnector(connector);
+        connector.start();
+        try (Socket socket = new Socket("localhost", connector.getLocalPort()))
+        {
+            OutputStream output = socket.getOutputStream();
+
+            String request = """
+            GET /abort HTTP/1.1\r
+            Host: test\r
+            \r
+            """;
+            output.write(request.getBytes(StandardCharsets.UTF_8));
+            output.flush();
+
+            BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
+            String line = in.readLine();
+            assertThat(line, is("HTTP/1.1 200 OK"));
+
+            boolean chunked = false;
+            while (!line.isEmpty())
+            {
+                line = in.readLine();
+                assertNotNull(line);
+                chunked |= line.equals("Transfer-Encoding: chunked");
+            }
+            assertFalse(chunked);
+
+            line = in.readLine();
+            assertThat(line, is("test"));
+
+            line = in.readLine();
+            assertNull(line);
+        }
+    }
+
     @Test
     public void testErrorCodeNoDefaultServletNonExistentErrorLocation() throws Exception
     {

jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java

@@ -488,7 +488,7 @@ public class Response implements HttpServletResponse
 
         switch (code)
         {
-            case -1 -> _channel.abort(new IOException(message));
+            case -1 -> _channel.abort(new org.eclipse.jetty.server.Request.Handler.AbortException(message));
             case HttpStatus.PROCESSING_102 -> sendProcessing();
             case HttpStatus.EARLY_HINTS_103 -> sendEarlyHint();
             default -> _channel.getState().sendError(code, message);

jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ErrorPageTest.java

@@ -13,8 +13,12 @@
 
 package org.eclipse.jetty.ee9.servlet;
 
+import java.io.BufferedReader;
 import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.OutputStream;
 import java.io.PrintWriter;
+import java.net.Socket;
 import java.nio.charset.StandardCharsets;
 import java.util.EnumSet;
 import java.util.concurrent.ConcurrentHashMap;
@@ -29,7 +33,6 @@ import jakarta.servlet.DispatcherType;
 import jakarta.servlet.Filter;
 import jakarta.servlet.FilterChain;
 import jakarta.servlet.FilterConfig;
-import jakarta.servlet.RequestDispatcher;
 import jakarta.servlet.Servlet;
 import jakarta.servlet.ServletException;
 import jakarta.servlet.ServletRequest;
@@ -49,10 +52,10 @@ import org.eclipse.jetty.http.HttpTester;
 import org.eclipse.jetty.logging.StacklessLogging;
 import org.eclipse.jetty.server.LocalConnector;
 import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
 import org.hamcrest.Matchers;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -62,6 +65,7 @@ import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 //@Disabled // TODO
@@ -105,6 +109,7 @@ public class ErrorPageTest
         _context.addServlet(ErrorAndStatusServlet.class, "/error-and-status/*");
         _context.addServlet(ErrorContentTypeCharsetWriterInitializedServlet.class, "/error-mime-charset-writer/*");
         _context.addServlet(ExceptionServlet.class, "/exception-servlet");
+        _context.addServlet(AbortServlet.class, "/abort");
 
         HandlerWrapper noopHandler = new HandlerWrapper()
         {
@@ -300,6 +305,34 @@ public class ErrorPageTest
         assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /fail/code"));
     }
 
+    @Test
+    public void testAbortWithSendError() throws Exception
+    {
+        ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.NO_SECURITY | ServletContextHandler.NO_SESSIONS);
+        contextHandler.setContextPath("/");
+
+        ServerConnector connector = new ServerConnector(_server);
+        connector.setPort(0);
+        _server.addConnector(connector);
+        connector.start();
+        try (Socket socket = new Socket("localhost", connector.getLocalPort()))
+        {
+            OutputStream output = socket.getOutputStream();
+
+            String request = """
+            GET /abort HTTP/1.1\r
+            Host: test\r
+            \r
+            """;
+            output.write(request.getBytes(StandardCharsets.UTF_8));
+            output.flush();
+
+            BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
+            String line = in.readLine();
+            assertNull(line);
+        }
+    }
+
     @Test
     public void testErrorException() throws Exception
     {
@@ -871,4 +904,13 @@ public class ErrorPageTest
             super(rootCause);
         }
     }
+
+    public static class AbortServlet extends HttpServlet
+    {
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException
+        {
+            response.sendError(-1);
+        }
+    }
 }