Fixes #11370 - IllegalStateException when last write fails.

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

@@ -143,8 +143,8 @@ public void write(boolean last, ByteBuffer content, Callback callback)
             {
                 if (_responseLimit >= 0 && (_written + content.remaining())  > _responseLimit)
                 {
-                    callback.failed(new HttpException.RuntimeException(HttpStatus.INTERNAL_SERVER_ERROR_500, "Response body is too large: " +
-                        _written + content.remaining() + ">" + _responseLimit));
+                    String message = "Response body is too large: %d>%d".formatted(_written + content.remaining(), _responseLimit);
+                    callback.failed(new HttpException.RuntimeException(HttpStatus.INTERNAL_SERVER_ERROR_500, message));
                     return;
                 }
                 _written += content.remaining();

jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/SizeLimitHandlerTest.java

@@ -80,7 +80,7 @@ public void testSmallGET() throws Exception
         _contextHandler.setHandler(new Handler.Abstract()
         {
             @Override
-            public boolean handle(Request request, Response response, Callback callback) throws Exception
+            public boolean handle(Request request, Response response, Callback callback)
             {
                 response.write(true, BufferUtil.toBuffer("Hello World"), callback);
                 return true;
@@ -99,7 +99,7 @@ public void testLargeGETContentLengthKnown() throws Exception
         _contextHandler.setHandler(new Handler.Abstract()
         {
             @Override
-            public boolean handle(Request request, Response response, Callback callback) throws Exception
+            public boolean handle(Request request, Response response, Callback callback)
             {
                 response.getHeaders().put(HttpHeader.CONTENT_LENGTH, 8 * 1024 + 1);
                 fail();
@@ -120,9 +120,8 @@ public void testLargeGETSingleByteBuffer() throws Exception
         _contextHandler.setHandler(new Handler.Abstract()
         {
             @Override
-            public boolean handle(Request request, Response response, Callback callback) throws Exception
+            public boolean handle(Request request, Response response, Callback callback)
             {
-
                 response.write(true, ByteBuffer.wrap(new byte[8 * 1024 + 1]), callback);
                 return true;
             }
@@ -141,7 +140,7 @@ public void testLargeGETManyWrites() throws Exception
         _contextHandler.setHandler(new Handler.Abstract()
         {
             @Override
-            public boolean handle(Request request, Response response, Callback callback) throws Exception
+            public boolean handle(Request request, Response response, Callback callback)
             {
                 byte[] data = new byte[1024];
                 Arrays.fill(data, (byte)'X');
@@ -199,10 +198,12 @@ public boolean handle(Request request, Response response, Callback callback) thr
         });
         _server.start();
         HttpTester.Response response = HttpTester.parseResponse(
-            _local.getResponse("POST /ctx/hello HTTP/1.0\r\n" +
-                "Content-Length: 8\r\n" +
-                "\r\n" +
-                "123456\r\n"));
+            _local.getResponse("""
+                POST /ctx/hello HTTP/1.0\r
+                Content-Length: 8\r
+                \r
+                123456\r
+                """));
         assertThat(response.getStatus(), equalTo(200));
         assertThat(response.getContent(), containsString("OK 8"));
     }
@@ -223,10 +224,11 @@ public boolean handle(Request request, Response response, Callback callback) thr
         });
         _server.start();
         HttpTester.Response response = HttpTester.parseResponse(
-            _local.getResponse("POST /ctx/hello HTTP/1.0\r\n" +
-                "Content-Length: 32768\r\n" +
-                "\r\n" +
-                "123456..."));
+            _local.getResponse("""
+                POST /ctx/hello HTTP/1.0\r
+                Content-Length: 32768\r
+                \r
+                123456..."""));
         assertThat(response.getStatus(), equalTo(413));
         assertThat(response.getContent(), containsString("32768>8192"));
     }
@@ -247,21 +249,23 @@ public boolean handle(Request request, Response response, Callback callback) thr
         });
         _server.start();
 
-        try (LocalConnector.LocalEndPoint endp = _local.executeRequest(
-            "POST /ctx/hello HTTP/1.1\r\n" +
-                "Host: localhost\r\n" +
-                "Transfer-Encoding: chunked\r\n" +
-                "\r\n"))
+        try (LocalConnector.LocalEndPoint endPoint = _local.executeRequest(
+            """
+                POST /ctx/hello HTTP/1.1\r
+                Host: localhost\r
+                Transfer-Encoding: chunked\r
+                \r
+                """))
         {
             byte[] data = new byte[1024];
             Arrays.fill(data, (byte)'X');
             data[1023] = (byte)'\n';
             String text = new String(data, 0, 1024, Charset.defaultCharset());
 
             for (int i = 0; i < 9; i++)
-                endp.addInput("400\r\n" + text + "\r\n");
+                endPoint.addInput("400\r\n" + text + "\r\n");
 
-            HttpTester.Response response = HttpTester.parseResponse(endp.getResponse());
+            HttpTester.Response response = HttpTester.parseResponse(endPoint.getResponse());
 
             assertThat(response.getStatus(), equalTo(413));
             assertThat(response.getContent(), containsString("&gt;8192"));
@@ -275,7 +279,7 @@ public void testMultipleRequests() throws Exception
         _contextHandler.setHandler(new Handler.Abstract()
         {
             @Override
-            public boolean handle(Request request, Response response, Callback callback) throws Exception
+            public boolean handle(Request request, Response response, Callback callback)
             {
                 response.write(true, BufferUtil.toBuffer(message), callback);
                 return true;

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

@@ -432,7 +432,7 @@ public void complete(Callback callback)
         }
 
         if (content != null)
-            channelWrite(content, true, new CompleteWriteCompleteCB());
+            channelWrite(content, true, new WriteCompleteCB());
     }
 
     /**
@@ -1766,15 +1766,4 @@ public InvocationType getInvocationType()
             return InvocationType.NON_BLOCKING;
         }
     }
-
-    private class CompleteWriteCompleteCB extends WriteCompleteCB
-    {
-        @Override
-        public void failed(Throwable x)
-        {
-            // TODO why is this needed for h2/h3?
-            HttpOutput.this._servletChannel.abort(x);
-            super.failed(x);
-        }
-    }
 }

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

@@ -1051,6 +1051,9 @@ protected void completed(Throwable failure)
             if (_requestState != RequestState.COMPLETING)
                 throw new IllegalStateException(this.getStatusStringLocked());
 
+            if (failure != null)
+                abortResponse(failure);
+
             if (_event == null)
             {
                 _requestState = RequestState.COMPLETED;

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

@@ -18,6 +18,7 @@
 import java.io.IOException;
 import java.net.URI;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.zip.GZIPInputStream;
 import java.util.zip.GZIPOutputStream;
@@ -40,7 +41,6 @@
 import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.IO;
 import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -49,6 +49,7 @@
 import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.lessThan;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public class SizeLimitHandlerServletTest
 {
@@ -57,8 +58,7 @@ public class SizeLimitHandlerServletTest
     private ServerConnector _connector;
     private HttpClient _client;
 
-    @BeforeEach
-    public void before() throws Exception
+    private void start(HttpServlet servlet) throws Exception
     {
         _server = new Server();
         _connector = new ServerConnector(_server);
@@ -71,15 +71,7 @@ public void before() throws Exception
         sizeLimitHandler.setHandler(gzipHandler);
         contextHandler.insertHandler(sizeLimitHandler);
 
-        contextHandler.addServlet(new HttpServlet()
-        {
-            @Override
-            protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException
-            {
-                String requestContent = IO.toString(req.getInputStream());
-                resp.getWriter().print(requestContent);
-            }
-        }, "/");
+        contextHandler.addServlet(servlet, "/");
 
         _server.setHandler(contextHandler);
         _server.start();
@@ -99,6 +91,16 @@ public void after() throws Exception
     @Test
     public void testGzippedEcho() throws Exception
     {
+        start(new HttpServlet()
+        {
+            @Override
+            protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException
+            {
+                String requestContent = IO.toString(req.getInputStream());
+                resp.getWriter().print(requestContent);
+            }
+        });
+
         String content = "x".repeat(SIZE_LIMIT * 2);
 
         URI uri = URI.create("http://localhost:" + _connector.getLocalPort());
@@ -123,6 +125,16 @@ public void testGzippedEcho() throws Exception
     @Test
     public void testNormalEcho() throws Exception
     {
+        start(new HttpServlet()
+        {
+            @Override
+            protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException
+            {
+                String requestContent = IO.toString(req.getInputStream());
+                resp.getWriter().print(requestContent);
+            }
+        });
+
         String content = "x".repeat(SIZE_LIMIT * 2);
 
         URI uri = URI.create("http://localhost:" + _connector.getLocalPort());
@@ -135,6 +147,65 @@ public void testNormalEcho() throws Exception
     @Test
     public void testGzipEchoNoAcceptEncoding() throws Exception
     {
+        start(new HttpServlet()
+        {
+            @Override
+            protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException
+            {
+                String requestContent = IO.toString(req.getInputStream());
+                // The content will be buffered, and the implementation
+                // will try to flush it, failing because of SizeLimitHandler.
+                resp.getWriter().print(requestContent);
+            }
+        });
+
+        String content = "x".repeat(SIZE_LIMIT * 2);
+        URI uri = URI.create("http://localhost:" + _connector.getLocalPort());
+
+        StringBuilder contentReceived = new StringBuilder();
+        CompletableFuture<Result> resultFuture = new CompletableFuture<>();
+        _client.POST(uri)
+            .headers(httpFields -> httpFields.add(HttpHeader.CONTENT_ENCODING, "gzip"))
+            .body(gzipContent(content))
+            .onResponseContentAsync((response, chunk, demander) ->
+            {
+                contentReceived.append(BufferUtil.toString(chunk.getByteBuffer()));
+                demander.run();
+            })
+            .send(resultFuture::complete);
+
+        Result result = resultFuture.get(5, TimeUnit.SECONDS);
+        assertNotNull(result);
+        assertThat(result.getResponse().getStatus(), equalTo(HttpStatus.INTERNAL_SERVER_ERROR_500));
+        assertThat(contentReceived.toString(), containsString("Response body is too large"));
+    }
+
+    @Test
+    public void testGzipEchoNoAcceptEncodingFlush() throws Exception
+    {
+        CountDownLatch flushFailureLatch = new CountDownLatch(1);
+        start(new HttpServlet()
+        {
+            @Override
+            protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException
+            {
+                String requestContent = IO.toString(req.getInputStream());
+                // The content will be buffered.
+                resp.getWriter().print(requestContent);
+                try
+                {
+                    // The flush will fail because exceeds
+                    // the SizeLimitHandler configuration.
+                    resp.flushBuffer();
+                }
+                catch (Throwable x)
+                {
+                    flushFailureLatch.countDown();
+                    throw x;
+                }
+            }
+        });
+
         String content = "x".repeat(SIZE_LIMIT * 2);
         URI uri = URI.create("http://localhost:" + _connector.getLocalPort());
 
@@ -150,6 +221,7 @@ public void testGzipEchoNoAcceptEncoding() throws Exception
             })
             .send(resultFuture::complete);
 
+        assertTrue(flushFailureLatch.await(5, TimeUnit.SECONDS));
 
         Result result = resultFuture.get(5, TimeUnit.SECONDS);
         assertNotNull(result);