Do not use HttpStream.Wrapper in SizeLimitHandler

documentation/jetty-documentation/src/main/asciidoc/programming-guide/server/http/server-http-handler-use.adoc

@@ -224,6 +224,27 @@ Server
         └── ContextHandler N
 ----
 
+[[pg-server-http-handler-use-sizelimit]]
+====== SizeLimitHandler
+
+`SizeLimitHandler` tracks the sizes of request content and response content, and fails the request processing with an HTTP status code of link:https://www.rfc-editor.org/rfc/rfc9110.html#name-413-content-too-large[`413 Content Too Large`].
+
+Server applications can set up the `SizeLimitHandler` before or after handlers that modify the request content or response content such as xref:pg-server-http-handler-use-gzip[`GzipHandler`].
+When `SizeLimitHandler` is before `GzipHandler` in the `Handler` tree, it will limit the compressed content; when it is after, it will limit the uncompressed content.
+
+The `Handler` tree structure look like the following, to limit uncompressed content:
+
+[source,screen]
+----
+Server
+└── GzipHandler
+    └── SizeLimitHandler
+        └── ContextHandlerCollection
+            ├── ContextHandler 1
+            :── ...
+            └── ContextHandler N
+----
+
 [[pg-server-http-handler-use-statistics]]
 ====== StatisticsHandler
 

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

@@ -21,27 +21,24 @@
 import org.eclipse.jetty.http.HttpFields;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpStatus;
-import org.eclipse.jetty.http.MetaData;
 import org.eclipse.jetty.io.Content;
