From cd47a07463a656b685d5fb2bf2e99fdde6d11352 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Sat, 4 Feb 2023 00:48:12 +1100 Subject: [PATCH] use non-pooling RetainableByteBufferPool to work around performance bug Signed-off-by: Lachlan Roberts --- .../main/java/org/eclipse/jetty/http/MultiPart.java | 7 ++++--- .../ee10/servlet/ServletMultiPartFormData.java | 2 +- .../jetty/ee10/servlet/MultiPartServletTest.java | 13 ++++++++++--- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java index 30d442c0b3b..6ef5b7d9243 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java @@ -125,7 +125,7 @@ public class MultiPart private final String name; private final String fileName; private final HttpFields fields; - private final Content.Source contentSource; + private Content.Source contentSource; private Path path; private boolean temporary = true; @@ -140,7 +140,6 @@ public class MultiPart this.fileName = fileName; this.fields = fields != null ? fields : HttpFields.EMPTY; this.path = path; - this.contentSource = newContentSource(); } private Path getPath() @@ -189,6 +188,8 @@ public class MultiPart */ public Content.Source getContentSource() { + if (contentSource == null) + contentSource = newContentSource(); return contentSource; } @@ -350,8 +351,8 @@ public class MultiPart public ChunksPart(String name, String fileName, HttpFields fields, List content) { super(name, fileName, fields); + this.content = Objects.requireNonNull(content); content.forEach(Content.Chunk::retain); - this.content = content; } @Override diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletMultiPartFormData.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletMultiPartFormData.java index 55087110208..7301b4f1417 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletMultiPartFormData.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletMultiPartFormData.java @@ -138,7 +138,7 @@ public class ServletMultiPartFormData } } - formData.parse(Content.Chunk.from(buffer, false, byteBuffer -> retainable.release())); + formData.parse(Content.Chunk.from(buffer, false, retainable::release)); if (readEof) { formData.parse(Content.Chunk.EOF); diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/MultiPartServletTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/MultiPartServletTest.java index 5bbccf73da8..20d71007c4f 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/MultiPartServletTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/MultiPartServletTest.java @@ -50,6 +50,7 @@ import org.eclipse.jetty.http.MultiPart; import org.eclipse.jetty.http.MultiPartFormData; import org.eclipse.jetty.io.Content; import org.eclipse.jetty.io.EofException; +import org.eclipse.jetty.io.RetainableByteBufferPool; import org.eclipse.jetty.logging.StacklessLogging; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; @@ -96,7 +97,12 @@ public class MultiPartServletTest private void start(HttpServlet servlet, MultipartConfigElement config) throws Exception { - server = new Server(); + start(servlet, config, null); + } + + private void start(HttpServlet servlet, MultipartConfigElement config, RetainableByteBufferPool bufferPool) throws Exception + { + server = new Server(null, null, bufferPool); connector = new ServerConnector(server); server.addConnector(connector); @@ -128,6 +134,7 @@ public class MultiPartServletTest @Test public void testLargePart() throws Exception { + RetainableByteBufferPool bufferPool = new RetainableByteBufferPool.NonPooling(); start(new HttpServlet() { @Override @@ -135,7 +142,7 @@ public class MultiPartServletTest { req.getParameterMap(); } - }, new MultipartConfigElement(tmpDirString)); + }, new MultipartConfigElement(tmpDirString), bufferPool); OutputStreamRequestContent content = new OutputStreamRequestContent(); MultiPartRequestContent multiPart = new MultiPartRequestContent(); @@ -159,7 +166,7 @@ public class MultiPartServletTest } content.close(); - Response response = listener.get(2, TimeUnit.MINUTES); + Response response = listener.get(30, TimeUnit.MINUTES); assertThat(response.getStatus(), equalTo(HttpStatus.BAD_REQUEST_400)); String responseContent = IO.toString(listener.getInputStream()); assertThat(responseContent, containsString("Unable to parse form content"));