From 4a35731fb047a03e0cfbfb6a3ec6f31f54fac1c3 Mon Sep 17 00:00:00 2001
From: gregw A {@link CompletableFuture} that is completed when a multipart/form-data content
- * has been parsed asynchronously from a {@link Content.Source} via {@link #parse(Content.Source)}
- * or from one or more {@link Content.Chunk}s via {@link #parse(Content.Chunk)}.
Once the parsing of the multipart/form-data content completes successfully, * objects of this class are completed with a {@link Parts} object.
*Objects of this class may be configured to save multipart files in a configurable
@@ -68,7 +67,7 @@
*
* @see Parts
*/
-public class MultiPartFormData extends CompletableFuture
Returns this {@code MultiPartFormData} object, - * so that it can be used in the typical "fluent" - * style of {@link CompletableFuture}.
- * - * @param content the multipart/form-data content to parse - * @return this {@code MultiPartFormData} object - */ - public MultiPartFormData parse(Content.Source content) - { - new Runnable() - { - @Override - public void run() - { - while (true) - { - Content.Chunk chunk = content.read(); - if (chunk == null) - { - content.demand(this); - return; - } - if (Content.Chunk.isFailure(chunk)) - { - listener.onFailure(chunk.getFailure()); - return; - } - parse(chunk); - chunk.release(); - if (isDone()) - return; - if (chunk.isLast()) - { - listener.onFailure(new EOFException()); - return; - } - } - } - }.run(); - return this; - } - /** *Parses the given chunk containing multipart/form-data bytes.
*One or more chunks may be passed to this method, until the parsing
@@ -147,16 +104,17 @@ public void run()
*
* @param chunk the {@link Content.Chunk} to parse.
*/
- public void parse(Content.Chunk chunk)
+ @Override
+ protected MultiPartFormData.Parts parse(Content.Chunk chunk)
{
if (listener.isFailed())
- return;
+ return null;
length += chunk.getByteBuffer().remaining();
long max = getMaxLength();
if (max > 0 && length > max)
- listener.onFailure(new IllegalStateException("max length exceeded: %d".formatted(max)));
- else
- parser.parse(chunk);
+ throw new IllegalStateException("max length exceeded: %d".formatted(max));
+ parser.parse(chunk);
+ return parts;
}
/**
@@ -550,8 +508,8 @@ public void onComplete()
try (AutoLock ignored = lock.lock())
{
result = List.copyOf(parts);
+ MultiPartFormData.this.parts = new Parts(result);
}
- complete(new Parts(result));
}
Charset getDefaultCharset()
@@ -579,7 +537,7 @@ int getPartsSize()
public void onFailure(Throwable failure)
{
super.onFailure(failure);
- completeExceptionally(failure);
+ completeExceptionally(failure); // TODO not here
}
private void fail(Throwable cause)
diff --git a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartFormDataTest.java b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartFormDataTest.java
index a1f771f1ca28..3fa1ffa2ec07 100644
--- a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartFormDataTest.java
+++ b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartFormDataTest.java
@@ -16,7 +16,6 @@
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.charset.Charset;
-import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
@@ -26,9 +25,10 @@
import java.util.concurrent.atomic.AtomicInteger;
import org.eclipse.jetty.io.Content;
+import org.eclipse.jetty.io.content.AsyncContent;
import org.eclipse.jetty.toolchain.test.FS;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
-import org.eclipse.jetty.util.BufferUtil;
+import org.eclipse.jetty.util.Callback;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
@@ -71,32 +71,19 @@ public void dispose()
int leaks = 0;
for (Content.Chunk chunk : _allocatedChunks)
{
- // Any release that does not return true is a leak.
- if (!chunk.release())
- leaks++;
+ // Any release that does not throw or return true is a leak.
+ try
+ {
+ if (!chunk.release())
+ leaks++;
+ }
+ catch (IllegalStateException ignored)
+ {
+ }
}
assertThat("Leaked " + leaks + "/" + _allocatedChunks.size() + " chunk(s)", leaks, is(0));
}
- Content.Chunk asChunk(String data, boolean last)
- {
- byte[] b = data.getBytes(StandardCharsets.UTF_8);
- ByteBuffer buffer = BufferUtil.allocate(b.length);
- BufferUtil.append(buffer, b);
- Content.Chunk chunk = Content.Chunk.from(buffer, last);
- _allocatedChunks.add(chunk);
- return chunk;
- }
-
- Content.Chunk asChunk(ByteBuffer data, boolean last)
- {
- ByteBuffer buffer = BufferUtil.allocate(data.remaining());
- BufferUtil.append(buffer, data);
- Content.Chunk chunk = Content.Chunk.from(buffer, last);
- _allocatedChunks.add(chunk);
- return chunk;
- }
-
@Test
public void testBadMultiPart() throws Exception
{
@@ -109,12 +96,14 @@ public void testBadMultiPart() throws Exception
"Content-Disposition: form-data; name=\"fileup\"; filename=\"test.upload\"\r\n" +
"\r\n";
- MultiPartFormData formData = new MultiPartFormData(boundary);
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, boundary);
formData.setFilesDirectory(_tmpDir);
formData.setMaxFileSize(1024);
formData.setMaxLength(3072);
formData.setMaxMemoryFileSize(50);
- formData.parse(asChunk(str, true));
+ Content.Sink.write(source, true, str, Callback.NOOP);
+ formData.parse();
formData.handle((parts, failure) ->
{
@@ -139,12 +128,14 @@ public void testFinalBoundaryOnly() throws Exception
eol +
"--" + boundary + "--" + eol;
- MultiPartFormData formData = new MultiPartFormData(boundary);
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, boundary);
formData.setFilesDirectory(_tmpDir);
formData.setMaxFileSize(1024);
formData.setMaxLength(3072);
formData.setMaxMemoryFileSize(50);
- formData.parse(asChunk(str, true));
+ Content.Sink.write(source, true, str, Callback.NOOP);
+ formData.parse();
formData.whenComplete((parts, failure) ->
{
@@ -165,12 +156,14 @@ public void testEmpty() throws Exception
String str = eol +
"--" + boundary + "--" + eol;
- MultiPartFormData formData = new MultiPartFormData(boundary);
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, boundary);
formData.setFilesDirectory(_tmpDir);
formData.setMaxFileSize(1024);
formData.setMaxLength(3072);
formData.setMaxMemoryFileSize(50);
- formData.parse(asChunk(str, true));
+ Content.Sink.write(source, true, str, Callback.NOOP);
+ formData.parse();
formData.whenComplete((parts, failure) ->
{
@@ -213,12 +206,14 @@ public void testEmptyStringBoundary() throws Exception
----\r
""";
- MultiPartFormData formData = new MultiPartFormData("");
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, "");
formData.setFilesDirectory(_tmpDir);
formData.setMaxFileSize(1024);
formData.setMaxLength(3072);
formData.setMaxMemoryFileSize(50);
- formData.parse(asChunk(str, true));
+ Content.Sink.write(source, true, str, Callback.NOOP);
+ formData.parse();
try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS))
{
@@ -253,8 +248,10 @@ public void testEmptyStringBoundary() throws Exception
@Test
public void testNoBody() throws Exception
{
- MultiPartFormData formData = new MultiPartFormData("boundary");
- formData.parse(Content.Chunk.EOF);
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, "boundary");
+ source.close();
+ formData.parse();
formData.handle((parts, failure) ->
{
@@ -268,9 +265,11 @@ public void testNoBody() throws Exception
@Test
public void testBodyWithOnlyCRLF() throws Exception
{
- MultiPartFormData formData = new MultiPartFormData("boundary");
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, "boundary");
String body = " \n\n\n\r\n\r\n\r\n\r\n";
- formData.parse(asChunk(body, true));
+ Content.Sink.write(source, true, body, Callback.NOOP);
+ formData.parse();
formData.handle((parts, failure) ->
{
@@ -285,7 +284,7 @@ public void testBodyWithOnlyCRLF() throws Exception
public void testLeadingWhitespaceBodyWithCRLF() throws Exception
{
String body = """
-
+
\r
\r
@@ -303,12 +302,14 @@ public void testLeadingWhitespaceBodyWithCRLF() throws Exception
--AaB03x--\r
""";
- MultiPartFormData formData = new MultiPartFormData("AaB03x");
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, "AaB03x");
formData.setFilesDirectory(_tmpDir);
formData.setMaxFileSize(1024);
formData.setMaxLength(3072);
formData.setMaxMemoryFileSize(50);
- formData.parse(asChunk(body, true));
+ Content.Sink.write(source, true, body, Callback.NOOP);
+ formData.parse();
try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS))
{
@@ -340,12 +341,14 @@ public void testLeadingWhitespaceBodyWithoutCRLF() throws Exception
--AaB03x--\r
""";
- MultiPartFormData formData = new MultiPartFormData("AaB03x");
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, "AaB03x");
formData.setFilesDirectory(_tmpDir);
formData.setMaxFileSize(1024);
formData.setMaxLength(3072);
formData.setMaxMemoryFileSize(50);
- formData.parse(asChunk(body, true));
+ Content.Sink.write(source, true, body, Callback.NOOP);
+ formData.parse();
try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS))
{
@@ -361,7 +364,8 @@ public void testLeadingWhitespaceBodyWithoutCRLF() throws Exception
@Test
public void testDefaultLimits() throws Exception
{
- MultiPartFormData formData = new MultiPartFormData("AaB03x");
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, "AaB03x");
formData.setFilesDirectory(_tmpDir);
String body = """
--AaB03x\r
@@ -371,7 +375,8 @@ public void testDefaultLimits() throws Exception
ABCDEFGHIJKLMNOPQRSTUVWXYZ\r
--AaB03x--\r
""";
- formData.parse(asChunk(body, true));
+ Content.Sink.write(source, true, body, Callback.NOOP);
+ formData.parse();
try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS))
{
@@ -390,7 +395,8 @@ public void testDefaultLimits() throws Exception
@Test
public void testRequestContentTooBig() throws Exception
{
- MultiPartFormData formData = new MultiPartFormData("AaB03x");
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, "AaB03x");
formData.setFilesDirectory(_tmpDir);
formData.setMaxLength(16);
@@ -402,7 +408,8 @@ public void testRequestContentTooBig() throws Exception
ABCDEFGHIJKLMNOPQRSTUVWXYZ\r
--AaB03x--\r
""";
- formData.parse(asChunk(body, true));
+ Content.Sink.write(source, true, body, Callback.NOOP);
+ formData.parse();
formData.handle((parts, failure) ->
{
@@ -416,7 +423,8 @@ public void testRequestContentTooBig() throws Exception
@Test
public void testFileTooBig() throws Exception
{
- MultiPartFormData formData = new MultiPartFormData("AaB03x");
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, "AaB03x");
formData.setFilesDirectory(_tmpDir);
formData.setMaxFileSize(16);
@@ -428,7 +436,8 @@ public void testFileTooBig() throws Exception
ABCDEFGHIJKLMNOPQRSTUVWXYZ\r
--AaB03x--\r
""";
- formData.parse(asChunk(body, true));
+ Content.Sink.write(source, true, body, Callback.NOOP);
+ formData.parse();
formData.handle((parts, failure) ->
{
@@ -442,7 +451,8 @@ public void testFileTooBig() throws Exception
@Test
public void testTwoFilesOneInMemoryOneOnDisk() throws Exception
{
- MultiPartFormData formData = new MultiPartFormData("AaB03x");
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, "AaB03x");
formData.setFilesDirectory(_tmpDir);
String chunk = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
formData.setMaxMemoryFileSize(chunk.length() + 1);
@@ -460,7 +470,8 @@ public void testTwoFilesOneInMemoryOneOnDisk() throws Exception
$C$C$C$C\r
--AaB03x--\r
""".replace("$C", chunk);
- formData.parse(asChunk(body, true));
+ Content.Sink.write(source, true, body, Callback.NOOP);
+ formData.parse();
try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS))
{
@@ -482,7 +493,8 @@ public void testTwoFilesOneInMemoryOneOnDisk() throws Exception
@Test
public void testPartWrite() throws Exception
{
- MultiPartFormData formData = new MultiPartFormData("AaB03x");
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, "AaB03x");
formData.setFilesDirectory(_tmpDir);
String chunk = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
formData.setMaxMemoryFileSize(chunk.length() + 1);
@@ -500,7 +512,8 @@ public void testPartWrite() throws Exception
$C$C$C$C\r
--AaB03x--\r
""".replace("$C", chunk);
- formData.parse(asChunk(body, true));
+ Content.Sink.write(source, true, body, Callback.NOOP);
+ formData.parse();
try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS))
{
@@ -528,7 +541,8 @@ public void testPartWrite() throws Exception
@Test
public void testPathPartDelete() throws Exception
{
- MultiPartFormData formData = new MultiPartFormData("AaB03x");
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, "AaB03x");
formData.setFilesDirectory(_tmpDir);
String body = """
@@ -539,7 +553,8 @@ public void testPathPartDelete() throws Exception
ABCDEFGHIJKLMNOPQRSTUVWXYZ\r
--AaB03x--\r
""";
- formData.parse(asChunk(body, true));
+ Content.Sink.write(source, true, body, Callback.NOOP);
+ formData.parse();
try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS))
{
@@ -559,7 +574,8 @@ public void testPathPartDelete() throws Exception
@Test
public void testAbort()
{
- MultiPartFormData formData = new MultiPartFormData("AaB03x");
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, "AaB03x");
formData.setFilesDirectory(_tmpDir);
formData.setMaxMemoryFileSize(32);
@@ -575,14 +591,15 @@ public void testAbort()
--AaB03x--\r
""";
// Parse only part of the content.
- formData.parse(asChunk(body, false));
+ Content.Sink.write(source, false, body, Callback.NOOP);
+ formData.parse();
assertEquals(1, formData.getPartsSize());
// Abort MultiPartFormData.
formData.completeExceptionally(new IOException());
// Parse the rest of the content.
- formData.parse(asChunk(terminator, true));
+ Content.Sink.write(source, true, terminator, Callback.NOOP);
// Try to get the parts, it should fail.
assertThrows(ExecutionException.class, () -> formData.get(5, TimeUnit.SECONDS));
@@ -592,7 +609,8 @@ public void testAbort()
@Test
public void testMaxHeaderLength() throws Exception
{
- MultiPartFormData formData = new MultiPartFormData("AaB03x");
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, "AaB03x");
formData.setFilesDirectory(_tmpDir);
formData.setPartHeadersMaxLength(32);
@@ -604,7 +622,8 @@ public void testMaxHeaderLength() throws Exception
ABCDEFGHIJKLMNOPQRSTUVWXYZ\r
--AaB03x--\r
""";
- formData.parse(asChunk(body, true));
+ formData.parse();
+ Content.Sink.write(source, true, body, Callback.NOOP);
formData.handle((parts, failure) ->
{
@@ -618,7 +637,8 @@ public void testMaxHeaderLength() throws Exception
@Test
public void testDefaultCharset() throws Exception
{
- MultiPartFormData formData = new MultiPartFormData("AaB03x");
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, "AaB03x");
formData.setFilesDirectory(_tmpDir);
formData.setMaxMemoryFileSize(-1);
@@ -645,11 +665,12 @@ public void testDefaultCharset() throws Exception
\r
--AaB03x--\r
""";
- formData.parse(asChunk(body1, false));
- formData.parse(asChunk(isoCedilla, false));
- formData.parse(asChunk(body2, false));
- formData.parse(asChunk(utfCedilla, false));
- formData.parse(asChunk(terminator, true));
+ formData.parse();
+ Content.Sink.write(source, false, body1, Callback.NOOP);
+ source.write(false, isoCedilla, Callback.NOOP);
+ Content.Sink.write(source, false, body2, Callback.NOOP);
+ source.write(false, utfCedilla, Callback.NOOP);
+ Content.Sink.write(source, true, terminator, Callback.NOOP);
try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS))
{
@@ -669,7 +690,8 @@ public void testDefaultCharset() throws Exception
@Test
public void testPartWithBackSlashInFileName() throws Exception
{
- MultiPartFormData formData = new MultiPartFormData("AaB03x");
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, "AaB03x");
formData.setFilesDirectory(_tmpDir);
formData.setMaxMemoryFileSize(-1);
@@ -681,7 +703,8 @@ public void testPartWithBackSlashInFileName() throws Exception
stuffaaa\r
--AaB03x--\r
""";
- formData.parse(asChunk(contents, true));
+ formData.parse();
+ Content.Sink.write(source, true, contents, Callback.NOOP);
try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS))
{
@@ -694,7 +717,8 @@ public void testPartWithBackSlashInFileName() throws Exception
@Test
public void testPartWithWindowsFileName() throws Exception
{
- MultiPartFormData formData = new MultiPartFormData("AaB03x");
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, "AaB03x");
formData.setFilesDirectory(_tmpDir);
formData.setMaxMemoryFileSize(-1);
@@ -706,7 +730,8 @@ public void testPartWithWindowsFileName() throws Exception
stuffaaa\r
--AaB03x--\r
""";
- formData.parse(asChunk(contents, true));
+ Content.Sink.write(source, true, contents, Callback.NOOP);
+ formData.parse();
try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS))
{
@@ -722,7 +747,8 @@ public void testPartWithWindowsFileName() throws Exception
@Disabled
public void testCorrectlyEncodedMSFilename() throws Exception
{
- MultiPartFormData formData = new MultiPartFormData("AaB03x");
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, "AaB03x");
formData.setFilesDirectory(_tmpDir);
formData.setMaxMemoryFileSize(-1);
@@ -734,7 +760,8 @@ public void testCorrectlyEncodedMSFilename() throws Exception
stuffaaa\r
--AaB03x--\r
""";
- formData.parse(asChunk(contents, true));
+ Content.Sink.write(source, true, contents, Callback.NOOP);
+ formData.parse();
try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS))
{
@@ -747,7 +774,8 @@ public void testCorrectlyEncodedMSFilename() throws Exception
@Test
public void testWriteFilesForPartWithoutFileName() throws Exception
{
- MultiPartFormData formData = new MultiPartFormData("AaB03x");
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, "AaB03x");
formData.setFilesDirectory(_tmpDir);
formData.setUseFilesForPartsWithoutFileName(true);
@@ -759,7 +787,8 @@ public void testWriteFilesForPartWithoutFileName() throws Exception
sssaaa\r
--AaB03x--\r
""";
- formData.parse(asChunk(body, true));
+ Content.Sink.write(source, true, body, Callback.NOOP);
+ formData.parse();
try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS))
{
@@ -775,7 +804,8 @@ public void testWriteFilesForPartWithoutFileName() throws Exception
@Test
public void testPartsWithSameName() throws Exception
{
- MultiPartFormData formData = new MultiPartFormData("AaB03x");
+ AsyncContent source = new TestContent();
+ MultiPartFormData formData = new MultiPartFormData(source, "AaB03x");
formData.setFilesDirectory(_tmpDir);
String sameNames = """
@@ -791,7 +821,8 @@ public void testPartsWithSameName() throws Exception
AAAAA\r
--AaB03x--\r
""";
- formData.parse(asChunk(sameNames, true));
+ Content.Sink.write(source, true, sameNames, Callback.NOOP);
+ formData.parse();
try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS))
{
@@ -810,4 +841,14 @@ public void testPartsWithSameName() throws Exception
assertEquals("AAAAA", part2.getContentAsString(formData.getDefaultCharset()));
}
}
+
+ private class TestContent extends AsyncContent
+ {
+ @Override
+ protected void offer(Content.Chunk chunk)
+ {
+ _allocatedChunks.add(chunk);
+ super.offer(chunk);
+ }
+ }
}
diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/AsyncContent.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/AsyncContent.java
index 6bd5eeebc32a..9ab80e4282cf 100644
--- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/AsyncContent.java
+++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/AsyncContent.java
@@ -79,7 +79,7 @@ public void write(boolean last, ByteBuffer byteBuffer, Callback callback)
* or succeeded if and only if the chunk is terminal, as non-terminal
* chunks have to bind the succeeding of the callback to their release.
*/
- private void offer(Content.Chunk chunk)
+ protected void offer(Content.Chunk chunk)
{
Throwable failure = null;
boolean wasEmpty = false;
diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceCompletableFuture.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceCompletableFuture.java
index 3fa4e7061a73..55c91c282cd9 100644
--- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceCompletableFuture.java
+++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceCompletableFuture.java
@@ -30,10 +30,10 @@ public ContentSourceCompletableFuture(Content.Source content)
_content = content;
}
- public boolean parse()
+ public CompletableFuture Parses the given chunk containing multipart/form-data bytes. One or more chunks may be passed to this method, until the parsing
- * of the multipart/form-data content completes. Returns the default charset as specified by
- * RFC 7578, section 4.6,
- * that is the charset specified by the part named {@code _charset_}. If that part is not present, returns {@code null}. Sets the directory where the files uploaded in the parts will be saved. Sets the maximum memory file size in bytes, after which files will be saved
- * in the directory specified by {@link #setFilesDirectory(Path)}. Use value {@code 0} to always save the files in the directory. Use value {@code -1} to never save the files in the directory. An ordered list of {@link MultiPart.Part}s that can
* be accessed by index or by name, or iterated over. Returns the {@link MultiPart.Part} at the given index, a number
* between {@code 0} included and the value returned by {@link #size()}
@@ -373,251 +182,445 @@ protected HttpFields customizePartHeaders(MultiPart.Part part)
}
}
- private class PartsListener extends MultiPart.AbstractPartsListener
+ public static class Parser
{
- private final AutoLock lock = new AutoLock();
- private final List Returns the default charset as specified by
+ * RFC 7578, section 4.6,
+ * that is the charset specified by the part named {@code _charset_}. If that part is not present, returns {@code null}. Sets the directory where the files uploaded in the parts will be saved. Sets the maximum memory file size in bytes, after which files will be saved
+ * in the directory specified by {@link #setFilesDirectory(Path)}. Use value {@code 0} to always save the files in the directory. Use value {@code -1} to never save the files in the directory. A {@link CompletableFuture} that is completed when a multipart/byteranges
- * content has been parsed asynchronously from a {@link Content.Source} via
- * {@link #parse(Content.Source)}. Once the parsing of the multipart/byteranges content completes successfully,
* objects of this class are completed with a {@link MultiPartByteRanges.Parts}
* object. Parses the given multipart/byteranges content. Returns this {@code MultiPartByteRanges} object,
- * so that it can be used in the typical "fluent" style
- * of {@link CompletableFuture}.
+ * An example usage to asynchronously read UTF-8 content is:
+ * An ordered list of {@link MultiPart.Part}s that can
* be accessed by index or by name, or iterated over. Servlet specific class for multipart content support. Use {@link #from(ServletApiRequest)} to
+ * Use {@link #from(ServletRequest)} to
* parse multipart request content into a {@link Parts} object that can
* be used to access Servlet {@link Part} objects. Parses the request content assuming it is a multipart content,
- * and returns a {@link Parts} objects that can be used to access
- * individual {@link Part}s. Parses the request content assuming it is a multipart content,
- * and returns a {@link Parts} objects that can be used to access
- * individual {@link Part}s.
+ * public static class FutureUtf8String extends ContentSourceCompletableFuture
*/
public abstract class ContentSourceCompletableFuture
- * public static class FutureUtf8String extends ContentSourceCompletableFuture+ * public static class FutureUtf8String extends ContentSourceCompletableFuture<String> * { * Utf8StringBuilder builder = new Utf8StringBuilder(); * @@ -34,6 +34,7 @@ * super(content); * } * + * @Override * protected String parse(Content.Chunk chunk) throws Throwable * { * if (chunk.hasRemaining()) diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java index 03740552f2fa..9dbf56b12659 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java @@ -675,6 +675,7 @@ public Object getAttribute(String name) if (_async != null) { // This switch works by allowing the attribute to get underneath any dispatch wrapper. + // Note that there are further servlet specific attributes in ServletContextRequest return switch (name) { case AsyncContext.ASYNC_REQUEST_URI -> getRequestURI(); diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java index b09fc4106511..6b414768c97c 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java @@ -239,6 +239,8 @@ public Object getAttribute(String name) case "jakarta.servlet.request.key_size" -> super.getAttribute(SecureRequestCustomizer.KEY_SIZE_ATTRIBUTE); case "jakarta.servlet.request.ssl_session_id" -> super.getAttribute(SecureRequestCustomizer.SSL_SESSION_ID_ATTRIBUTE); case "jakarta.servlet.request.X509Certificate" -> super.getAttribute(SecureRequestCustomizer.PEER_CERTIFICATES_ATTRIBUTE); + case ServletContextRequest.MULTIPART_CONFIG_ELEMENT -> _matchedResource.getResource().getServletHolder().getMultipartConfig(); + default -> super.getAttribute(name); }; } @@ -255,6 +257,8 @@ public Set getAttributeNameSet() names.add("jakarta.servlet.request.ssl_session_id"); if (names.contains(SecureRequestCustomizer.PEER_CERTIFICATES_ATTRIBUTE)) names.add("jakarta.servlet.request.X509Certificate"); + if (_matchedResource.getResource().getServletHolder().getMultipartConfig() != null) + names.add(ServletContextRequest.MULTIPART_CONFIG_ELEMENT); return names; } diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHolder.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHolder.java index a3079dc50cc3..2404db1c6425 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHolder.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHolder.java @@ -74,7 +74,7 @@ public class ServletHolder extends Holder implements Comparable _roleMap; private String _forcedPath; private String _runAsRole; - private ServletRegistration.Dynamic _registration; + private ServletHolder.Registration _registration; private JspContainer _jspContainer; private volatile Servlet _servlet; @@ -155,6 +155,11 @@ public ServletHolder(Class extends Servlet> servlet) setHeldClass(servlet); } + public Object getMultipartConfig() + { + return _registration == null ? null : _registration.getMultipartConfig(); + } + /** * @return The unavailable exception or null if not unavailable */ @@ -710,14 +715,6 @@ protected void prepare(ServletRequest request, ServletResponse response) throws { // Ensure the servlet is initialized prior to any filters being invoked getServlet(); - - // Check for multipart config - if (_registration != null) - { - MultipartConfigElement mpce = ((Registration)_registration).getMultipartConfig(); - if (mpce != null) - request.setAttribute(ServletContextRequest.MULTIPART_CONFIG_ELEMENT, mpce); - } } /** @@ -1015,7 +1012,7 @@ public Set setServletSecurity(ServletSecurityElement securityElement) } } - public ServletRegistration.Dynamic getRegistration() + public ServletHolder.Registration getRegistration() { if (_registration == null) _registration = new Registration(); 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 1405f06b346c..a4a128d415b0 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 @@ -49,6 +49,7 @@ */ public class ServletMultiPartFormData { + /** * Get future {@link ServletMultiPartFormData.Parts} from a servlet request. * @param servletRequest A servlet request 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 e08cbd599ec2..a460f0b5a9a7 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 @@ -108,16 +108,17 @@ private void start(HttpServlet servlet, MultipartConfigElement config, ByteBuffe connector = new ServerConnector(server); server.addConnector(connector); - ServletContextHandler contextHandler = new ServletContextHandler("/"); + ServletContextHandler servletContextHandler = new ServletContextHandler("/"); ServletHolder servletHolder = new ServletHolder(servlet); servletHolder.getRegistration().setMultipartConfig(config); - contextHandler.addServlet(servletHolder, "/"); + servletContextHandler.addServlet(servletHolder, "/"); + server.setHandler(servletContextHandler); + GzipHandler gzipHandler = new GzipHandler(); gzipHandler.addIncludedMimeTypes("multipart/form-data"); gzipHandler.setMinGzipSize(32); - gzipHandler.setHandler(contextHandler); - server.setHandler(gzipHandler); + servletContextHandler.insertHandler(gzipHandler); server.start(); From 7b615e068c9d4bd24c32f087b88512828b088a34 Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 29 Jun 2023 18:40:29 +0200 Subject: [PATCH 12/19] Experiment with a fully async ContentSourceCompletableFuture Fixed parts echo test Tested with simple preload handler --- .../servlet/ServletMultiPartFormData.java | 1 - .../ee10/servlet/MultiPartServletTest.java | 149 ++++++++++++------ 2 files changed, 102 insertions(+), 48 deletions(-) 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 a4a128d415b0..1405f06b346c 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 @@ -49,7 +49,6 @@ */ public class ServletMultiPartFormData { - /** * Get future {@link ServletMultiPartFormData.Parts} from a servlet request. * @param servletRequest A servlet request 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 a460f0b5a9a7..953bb4a98062 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 @@ -22,6 +22,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Map; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -42,26 +43,32 @@ import org.eclipse.jetty.client.OutputStreamRequestContent; import org.eclipse.jetty.client.Response; import org.eclipse.jetty.client.StringRequestContent; +import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpTester; +import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.http.MultiPart; import org.eclipse.jetty.http.MultiPartFormData; -import org.eclipse.jetty.io.ByteBufferPool; +import org.eclipse.jetty.io.Content; import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.io.content.InputStreamContentSource; import org.eclipse.jetty.logging.StacklessLogging; +import org.eclipse.jetty.server.Handler; +import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.handler.gzip.GzipHandler; +import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.component.LifeCycle; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import static java.nio.charset.StandardCharsets.UTF_8; import static org.hamcrest.MatcherAssert.assertThat; @@ -92,19 +99,10 @@ public void before() throws Exception tmpDirString = tmpDir.toAbsolutePath().toString(); } - private void start(HttpServlet servlet) throws Exception + private void start(HttpServlet servlet, MultipartConfigElement config, boolean preload) throws Exception { - start(servlet, new MultipartConfigElement(tmpDirString, MAX_FILE_SIZE, -1, 0)); - } - - private void start(HttpServlet servlet, MultipartConfigElement config) throws Exception - { - start(servlet, config, null); - } - - private void start(HttpServlet servlet, MultipartConfigElement config, ByteBufferPool bufferPool) throws Exception - { - server = new Server(null, null, bufferPool); + config = config == null ? new MultipartConfigElement(tmpDirString, MAX_FILE_SIZE, -1, 0) : config; + server = new Server(null, null, null); connector = new ServerConnector(server); server.addConnector(connector); @@ -114,10 +112,43 @@ private void start(HttpServlet servlet, MultipartConfigElement config, ByteBuffe servletContextHandler.addServlet(servletHolder, "/"); server.setHandler(servletContextHandler); - GzipHandler gzipHandler = new GzipHandler(); gzipHandler.addIncludedMimeTypes("multipart/form-data"); gzipHandler.setMinGzipSize(32); + + // User a very simple preload handler + if (preload) + { + gzipHandler.setHandler(new Handler.Wrapper() + { + @Override + public boolean handle(Request request, org.eclipse.jetty.server.Response response, Callback callback) throws Exception + { + String contentType = request.getHeaders().get(HttpHeader.CONTENT_TYPE); + if (contentType == null || !MimeTypes.Type.MULTIPART_FORM_DATA.is(HttpField.valueParameters(contentType, null))) + return super.handle(request, response, callback); + + CompletableFuture futureParts = ServletMultiPartFormData.from(Request.as(request, ServletContextRequest.class).getServletApiRequest()); + if (futureParts.isDone()) + return super.handle(request, response, callback); + + futureParts.whenComplete((parts, failure) -> + { + try + { + if (!super.handle(request, response, callback)) + callback.failed(new IllegalStateException("Not Handled")); + } + catch (Throwable x) + { + callback.failed(x); + } + }); + return true; + } + }); + } + servletContextHandler.insertHandler(gzipHandler); server.start(); @@ -134,17 +165,18 @@ public void stop() throws Exception IO.delete(tmpDir.toFile()); } - @Test - public void testLargePart() throws Exception + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testLargePart(boolean preload) throws Exception { start(new HttpServlet() { @Override - protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + protected void service(HttpServletRequest req, HttpServletResponse resp) { req.getParameterMap(); } - }, new MultipartConfigElement(tmpDirString)); + }, new MultipartConfigElement(tmpDirString), preload); OutputStreamRequestContent content = new OutputStreamRequestContent(); MultiPartRequestContent multiPart = new MultiPartRequestContent(); @@ -175,8 +207,9 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) throws }); } - @Test - public void testManyParts() throws Exception + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testManyParts(boolean preload) throws Exception { start(new HttpServlet() { @@ -185,7 +218,7 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) throws { req.getParameterMap(); } - }, new MultipartConfigElement(tmpDirString)); + }, new MultipartConfigElement(tmpDirString), preload); byte[] byteArray = new byte[1024]; Arrays.fill(byteArray, (byte)1); @@ -213,8 +246,9 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) throws }); } - @Test - public void testMaxRequestSize() throws Exception + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testMaxRequestSize(boolean preload) throws Exception { start(new HttpServlet() { @@ -223,7 +257,7 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) throws { req.getParameterMap(); } - }, new MultipartConfigElement(tmpDirString, -1, 1024, 1024 * 1024 * 8)); + }, new MultipartConfigElement(tmpDirString, -1, 1024, 1024 * 1024 * 8), preload); OutputStreamRequestContent content = new OutputStreamRequestContent(); MultiPartRequestContent multiPart = new MultiPartRequestContent(); @@ -282,13 +316,14 @@ private static void assert400orEof(InputStreamResponseListener listener, Consume checkbody.accept(responseContent); } - @Test - public void testSimpleMultiPart() throws Exception + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testSimpleMultiPart(boolean preload) throws Exception { start(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response1) throws ServletException, IOException { Collection parts = request.getParts(); assertNotNull(parts); @@ -298,10 +333,10 @@ protected void service(HttpServletRequest request, HttpServletResponse response) Collection headerNames = part.getHeaderNames(); assertNotNull(headerNames); assertEquals(2, headerNames.size()); - String content = IO.toString(part.getInputStream(), UTF_8); - assertEquals("content1", content); + String content1 = IO.toString(part.getInputStream(), UTF_8); + assertEquals("content1", content1); } - }); + }, null, preload); try (Socket socket = new Socket("localhost", connector.getLocalPort())) { @@ -333,21 +368,23 @@ protected void service(HttpServletRequest request, HttpServletResponse response) } } - @Test - public void testTempFilesDeletedOnError() throws Exception + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testTempFilesDeletedOnError(boolean preload) throws Exception { byte[] bytes = new byte[2 * MAX_FILE_SIZE]; Arrays.fill(bytes, (byte)1); + // Should throw as the max file size is exceeded. start(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response1) throws ServletException, IOException { // Should throw as the max file size is exceeded. request.getParts(); } - }); + }, null, preload); MultiPartRequestContent multiPart = new MultiPartRequestContent(); multiPart.addPart(new MultiPart.ContentSourcePart("largePart", "largeFile.bin", HttpFields.EMPTY, new BytesRequestContent(bytes))); @@ -370,18 +407,34 @@ protected void service(HttpServletRequest request, HttpServletResponse response) assertThat(fileList.length, is(0)); } - @Test - public void testMultiPartGzip() throws Exception + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testMultiPartGzip(boolean preload) throws Exception { start(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException + protected void service(HttpServletRequest request, HttpServletResponse response1) throws IOException, ServletException { - response.setContentType(request.getContentType()); - IO.copy(request.getInputStream(), response.getOutputStream()); + String contentType1 = request.getContentType(); + response1.setContentType(contentType1); + response1.flushBuffer(); + + MultiPartRequestContent echoParts = new MultiPartRequestContent(MultiPart.extractBoundary(contentType1)); + Collection servletParts = request.getParts(); + for (Part part : servletParts) + { + HttpFields.Mutable partHeaders = HttpFields.build(); + for (String h1 : part.getHeaderNames()) + partHeaders.add(h1, part.getHeader(h1)); + + echoParts.addPart(new MultiPart.ContentSourcePart(part.getName(), part.getSubmittedFileName(), partHeaders, new InputStreamContentSource(part.getInputStream()))); + } + echoParts.close(); + IO.copy(Content.Source.asInputStream(echoParts), response1.getOutputStream()); } - }); + }, null, preload); + // Do not automatically handle gzip. client.getContentDecoderFactories().clear(); @@ -418,8 +471,9 @@ protected void service(HttpServletRequest request, HttpServletResponse response) assertThat(parts.get(0).getContentAsString(UTF_8), is(contentString)); } - @Test - public void testDoubleReadFromPart() throws Exception + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testDoubleReadFromPart(boolean preload) throws Exception { start(new HttpServlet() { @@ -433,7 +487,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S resp.getWriter().println("Part: name=" + part.getName() + ", size=" + part.getSize() + ", content=" + IO.toString(part.getInputStream())); } } - }); + }, null, preload); String contentString = "the quick brown fox jumps over the lazy dog, " + "the quick brown fox jumps over the lazy dog"; @@ -453,8 +507,9 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S "Part: name=myPart, size=88, content=the quick brown fox jumps over the lazy dog, the quick brown fox jumps over the lazy dog")); } - @Test - public void testPartAsParameter() throws Exception + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testPartAsParameter(boolean preload) throws Exception { start(new HttpServlet() { @@ -469,7 +524,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S resp.getWriter().println("Parameter: " + entry.getKey() + "=" + entry.getValue()[0]); } } - }); + }, null, preload); String contentString = "the quick brown fox jumps over the lazy dog, " + "the quick brown fox jumps over the lazy dog"; From 5eaf3d7b96989e8b6bb3b7e1836ca798f5aeb391 Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 29 Jun 2023 23:04:30 +0200 Subject: [PATCH 13/19] Experiment with a fully async ContentSourceCompletableFuture removed usage of DelayedHandler --- .../handler/MultiPartFormDataHandlerTest.java | 69 ------------------- 1 file changed, 69 deletions(-) diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/MultiPartFormDataHandlerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/MultiPartFormDataHandlerTest.java index 0d1e657b76ba..e9087a0a8ad3 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/MultiPartFormDataHandlerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/MultiPartFormDataHandlerTest.java @@ -17,8 +17,6 @@ import java.nio.ByteBuffer; import java.nio.channels.SocketChannel; import java.nio.file.Path; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; @@ -44,7 +42,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -119,72 +116,6 @@ public boolean handle(Request request, Response response, Callback callback) } } - @Test - public void testDelayedUntilFormData() throws Exception - { - DelayedHandler delayedHandler = new DelayedHandler(); - CountDownLatch processLatch = new CountDownLatch(1); - delayedHandler.setHandler(new Handler.Abstract.NonBlocking() - { - @Override - public boolean handle(Request request, Response response, Callback callback) throws Exception - { - processLatch.countDown(); - MultiPartFormData.Parts parts = (MultiPartFormData.Parts)request.getAttribute(MultiPartFormData.Parts.class.getName()); - assertNotNull(parts); - MultiPart.Part part = parts.get(0); - Content.copy(part.getContentSource(), response, callback); - return true; - } - }); - start(delayedHandler); - - try (SocketChannel client = SocketChannel.open(new InetSocketAddress("localhost", connector.getLocalPort()))) - { - String contentBegin = """ - --A1B2C3 - Content-Disposition: form-data; name="part" - - """; - String contentMiddle = """ - 0123456789\ - """; - String contentEnd = """ - ABCDEF - --A1B2C3-- - """; - String header = """ - POST / HTTP/1.1 - Host: localhost - Content-Type: multipart/form-data; boundary=A1B2C3 - Content-Length: $L - - """.replace("$L", String.valueOf(contentBegin.length() + contentMiddle.length() + contentEnd.length())); - - client.write(UTF_8.encode(header)); - client.write(UTF_8.encode(contentBegin)); - - // Verify that the handler has not been called yet. - assertFalse(processLatch.await(1, TimeUnit.SECONDS)); - - client.write(UTF_8.encode(contentMiddle)); - - // Verify that the handler has not been called yet. - assertFalse(processLatch.await(1, TimeUnit.SECONDS)); - - // Finish to send the content. - client.write(UTF_8.encode(contentEnd)); - - // Verify that the handler has been called. - assertTrue(processLatch.await(5, TimeUnit.SECONDS)); - - HttpTester.Response response = HttpTester.parseResponse(HttpTester.from(client)); - assertNotNull(response); - assertEquals(HttpStatus.OK_200, response.getStatus()); - assertEquals("0123456789ABCDEF", response.getContent()); - } - } - @Test public void testEchoMultiPart() throws Exception { From 78be33da5474262f08009fb6fb48cdc88c7250ac Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 29 Jun 2023 23:54:24 +0200 Subject: [PATCH 14/19] Experiment with a fully async ContentSourceCompletableFuture ServletFormHandler --- .../org/eclipse/jetty/server/FormFields.java | 6 +-- .../ee10/servlet/ServletContextRequest.java | 4 +- .../servlet/ServletMultiPartFormData.java | 13 +++++- .../ee10/servlet/MultiPartServletTest.java | 40 +------------------ 4 files changed, 19 insertions(+), 44 deletions(-) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java index 1f7f949add17..a0c7ea2962c0 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java @@ -35,6 +35,8 @@ */ public class FormFields extends CompletableFuture implements Runnable { + // TODO convert to a ContentSourceCompletableFuture + public static final String MAX_FIELDS_ATTRIBUTE = "org.eclipse.jetty.server.Request.maxFormKeys"; public static final String MAX_LENGTH_ATTRIBUTE = "org.eclipse.jetty.server.Request.maxFormContentSize"; private static final CompletableFuture EMPTY = CompletableFuture.completedFuture(Fields.EMPTY); @@ -59,19 +61,15 @@ public static Charset getFormEncodedCharset(Request request) public static CompletableFuture from(Request request) { - // TODO make this attributes provided by the ContextRequest wrapper int maxFields = getRequestAttribute(request, FormFields.MAX_FIELDS_ATTRIBUTE); int maxLength = getRequestAttribute(request, FormFields.MAX_LENGTH_ATTRIBUTE); - return from(request, maxFields, maxLength); } public static CompletableFuture from(Request request, Charset charset) { - // TODO make this attributes provided by the ContextRequest wrapper int maxFields = getRequestAttribute(request, FormFields.MAX_FIELDS_ATTRIBUTE); int maxLength = getRequestAttribute(request, FormFields.MAX_LENGTH_ATTRIBUTE); - return from(request, charset, maxFields, maxLength); } diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java index 6b414768c97c..a3d53eee6297 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java @@ -32,6 +32,7 @@ import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.UriCompliance; import org.eclipse.jetty.http.pathmap.MatchedResource; +import org.eclipse.jetty.server.FormFields; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.SecureRequestCustomizer; @@ -240,7 +241,8 @@ public Object getAttribute(String name) case "jakarta.servlet.request.ssl_session_id" -> super.getAttribute(SecureRequestCustomizer.SSL_SESSION_ID_ATTRIBUTE); case "jakarta.servlet.request.X509Certificate" -> super.getAttribute(SecureRequestCustomizer.PEER_CERTIFICATES_ATTRIBUTE); case ServletContextRequest.MULTIPART_CONFIG_ELEMENT -> _matchedResource.getResource().getServletHolder().getMultipartConfig(); - + case FormFields.MAX_FIELDS_ATTRIBUTE -> getServletContext().getServletContextHandler().getMaxFormKeys(); + case FormFields.MAX_LENGTH_ATTRIBUTE -> getServletContext().getServletContextHandler().getMaxFormContentSize(); default -> super.getAttribute(name); }; } 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 1405f06b346c..4d1443a9454e 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 @@ -53,8 +53,20 @@ public class ServletMultiPartFormData * Get future {@link ServletMultiPartFormData.Parts} from a servlet request. * @param servletRequest A servlet request * @return A future {@link ServletMultiPartFormData.Parts}, which may have already been created and/or completed. + * @see #from(ServletRequest, String) */ public static CompletableFuture from(ServletRequest servletRequest) + { + return from(servletRequest, servletRequest.getContentType()); + } + + /** + * Get future {@link ServletMultiPartFormData.Parts} from a servlet request. + * @param servletRequest A servlet request + * @param contentType The contentType, passed as an optimization as it has likely already been retrieved. + * @return A future {@link ServletMultiPartFormData.Parts}, which may have already been created and/or completed. + */ + public static CompletableFuture from(ServletRequest servletRequest, String contentType) { // Look for an existing future (we use the future here rather than the parts as it can remember any failure). @SuppressWarnings("unchecked") @@ -69,7 +81,6 @@ public static CompletableFuture from(ServletRequest servletRequest) return CompletableFuture.failedFuture(new IllegalStateException("No multipart configuration element")); // Are we the right content type to produce our own parts? - String contentType = servletRequest.getContentType(); if (contentType == null || !MimeTypes.Type.MULTIPART_FORM_DATA.is(HttpField.valueParameters(contentType, null))) return CompletableFuture.failedFuture(new IllegalStateException("Not multipart Content-Type")); 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 953bb4a98062..cace3a6aaf6a 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 @@ -22,7 +22,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.Map; -import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -43,26 +42,21 @@ import org.eclipse.jetty.client.OutputStreamRequestContent; import org.eclipse.jetty.client.Response; import org.eclipse.jetty.client.StringRequestContent; -import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpTester; -import org.eclipse.jetty.http.MimeTypes; 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.content.InputStreamContentSource; import org.eclipse.jetty.logging.StacklessLogging; -import org.eclipse.jetty.server.Handler; -import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.handler.gzip.GzipHandler; -import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.component.LifeCycle; import org.junit.jupiter.api.AfterEach; @@ -116,38 +110,8 @@ private void start(HttpServlet servlet, MultipartConfigElement config, boolean p gzipHandler.addIncludedMimeTypes("multipart/form-data"); gzipHandler.setMinGzipSize(32); - // User a very simple preload handler if (preload) - { - gzipHandler.setHandler(new Handler.Wrapper() - { - @Override - public boolean handle(Request request, org.eclipse.jetty.server.Response response, Callback callback) throws Exception - { - String contentType = request.getHeaders().get(HttpHeader.CONTENT_TYPE); - if (contentType == null || !MimeTypes.Type.MULTIPART_FORM_DATA.is(HttpField.valueParameters(contentType, null))) - return super.handle(request, response, callback); - - CompletableFuture futureParts = ServletMultiPartFormData.from(Request.as(request, ServletContextRequest.class).getServletApiRequest()); - if (futureParts.isDone()) - return super.handle(request, response, callback); - - futureParts.whenComplete((parts, failure) -> - { - try - { - if (!super.handle(request, response, callback)) - callback.failed(new IllegalStateException("Not Handled")); - } - catch (Throwable x) - { - callback.failed(x); - } - }); - return true; - } - }); - } + gzipHandler.setHandler(new ServletFormHandler()); servletContextHandler.insertHandler(gzipHandler); @@ -214,7 +178,7 @@ public void testManyParts(boolean preload) throws Exception start(new HttpServlet() { @Override - protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + protected void service(HttpServletRequest req, HttpServletResponse resp) { req.getParameterMap(); } From 0a739fae8f76edc8cbae76eafc7797332a527951 Mon Sep 17 00:00:00 2001 From: gregw Date: Fri, 30 Jun 2023 09:00:30 +0200 Subject: [PATCH 15/19] Experiment with a fully async ContentSourceCompletableFuture Converted FormFields --- .../org/eclipse/jetty/server/FormFields.java | 195 +++++++----------- .../eclipse/jetty/server/FormFieldsTest.java | 86 ++++++++ .../server/handler/gzip/GzipHandlerTest.java | 5 +- .../ee10/servlet/PreloadFormHandler.java | 87 ++++++++ .../ee10/servlet/MultiPartServletTest.java | 2 +- 5 files changed, 252 insertions(+), 123 deletions(-) create mode 100644 jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/FormFieldsTest.java create mode 100644 jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/PreloadFormHandler.java diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java index a0c7ea2962c0..19a896d5f0c0 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java @@ -22,6 +22,8 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.content.ContentSourceCompletableFuture; +import org.eclipse.jetty.util.Attributes; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.CharsetStringBuilder; import org.eclipse.jetty.util.Fields; @@ -33,10 +35,8 @@ * A {@link CompletableFuture} that is completed once a {@code application/x-www-form-urlencoded} * content has been parsed asynchronously from the {@link Content.Source}. */ -public class FormFields extends CompletableFuture implements Runnable +public class FormFields extends ContentSourceCompletableFuture { - // TODO convert to a ContentSourceCompletableFuture - public static final String MAX_FIELDS_ATTRIBUTE = "org.eclipse.jetty.server.Request.maxFormKeys"; public static final String MAX_LENGTH_ATTRIBUTE = "org.eclipse.jetty.server.Request.maxFormContentSize"; private static final CompletableFuture EMPTY = CompletableFuture.completedFuture(Fields.EMPTY); @@ -88,24 +88,28 @@ public static CompletableFuture get(Request request) public static CompletableFuture from(Request request, int maxFields, int maxLength) { - Object attr = request.getAttribute(FormFields.class.getName()); + return from(request, getFormEncodedCharset(request), maxFields, maxLength); + } + + public static CompletableFuture from(Request request, Charset charset, int maxFields, int maxLength) + { + return from(request, request, charset, maxFields, maxLength); + } + + static CompletableFuture from(Content.Source source, Attributes attributes, Charset charset, int maxFields, int maxLength) + { + Object attr = attributes.getAttribute(FormFields.class.getName()); if (attr instanceof FormFields futureFormFields) return futureFormFields; else if (attr instanceof Fields fields) return CompletableFuture.completedFuture(fields); - Charset charset = getFormEncodedCharset(request); if (charset == null) return EMPTY; - return from(request, charset, maxFields, maxLength); - } - - public static CompletableFuture from(Request request, Charset charset, int maxFields, int maxLength) - { - FormFields futureFormFields = new FormFields(request, charset, maxFields, maxLength); - request.setAttribute(FormFields.class.getName(), futureFormFields); - futureFormFields.run(); + FormFields futureFormFields = new FormFields(source, charset, maxFields, maxLength); + attributes.setAttribute(FormFields.class.getName(), futureFormFields); + futureFormFields.parse(); return futureFormFields; } @@ -136,6 +140,7 @@ private static int getRequestAttribute(Request request, String attribute) public FormFields(Content.Source source, Charset charset, int maxFields, int maxSize) { + super(source); _source = source; _maxFields = maxFields; _maxLength = maxSize; @@ -144,137 +149,91 @@ public FormFields(Content.Source source, Charset charset, int maxFields, int max } @Override - public void run() + protected Fields parse(Content.Chunk chunk) throws CharacterCodingException { - Content.Chunk chunk = null; - try + String value = null; + ByteBuffer buffer = chunk.getByteBuffer(); + + do { - while (true) + loop: + while (BufferUtil.hasContent(buffer)) { - chunk = _source.read(); - if (chunk == null) - { - _source.demand(this); - return; - } - - if (Content.Chunk.isFailure(chunk)) + byte b = buffer.get(); + switch (_percent) { - completeExceptionally(chunk.getFailure()); - return; - } - - while (true) - { - Fields.Field field = parse(chunk); - if (field == null) - break; - if (_maxFields >= 0 && _fields.getSize() >= _maxFields) + case 1 -> { - chunk.release(); - // Do not double release if completeExceptionally() throws. - chunk = null; - completeExceptionally(new IllegalStateException("form with too many fields")); - return; + _percentCode = b; + _percent++; + continue; + } + case 2 -> + { + _builder.append(decodeHexByte((char)_percentCode, (char)b)); + _percent = 0; + continue; } - _fields.add(field); - } - - chunk.release(); - if (chunk.isLast()) - { - // Do not double release if complete() throws. - chunk = null; - complete(_fields); - return; } - } - } - catch (Throwable x) - { - if (chunk != null) - chunk.release(); - completeExceptionally(x); - } - } - protected Fields.Field parse(Content.Chunk chunk) throws CharacterCodingException - { - String value = null; - ByteBuffer buffer = chunk.getByteBuffer(); - loop: - while (BufferUtil.hasContent(buffer)) - { - byte b = buffer.get(); - switch (_percent) - { - case 1 -> - { - _percentCode = b; - _percent++; - continue; - } - case 2 -> + if (_name == null) { - _builder.append(decodeHexByte((char)_percentCode, (char)b)); - _percent = 0; - continue; + switch (b) + { + case '=' -> + { + _name = _builder.build(); + checkLength(_name); + } + case '+' -> _builder.append((byte)' '); + case '%' -> _percent++; + default -> _builder.append(b); + } } - } - - if (_name == null) - { - switch (b) + else { - case '=' -> + switch (b) { - _name = _builder.build(); - checkLength(_name); + case '&' -> + { + value = _builder.build(); + checkLength(value); + break loop; + } + case '+' -> _builder.append((byte)' '); + case '%' -> _percent++; + default -> _builder.append(b); } - case '+' -> _builder.append((byte)' '); - case '%' -> _percent++; - default -> _builder.append(b); } } - else + + if (_name != null) { - switch (b) + if (value == null && chunk.isLast()) { - case '&' -> + if (_percent > 0) { - value = _builder.build(); - checkLength(value); - break loop; + _builder.append((byte)'%'); + _builder.append(_percentCode); } - case '+' -> _builder.append((byte)' '); - case '%' -> _percent++; - default -> _builder.append(b); + value = _builder.build(); + checkLength(value); } - } - } - if (_name != null) - { - if (value == null && chunk.isLast()) - { - if (_percent > 0) + if (value != null) { - _builder.append((byte)'%'); - _builder.append(_percentCode); + Fields.Field field = new Fields.Field(_name, value); + _name = null; + value = null; + if (_maxFields > 0 && _fields.getSize() >= _maxFields) + throw new IllegalStateException("form with too many fields > " + _maxFields); + _fields.add(field); } - value = _builder.build(); - checkLength(value); - } - - if (value != null) - { - Fields.Field field = new Fields.Field(_name, value); - _name = null; - return field; } } + while (BufferUtil.hasContent(buffer)); - return null; + return chunk.isLast() ? _fields : null; } private void checkLength(String nameOrValue) diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/FormFieldsTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/FormFieldsTest.java new file mode 100644 index 000000000000..4d00dddb5efc --- /dev/null +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/FormFieldsTest.java @@ -0,0 +1,86 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.server; + +import java.nio.charset.Charset; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.stream.Stream; + +import org.eclipse.jetty.io.content.AsyncContent; +import org.eclipse.jetty.util.Attributes; +import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.Fields; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class FormFieldsTest +{ + public static Stream tests() + { + return Stream.of( + Arguments.of(List.of("name=value"), UTF_8, -1, -1, Map.of("name", "value")), + Arguments.of(List.of("name=value", ""), UTF_8, -1, -1, Map.of("name", "value")), + Arguments.of(List.of("name", "=value", ""), UTF_8, -1, -1, Map.of("name", "value")), + Arguments.of(List.of("n", "ame", "=", "value"), UTF_8, -1, -1, Map.of("name", "value")), + Arguments.of(List.of("n=v&X=Y"), UTF_8, 2, 4, Map.of("n", "v", "X", "Y")), + Arguments.of(List.of("name=f¤¤&X=Y"), UTF_8, -1, -1, Map.of("name", "f¤¤", "X", "Y")), + Arguments.of(List.of("n=v&X=Y"), UTF_8, 1, -1, null), + Arguments.of(List.of("n=v&X=Y"), UTF_8, -1, 3, null) + ); + } + + @ParameterizedTest + @MethodSource("tests") + public void testFormFields(List chunks, Charset charset, int maxFields, int maxLength, Map expected) + { + AsyncContent source = new AsyncContent(); + Attributes attributes = new Attributes.Mapped(); + CompletableFuture futureFields = FormFields.from(source, attributes, charset, maxFields, maxLength); + assertFalse(futureFields.isDone()); + + int last = chunks.size() - 1; + for (int i = 0; i <= last; i++) + source.write(i == last, BufferUtil.toBuffer(chunks.get(i), charset), Callback.NOOP); + + assertTrue(futureFields.isDone()); + + try + { + Map result = new HashMap<>(); + for (Fields.Field f : futureFields.get()) + result.put(f.getName(), f.getValue()); + + assertEquals(expected, result); + } + catch (AssertionError e) + { + throw e; + } + catch (Throwable e) + { + assertNull(expected); + } + } +} diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipHandlerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipHandlerTest.java index f7b9c918df9b..1c94cd813cb7 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipHandlerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipHandlerTest.java @@ -1984,11 +1984,8 @@ public static class DumpHandler extends Handler.Abstract public boolean handle(Request request, Response response, Callback callback) throws Exception { response.getHeaders().put(HttpHeader.CONTENT_TYPE, "text/plain"); - Fields queryParameters = Request.extractQueryParameters(request); - FormFields futureFormFields = new FormFields(request, StandardCharsets.UTF_8, -1, -1); - futureFormFields.run(); - Fields formParameters = futureFormFields.get(); + Fields formParameters = FormFields.from(request, UTF_8, -1, -1).get(); Fields parameters = Fields.combine(queryParameters, formParameters); String dump = parameters.stream().map(f -> "%s: %s\n".formatted(f.getName(), f.getValue())).collect(Collectors.joining()); diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/PreloadFormHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/PreloadFormHandler.java new file mode 100644 index 000000000000..bad6553cb664 --- /dev/null +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/PreloadFormHandler.java @@ -0,0 +1,87 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.ee10.servlet; + +import java.util.concurrent.CompletableFuture; + +import jakarta.servlet.ServletRequest; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.MimeTypes; +import org.eclipse.jetty.server.FormFields; +import org.eclipse.jetty.server.Handler; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.util.Callback; + +/** + * Handler to asynchronously preload & parse {@link MimeTypes.Type#FORM_ENCODED} and + * {@link MimeTypes.Type#MULTIPART_FORM_DATA} content prior to invoking the {@link ServletHandler}, + * which can then consume them with blocking APIs but without blocking. + * @see FormFields#from(Request) + * @see ServletMultiPartFormData#from(ServletRequest) + */ +public class PreloadFormHandler extends Handler.Wrapper +{ + public PreloadFormHandler() + { + this(null); + } + + public PreloadFormHandler(Handler handler) + { + super(handler); + } + + @Override + public boolean handle(Request request, org.eclipse.jetty.server.Response response, Callback callback) throws Exception + { + String contentType = request.getHeaders().get(HttpHeader.CONTENT_TYPE); + if (contentType == null) + return super.handle(request, response, callback); + + MimeTypes.Type mimeType = MimeTypes.getBaseType(contentType); + if (mimeType == null) + return super.handle(request, response, callback); + + CompletableFuture> future = switch (mimeType) + { + case FORM_ENCODED -> FormFields.from(request); + case MULTIPART_FORM_DATA -> ServletMultiPartFormData.from(Request.as(request, ServletContextRequest.class).getServletApiRequest(), contentType); + default -> null; + }; + + if (future == null) + return super.handle(request, response, callback); + + if (future.isDone()) + { + if (!super.handle(request, response, callback)) + callback.failed(new IllegalStateException("Not Handled")); + return true; + } + + future.whenComplete((result, failure) -> + { + try + { + if (!super.handle(request, response, callback)) + callback.failed(new IllegalStateException("Not Handled")); + } + catch (Throwable x) + { + callback.failed(x); + } + }); + return true; + } +} 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 cace3a6aaf6a..4edbf4c2b86e 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 @@ -111,7 +111,7 @@ private void start(HttpServlet servlet, MultipartConfigElement config, boolean p gzipHandler.setMinGzipSize(32); if (preload) - gzipHandler.setHandler(new ServletFormHandler()); + gzipHandler.setHandler(new PreloadFormHandler()); servletContextHandler.insertHandler(gzipHandler); From 91209b45bb92b1cab63ad394f92f431b8afbba49 Mon Sep 17 00:00:00 2001 From: gregw Date: Fri, 30 Jun 2023 11:54:22 +0200 Subject: [PATCH 16/19] Experiment with a fully async ContentSourceCompletableFuture updates from review --- .../jetty/docs/programming/ContentDocs.java | 4 +- .../jetty/http/MultiPartByteRanges.java | 2 +- .../eclipse/jetty/http/MultiPartFormData.java | 4 +- .../ContentSourceCompletableFuture.java | 42 +++++---- .../org/eclipse/jetty/server/FormFields.java | 91 +++++++++++++++---- .../eclipse/jetty/server/FormFieldsTest.java | 5 +- .../QuickStartGeneratorConfiguration.java | 2 +- .../jetty/ee10/servlet/Dispatcher.java | 9 ++ ...FormHandler.java => EagerFormHandler.java} | 18 ++-- .../ee10/servlet/ServletContextRequest.java | 8 +- .../jetty/ee10/servlet/ServletHolder.java | 12 +-- .../ee10/servlet/MultiPartServletTest.java | 38 ++++---- .../webapp/StandardDescriptorProcessor.java | 2 +- 13 files changed, 153 insertions(+), 84 deletions(-) rename jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/{PreloadFormHandler.java => EagerFormHandler.java} (84%) diff --git a/documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/ContentDocs.java b/documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/ContentDocs.java index 959767fb7882..3724c4480f84 100644 --- a/documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/ContentDocs.java +++ b/documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/ContentDocs.java @@ -216,13 +216,11 @@ public static void testFutureString() throws Exception Content.Sink.write(source, true, "Three", writeCallback); if (!writeCallback.isDone() || !future.isDone()) throw new IllegalStateException("Should be consumed"); - - System.err.println(future.get()); } public static class FutureUtf8String extends ContentSourceCompletableFuture { - Utf8StringBuilder builder = new Utf8StringBuilder(); + private final Utf8StringBuilder builder = new Utf8StringBuilder(); public FutureUtf8String(Content.Source content) { diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartByteRanges.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartByteRanges.java index b4c8fd3f74de..0efcc9aaf2b5 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartByteRanges.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartByteRanges.java @@ -52,7 +52,7 @@ * * @see Parts */ -public class MultiPartByteRanges extends CompletableFuture +public class MultiPartByteRanges { private MultiPartByteRanges() { diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartFormData.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartFormData.java index 4bfd612aa2dc..1f58648e6993 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartFormData.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartFormData.java @@ -224,7 +224,7 @@ protected Parts parse(Content.Chunk chunk) throws Throwable throw listener.failure; length += chunk.getByteBuffer().remaining(); long max = getMaxLength(); - if (max > 0 && length > max) + if (max >= 0 && length > max) throw new IllegalStateException("max length exceeded: %d".formatted(max)); parser.parse(chunk); if (listener.isFailed()) @@ -394,7 +394,7 @@ int getPartsSize() private class PartsListener extends MultiPart.AbstractPartsListener { - private final AutoLock lock = new AutoLock(); // TODO why do we need this lock? + private final AutoLock lock = new AutoLock(); private final List parts = new ArrayList<>(); private final List partChunks = new ArrayList<>(); private long fileSize; diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceCompletableFuture.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceCompletableFuture.java index c23922b852e8..ff0ef9dc7e33 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceCompletableFuture.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceCompletableFuture.java @@ -24,8 +24,8 @@ * * An example usage to asynchronously read UTF-8 content is: *
- *- * public static class FutureUtf8String extends ContentSourceCompletableFuture<String> + **/ public abstract class ContentSourceCompletableFuture{@code + * public static class FutureUtf8String extends ContentSourceCompletableFuture+ * new FutureUtf8String(source).thenAccept(System.err::println); + * }; * { * Utf8StringBuilder builder = new Utf8StringBuilder(); * @@ -34,7 +34,7 @@ * super(content); * } * - * @Override + * @Override * protected String parse(Content.Chunk chunk) throws Throwable * { * if (chunk.hasRemaining()) @@ -43,10 +43,8 @@ * } * } * ... - * { - * new FutureUtf8String(source).thenAccept(System.err::println); - * } - * extends CompletableFuture { @@ -57,25 +55,27 @@ public ContentSourceCompletableFuture(Content.Source content) _content = content; } - public CompletableFuture parse() - { - onContentAvailable(); - return this; - } - - private void onContentAvailable() + /** + * Progress the parsing, {@link Content.Source#read() reading} and/or {@link Content.Source#demand(Runnable) demanding} + * as necessary. + * + * This method must be called once to initiate the reading and parsing, + * and is then called to progress parsing in response to any {@link Content.Source#demand(Runnable) demand} calls. + *
+ */ + public void parse() { while (true) { Content.Chunk chunk = _content.read(); if (chunk == null) { - _content.demand(this::onContentAvailable); + _content.demand(this::parse); return; } if (Content.Chunk.isFailure(chunk)) { - if (!chunk.isLast() && ignoreTransientFailure(chunk.getFailure())) + if (!chunk.isLast() && onTransientFailure(chunk.getFailure())) continue; completeExceptionally(chunk.getFailure()); return; @@ -110,9 +110,11 @@ private void onContentAvailable() /** * Called to parse a {@link org.eclipse.jetty.io.Content.Chunk} - * @param chunk The chunk containing content to parse. The chunk will - * never be a - * {@link org.eclipse.jetty.io.Content.Chunk#isFailure(Content.Chunk) failure chunk}. + * @param chunk The chunk containing content to parse. The chunk will never be null nor a + * {@link org.eclipse.jetty.io.Content.Chunk#isFailure(Content.Chunk) failure chunk}. + * If references to the content of the chunk are to be held beyond the scope of this call, + * then implementations must call {@link Content.Chunk#retain()} and {@link Content.Chunk#release()} + * as appropriate. * @return The parsed {@code X} instance or null if parsing is not yet complete * @throws Throwable Thrown if there is an error parsing */ @@ -123,7 +125,7 @@ private void onContentAvailable() * {@link org.eclipse.jetty.io.Content.Chunk#isFailure(Content.Chunk) failure chunk} * @return True if the chunk can be ignored. */ - protected boolean ignoreTransientFailure(Throwable cause) + protected boolean onTransientFailure(Throwable cause) { return false; } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java index 19a896d5f0c0..806b43e688b9 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java @@ -59,6 +59,38 @@ public static Charset getFormEncodedCharset(Request request) return StringUtil.isEmpty(cs) ? StandardCharsets.UTF_8 : Charset.forName(cs); } + /** + * Set a {@link Fields} or related failure for the request + * @param request The request to which to associate the fields with + * @param fields A {@link CompletableFuture} that will provide either the fields or a failure. + */ + public static void set(Request request, CompletableFuturefields) + { + request.setAttribute(FormFields.class.getName(), fields); + } + + /** + * @param request The request to enquire from + * @return A {@link CompletableFuture} that will provide either the fields or a failure, or null if none set. + * @see #from(Request) + * + */ + public static CompletableFuture get(Request request) + { + Object attr = request.getAttribute(FormFields.class.getName()); + if (attr instanceof FormFields futureFormFields) + return futureFormFields; + return EMPTY; + } + + /** + * Find or create a {@link FormFields} from a {@link Content.Source}. + * @param request The {@link Request} in which to look for an existing {@link FormFields} attribute, + * using the classname as the attribute name, else the request is used + * as a {@link Content.Source} from which to read the fields and set the attribute. + * @return A {@link CompletableFuture} that will provide the {@link Fields} or a failure. + * @see #from(Content.Source, Attributes, Charset, int, int) + */ public static CompletableFuture from(Request request) { int maxFields = getRequestAttribute(request, FormFields.MAX_FIELDS_ATTRIBUTE); @@ -66,6 +98,15 @@ public static CompletableFuture from(Request request) return from(request, maxFields, maxLength); } + /** + * Find or create a {@link FormFields} from a {@link Content.Source}. + * @param request The {@link Request} in which to look for an existing {@link FormFields} attribute, + * using the classname as the attribute name, else the request is used + * as a {@link Content.Source} from which to read the fields and set the attribute. + * @param charset the {@link Charset} to use for byte to string conversion. + * @return A {@link CompletableFuture} that will provide the {@link Fields} or a failure. + * @see #from(Content.Source, Attributes, Charset, int, int) + */ public static CompletableFuture from(Request request, Charset charset) { int maxFields = getRequestAttribute(request, FormFields.MAX_FIELDS_ATTRIBUTE); @@ -73,29 +114,47 @@ public static CompletableFuture from(Request request, Charset charset) return from(request, charset, maxFields, maxLength); } - public static void set(Request request, CompletableFuture fields) - { - request.setAttribute(FormFields.class.getName(), fields); - } - - public static CompletableFuture get(Request request) - { - Object attr = request.getAttribute(FormFields.class.getName()); - if (attr instanceof FormFields futureFormFields) - return futureFormFields; - return EMPTY; - } - + /** + * Find or create a {@link FormFields} from a {@link Content.Source}. + * @param request The {@link Request} in which to look for an existing {@link FormFields} attribute, + * using the classname as the attribute name, else the request is used + * as a {@link Content.Source} from which to read the fields and set the attribute. + * @param maxFields The maximum number of fields to be parsed + * @param maxLength The maximum total size of the fields + * @return A {@link CompletableFuture} that will provide the {@link Fields} or a failure. + * @see #from(Content.Source, Attributes, Charset, int, int) + */ public static CompletableFuture from(Request request, int maxFields, int maxLength) { return from(request, getFormEncodedCharset(request), maxFields, maxLength); } + /** + * Find or create a {@link FormFields} from a {@link Content.Source}. + * @param request The {@link Request} in which to look for an existing {@link FormFields} attribute, + * using the classname as the attribute name, else the request is used + * as a {@link Content.Source} from which to read the fields and set the attribute. + * @param charset the {@link Charset} to use for byte to string conversion. + * @param maxFields The maximum number of fields to be parsed + * @param maxLength The maximum total size of the fields + * @return A {@link CompletableFuture} that will provide the {@link Fields} or a failure. + * @see #from(Content.Source, Attributes, Charset, int, int) + */ public static CompletableFuture from(Request request, Charset charset, int maxFields, int maxLength) { return from(request, request, charset, maxFields, maxLength); } + /** + * Find or create a {@link FormFields} from a {@link Content.Source}. + * @param source The {@link Content.Source} from which to read the fields. + * @param attributes The {@link Attributes} in which to look for an existing {@link FormFields}, else in which to + * add the fields, using the classname as the attribute name. + * @param charset the {@link Charset} to use for byte to string conversion. + * @param maxFields The maximum number of fields to be parsed + * @param maxLength The maximum total size of the fields + * @return A {@link CompletableFuture} that will provide the {@link Fields} or a failure. + */ static CompletableFuture from(Content.Source source, Attributes attributes, Charset charset, int maxFields, int maxLength) { Object attr = attributes.getAttribute(FormFields.class.getName()); @@ -128,7 +187,6 @@ private static int getRequestAttribute(Request request, String attribute) } } - private final Content.Source _source; private final Fields _fields; private final CharsetStringBuilder _builder; private final int _maxFields; @@ -138,10 +196,9 @@ private static int getRequestAttribute(Request request, String attribute) private int _percent = 0; private byte _percentCode; - public FormFields(Content.Source source, Charset charset, int maxFields, int maxSize) + private FormFields(Content.Source source, Charset charset, int maxFields, int maxSize) { super(source); - _source = source; _maxFields = maxFields; _maxLength = maxSize; _builder = CharsetStringBuilder.forCharset(charset); @@ -225,7 +282,7 @@ protected Fields parse(Content.Chunk chunk) throws CharacterCodingException Fields.Field field = new Fields.Field(_name, value); _name = null; value = null; - if (_maxFields > 0 && _fields.getSize() >= _maxFields) + if (_maxFields >= 0 && _fields.getSize() >= _maxFields) throw new IllegalStateException("form with too many fields > " + _maxFields); _fields.add(field); } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/FormFieldsTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/FormFieldsTest.java index 4d00dddb5efc..e6ecab2b15a7 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/FormFieldsTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/FormFieldsTest.java @@ -25,6 +25,7 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.Fields; +import org.eclipse.jetty.util.FutureCallback; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -61,9 +62,11 @@ public void testFormFields(List chunks, Charset charset, int maxFields, assertFalse(futureFields.isDone()); int last = chunks.size() - 1; + FutureCallback eof = new FutureCallback(); for (int i = 0; i <= last; i++) - source.write(i == last, BufferUtil.toBuffer(chunks.get(i), charset), Callback.NOOP); + source.write(i == last, BufferUtil.toBuffer(chunks.get(i), charset), i == last ? eof : Callback.NOOP); + assertTrue(eof.isDone()); assertTrue(futureFields.isDone()); try diff --git a/jetty-ee10/jetty-ee10-quickstart/src/main/java/org/eclipse/jetty/ee10/quickstart/QuickStartGeneratorConfiguration.java b/jetty-ee10/jetty-ee10-quickstart/src/main/java/org/eclipse/jetty/ee10/quickstart/QuickStartGeneratorConfiguration.java index 14bd1ce54f6c..f99ebd5ad828 100644 --- a/jetty-ee10/jetty-ee10-quickstart/src/main/java/org/eclipse/jetty/ee10/quickstart/QuickStartGeneratorConfiguration.java +++ b/jetty-ee10/jetty-ee10-quickstart/src/main/java/org/eclipse/jetty/ee10/quickstart/QuickStartGeneratorConfiguration.java @@ -764,7 +764,7 @@ private void outholder(XmlAppendable out, MetaData md, ServletHolder holder) thr } //multipart-config - MultipartConfigElement multipartConfig = ((ServletHolder.Registration)holder.getRegistration()).getMultipartConfig(); + MultipartConfigElement multipartConfig = holder.getRegistration().getMultipartConfigElement(); if (multipartConfig != null) { out.openTag("multipart-config", origin(md, holder.getName() + ".servlet.multipart-config")); diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/Dispatcher.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/Dispatcher.java index d1bc1702c1f0..4cf48e546e76 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/Dispatcher.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/Dispatcher.java @@ -309,6 +309,15 @@ public Object getAttribute(String name) { return null; } + case ServletContextRequest.MULTIPART_CONFIG_ELEMENT -> + { + // If we already have future parts, return the configuration of the wrapped request. + if (super.getAttribute(ServletMultiPartFormData.class.getName()) != null) + return super.getAttribute(name); + // otherwise, return the configuration of this mapping + return _mappedServlet.getServletHolder().getMultipartConfigElement(); + } + default -> { return super.getAttribute(name); diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/PreloadFormHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/EagerFormHandler.java similarity index 84% rename from jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/PreloadFormHandler.java rename to jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/EagerFormHandler.java index bad6553cb664..64940c0cda7c 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/PreloadFormHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/EagerFormHandler.java @@ -24,20 +24,20 @@ import org.eclipse.jetty.util.Callback; /** - * Handler to asynchronously preload & parse {@link MimeTypes.Type#FORM_ENCODED} and + * Handler to eagerly and asynchronously read & parse {@link MimeTypes.Type#FORM_ENCODED} and * {@link MimeTypes.Type#MULTIPART_FORM_DATA} content prior to invoking the {@link ServletHandler}, * which can then consume them with blocking APIs but without blocking. * @see FormFields#from(Request) * @see ServletMultiPartFormData#from(ServletRequest) */ -public class PreloadFormHandler extends Handler.Wrapper +public class EagerFormHandler extends Handler.Wrapper { - public PreloadFormHandler() + public EagerFormHandler() { this(null); } - public PreloadFormHandler(Handler handler) + public EagerFormHandler(Handler handler) { super(handler); } @@ -63,15 +63,11 @@ public boolean handle(Request request, org.eclipse.jetty.server.Response respons if (future == null) return super.handle(request, response, callback); - if (future.isDone()) - { - if (!super.handle(request, response, callback)) - callback.failed(new IllegalStateException("Not Handled")); - return true; - } - future.whenComplete((result, failure) -> { + // The result and failure are not handled here. Rather we call the next handler + // to allow the normal processing to handle the result or failure, which will be + // provided via the attribute to ServletApiRequest#getParts() try { if (!super.handle(request, response, callback)) diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java index a3d53eee6297..17da4f12ff6e 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java @@ -240,7 +240,7 @@ public Object getAttribute(String name) case "jakarta.servlet.request.key_size" -> super.getAttribute(SecureRequestCustomizer.KEY_SIZE_ATTRIBUTE); case "jakarta.servlet.request.ssl_session_id" -> super.getAttribute(SecureRequestCustomizer.SSL_SESSION_ID_ATTRIBUTE); case "jakarta.servlet.request.X509Certificate" -> super.getAttribute(SecureRequestCustomizer.PEER_CERTIFICATES_ATTRIBUTE); - case ServletContextRequest.MULTIPART_CONFIG_ELEMENT -> _matchedResource.getResource().getServletHolder().getMultipartConfig(); + case ServletContextRequest.MULTIPART_CONFIG_ELEMENT -> _matchedResource.getResource().getServletHolder().getMultipartConfigElement(); case FormFields.MAX_FIELDS_ATTRIBUTE -> getServletContext().getServletContextHandler().getMaxFormKeys(); case FormFields.MAX_LENGTH_ATTRIBUTE -> getServletContext().getServletContextHandler().getMaxFormContentSize(); default -> super.getAttribute(name); @@ -259,8 +259,12 @@ public Set getAttributeNameSet() names.add("jakarta.servlet.request.ssl_session_id"); if (names.contains(SecureRequestCustomizer.PEER_CERTIFICATES_ATTRIBUTE)) names.add("jakarta.servlet.request.X509Certificate"); - if (_matchedResource.getResource().getServletHolder().getMultipartConfig() != null) + if (_matchedResource.getResource().getServletHolder().getMultipartConfigElement() != null) names.add(ServletContextRequest.MULTIPART_CONFIG_ELEMENT); + if (getServletContext().getServletContextHandler().getMaxFormKeys() >= 0) + names.add(FormFields.MAX_FIELDS_ATTRIBUTE); + if (getServletContext().getServletContextHandler().getMaxFormContentSize() >= 0L) + names.add(FormFields.MAX_FIELDS_ATTRIBUTE); return names; } diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHolder.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHolder.java index 2404db1c6425..a290c7d8d8b4 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHolder.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHolder.java @@ -155,9 +155,9 @@ public ServletHolder(Class extends Servlet> servlet) setHeldClass(servlet); } - public Object getMultipartConfig() + public MultipartConfigElement getMultipartConfigElement() { - return _registration == null ? null : _registration.getMultipartConfig(); + return _registration == null ? null : _registration.getMultipartConfigElement(); } /** @@ -916,7 +916,7 @@ public String getServletName() public class Registration extends HolderRegistration implements ServletRegistration.Dynamic { - protected MultipartConfigElement _multipartConfig; + protected MultipartConfigElement _multipartConfigElement; @Override public Set addMapping(String... urlPatterns) @@ -991,12 +991,12 @@ public int getInitOrder() @Override public void setMultipartConfig(MultipartConfigElement element) { - _multipartConfig = element; + _multipartConfigElement = element; } - public MultipartConfigElement getMultipartConfig() + public MultipartConfigElement getMultipartConfigElement() { - return _multipartConfig; + return _multipartConfigElement; } @Override 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 4edbf4c2b86e..9e46e56f6789 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 @@ -93,7 +93,7 @@ public void before() throws Exception tmpDirString = tmpDir.toAbsolutePath().toString(); } - private void start(HttpServlet servlet, MultipartConfigElement config, boolean preload) throws Exception + private void start(HttpServlet servlet, MultipartConfigElement config, boolean eager) throws Exception { config = config == null ? new MultipartConfigElement(tmpDirString, MAX_FILE_SIZE, -1, 0) : config; server = new Server(null, null, null); @@ -110,8 +110,8 @@ private void start(HttpServlet servlet, MultipartConfigElement config, boolean p gzipHandler.addIncludedMimeTypes("multipart/form-data"); gzipHandler.setMinGzipSize(32); - if (preload) - gzipHandler.setHandler(new PreloadFormHandler()); + if (eager) + gzipHandler.setHandler(new EagerFormHandler()); servletContextHandler.insertHandler(gzipHandler); @@ -131,7 +131,7 @@ public void stop() throws Exception @ParameterizedTest @ValueSource(booleans = {true, false}) - public void testLargePart(boolean preload) throws Exception + public void testLargePart(boolean eager) throws Exception { start(new HttpServlet() { @@ -140,7 +140,7 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) { req.getParameterMap(); } - }, new MultipartConfigElement(tmpDirString), preload); + }, new MultipartConfigElement(tmpDirString), eager); OutputStreamRequestContent content = new OutputStreamRequestContent(); MultiPartRequestContent multiPart = new MultiPartRequestContent(); @@ -173,7 +173,7 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) @ParameterizedTest @ValueSource(booleans = {true, false}) - public void testManyParts(boolean preload) throws Exception + public void testManyParts(boolean eager) throws Exception { start(new HttpServlet() { @@ -182,7 +182,7 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) { req.getParameterMap(); } - }, new MultipartConfigElement(tmpDirString), preload); + }, new MultipartConfigElement(tmpDirString), eager); byte[] byteArray = new byte[1024]; Arrays.fill(byteArray, (byte)1); @@ -212,7 +212,7 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) @ParameterizedTest @ValueSource(booleans = {true, false}) - public void testMaxRequestSize(boolean preload) throws Exception + public void testMaxRequestSize(boolean eager) throws Exception { start(new HttpServlet() { @@ -221,7 +221,7 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) throws { req.getParameterMap(); } - }, new MultipartConfigElement(tmpDirString, -1, 1024, 1024 * 1024 * 8), preload); + }, new MultipartConfigElement(tmpDirString, -1, 1024, 1024 * 1024 * 8), eager); OutputStreamRequestContent content = new OutputStreamRequestContent(); MultiPartRequestContent multiPart = new MultiPartRequestContent(); @@ -282,7 +282,7 @@ private static void assert400orEof(InputStreamResponseListener listener, Consume @ParameterizedTest @ValueSource(booleans = {true, false}) - public void testSimpleMultiPart(boolean preload) throws Exception + public void testSimpleMultiPart(boolean eager) throws Exception { start(new HttpServlet() { @@ -300,7 +300,7 @@ protected void service(HttpServletRequest request, HttpServletResponse response1 String content1 = IO.toString(part.getInputStream(), UTF_8); assertEquals("content1", content1); } - }, null, preload); + }, null, eager); try (Socket socket = new Socket("localhost", connector.getLocalPort())) { @@ -334,7 +334,7 @@ protected void service(HttpServletRequest request, HttpServletResponse response1 @ParameterizedTest @ValueSource(booleans = {true, false}) - public void testTempFilesDeletedOnError(boolean preload) throws Exception + public void testTempFilesDeletedOnError(boolean eager) throws Exception { byte[] bytes = new byte[2 * MAX_FILE_SIZE]; Arrays.fill(bytes, (byte)1); @@ -348,7 +348,7 @@ protected void service(HttpServletRequest request, HttpServletResponse response1 // Should throw as the max file size is exceeded. request.getParts(); } - }, null, preload); + }, null, eager); MultiPartRequestContent multiPart = new MultiPartRequestContent(); multiPart.addPart(new MultiPart.ContentSourcePart("largePart", "largeFile.bin", HttpFields.EMPTY, new BytesRequestContent(bytes))); @@ -373,7 +373,7 @@ protected void service(HttpServletRequest request, HttpServletResponse response1 @ParameterizedTest @ValueSource(booleans = {true, false}) - public void testMultiPartGzip(boolean preload) throws Exception + public void testMultiPartGzip(boolean eager) throws Exception { start(new HttpServlet() { @@ -397,7 +397,7 @@ protected void service(HttpServletRequest request, HttpServletResponse response1 echoParts.close(); IO.copy(Content.Source.asInputStream(echoParts), response1.getOutputStream()); } - }, null, preload); + }, null, eager); // Do not automatically handle gzip. client.getContentDecoderFactories().clear(); @@ -437,7 +437,7 @@ protected void service(HttpServletRequest request, HttpServletResponse response1 @ParameterizedTest @ValueSource(booleans = {true, false}) - public void testDoubleReadFromPart(boolean preload) throws Exception + public void testDoubleReadFromPart(boolean eager) throws Exception { start(new HttpServlet() { @@ -451,7 +451,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S resp.getWriter().println("Part: name=" + part.getName() + ", size=" + part.getSize() + ", content=" + IO.toString(part.getInputStream())); } } - }, null, preload); + }, null, eager); String contentString = "the quick brown fox jumps over the lazy dog, " + "the quick brown fox jumps over the lazy dog"; @@ -473,7 +473,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S @ParameterizedTest @ValueSource(booleans = {true, false}) - public void testPartAsParameter(boolean preload) throws Exception + public void testPartAsParameter(boolean eager) throws Exception { start(new HttpServlet() { @@ -488,7 +488,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S resp.getWriter().println("Parameter: " + entry.getKey() + "=" + entry.getValue()[0]); } } - }, null, preload); + }, null, eager); String contentString = "the quick brown fox jumps over the lazy dog, " + "the quick brown fox jumps over the lazy dog"; diff --git a/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/StandardDescriptorProcessor.java b/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/StandardDescriptorProcessor.java index 0e4e9f8d2b03..75a8ec270979 100644 --- a/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/StandardDescriptorProcessor.java +++ b/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/StandardDescriptorProcessor.java @@ -588,7 +588,7 @@ public void visitServlet(WebAppContext context, Descriptor descriptor, XmlParser case WebFragment: { //another fragment set the value, this fragment's values must match exactly or it is an error - MultipartConfigElement cfg = ((ServletHolder.Registration)holder.getRegistration()).getMultipartConfig(); + MultipartConfigElement cfg = holder.getRegistration().getMultipartConfigElement(); if (cfg.getMaxFileSize() != element.getMaxFileSize()) throw new IllegalStateException("Conflicting multipart-config max-file-size for servlet " + name + " in " + descriptor.getURI()); From 1b4703990dd587c19854a8c641052dbd19ab43b0 Mon Sep 17 00:00:00 2001 From: gregw Date: Fri, 30 Jun 2023 11:57:05 +0200 Subject: [PATCH 17/19] Experiment with a fully async ContentSourceCompletableFuture updates from review --- .../java/org/eclipse/jetty/ee10/servlet/EagerFormHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/EagerFormHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/EagerFormHandler.java index 64940c0cda7c..015d051edda4 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/EagerFormHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/EagerFormHandler.java @@ -24,7 +24,7 @@ import org.eclipse.jetty.util.Callback; /** - * Handler to eagerly and asynchronously read & parse {@link MimeTypes.Type#FORM_ENCODED} and + * Handler to eagerly and asynchronously read and parse {@link MimeTypes.Type#FORM_ENCODED} and * {@link MimeTypes.Type#MULTIPART_FORM_DATA} content prior to invoking the {@link ServletHandler}, * which can then consume them with blocking APIs but without blocking. * @see FormFields#from(Request) From dff02022e9ddda42776a71d109f7968ed178b685 Mon Sep 17 00:00:00 2001 From: gregw Date: Fri, 30 Jun 2023 12:33:54 +0200 Subject: [PATCH 18/19] Experiment with a fully async ContentSourceCompletableFuture updates from review --- .../src/main/java/org/eclipse/jetty/server/FormFields.java | 5 +++-- .../test/java/org/eclipse/jetty/server/FormFieldsTest.java | 7 +++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java index 806b43e688b9..eaf96ccb5b87 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java @@ -148,8 +148,9 @@ public static CompletableFuture from(Request request, Charset charset, i /** * Find or create a {@link FormFields} from a {@link Content.Source}. * @param source The {@link Content.Source} from which to read the fields. - * @param attributes The {@link Attributes} in which to look for an existing {@link FormFields}, else in which to - * add the fields, using the classname as the attribute name. + * @param attributes The {@link Attributes} in which to look for an existing {@link CompletableFuture} of + * {@link FormFields}, using the classname as the attribute name. If not found the attribute + * is set with the created {@link CompletableFuture} of {@link FormFields}. * @param charset the {@link Charset} to use for byte to string conversion. * @param maxFields The maximum number of fields to be parsed * @param maxLength The maximum total size of the fields diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/FormFieldsTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/FormFieldsTest.java index e6ecab2b15a7..cfd0b1ec650c 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/FormFieldsTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/FormFieldsTest.java @@ -18,6 +18,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; import java.util.stream.Stream; import org.eclipse.jetty.io.content.AsyncContent; @@ -55,6 +56,7 @@ public static Stream tests() @ParameterizedTest @MethodSource("tests") public void testFormFields(List chunks, Charset charset, int maxFields, int maxLength, Map expected) + throws Exception { AsyncContent source = new AsyncContent(); Attributes attributes = new Attributes.Mapped(); @@ -66,11 +68,12 @@ public void testFormFields(List chunks, Charset charset, int maxFields, for (int i = 0; i <= last; i++) source.write(i == last, BufferUtil.toBuffer(chunks.get(i), charset), i == last ? eof : Callback.NOOP); - assertTrue(eof.isDone()); - assertTrue(futureFields.isDone()); try { + eof.get(10, TimeUnit.SECONDS); + assertTrue(futureFields.isDone()); + Map result = new HashMap<>(); for (Fields.Field f : futureFields.get()) result.put(f.getName(), f.getValue()); From e45daca3022f73927aeb1831a31a16789d3b122f Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 30 Jun 2023 16:03:25 +0200 Subject: [PATCH 19/19] Improved javadocs of ContentSourceCompletableFuture. Signed-off-by: Simone Bordet --- .../ContentSourceCompletableFuture.java | 81 +++++++++++-------- 1 file changed, 46 insertions(+), 35 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceCompletableFuture.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceCompletableFuture.java index ff0ef9dc7e33..f44d92832618 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceCompletableFuture.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceCompletableFuture.java @@ -19,31 +19,36 @@ import org.eclipse.jetty.io.Content; /** - * A utility class to convert content from a {@link Content.Source} to an instance - * available via a {@link CompletableFuture}. - * - * An example usage to asynchronously read UTF-8 content is: - *
+ *A utility class to convert content from a {@link Content.Source} to an instance + * available via a {@link CompletableFuture}.
+ *An example usage to asynchronously read UTF-8 content is:
*{@code - * public static class FutureUtf8String extends ContentSourceCompletableFuture*/ public abstract class ContentSourceCompletableFuture; + * public static class CompletableUTF8String extends ContentSourceCompletableFuture ; + * { + * private final Utf8StringBuilder builder = new Utf8StringBuilder(); + * + * public CompletableUTF8String(Content.Source content) + * { + * super(content); + * } + * + * @Override + * protected String parse(Content.Chunk chunk) throws Throwable * { - * Utf8StringBuilder builder = new Utf8StringBuilder(); + * // Accumulate the chunk bytes. + * if (chunk.hasRemaining()) + * builder.append(chunk.getByteBuffer()); * - * public FutureUtf8String(Content.Source content) - * { - * super(content); - * } + * // Not the last chunk, the result is not ready yet. + * if (!chunk.isLast()) + * return null; * - * @Override - * protected String parse(Content.Chunk chunk) throws Throwable - * { - * if (chunk.hasRemaining()) - * builder.append(chunk.getByteBuffer()); - * return chunk.isLast() ? builder.takeCompleteString(IllegalStateException::new) : null; - * } + * // The result is ready. + * return builder.takeCompleteString(IllegalStateException::new); * } - * ... - * new FutureUtf8String(source).thenAccept(System.err::println); + * } + * + * new CompletableUTF8String(source).thenAccept(System.err::println); * } extends CompletableFuture @@ -56,12 +61,15 @@ public ContentSourceCompletableFuture(Content.Source content) } /** - * Progress the parsing, {@link Content.Source#read() reading} and/or {@link Content.Source#demand(Runnable) demanding} - * as necessary. - * - * This method must be called once to initiate the reading and parsing, - * and is then called to progress parsing in response to any {@link Content.Source#demand(Runnable) demand} calls. - *
+ *Initiates the parsing of the {@link Content.Source}.
+ *For every valid chunk that is read, {@link #parse(Content.Chunk)} + * is called, until a result is produced that is used to + * complete this {@link CompletableFuture}.
+ *Internally, this method is called multiple times to progress + * the parsing in response to {@link Content.Source#demand(Runnable)} + * calls.
+ *Exceptions thrown during parsing result in this + * {@link CompletableFuture} to be completed exceptionally.
*/ public void parse() { @@ -109,21 +117,24 @@ public void parse() } /** - * Called to parse a {@link org.eclipse.jetty.io.Content.Chunk} - * @param chunk The chunk containing content to parse. The chunk will never be null nor a + *Called by {@link #parse()} to parse a {@link org.eclipse.jetty.io.Content.Chunk}.
+ * + * @param chunk The chunk containing content to parse. The chunk will never be {@code null} nor a * {@link org.eclipse.jetty.io.Content.Chunk#isFailure(Content.Chunk) failure chunk}. - * If references to the content of the chunk are to be held beyond the scope of this call, - * then implementations must call {@link Content.Chunk#retain()} and {@link Content.Chunk#release()} - * as appropriate. - * @return The parsed {@code X} instance or null if parsing is not yet complete - * @throws Throwable Thrown if there is an error parsing + * If the chunk is stored away to be used later beyond the scope of this call, + * then implementations must call {@link Content.Chunk#retain()} and + * {@link Content.Chunk#release()} as appropriate. + * @return The parsed {@code X} result instance or {@code null} if parsing is not yet complete + * @throws Throwable If there is an error parsing */ protected abstract X parse(Content.Chunk chunk) throws Throwable; /** - * @param cause A {@link Content.Chunk#isLast() non-last} + *Callback method that informs the parsing about how to handle transient failures.
+ * + * @param cause A transient failure obtained by reading a {@link Content.Chunk#isLast() non-last} * {@link org.eclipse.jetty.io.Content.Chunk#isFailure(Content.Chunk) failure chunk} - * @return True if the chunk can be ignored. + * @return {@code true} if the transient failure can be ignored, {@code false} otherwise */ protected boolean onTransientFailure(Throwable cause) {