+import org.eclipse.jetty.server.handler.gzip.GzipHandler;
 import org.eclipse.jetty.util.Callback;
 
 /**
- * A handler that can limit the size of message bodies in requests and responses.
- *
+ * <p>A {@link Handler} that can limit the size of message bodies in requests and responses.</p>
  * <p>The optional request and response limits are imposed by checking the {@code Content-Length}
- * header or observing the actual bytes seen by the handler. Handler order is important, in as much
- * as if this handler is before a the {@link org.eclipse.jetty.server.handler.gzip.GzipHandler},
- * then it will limit compressed sized, if it as after the {@link
- * org.eclipse.jetty.server.handler.gzip.GzipHandler} then the limit is applied to uncompressed
- * bytes. If a size limit is exceeded then {@link BadMessageException} is thrown with a {@link
- * org.eclipse.jetty.http.HttpStatus#PAYLOAD_TOO_LARGE_413} status.
+ * header or observing the actual bytes seen by this Handler.</p>
+ * <p>Handler order is important; for example, if this handler is before the {@link GzipHandler},
+ * then it will limit compressed sizes, if it as after the {@link GzipHandler} then it will limit
+ * uncompressed sizes.</p>
+ * <p>If a size limit is exceeded then {@link BadMessageException} is thrown with a
+ * {@link HttpStatus#PAYLOAD_TOO_LARGE_413} status.</p>
  */
 public class SizeLimitHandler extends Handler.Wrapper
 {
     private final long _requestLimit;
     private final long _responseLimit;
-    private long _read = 0;
-    private long _written = 0;
 
     /**
      * @param requestLimit The request body size limit in bytes or -1 for no limit
@@ -68,76 +65,92 @@ public boolean handle(Request request, Response response, Callback callback) thr
             }
         }
 
-        HttpFields.Mutable.Wrapper httpFields = new HttpFields.Mutable.Wrapper(response.getHeaders())
+        SizeLimitRequestWrapper wrappedRequest = new SizeLimitRequestWrapper(request);
+        SizeLimitResponseWrapper wrappedResponse = new SizeLimitResponseWrapper(wrappedRequest, response);
+        return super.handle(wrappedRequest, wrappedResponse, callback);
+    }
+
+    private class SizeLimitRequestWrapper extends Request.Wrapper
+    {
+        private long _read = 0;
+
+        public SizeLimitRequestWrapper(Request wrapped)
         {
-            @Override
-            public HttpField onAddField(HttpField field)
+            super(wrapped);
+        }
+
+        @Override
+        public Content.Chunk read()
+        {
+            Content.Chunk chunk = super.read();
+            if (chunk == null)
+                return null;
+            if (chunk.getFailure() != null)
+                return chunk;
+
+            // Check request content limit.
+            ByteBuffer content = chunk.getByteBuffer();
+            if (content != null && content.remaining() > 0)
             {
-                if (field.getHeader().is(HttpHeader.CONTENT_LENGTH.asString()))
+                _read += content.remaining();
+                if (_requestLimit >= 0 && _read > _requestLimit)
                 {
-                    long contentLength = field.getLongValue();
-                    if (_responseLimit >= 0 && contentLength > _responseLimit)
-                        throw new HttpException.RuntimeException(HttpStatus.INTERNAL_SERVER_ERROR_500, "Response body is too large: " + contentLength + ">" + _responseLimit);
+                    BadMessageException e = new BadMessageException(HttpStatus.PAYLOAD_TOO_LARGE_413, "Request body is too large: " + _read + ">" + _requestLimit);
+                    getWrapped().fail(e);
+                    return null;
                 }
-                return super.onAddField(field);
             }
-        };
 
-        response = new Response.Wrapper(request, response)
-        {
-            @Override
-            public HttpFields.Mutable getHeaders()
-            {
-                return httpFields;
-            }
-        };
+            return chunk;
+        }
+    }
 
-        request.addHttpStreamWrapper(httpStream -> new HttpStream.Wrapper(httpStream)
+    private class SizeLimitResponseWrapper extends Response.Wrapper
+    {
+        private final HttpFields.Mutable _httpFields;
+        private long _written = 0;
+
+        public SizeLimitResponseWrapper(Request request, Response wrapped)
         {
-            @Override
-            public Content.Chunk read()
-            {
-                Content.Chunk chunk = super.read();
-                if (chunk == null)
-                    return null;
-                if (chunk.getFailure() != null)
-                    return chunk;
+            super(request, wrapped);
 
-                // Check request content limit.
-                ByteBuffer content = chunk.getByteBuffer();
-                if (content != null && content.remaining() > 0)
+            _httpFields = new HttpFields.Mutable.Wrapper(wrapped.getHeaders())
+            {
+                @Override
+                public HttpField onAddField(HttpField field)
                 {
-                    _read += content.remaining();
-                    if (_requestLimit >= 0 && _read > _requestLimit)
+                    if (field.getHeader().is(HttpHeader.CONTENT_LENGTH.asString()))
                     {
-                        BadMessageException e = new BadMessageException(HttpStatus.PAYLOAD_TOO_LARGE_413, "Request body is too large: " + _read + ">" + _requestLimit);
-                        request.fail(e);
-                        return null;
+                        long contentLength = field.getLongValue();
+                        if (_responseLimit >= 0 && contentLength > _responseLimit)
+                            throw new HttpException.RuntimeException(HttpStatus.INTERNAL_SERVER_ERROR_500, "Response body is too large: " + contentLength + ">" + _responseLimit);
                     }
+                    return super.onAddField(field);
                 }
+            };
+        }
 
-                return chunk;
-            }
+        @Override
+        public HttpFields.Mutable getHeaders()
+        {
+            return _httpFields;
+        }
 
-            @Override
-            public void send(MetaData.Request request, MetaData.Response response, boolean last, ByteBuffer content, Callback callback)
+        @Override
+        public void write(boolean last, ByteBuffer content, Callback callback)
+        {
+            if (content != null && content.remaining() > 0)
             {
-                // Check response content limit.
-                if (content != null && content.remaining() > 0)
+                if (_responseLimit >= 0 && (_written + content.remaining())  > _responseLimit)
                 {
-                    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));
-                        return;
-                    }
-                    _written += content.remaining();
+                    callback.failed(new HttpException.RuntimeException(HttpStatus.INTERNAL_SERVER_ERROR_500, "Response body is too large: " +
+                        _written + content.remaining() + ">" + _responseLimit));
+                    return;
                 }
-
-                super.send(request, response, last, content, callback);
+                _written += content.remaining();
             }
-        });
 
-        return super.handle(request, response, callback);
+            super.write(last, content, callback);
+        }
     }
 }

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

@@ -267,4 +267,29 @@ public boolean handle(Request request, Response response, Callback callback) thr
             assertThat(response.getContent(), containsString(">8192"));
         }
     }
+
+    @Test
+    public void testMultipleRequests() throws Exception
+    {
+        String message = "x".repeat(1024);
+        _contextHandler.setHandler(new Handler.Abstract()
+        {
+            @Override
+            public boolean handle(Request request, Response response, Callback callback) throws Exception
+            {
+                response.write(true, BufferUtil.toBuffer(message), callback);
+                return true;
+            }
+        });
+
+        _server.start();
+
+        for (int i = 0; i < 1000; i++)
+        {
+            HttpTester.Response response = HttpTester.parseResponse(
+                _local.getResponse("GET /ctx/hello HTTP/1.0\r\n\r\n"));
+            assertThat(response.getStatus(), equalTo(200));
+            assertThat(response.getContent(), equalTo(message));
+        }
+    }
 }

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

@@ -15,12 +15,10 @@
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
-import java.io.EOFException;
 import java.io.IOException;
 import java.net.URI;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicInteger;
 import java.util.zip.GZIPInputStream;
 import java.util.zip.GZIPOutputStream;
 
@@ -31,23 +29,26 @@
 import org.eclipse.jetty.client.ContentResponse;
 import org.eclipse.jetty.client.HttpClient;
 import org.eclipse.jetty.client.Request;
+import org.eclipse.jetty.client.Result;
 import org.eclipse.jetty.client.StringRequestContent;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
 import org.eclipse.jetty.server.SizeLimitHandler;
 import org.eclipse.jetty.server.handler.gzip.GzipHandler;
+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;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
-import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.lessThan;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 public class SizeLimitHandlerServletTest
 {
@@ -137,21 +138,23 @@ public void testGzipEchoNoAcceptEncoding() throws Exception
         String content = "x".repeat(SIZE_LIMIT * 2);
         URI uri = URI.create("http://localhost:" + _connector.getLocalPort());
 
-        AtomicInteger contentReceived = new AtomicInteger();
-        CompletableFuture<Throwable> failure = new CompletableFuture<>();
+        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.addAndGet(chunk.getByteBuffer().remaining());
-                chunk.release();
+                contentReceived.append(BufferUtil.toString(chunk.getByteBuffer()));
                 demander.run();
-            }).send(result -> failure.complete(result.getFailure()));
+            })
+            .send(resultFuture::complete);
+
 
-        Throwable exception = failure.get(5, TimeUnit.SECONDS);
-        assertThat(exception, instanceOf(EOFException.class));
-        assertThat(contentReceived.get(), lessThan(SIZE_LIMIT));
+        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"));
     }
 
     public static Request.Content gzipContent(String content) throws Exception