From ec2dbe73a8fb29243820ad822074808c422d056c Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 30 Jun 2023 17:01:16 +0200 Subject: [PATCH] Fully async Multipart Form handling (#9975) A fully async ContentSourceCompletableFuture for use by MultiPartFormData and MultiPartByteRanges Restructure MultiPartFormData to have a Parser class --------- Signed-off-by: Simone Bordet Co-authored-by: Simone Bordet --- .../jetty/docs/programming/ContentDocs.java | 91 ++ .../util/MultiPartRequestContentTest.java | 6 +- .../jetty/http/MultiPartByteRanges.java | 228 +++-- .../eclipse/jetty/http/MultiPartFormData.java | 827 +++++++++--------- .../jetty/http/MultiPartFormDataTest.java | 238 ++--- .../ContentSourceCompletableFuture.java | 144 +++ .../org/eclipse/jetty/server/FormFields.java | 299 ++++--- .../jetty/server/handler/DelayedHandler.java | 102 --- .../eclipse/jetty/server/FormFieldsTest.java | 92 ++ .../jetty/server/MultiPartByteRangesTest.java | 5 +- .../handler/MultiPartFormDataHandlerTest.java | 110 +-- .../ResourceHandlerByteRangesTest.java | 2 +- .../server/handler/gzip/GzipHandlerTest.java | 5 +- .../QuickStartGeneratorConfiguration.java | 2 +- .../jetty/ee10/servlet/Dispatcher.java | 9 + .../jetty/ee10/servlet/EagerFormHandler.java | 83 ++ .../jetty/ee10/servlet/ServletApiRequest.java | 137 +-- .../ee10/servlet/ServletContextRequest.java | 10 + .../jetty/ee10/servlet/ServletHolder.java | 25 +- .../servlet/ServletMultiPartFormData.java | 186 ++-- .../ee10/servlet/MultiPartServletTest.java | 139 +-- .../webapp/StandardDescriptorProcessor.java | 2 +- 22 files changed, 1515 insertions(+), 1227 deletions(-) create mode 100644 jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceCompletableFuture.java 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/EagerFormHandler.java 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 aa3364180f5..3724c4480f8 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 @@ -13,11 +13,18 @@ package org.eclipse.jetty.docs.programming; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.util.concurrent.CompletableFuture; + import org.eclipse.jetty.io.Content; import org.eclipse.jetty.io.content.AsyncContent; +import org.eclipse.jetty.io.content.ContentSourceCompletableFuture; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.CharsetStringBuilder; import org.eclipse.jetty.util.FutureCallback; +import org.eclipse.jetty.util.Utf8StringBuilder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -146,8 +153,92 @@ public class ContentDocs throw new IllegalStateException("EOF expected"); } + public static class FutureString extends CompletableFuture + { + private final CharsetStringBuilder text; + private final Content.Source source; + + public FutureString(Content.Source source, Charset charset) + { + this.source = source; + this.text = CharsetStringBuilder.forCharset(charset); + source.demand(this::onContentAvailable); + } + + private void onContentAvailable() + { + while (true) + { + Content.Chunk chunk = source.read(); + if (chunk == null) + { + source.demand(this::onContentAvailable); + return; + } + + try + { + if (Content.Chunk.isFailure(chunk)) + throw chunk.getFailure(); + + if (chunk.hasRemaining()) + text.append(chunk.getByteBuffer()); + + if (chunk.isLast() && complete(text.build())) + return; + } + catch (Throwable e) + { + completeExceptionally(e); + } + finally + { + chunk.release(); + } + } + } + } + + public static void testFutureString() throws Exception + { + AsyncContent source = new AsyncContent(); + FutureString future = new FutureString(source, StandardCharsets.UTF_8); + if (future.isDone()) + throw new IllegalStateException(); + + Callback.Completable writeCallback = new Callback.Completable(); + Content.Sink.write(source, false, "One", writeCallback); + if (!writeCallback.isDone() || future.isDone()) + throw new IllegalStateException("Should be consumed"); + Content.Sink.write(source, false, "Two", writeCallback); + if (!writeCallback.isDone() || future.isDone()) + throw new IllegalStateException("Should be consumed"); + Content.Sink.write(source, true, "Three", writeCallback); + if (!writeCallback.isDone() || !future.isDone()) + throw new IllegalStateException("Should be consumed"); + } + + public static class FutureUtf8String extends ContentSourceCompletableFuture + { + private final Utf8StringBuilder builder = new Utf8StringBuilder(); + + public FutureUtf8String(Content.Source content) + { + super(content); + } + + @Override + protected String parse(Content.Chunk chunk) throws Throwable + { + if (chunk.hasRemaining()) + builder.append(chunk.getByteBuffer()); + return chunk.isLast() ? builder.takeCompleteString(IllegalStateException::new) : null; + } + } + public static void main(String... args) throws Exception { testEcho(); + testFutureString(); } } diff --git a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/util/MultiPartRequestContentTest.java b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/util/MultiPartRequestContentTest.java index 1a03575314f..f6b598f848f 100644 --- a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/util/MultiPartRequestContentTest.java +++ b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/util/MultiPartRequestContentTest.java @@ -439,12 +439,12 @@ public class MultiPartRequestContentTest extends AbstractHttpClientServerTest String contentType = request.getHeaders().get(HttpHeader.CONTENT_TYPE); assertEquals("multipart/form-data", HttpField.valueParameters(contentType, null)); String boundary = MultiPart.extractBoundary(contentType); - MultiPartFormData formData = new MultiPartFormData(boundary); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser(boundary); formData.setFilesDirectory(tmpDir); - formData.parse(request); + try { - process(formData.join()); // May block waiting for multipart form data. + process(formData.parse(request).join()); // May block waiting for multipart form data. response.write(true, BufferUtil.EMPTY_BUFFER, callback); } catch (Exception x) 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 944d9f1dee5..0efcc9aaf2b 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 @@ -23,12 +23,12 @@ import java.util.List; import java.util.concurrent.CompletableFuture; import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.content.ContentSourceCompletableFuture; import org.eclipse.jetty.util.thread.AutoLock; /** *

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)}.

+ * has been parsed asynchronously from a {@link Content.Source}.

*

Once the parsing of the multipart/byteranges content completes successfully, * objects of this class are completed with a {@link MultiPartByteRanges.Parts} * object.

@@ -52,75 +52,10 @@ import org.eclipse.jetty.util.thread.AutoLock; * * @see Parts */ -public class MultiPartByteRanges extends CompletableFuture +public class MultiPartByteRanges { - private final PartsListener listener = new PartsListener(); - private final MultiPart.Parser parser; - - public MultiPartByteRanges(String boundary) + private MultiPartByteRanges() { - this.parser = new MultiPart.Parser(boundary, listener); - } - - /** - * @return the boundary string - */ - public String getBoundary() - { - return parser.getBoundary(); - } - - @Override - public boolean completeExceptionally(Throwable failure) - { - listener.fail(failure); - return super.completeExceptionally(failure); - } - - /** - *

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}.

- * - * @param content the multipart/byteranges content to parse - * @return this {@code MultiPartByteRanges} object - */ - public MultiPartByteRanges 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 (chunk.isLast() || isDone()) - return; - } - } - }.run(); - return this; - } - - private void parse(Content.Chunk chunk) - { - if (listener.isFailed()) - return; - parser.parse(chunk); } /** @@ -267,76 +202,123 @@ public class MultiPartByteRanges extends CompletableFuture partChunks = new ArrayList<>(); - private final List parts = new ArrayList<>(); - private Throwable failure; + private final PartsListener listener = new PartsListener(); + private final MultiPart.Parser parser; + private Parts parts; - private boolean isFailed() + public Parser(String boundary) { - try (AutoLock ignored = lock.lock()) - { - return failure != null; - } + parser = new MultiPart.Parser(boundary, listener); } - @Override - public void onPartContent(Content.Chunk chunk) + public CompletableFuture parse(Content.Source content) { - try (AutoLock ignored = lock.lock()) + ContentSourceCompletableFuture futureParts = new ContentSourceCompletableFuture<>(content) { - // Retain the chunk because it is stored for later use. - chunk.retain(); - partChunks.add(chunk); - } + @Override + protected MultiPartByteRanges.Parts parse(Content.Chunk chunk) throws Throwable + { + if (listener.isFailed()) + throw listener.failure; + parser.parse(chunk); + if (listener.isFailed()) + throw listener.failure; + return parts; + } + + @Override + public boolean completeExceptionally(Throwable failure) + { + boolean failed = super.completeExceptionally(failure); + if (failed) + listener.fail(failure); + return failed; + } + }; + futureParts.parse(); + return futureParts; } - @Override - public void onPart(String name, String fileName, HttpFields headers) + /** + * @return the boundary string + */ + public String getBoundary() { - try (AutoLock ignored = lock.lock()) - { - parts.add(new MultiPart.ChunksPart(name, fileName, headers, List.copyOf(partChunks))); - partChunks.forEach(Content.Chunk::release); - partChunks.clear(); - } + return parser.getBoundary(); } - @Override - public void onComplete() + private class PartsListener extends MultiPart.AbstractPartsListener { - super.onComplete(); - List copy; - try (AutoLock ignored = lock.lock()) - { - copy = List.copyOf(parts); - } - complete(new Parts(getBoundary(), copy)); - } + private final AutoLock lock = new AutoLock(); + private final List partChunks = new ArrayList<>(); + private final List parts = new ArrayList<>(); + private Throwable failure; - @Override - public void onFailure(Throwable failure) - { - super.onFailure(failure); - completeExceptionally(failure); - } - - private void fail(Throwable cause) - { - List partsToFail; - try (AutoLock ignored = lock.lock()) + private boolean isFailed() { - if (failure != null) - return; - failure = cause; - partsToFail = List.copyOf(parts); - parts.clear(); - partChunks.forEach(Content.Chunk::release); - partChunks.clear(); + try (AutoLock ignored = lock.lock()) + { + return failure != null; + } + } + + @Override + public void onPartContent(Content.Chunk chunk) + { + try (AutoLock ignored = lock.lock()) + { + // Retain the chunk because it is stored for later use. + chunk.retain(); + partChunks.add(chunk); + } + } + + @Override + public void onPart(String name, String fileName, HttpFields headers) + { + try (AutoLock ignored = lock.lock()) + { + parts.add(new MultiPart.ChunksPart(name, fileName, headers, List.copyOf(partChunks))); + partChunks.forEach(Content.Chunk::release); + partChunks.clear(); + } + } + + @Override + public void onComplete() + { + super.onComplete(); + List copy; + try (AutoLock ignored = lock.lock()) + { + copy = List.copyOf(parts); + Parser.this.parts = new Parts(getBoundary(), copy); + } + } + + @Override + public void onFailure(Throwable failure) + { + fail(failure); + } + + private void fail(Throwable cause) + { + List partsToFail; + try (AutoLock ignored = lock.lock()) + { + if (failure != null) + return; + failure = cause; + partsToFail = List.copyOf(parts); + parts.clear(); + partChunks.forEach(Content.Chunk::release); + partChunks.clear(); + } + partsToFail.forEach(p -> p.fail(cause)); } - partsToFail.forEach(p -> p.fail(cause)); } } } 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 851e4c08551..1f58648e699 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 @@ -26,8 +26,11 @@ import java.util.Iterator; import java.util.List; import java.util.Objects; import java.util.concurrent.CompletableFuture; +import java.util.function.Function; import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.content.ContentSourceCompletableFuture; +import org.eclipse.jetty.util.Attributes; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; @@ -37,8 +40,7 @@ import static java.nio.charset.StandardCharsets.US_ASCII; /** *

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)}.

+ * has been parsed asynchronously from a {@link Content.Source}.

*

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 @@ -67,241 +69,31 @@ import static java.nio.charset.StandardCharsets.US_ASCII; * * @see Parts */ -public class MultiPartFormData extends CompletableFuture +public class MultiPartFormData { private static final Logger LOG = LoggerFactory.getLogger(MultiPartFormData.class); - private final PartsListener listener = new PartsListener(); - private final MultiPart.Parser parser; - private boolean useFilesForPartsWithoutFileName; - private Path filesDirectory; - private long maxFileSize = -1; - private long maxMemoryFileSize; - private long maxLength = -1; - private long length; - - public MultiPartFormData(String boundary) + private MultiPartFormData() { - parser = new MultiPart.Parser(Objects.requireNonNull(boundary), listener); } - /** - * @return the boundary string - */ - public String getBoundary() + public static CompletableFuture from(Attributes attributes, String boundary, Function> parse) { - return parser.getBoundary(); - } - - /** - *

Parses the given multipart/form-data content.

- *

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() + @SuppressWarnings("unchecked") + CompletableFuture futureParts = (CompletableFuture)attributes.getAttribute(MultiPartFormData.class.getName()); + if (futureParts == null) { - @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 (chunk.isLast() || isDone()) - 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 - * of the multipart/form-data content completes.

- * - * @param chunk the {@link Content.Chunk} to parse. - */ - public void parse(Content.Chunk chunk) - { - if (listener.isFailed()) - return; - 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); - } - - /** - *

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}.

- * - * @return the default charset specified by the {@code _charset_} part, - * or null if that part is not present - */ - public Charset getDefaultCharset() - { - return listener.getDefaultCharset(); - } - - /** - * @return the max length of a {@link MultiPart.Part} headers, in bytes, or -1 for unlimited length - */ - public int getPartHeadersMaxLength() - { - return parser.getPartHeadersMaxLength(); - } - - /** - * @param partHeadersMaxLength the max length of a {@link MultiPart.Part} headers, in bytes, or -1 for unlimited length - */ - public void setPartHeadersMaxLength(int partHeadersMaxLength) - { - parser.setPartHeadersMaxLength(partHeadersMaxLength); - } - - /** - * @return whether parts without fileName may be stored as files - */ - public boolean isUseFilesForPartsWithoutFileName() - { - return useFilesForPartsWithoutFileName; - } - - /** - * @param useFilesForPartsWithoutFileName whether parts without fileName may be stored as files - */ - public void setUseFilesForPartsWithoutFileName(boolean useFilesForPartsWithoutFileName) - { - this.useFilesForPartsWithoutFileName = useFilesForPartsWithoutFileName; - } - - /** - * @return the directory where files are saved - */ - public Path getFilesDirectory() - { - return filesDirectory; - } - - /** - *

Sets the directory where the files uploaded in the parts will be saved.

- * - * @param filesDirectory the directory where files are saved - */ - public void setFilesDirectory(Path filesDirectory) - { - this.filesDirectory = filesDirectory; - } - - /** - * @return the maximum file size in bytes, or -1 for unlimited file size - */ - public long getMaxFileSize() - { - return maxFileSize; - } - - /** - * @param maxFileSize the maximum file size in bytes, or -1 for unlimited file size - */ - public void setMaxFileSize(long maxFileSize) - { - this.maxFileSize = maxFileSize; - } - - /** - * @return the maximum memory file size in bytes, or -1 for unlimited memory file size - */ - public long getMaxMemoryFileSize() - { - return maxMemoryFileSize; - } - - /** - *

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.

- * - * @param maxMemoryFileSize the maximum memory file size in bytes, or -1 for unlimited memory file size - */ - public void setMaxMemoryFileSize(long maxMemoryFileSize) - { - this.maxMemoryFileSize = maxMemoryFileSize; - } - - /** - * @return the maximum length in bytes of the whole multipart content, or -1 for unlimited length - */ - public long getMaxLength() - { - return maxLength; - } - - /** - * @param maxLength the maximum length in bytes of the whole multipart content, or -1 for unlimited length - */ - public void setMaxLength(long maxLength) - { - this.maxLength = maxLength; - } - - /** - * @return the maximum number of parts that can be parsed from the multipart content. - */ - public long getMaxParts() - { - return parser.getMaxParts(); - } - - /** - * @param maxParts the maximum number of parts that can be parsed from the multipart content. - */ - public void setMaxParts(long maxParts) - { - parser.setMaxParts(maxParts); - } - - @Override - public boolean completeExceptionally(Throwable failure) - { - listener.fail(failure); - return super.completeExceptionally(failure); - } - - // Only used for testing. - int getPartsSize() - { - return listener.getPartsSize(); + futureParts = parse.apply(new Parser(boundary)); + attributes.setAttribute(MultiPartFormData.class.getName(), futureParts); + } + return futureParts; } /** *

An ordered list of {@link MultiPart.Part}s that can * be accessed by index or by name, or iterated over.

*/ - public class Parts implements Iterable, Closeable + public static class Parts implements Iterable, Closeable { private final List parts; @@ -310,11 +102,6 @@ public class MultiPartFormData extends CompletableFutureReturns the {@link MultiPart.Part} at the given index, a number * between {@code 0} included and the value returned by {@link #size()} @@ -409,251 +196,447 @@ public class MultiPartFormData extends CompletableFuture parts = new ArrayList<>(); - private final List partChunks = new ArrayList<>(); - private long fileSize; - private long memoryFileSize; - private Path filePath; - private SeekableByteChannel fileChannel; - private Throwable failure; + private final PartsListener listener = new PartsListener(); + private final MultiPart.Parser parser; + private boolean useFilesForPartsWithoutFileName; + private Path filesDirectory; + private long maxFileSize = -1; + private long maxMemoryFileSize; + private long maxLength = -1; + private long length; + private Parts parts; - @Override - public void onPartContent(Content.Chunk chunk) + public Parser(String boundary) { - ByteBuffer buffer = chunk.getByteBuffer(); - String fileName = getFileName(); - if (fileName != null || isUseFilesForPartsWithoutFileName()) + parser = new MultiPart.Parser(Objects.requireNonNull(boundary), listener); + } + + public CompletableFuture parse(Content.Source content) + { + ContentSourceCompletableFuture futureParts = new ContentSourceCompletableFuture<>(content) { - long maxFileSize = getMaxFileSize(); - fileSize += buffer.remaining(); - if (maxFileSize >= 0 && fileSize > maxFileSize) + @Override + protected Parts parse(Content.Chunk chunk) throws Throwable { - onFailure(new IllegalStateException("max file size exceeded: %d".formatted(maxFileSize))); - return; + if (listener.isFailed()) + throw listener.failure; + length += chunk.getByteBuffer().remaining(); + long max = getMaxLength(); + if (max >= 0 && length > max) + throw new IllegalStateException("max length exceeded: %d".formatted(max)); + parser.parse(chunk); + if (listener.isFailed()) + throw listener.failure; + return parts; } - long maxMemoryFileSize = getMaxMemoryFileSize(); - if (maxMemoryFileSize >= 0) + @Override + public boolean completeExceptionally(Throwable failure) { - memoryFileSize += buffer.remaining(); - if (memoryFileSize > maxMemoryFileSize) - { - try - { - // Must save to disk. - if (ensureFileChannel()) - { - // Write existing memory chunks. - List partChunks; - try (AutoLock ignored = lock.lock()) - { - partChunks = List.copyOf(this.partChunks); - } - for (Content.Chunk c : partChunks) - { - write(c.getByteBuffer()); - } - } - write(buffer); - if (chunk.isLast()) - close(); - } - catch (Throwable x) - { - onFailure(x); - } - - try (AutoLock ignored = lock.lock()) - { - partChunks.forEach(Content.Chunk::release); - partChunks.clear(); - } - return; - } + boolean failed = super.completeExceptionally(failure); + if (failed) + listener.fail(failure); + return failed; } - } - // Retain the chunk because it is stored for later use. - chunk.retain(); - try (AutoLock ignored = lock.lock()) - { - partChunks.add(chunk); - } + }; + futureParts.parse(); + return futureParts; } - private void write(ByteBuffer buffer) throws Exception + /** + * @return the boundary string + */ + public String getBoundary() { - int remaining = buffer.remaining(); - while (remaining > 0) - { - SeekableByteChannel channel = fileChannel(); - if (channel == null) - throw new IllegalStateException(); - int written = channel.write(buffer); - if (written == 0) - throw new NonWritableChannelException(); - remaining -= written; - } + return parser.getBoundary(); } - private void close() + /** + *

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}.

+ * + * @return the default charset specified by the {@code _charset_} part, + * or null if that part is not present + */ + public Charset getDefaultCharset() { - try - { - Closeable closeable = fileChannel(); - if (closeable != null) - closeable.close(); - } - catch (Throwable x) - { - onFailure(x); - } + return listener.getDefaultCharset(); } - @Override - public void onPart(String name, String fileName, HttpFields headers) + /** + * @return the max length of a {@link MultiPart.Part} headers, in bytes, or -1 for unlimited length + */ + public int getPartHeadersMaxLength() { - fileSize = 0; - memoryFileSize = 0; - try (AutoLock ignored = lock.lock()) - { - MultiPart.Part part; - if (fileChannel != null) - part = new MultiPart.PathPart(name, fileName, headers, filePath); - else - part = new MultiPart.ChunksPart(name, fileName, headers, List.copyOf(partChunks)); - // Reset part-related state. - filePath = null; - fileChannel = null; - partChunks.forEach(Content.Chunk::release); - partChunks.clear(); - // Store the new part. - parts.add(part); - } + return parser.getPartHeadersMaxLength(); } - @Override - public void onComplete() + /** + * @param partHeadersMaxLength the max length of a {@link MultiPart.Part} headers, in bytes, or -1 for unlimited length + */ + public void setPartHeadersMaxLength(int partHeadersMaxLength) { - super.onComplete(); - List result; - try (AutoLock ignored = lock.lock()) - { - result = List.copyOf(parts); - } - complete(new Parts(result)); + parser.setPartHeadersMaxLength(partHeadersMaxLength); } - Charset getDefaultCharset() + /** + * @return whether parts without fileName may be stored as files + */ + public boolean isUseFilesForPartsWithoutFileName() { - try (AutoLock ignored = lock.lock()) - { - return parts.stream() - .filter(part -> "_charset_".equals(part.getName())) - .map(part -> part.getContentAsString(US_ASCII)) - .map(Charset::forName) - .findFirst() - .orElse(null); - } + return useFilesForPartsWithoutFileName; } + /** + * @param useFilesForPartsWithoutFileName whether parts without fileName may be stored as files + */ + public void setUseFilesForPartsWithoutFileName(boolean useFilesForPartsWithoutFileName) + { + this.useFilesForPartsWithoutFileName = useFilesForPartsWithoutFileName; + } + + /** + * @return the directory where files are saved + */ + public Path getFilesDirectory() + { + return filesDirectory; + } + + /** + *

Sets the directory where the files uploaded in the parts will be saved.

+ * + * @param filesDirectory the directory where files are saved + */ + public void setFilesDirectory(Path filesDirectory) + { + this.filesDirectory = filesDirectory; + } + + /** + * @return the maximum file size in bytes, or -1 for unlimited file size + */ + public long getMaxFileSize() + { + return maxFileSize; + } + + /** + * @param maxFileSize the maximum file size in bytes, or -1 for unlimited file size + */ + public void setMaxFileSize(long maxFileSize) + { + this.maxFileSize = maxFileSize; + } + + /** + * @return the maximum memory file size in bytes, or -1 for unlimited memory file size + */ + public long getMaxMemoryFileSize() + { + return maxMemoryFileSize; + } + + /** + *

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.

+ * + * @param maxMemoryFileSize the maximum memory file size in bytes, or -1 for unlimited memory file size + */ + public void setMaxMemoryFileSize(long maxMemoryFileSize) + { + this.maxMemoryFileSize = maxMemoryFileSize; + } + + /** + * @return the maximum length in bytes of the whole multipart content, or -1 for unlimited length + */ + public long getMaxLength() + { + return maxLength; + } + + /** + * @param maxLength the maximum length in bytes of the whole multipart content, or -1 for unlimited length + */ + public void setMaxLength(long maxLength) + { + this.maxLength = maxLength; + } + + /** + * @return the maximum number of parts that can be parsed from the multipart content. + */ + public long getMaxParts() + { + return parser.getMaxParts(); + } + + /** + * @param maxParts the maximum number of parts that can be parsed from the multipart content. + */ + public void setMaxParts(long maxParts) + { + parser.setMaxParts(maxParts); + } + + // Only used for testing. int getPartsSize() { - try (AutoLock ignored = lock.lock()) - { - return parts.size(); - } + return listener.getPartsSize(); } - @Override - public void onFailure(Throwable failure) + private class PartsListener extends MultiPart.AbstractPartsListener { - super.onFailure(failure); - completeExceptionally(failure); - } + private final AutoLock lock = new AutoLock(); + private final List parts = new ArrayList<>(); + private final List partChunks = new ArrayList<>(); + private long fileSize; + private long memoryFileSize; + private Path filePath; + private SeekableByteChannel fileChannel; + private Throwable failure; - private void fail(Throwable cause) - { - List partsToFail; - try (AutoLock ignored = lock.lock()) + @Override + public void onPartContent(Content.Chunk chunk) { - if (failure != null) - return; - failure = cause; - partsToFail = List.copyOf(parts); - parts.clear(); - partChunks.forEach(Content.Chunk::release); - partChunks.clear(); - } - partsToFail.forEach(p -> p.fail(cause)); - close(); - delete(); - } + ByteBuffer buffer = chunk.getByteBuffer(); + String fileName = getFileName(); + if (fileName != null || isUseFilesForPartsWithoutFileName()) + { + long maxFileSize = getMaxFileSize(); + fileSize += buffer.remaining(); + if (maxFileSize >= 0 && fileSize > maxFileSize) + { + onFailure(new IllegalStateException("max file size exceeded: %d".formatted(maxFileSize))); + return; + } - private SeekableByteChannel fileChannel() - { - try (AutoLock ignored = lock.lock()) - { - return fileChannel; - } - } + long maxMemoryFileSize = getMaxMemoryFileSize(); + if (maxMemoryFileSize >= 0) + { + memoryFileSize += buffer.remaining(); + if (memoryFileSize > maxMemoryFileSize) + { + try + { + // Must save to disk. + if (ensureFileChannel()) + { + // Write existing memory chunks. + List partChunks; + try (AutoLock ignored = lock.lock()) + { + partChunks = List.copyOf(this.partChunks); + } + for (Content.Chunk c : partChunks) + { + write(c.getByteBuffer()); + } + } + write(buffer); + if (chunk.isLast()) + close(); + } + catch (Throwable x) + { + onFailure(x); + } - private void delete() - { - try - { - Path path = null; + try (AutoLock ignored = lock.lock()) + { + partChunks.forEach(Content.Chunk::release); + partChunks.clear(); + } + return; + } + } + } + // Retain the chunk because it is stored for later use. + chunk.retain(); try (AutoLock ignored = lock.lock()) { - if (filePath != null) - path = filePath; + partChunks.add(chunk); + } + } + + private void write(ByteBuffer buffer) throws Exception + { + int remaining = buffer.remaining(); + while (remaining > 0) + { + SeekableByteChannel channel = fileChannel(); + if (channel == null) + throw new IllegalStateException(); + int written = channel.write(buffer); + if (written == 0) + throw new NonWritableChannelException(); + remaining -= written; + } + } + + private void close() + { + try + { + Closeable closeable = fileChannel(); + if (closeable != null) + closeable.close(); + } + catch (Throwable x) + { + onFailure(x); + } + } + + @Override + public void onPart(String name, String fileName, HttpFields headers) + { + fileSize = 0; + memoryFileSize = 0; + try (AutoLock ignored = lock.lock()) + { + MultiPart.Part part; + if (fileChannel != null) + part = new MultiPart.PathPart(name, fileName, headers, filePath); + else + part = new MultiPart.ChunksPart(name, fileName, headers, List.copyOf(partChunks)); + // Reset part-related state. filePath = null; fileChannel = null; + partChunks.forEach(Content.Chunk::release); + partChunks.clear(); + // Store the new part. + parts.add(part); } - if (path != null) - Files.delete(path); } - catch (Throwable x) - { - if (LOG.isTraceEnabled()) - LOG.trace("IGNORED", x); - } - } - private boolean isFailed() - { - try (AutoLock ignored = lock.lock()) + @Override + public void onComplete() { - return failure != null; + super.onComplete(); + List result; + try (AutoLock ignored = lock.lock()) + { + result = List.copyOf(parts); + Parser.this.parts = new Parts(result); + } } - } - private boolean ensureFileChannel() - { - try (AutoLock ignored = lock.lock()) + Charset getDefaultCharset() { - if (fileChannel != null) - return false; - createFileChannel(); - return true; + try (AutoLock ignored = lock.lock()) + { + return parts.stream() + .filter(part -> "_charset_".equals(part.getName())) + .map(part -> part.getContentAsString(US_ASCII)) + .map(Charset::forName) + .findFirst() + .orElse(null); + } } - } - private void createFileChannel() - { - try (AutoLock ignored = lock.lock()) + int getPartsSize() { - Path directory = getFilesDirectory(); - Files.createDirectories(directory); - String fileName = "MultiPart"; - filePath = Files.createTempFile(directory, fileName, ""); - fileChannel = Files.newByteChannel(filePath, StandardOpenOption.WRITE, StandardOpenOption.APPEND); + try (AutoLock ignored = lock.lock()) + { + return parts.size(); + } } - catch (Throwable x) + + @Override + public void onFailure(Throwable failure) { - onFailure(x); + fail(failure); + } + + private void fail(Throwable cause) + { + List partsToFail; + try (AutoLock ignored = lock.lock()) + { + if (failure != null) + return; + failure = cause; + partsToFail = List.copyOf(parts); + parts.clear(); + partChunks.forEach(Content.Chunk::release); + partChunks.clear(); + } + partsToFail.forEach(p -> p.fail(cause)); + close(); + delete(); + } + + private SeekableByteChannel fileChannel() + { + try (AutoLock ignored = lock.lock()) + { + return fileChannel; + } + } + + private void delete() + { + try + { + Path path = null; + try (AutoLock ignored = lock.lock()) + { + if (filePath != null) + path = filePath; + filePath = null; + fileChannel = null; + } + if (path != null) + Files.delete(path); + } + catch (Throwable x) + { + if (LOG.isTraceEnabled()) + LOG.trace("IGNORED", x); + } + } + + private boolean isFailed() + { + try (AutoLock ignored = lock.lock()) + { + return failure != null; + } + } + + private boolean ensureFileChannel() + { + try (AutoLock ignored = lock.lock()) + { + if (fileChannel != null) + return false; + createFileChannel(); + return true; + } + } + + private void createFileChannel() + { + try (AutoLock ignored = lock.lock()) + { + Path directory = getFilesDirectory(); + Files.createDirectories(directory); + String fileName = "MultiPart"; + filePath = Files.createTempFile(directory, fileName, ""); + fileChannel = Files.newByteChannel(filePath, StandardOpenOption.WRITE, StandardOpenOption.APPEND); + } + catch (Throwable x) + { + onFailure(x); + } } } } 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 a1f771f1ca2..5ccff5406a6 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,19 +16,20 @@ package org.eclipse.jetty.http; 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; import java.util.List; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; 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 +72,19 @@ public class MultiPartFormDataTest 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,14 +97,14 @@ public class MultiPartFormDataTest "Content-Disposition: form-data; name=\"fileup\"; filename=\"test.upload\"\r\n" + "\r\n"; - MultiPartFormData formData = new MultiPartFormData(boundary); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser(boundary); formData.setFilesDirectory(_tmpDir); formData.setMaxFileSize(1024); formData.setMaxLength(3072); formData.setMaxMemoryFileSize(50); - formData.parse(asChunk(str, true)); - - formData.handle((parts, failure) -> + Content.Sink.write(source, true, str, Callback.NOOP); + formData.parse(source).handle((parts, failure) -> { assertNull(parts); assertInstanceOf(BadMessageException.class, failure); @@ -139,14 +127,14 @@ public class MultiPartFormDataTest eol + "--" + boundary + "--" + eol; - MultiPartFormData formData = new MultiPartFormData(boundary); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser(boundary); formData.setFilesDirectory(_tmpDir); formData.setMaxFileSize(1024); formData.setMaxLength(3072); formData.setMaxMemoryFileSize(50); - formData.parse(asChunk(str, true)); - - formData.whenComplete((parts, failure) -> + Content.Sink.write(source, true, str, Callback.NOOP); + formData.parse(source).whenComplete((parts, failure) -> { // No errors and no parts. assertNull(failure); @@ -165,14 +153,14 @@ public class MultiPartFormDataTest String str = eol + "--" + boundary + "--" + eol; - MultiPartFormData formData = new MultiPartFormData(boundary); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser(boundary); formData.setFilesDirectory(_tmpDir); formData.setMaxFileSize(1024); formData.setMaxLength(3072); formData.setMaxMemoryFileSize(50); - formData.parse(asChunk(str, true)); - - formData.whenComplete((parts, failure) -> + Content.Sink.write(source, true, str, Callback.NOOP); + formData.parse(source).whenComplete((parts, failure) -> { // No errors and no parts. assertNull(failure); @@ -213,14 +201,14 @@ public class MultiPartFormDataTest ----\r """; - MultiPartFormData formData = new MultiPartFormData(""); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser(""); formData.setFilesDirectory(_tmpDir); formData.setMaxFileSize(1024); formData.setMaxLength(3072); formData.setMaxMemoryFileSize(50); - formData.parse(asChunk(str, true)); - - try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS)) + Content.Sink.write(source, true, str, Callback.NOOP); + try (MultiPartFormData.Parts parts = formData.parse(source).get(5, TimeUnit.SECONDS)) { assertThat(parts.size(), is(4)); @@ -253,10 +241,10 @@ public class MultiPartFormDataTest @Test public void testNoBody() throws Exception { - MultiPartFormData formData = new MultiPartFormData("boundary"); - formData.parse(Content.Chunk.EOF); - - formData.handle((parts, failure) -> + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser("boundary"); + source.close(); + formData.parse(source).handle((parts, failure) -> { assertNull(parts); assertNotNull(failure); @@ -268,11 +256,11 @@ public class MultiPartFormDataTest @Test public void testBodyWithOnlyCRLF() throws Exception { - MultiPartFormData formData = new MultiPartFormData("boundary"); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser("boundary"); String body = " \n\n\n\r\n\r\n\r\n\r\n"; - formData.parse(asChunk(body, true)); - - formData.handle((parts, failure) -> + Content.Sink.write(source, true, body, Callback.NOOP); + formData.parse(source).handle((parts, failure) -> { assertNull(parts); assertNotNull(failure); @@ -285,7 +273,7 @@ public class MultiPartFormDataTest public void testLeadingWhitespaceBodyWithCRLF() throws Exception { String body = """ - + \r \r @@ -303,14 +291,14 @@ public class MultiPartFormDataTest --AaB03x--\r """; - MultiPartFormData formData = new MultiPartFormData("AaB03x"); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x"); formData.setFilesDirectory(_tmpDir); formData.setMaxFileSize(1024); formData.setMaxLength(3072); formData.setMaxMemoryFileSize(50); - formData.parse(asChunk(body, true)); - - try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS)) + Content.Sink.write(source, true, body, Callback.NOOP); + try (MultiPartFormData.Parts parts = formData.parse(source).get(5, TimeUnit.SECONDS)) { assertThat(parts.size(), is(2)); MultiPart.Part part1 = parts.getFirst("field1"); @@ -340,14 +328,14 @@ public class MultiPartFormDataTest --AaB03x--\r """; - MultiPartFormData formData = new MultiPartFormData("AaB03x"); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x"); formData.setFilesDirectory(_tmpDir); formData.setMaxFileSize(1024); formData.setMaxLength(3072); formData.setMaxMemoryFileSize(50); - formData.parse(asChunk(body, true)); - - try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS)) + Content.Sink.write(source, true, body, Callback.NOOP); + try (MultiPartFormData.Parts parts = formData.parse(source).get(5, TimeUnit.SECONDS)) { // The first boundary must be on a new line, so the first "part" is not recognized as such. assertThat(parts.size(), is(1)); @@ -361,7 +349,8 @@ public class MultiPartFormDataTest @Test public void testDefaultLimits() throws Exception { - MultiPartFormData formData = new MultiPartFormData("AaB03x"); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x"); formData.setFilesDirectory(_tmpDir); String body = """ --AaB03x\r @@ -371,9 +360,8 @@ public class MultiPartFormDataTest ABCDEFGHIJKLMNOPQRSTUVWXYZ\r --AaB03x--\r """; - formData.parse(asChunk(body, true)); - - try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS)) + Content.Sink.write(source, true, body, Callback.NOOP); + try (MultiPartFormData.Parts parts = formData.parse(source).get(5, TimeUnit.SECONDS)) { assertThat(parts.size(), is(1)); MultiPart.Part part = parts.get(0); @@ -390,7 +378,8 @@ public class MultiPartFormDataTest @Test public void testRequestContentTooBig() throws Exception { - MultiPartFormData formData = new MultiPartFormData("AaB03x"); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x"); formData.setFilesDirectory(_tmpDir); formData.setMaxLength(16); @@ -402,9 +391,8 @@ public class MultiPartFormDataTest ABCDEFGHIJKLMNOPQRSTUVWXYZ\r --AaB03x--\r """; - formData.parse(asChunk(body, true)); - - formData.handle((parts, failure) -> + Content.Sink.write(source, true, body, Callback.NOOP); + formData.parse(source).handle((parts, failure) -> { assertNull(parts); assertNotNull(failure); @@ -416,7 +404,8 @@ public class MultiPartFormDataTest @Test public void testFileTooBig() throws Exception { - MultiPartFormData formData = new MultiPartFormData("AaB03x"); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x"); formData.setFilesDirectory(_tmpDir); formData.setMaxFileSize(16); @@ -428,9 +417,8 @@ public class MultiPartFormDataTest ABCDEFGHIJKLMNOPQRSTUVWXYZ\r --AaB03x--\r """; - formData.parse(asChunk(body, true)); - - formData.handle((parts, failure) -> + Content.Sink.write(source, true, body, Callback.NOOP); + formData.parse(source).handle((parts, failure) -> { assertNull(parts); assertNotNull(failure); @@ -442,7 +430,8 @@ public class MultiPartFormDataTest @Test public void testTwoFilesOneInMemoryOneOnDisk() throws Exception { - MultiPartFormData formData = new MultiPartFormData("AaB03x"); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x"); formData.setFilesDirectory(_tmpDir); String chunk = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; formData.setMaxMemoryFileSize(chunk.length() + 1); @@ -460,9 +449,8 @@ public class MultiPartFormDataTest $C$C$C$C\r --AaB03x--\r """.replace("$C", chunk); - formData.parse(asChunk(body, true)); - - try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS)) + Content.Sink.write(source, true, body, Callback.NOOP); + try (MultiPartFormData.Parts parts = formData.parse(source).get(5, TimeUnit.SECONDS)) { assertNotNull(parts); assertEquals(2, parts.size()); @@ -482,7 +470,8 @@ public class MultiPartFormDataTest @Test public void testPartWrite() throws Exception { - MultiPartFormData formData = new MultiPartFormData("AaB03x"); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x"); formData.setFilesDirectory(_tmpDir); String chunk = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; formData.setMaxMemoryFileSize(chunk.length() + 1); @@ -500,9 +489,8 @@ public class MultiPartFormDataTest $C$C$C$C\r --AaB03x--\r """.replace("$C", chunk); - formData.parse(asChunk(body, true)); - - try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS)) + Content.Sink.write(source, true, body, Callback.NOOP); + try (MultiPartFormData.Parts parts = formData.parse(source).get(5, TimeUnit.SECONDS)) { assertNotNull(parts); assertEquals(2, parts.size()); @@ -528,7 +516,8 @@ public class MultiPartFormDataTest @Test public void testPathPartDelete() throws Exception { - MultiPartFormData formData = new MultiPartFormData("AaB03x"); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x"); formData.setFilesDirectory(_tmpDir); String body = """ @@ -539,9 +528,8 @@ public class MultiPartFormDataTest ABCDEFGHIJKLMNOPQRSTUVWXYZ\r --AaB03x--\r """; - formData.parse(asChunk(body, true)); - - try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS)) + Content.Sink.write(source, true, body, Callback.NOOP); + try (MultiPartFormData.Parts parts = formData.parse(source).get(5, TimeUnit.SECONDS)) { assertNotNull(parts); assertEquals(1, parts.size()); @@ -559,7 +547,8 @@ public class MultiPartFormDataTest @Test public void testAbort() { - MultiPartFormData formData = new MultiPartFormData("AaB03x"); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x"); formData.setFilesDirectory(_tmpDir); formData.setMaxMemoryFileSize(32); @@ -575,24 +564,27 @@ public class MultiPartFormDataTest --AaB03x--\r """; // Parse only part of the content. - formData.parse(asChunk(body, false)); + Content.Sink.write(source, false, body, Callback.NOOP); + + CompletableFuture futureParts = formData.parse(source); assertEquals(1, formData.getPartsSize()); // Abort MultiPartFormData. - formData.completeExceptionally(new IOException()); + futureParts.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)); + assertThrows(ExecutionException.class, () -> futureParts.get(5, TimeUnit.SECONDS)); assertEquals(0, formData.getPartsSize()); } @Test public void testMaxHeaderLength() throws Exception { - MultiPartFormData formData = new MultiPartFormData("AaB03x"); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x"); formData.setFilesDirectory(_tmpDir); formData.setPartHeadersMaxLength(32); @@ -604,9 +596,8 @@ public class MultiPartFormDataTest ABCDEFGHIJKLMNOPQRSTUVWXYZ\r --AaB03x--\r """; - formData.parse(asChunk(body, true)); - - formData.handle((parts, failure) -> + Content.Sink.write(source, true, body, Callback.NOOP); + formData.parse(source).handle((parts, failure) -> { assertNull(parts); assertNotNull(failure); @@ -618,7 +609,8 @@ public class MultiPartFormDataTest @Test public void testDefaultCharset() throws Exception { - MultiPartFormData formData = new MultiPartFormData("AaB03x"); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x"); formData.setFilesDirectory(_tmpDir); formData.setMaxMemoryFileSize(-1); @@ -645,13 +637,14 @@ public class MultiPartFormDataTest \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)); + CompletableFuture futureParts = formData.parse(source); + 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)) + try (MultiPartFormData.Parts parts = futureParts.get(5, TimeUnit.SECONDS)) { Charset defaultCharset = formData.getDefaultCharset(); assertEquals(ISO_8859_1, defaultCharset); @@ -669,7 +662,8 @@ public class MultiPartFormDataTest @Test public void testPartWithBackSlashInFileName() throws Exception { - MultiPartFormData formData = new MultiPartFormData("AaB03x"); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x"); formData.setFilesDirectory(_tmpDir); formData.setMaxMemoryFileSize(-1); @@ -681,9 +675,9 @@ public class MultiPartFormDataTest stuffaaa\r --AaB03x--\r """; - formData.parse(asChunk(contents, true)); + Content.Sink.write(source, true, contents, Callback.NOOP); - try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS)) + try (MultiPartFormData.Parts parts = formData.parse(source).get(5, TimeUnit.SECONDS)) { assertThat(parts.size(), is(1)); MultiPart.Part part = parts.get(0); @@ -694,7 +688,8 @@ public class MultiPartFormDataTest @Test public void testPartWithWindowsFileName() throws Exception { - MultiPartFormData formData = new MultiPartFormData("AaB03x"); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x"); formData.setFilesDirectory(_tmpDir); formData.setMaxMemoryFileSize(-1); @@ -706,9 +701,8 @@ public class MultiPartFormDataTest stuffaaa\r --AaB03x--\r """; - formData.parse(asChunk(contents, true)); - - try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS)) + Content.Sink.write(source, true, contents, Callback.NOOP); + try (MultiPartFormData.Parts parts = formData.parse(source).get(5, TimeUnit.SECONDS)) { assertThat(parts.size(), is(1)); MultiPart.Part part = parts.get(0); @@ -722,7 +716,8 @@ public class MultiPartFormDataTest @Disabled public void testCorrectlyEncodedMSFilename() throws Exception { - MultiPartFormData formData = new MultiPartFormData("AaB03x"); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x"); formData.setFilesDirectory(_tmpDir); formData.setMaxMemoryFileSize(-1); @@ -734,9 +729,8 @@ public class MultiPartFormDataTest stuffaaa\r --AaB03x--\r """; - formData.parse(asChunk(contents, true)); - - try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS)) + Content.Sink.write(source, true, contents, Callback.NOOP); + try (MultiPartFormData.Parts parts = formData.parse(source).get(5, TimeUnit.SECONDS)) { assertThat(parts.size(), is(1)); MultiPart.Part part = parts.get(0); @@ -747,7 +741,8 @@ public class MultiPartFormDataTest @Test public void testWriteFilesForPartWithoutFileName() throws Exception { - MultiPartFormData formData = new MultiPartFormData("AaB03x"); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x"); formData.setFilesDirectory(_tmpDir); formData.setUseFilesForPartsWithoutFileName(true); @@ -759,9 +754,8 @@ public class MultiPartFormDataTest sssaaa\r --AaB03x--\r """; - formData.parse(asChunk(body, true)); - - try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS)) + Content.Sink.write(source, true, body, Callback.NOOP); + try (MultiPartFormData.Parts parts = formData.parse(source).get(5, TimeUnit.SECONDS)) { assertThat(parts.size(), is(1)); MultiPart.Part part = parts.get(0); @@ -775,7 +769,8 @@ public class MultiPartFormDataTest @Test public void testPartsWithSameName() throws Exception { - MultiPartFormData formData = new MultiPartFormData("AaB03x"); + AsyncContent source = new TestContent(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser("AaB03x"); formData.setFilesDirectory(_tmpDir); String sameNames = """ @@ -791,9 +786,8 @@ public class MultiPartFormDataTest AAAAA\r --AaB03x--\r """; - formData.parse(asChunk(sameNames, true)); - - try (MultiPartFormData.Parts parts = formData.get(5, TimeUnit.SECONDS)) + Content.Sink.write(source, true, sameNames, Callback.NOOP); + try (MultiPartFormData.Parts parts = formData.parse(source).get(5, TimeUnit.SECONDS)) { assertEquals(2, parts.size()); @@ -810,4 +804,16 @@ public class MultiPartFormDataTest assertEquals("AAAAA", part2.getContentAsString(formData.getDefaultCharset())); } } + + private class TestContent extends AsyncContent + { + @Override + public Content.Chunk read() + { + Content.Chunk chunk = super.read(); + if (chunk != null && chunk.canRetain()) + _allocatedChunks.add(chunk); + return chunk; + } + } } 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 new file mode 100644 index 00000000000..f44d9283261 --- /dev/null +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceCompletableFuture.java @@ -0,0 +1,144 @@ +// +// ======================================================================== +// 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.io.content; + +import java.io.EOFException; +import java.util.concurrent.CompletableFuture; + +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:

+ * 
{@code
+ * 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
+ *     {
+ *         // Accumulate the chunk bytes.
+ *         if (chunk.hasRemaining())
+ *             builder.append(chunk.getByteBuffer());
+ *
+ *         // Not the last chunk, the result is not ready yet.
+ *         if (!chunk.isLast())
+ *             return null;
+ *
+ *         // The result is ready.
+ *         return builder.takeCompleteString(IllegalStateException::new);
+ *     }
+ * }
+ * 
+ * new CompletableUTF8String(source).thenAccept(System.err::println);
+ * }
+ */ +public abstract class ContentSourceCompletableFuture extends CompletableFuture +{ + private final Content.Source _content; + + public ContentSourceCompletableFuture(Content.Source content) + { + _content = content; + } + + /** + *

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() + { + while (true) + { + Content.Chunk chunk = _content.read(); + if (chunk == null) + { + _content.demand(this::parse); + return; + } + if (Content.Chunk.isFailure(chunk)) + { + if (!chunk.isLast() && onTransientFailure(chunk.getFailure())) + continue; + completeExceptionally(chunk.getFailure()); + return; + } + + try + { + X x = parse(chunk); + if (x != null) + { + complete(x); + return; + } + } + catch (Throwable failure) + { + completeExceptionally(failure); + return; + } + finally + { + chunk.release(); + } + + if (chunk.isLast()) + { + completeExceptionally(new EOFException()); + return; + } + } + } + + /** + *

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 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; + + /** + *

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 {@code true} if the transient failure can be ignored, {@code false} otherwise + */ + 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 1f7f949add1..eaf96ccb5b8 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 java.util.concurrent.CompletableFuture; 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,7 +35,7 @@ import static org.eclipse.jetty.util.UrlEncoded.decodeHexByte; * 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 { 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"; @@ -57,29 +59,22 @@ public class FormFields extends CompletableFuture implements Runnable return StringUtil.isEmpty(cs) ? StandardCharsets.UTF_8 : Charset.forName(cs); } - 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); - } - + /** + * 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, CompletableFuture fields) { 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()); @@ -88,26 +83,93 @@ public class FormFields extends CompletableFuture implements Runnable 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); + int maxLength = getRequestAttribute(request, FormFields.MAX_LENGTH_ATTRIBUTE); + 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); + int maxLength = getRequestAttribute(request, FormFields.MAX_LENGTH_ATTRIBUTE); + return from(request, charset, 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 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) { - Object attr = request.getAttribute(FormFields.class.getName()); + 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 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 + * @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()); 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; } @@ -126,7 +188,6 @@ public class FormFields extends CompletableFuture implements Runnable } } - private final Content.Source _source; private final Fields _fields; private final CharsetStringBuilder _builder; private final int _maxFields; @@ -136,9 +197,9 @@ public class FormFields extends CompletableFuture implements Runnable 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) { - _source = source; + super(source); _maxFields = maxFields; _maxLength = maxSize; _builder = CharsetStringBuilder.forCharset(charset); @@ -146,137 +207,91 @@ public class FormFields extends CompletableFuture implements Runnable } @Override - public void run() - { - Content.Chunk chunk = null; - try - { - while (true) - { - chunk = _source.read(); - if (chunk == null) - { - _source.demand(this); - return; - } - - if (Content.Chunk.isFailure(chunk)) - { - completeExceptionally(chunk.getFailure()); - return; - } - - while (true) - { - Fields.Field field = parse(chunk); - if (field == null) - break; - if (_maxFields >= 0 && _fields.getSize() >= _maxFields) - { - chunk.release(); - // Do not double release if completeExceptionally() throws. - chunk = null; - completeExceptionally(new IllegalStateException("form with too many fields")); - return; - } - _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 + protected Fields parse(Content.Chunk chunk) throws CharacterCodingException { String value = null; ByteBuffer buffer = chunk.getByteBuffer(); - loop: - while (BufferUtil.hasContent(buffer)) + + do { - byte b = buffer.get(); - switch (_percent) + loop: + while (BufferUtil.hasContent(buffer)) { - case 1 -> + byte b = buffer.get(); + switch (_percent) { - _percentCode = b; - _percent++; - continue; + case 1 -> + { + _percentCode = b; + _percent++; + continue; + } + case 2 -> + { + _builder.append(decodeHexByte((char)_percentCode, (char)b)); + _percent = 0; + 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); + } + } + else + { + switch (b) + { + case '&' -> + { + value = _builder.build(); + checkLength(value); + break loop; + } + case '+' -> _builder.append((byte)' '); + case '%' -> _percent++; + default -> _builder.append(b); + } } } - if (_name == null) + if (_name != null) { - switch (b) + if (value == null && chunk.isLast()) { - case '=' -> + if (_percent > 0) { - _name = _builder.build(); - checkLength(_name); + _builder.append((byte)'%'); + _builder.append(_percentCode); } - case '+' -> _builder.append((byte)' '); - case '%' -> _percent++; - default -> _builder.append(b); + value = _builder.build(); + checkLength(value); } - } - else - { - switch (b) + + if (value != null) { - case '&' -> - { - value = _builder.build(); - checkLength(value); - break loop; - } - case '+' -> _builder.append((byte)' '); - case '%' -> _percent++; - default -> _builder.append(b); + 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); } } } + while (BufferUtil.hasContent(buffer)); - if (_name != null) - { - if (value == null && chunk.isLast()) - { - if (_percent > 0) - { - _builder.append((byte)'%'); - _builder.append(_percentCode); - } - value = _builder.build(); - checkLength(value); - } - - if (value != null) - { - Fields.Field field = new Fields.Field(_name, value); - _name = null; - return field; - } - } - - return null; + return chunk.isLast() ? _fields : null; } private void checkLength(String nameOrValue) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/DelayedHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/DelayedHandler.java index 5f681da33ec..a978b054f20 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/DelayedHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/DelayedHandler.java @@ -13,7 +13,6 @@ package org.eclipse.jetty.server.handler; -import java.io.IOException; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.Objects; @@ -25,8 +24,6 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.HttpStatus; 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.server.FormFields; import org.eclipse.jetty.server.Handler; @@ -115,7 +112,6 @@ public class DelayedHandler extends Handler.Wrapper return switch (mimeType) { case FORM_ENCODED -> new UntilFormDelayedProcess(handler, request, response, callback, contentType); - case MULTIPART_FORM_DATA -> new UntilMultiPartDelayedProcess(handler, request, response, callback, contentType); default -> new UntilContentDelayedProcess(handler, request, response, callback); }; } @@ -270,102 +266,4 @@ public class DelayedHandler extends Handler.Wrapper Response.writeError(getRequest(), getResponse(), getCallback(), x); } } - - protected static class UntilMultiPartDelayedProcess extends DelayedProcess - { - private final MultiPartFormData _formData; - - public UntilMultiPartDelayedProcess(Handler handler, Request wrapped, Response response, Callback callback, String contentType) - { - super(handler, wrapped, response, callback); - String boundary = MultiPart.extractBoundary(contentType); - _formData = boundary == null ? null : new MultiPartFormData(boundary); - } - - private void process(MultiPartFormData.Parts parts, Throwable x) - { - if (x == null) - { - getRequest().setAttribute(MultiPartFormData.Parts.class.getName(), parts); - super.process(); - } - else - { - Response.writeError(getRequest(), getResponse(), getCallback(), x); - } - } - - private void executeProcess(MultiPartFormData.Parts parts, Throwable x) - { - if (x == null) - { - // We must execute here as even though we have consumed all the input, we are probably - // invoked in a demand runnable that is serialized with any write callbacks that might be done in process - getRequest().getContext().execute(() -> process(parts, x)); - } - else - { - Response.writeError(getRequest(), getResponse(), getCallback(), x); - } - } - - @Override - public void delay() - { - if (_formData == null) - { - this.process(); - } - else - { - _formData.setFilesDirectory(getRequest().getContext().getTempDirectory().toPath()); - readAndParse(); - // if we are done already, then we are still in the scope of the original process call and can - // process directly, otherwise we must execute a call to process as we are within a serialized - // demand callback. - if (_formData.isDone()) - { - try - { - MultiPartFormData.Parts parts = _formData.join(); - process(parts, null); - } - catch (Throwable t) - { - process(null, t); - } - } - else - { - _formData.whenComplete(this::executeProcess); - } - } - } - - private void readAndParse() - { - while (!_formData.isDone()) - { - Content.Chunk chunk = getRequest().read(); - if (chunk == null) - { - getRequest().demand(this::readAndParse); - return; - } - if (Content.Chunk.isFailure(chunk)) - { - _formData.completeExceptionally(chunk.getFailure()); - return; - } - _formData.parse(chunk); - chunk.release(); - if (chunk.isLast()) - { - if (!_formData.isDone()) - process(null, new IOException("Incomplete multipart")); - return; - } - } - } - } } 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 00000000000..cfd0b1ec650 --- /dev/null +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/FormFieldsTest.java @@ -0,0 +1,92 @@ +// +// ======================================================================== +// 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.concurrent.TimeUnit; +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.eclipse.jetty.util.FutureCallback; +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) + throws Exception + { + 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; + FutureCallback eof = new FutureCallback(); + for (int i = 0; i <= last; i++) + source.write(i == last, BufferUtil.toBuffer(chunks.get(i), charset), i == last ? eof : Callback.NOOP); + + + 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()); + + 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/MultiPartByteRangesTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/MultiPartByteRangesTest.java index 6e787edc799..745de4c3ada 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/MultiPartByteRangesTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/MultiPartByteRangesTest.java @@ -119,9 +119,8 @@ public class MultiPartByteRangesTest assertNotNull(contentType); String boundary = MultiPart.extractBoundary(contentType); - MultiPartByteRanges byteRanges = new MultiPartByteRanges(boundary); - byteRanges.parse(new ByteBufferContentSource(ByteBuffer.wrap(response.getContentBytes()))); - MultiPartByteRanges.Parts parts = byteRanges.join(); + MultiPartByteRanges.Parser byteRanges = new MultiPartByteRanges.Parser(boundary); + MultiPartByteRanges.Parts parts = byteRanges.parse(new ByteBufferContentSource(ByteBuffer.wrap(response.getContentBytes()))).join(); assertEquals(3, parts.size()); MultiPart.Part part1 = parts.get(0); 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 d46555dd6c4..e9087a0a8ad 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.net.InetSocketAddress; 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 org.junit.jupiter.api.extension.ExtendWith; 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; @@ -79,7 +76,8 @@ public class MultiPartFormDataHandlerTest public boolean handle(Request request, Response response, Callback callback) { String boundary = MultiPart.extractBoundary(request.getHeaders().get(HttpHeader.CONTENT_TYPE)); - new MultiPartFormData(boundary).parse(request) + new MultiPartFormData.Parser(boundary) + .parse(request) .whenComplete((parts, failure) -> { if (parts != null) @@ -118,72 +116,6 @@ public class MultiPartFormDataHandlerTest } } - @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 { @@ -193,13 +125,15 @@ public class MultiPartFormDataHandlerTest public boolean handle(Request request, Response response, Callback callback) { String boundary = MultiPart.extractBoundary(request.getHeaders().get(HttpHeader.CONTENT_TYPE)); - new MultiPartFormData(boundary).parse(request) + + new MultiPartFormData.Parser(boundary) + .parse(request) .whenComplete((parts, failure) -> { if (parts != null) { - response.getHeaders().put(HttpHeader.CONTENT_TYPE, "multipart/form-data; boundary=\"%s\"".formatted(parts.getMultiPartFormData().getBoundary())); - MultiPartFormData.ContentSource source = new MultiPartFormData.ContentSource(parts.getMultiPartFormData().getBoundary()); + response.getHeaders().put(HttpHeader.CONTENT_TYPE, "multipart/form-data; boundary=\"%s\"".formatted(boundary)); + MultiPartFormData.ContentSource source = new MultiPartFormData.ContentSource(boundary); source.setPartHeadersMaxLength(1024); parts.forEach(source::addPart); source.close(); @@ -310,22 +244,22 @@ public class MultiPartFormDataHandlerTest String boundary = MultiPart.extractBoundary(value); assertNotNull(boundary); - MultiPartFormData formData = new MultiPartFormData(boundary); + ByteBufferContentSource byteBufferContentSource = new ByteBufferContentSource(ByteBuffer.wrap(response.getContentBytes())); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser(boundary); formData.setFilesDirectory(tempDir); - formData.parse(new ByteBufferContentSource(ByteBuffer.wrap(response.getContentBytes()))); - MultiPartFormData.Parts parts = formData.join(); - - assertEquals(2, parts.size()); - MultiPart.Part part1 = parts.get(0); - assertEquals("part1", part1.getName()); - assertEquals("hello", part1.getContentAsString(UTF_8)); - MultiPart.Part part2 = parts.get(1); - assertEquals("part2", part2.getName()); - assertEquals("file2.bin", part2.getFileName()); - HttpFields headers2 = part2.getHeaders(); - assertEquals(2, headers2.size()); - assertEquals("application/octet-stream", headers2.get(HttpHeader.CONTENT_TYPE)); - assertEquals(32, part2.getContentSource().getLength()); + try (MultiPartFormData.Parts parts = formData.parse(byteBufferContentSource).join()) + { + assertEquals(2, parts.size()); + MultiPart.Part part1 = parts.get(0); + assertEquals("part1", part1.getName()); + assertEquals("hello", part1.getContentAsString(UTF_8)); + MultiPart.Part part2 = parts.get(1); + assertEquals("part2", part2.getName()); + assertEquals("file2.bin", part2.getFileName()); + HttpFields headers2 = part2.getHeaders(); + assertEquals(2, headers2.size()); + assertEquals("application/octet-stream", headers2.get(HttpHeader.CONTENT_TYPE)); + } } } } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerByteRangesTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerByteRangesTest.java index f335ace1f6e..70b07e1f5fc 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerByteRangesTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerByteRangesTest.java @@ -175,7 +175,7 @@ public class ResourceHandlerByteRangesTest String contentType = response.get(HttpHeader.CONTENT_TYPE); assertThat(contentType, startsWith(responseContentType)); String boundary = MultiPart.extractBoundary(contentType); - MultiPartByteRanges.Parts parts = new MultiPartByteRanges(boundary) + MultiPartByteRanges.Parts parts = new MultiPartByteRanges.Parser(boundary) .parse(new ByteBufferContentSource(response.getContentByteBuffer())) .join(); assertEquals(2, parts.size()); 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 f7b9c918df9..1c94cd813cb 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 class GzipHandlerTest 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-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 14bd1ce54f6..f99ebd5ad82 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 @@ public class QuickStartGeneratorConfiguration extends AbstractConfiguration } //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 d1bc1702c1f..4cf48e546e7 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 class Dispatcher implements RequestDispatcher { 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/EagerFormHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/EagerFormHandler.java new file mode 100644 index 00000000000..015d051edda --- /dev/null +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/EagerFormHandler.java @@ -0,0 +1,83 @@ +// +// ======================================================================== +// 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 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) + * @see ServletMultiPartFormData#from(ServletRequest) + */ +public class EagerFormHandler extends Handler.Wrapper +{ + public EagerFormHandler() + { + this(null); + } + + public EagerFormHandler(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); + + 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)) + callback.failed(new IllegalStateException("Not Handled")); + } + catch (Throwable x) + { + callback.failed(x); + } + }); + return true; + } +} 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 d1e1d1ab8ad..9dbf56b1265 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 @@ -34,11 +34,11 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import jakarta.servlet.AsyncContext; import jakarta.servlet.DispatcherType; -import jakarta.servlet.MultipartConfigElement; import jakarta.servlet.RequestDispatcher; import jakarta.servlet.ServletConnection; import jakarta.servlet.ServletContext; @@ -489,33 +489,26 @@ public class ServletApiRequest implements HttpServletRequest { if (_parts == null) { - String contentType = getContentType(); - if (contentType == null || !MimeTypes.Type.MULTIPART_FORM_DATA.is(HttpField.valueParameters(contentType, null))) - throw new ServletException("Unsupported Content-Type [%s], expected [%s]".formatted(contentType, MimeTypes.Type.MULTIPART_FORM_DATA.asString())); - - MultipartConfigElement config = (MultipartConfigElement)getAttribute(ServletContextRequest.MULTIPART_CONFIG_ELEMENT); - if (config == null) - throw new IllegalStateException("No multipart config for servlet"); - - ServletContextHandler contextHandler = getServletRequestInfo().getServletContext().getServletContextHandler(); - int maxFormContentSize = contextHandler.getMaxFormContentSize(); - int maxFormKeys = contextHandler.getMaxFormKeys(); - - _parts = ServletMultiPartFormData.from(this, maxFormKeys); - Collection parts = _parts.getParts(); - - String formCharset = null; - Part charsetPart = _parts.getPart("_charset_"); - if (charsetPart != null) + try { - try (InputStream is = charsetPart.getInputStream()) - { - formCharset = IO.toString(is, StandardCharsets.UTF_8); - } - } + CompletableFuture futureServletMultiPartFormData = ServletMultiPartFormData.from(this); - /* - Select Charset to use for this part. (NOTE: charset behavior is for the part value only and not the part header/field names) + _parts = futureServletMultiPartFormData.get(); + + Collection parts = _parts.getParts(); + + String formCharset = null; + Part charsetPart = _parts.getPart("_charset_"); + if (charsetPart != null) + { + try (InputStream is = charsetPart.getInputStream()) + { + formCharset = IO.toString(is, StandardCharsets.UTF_8); + } + } + + /* + Select Charset to use for this part. (NOTE: charset behavior is for the part value only and not the part header/field names) 1. Use the part specific charset as provided in that part's Content-Type header; else 2. Use the overall default charset. Determined by: a. if part name _charset_ exists, use that part's value. @@ -523,38 +516,66 @@ public class ServletApiRequest implements HttpServletRequest (note, this can be either from the charset field on the request Content-Type header, or from a manual call to request.setCharacterEncoding()) c. use utf-8. - */ - Charset defaultCharset; - if (formCharset != null) - defaultCharset = Charset.forName(formCharset); - else if (getCharacterEncoding() != null) - defaultCharset = Charset.forName(getCharacterEncoding()); - else - defaultCharset = StandardCharsets.UTF_8; + */ + Charset defaultCharset; + if (formCharset != null) + defaultCharset = Charset.forName(formCharset); + else if (getCharacterEncoding() != null) + defaultCharset = Charset.forName(getCharacterEncoding()); + else + defaultCharset = StandardCharsets.UTF_8; - long formContentSize = 0; - for (Part p : parts) - { - if (p.getSubmittedFileName() == null) + // Recheck some constraints here, just in case the preloaded parts were not properly configured. + ServletContextHandler servletContextHandler = getServletRequestInfo().getServletContext().getServletContextHandler(); + long maxFormContentSize = servletContextHandler.getMaxFormContentSize(); + int maxFormKeys = servletContextHandler.getMaxFormKeys(); + + long formContentSize = 0; + int count = 0; + for (Part p : parts) { - formContentSize = Math.addExact(formContentSize, p.getSize()); - if (maxFormContentSize >= 0 && formContentSize > maxFormContentSize) - throw new IllegalStateException("Form is larger than max length " + maxFormContentSize); + if (maxFormKeys > 0 && ++count > maxFormKeys) + throw new IllegalStateException("Too many form keys > " + maxFormKeys); - // Servlet Spec 3.0 pg 23, parts without filename must be put into params. - String charset = null; - if (p.getContentType() != null) - charset = MimeTypes.getCharsetFromContentType(p.getContentType()); - - try (InputStream is = p.getInputStream()) + if (p.getSubmittedFileName() == null) { - String content = IO.toString(is, charset == null ? defaultCharset : Charset.forName(charset)); - if (_contentParameters == null) - _contentParameters = new Fields(); - _contentParameters.add(p.getName(), content); + formContentSize = Math.addExact(formContentSize, p.getSize()); + if (maxFormContentSize >= 0 && formContentSize > maxFormContentSize) + throw new IllegalStateException("Form is larger than max length " + maxFormContentSize); + + // Servlet Spec 3.0 pg 23, parts without filename must be put into params. + String charset = null; + if (p.getContentType() != null) + charset = MimeTypes.getCharsetFromContentType(p.getContentType()); + + try (InputStream is = p.getInputStream()) + { + String content = IO.toString(is, charset == null ? defaultCharset : Charset.forName(charset)); + if (_contentParameters == null) + _contentParameters = new Fields(); + _contentParameters.add(p.getName(), content); + } } } } + catch (Throwable t) + { + if (LOG.isDebugEnabled()) + LOG.debug("getParts", t); + + Throwable cause; + if (t instanceof ExecutionException ee) + cause = ee.getCause(); + else if (t instanceof ServletException se) + cause = se.getCause(); + else + cause = t; + + if (cause instanceof IOException ioException) + throw ioException; + + throw new ServletException(new BadMessageException("bad multipart", cause)); + } } return _parts.getParts(); @@ -654,6 +675,7 @@ public class ServletApiRequest implements HttpServletRequest 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(); @@ -867,13 +889,24 @@ public class ServletApiRequest implements HttpServletRequest { getParts(); } - catch (IOException | ServletException e) + catch (IOException e) { String msg = "Unable to extract content parameters"; if (LOG.isDebugEnabled()) LOG.debug(msg, e); throw new RuntimeIOException(msg, e); } + catch (ServletException e) + { + Throwable cause = e.getCause(); + if (cause instanceof BadMessageException badMessageException) + throw badMessageException; + + String msg = "Unable to extract content parameters"; + if (LOG.isDebugEnabled()) + LOG.debug(msg, e); + throw new RuntimeIOException(msg, e); + } } else { 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 b09fc410651..17da4f12ff6 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.HttpFields; 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; @@ -239,6 +240,9 @@ public class ServletContextRequest extends ContextRequest implements ServletCont 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().getMultipartConfigElement(); + case FormFields.MAX_FIELDS_ATTRIBUTE -> getServletContext().getServletContextHandler().getMaxFormKeys(); + case FormFields.MAX_LENGTH_ATTRIBUTE -> getServletContext().getServletContextHandler().getMaxFormContentSize(); default -> super.getAttribute(name); }; } @@ -255,6 +259,12 @@ public class ServletContextRequest extends ContextRequest implements ServletCont 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().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 a3079dc50cc..a290c7d8d8b 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 class ServletHolder extends Holder implements Comparable implements Comparable implements Comparable addMapping(String... urlPatterns) @@ -994,12 +991,12 @@ public class ServletHolder extends Holder implements Comparable implements ComparableServlet 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.

* @@ -49,106 +50,103 @@ import org.eclipse.jetty.util.StringUtil; public class ServletMultiPartFormData { /** - *

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.

- * - * @param request the HTTP request with multipart content - * @return a {@link Parts} object to access the individual {@link Part}s - * @throws IOException if reading the request content fails - * @see org.eclipse.jetty.server.handler.DelayedHandler + * 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 Parts from(ServletApiRequest request) throws IOException + public static CompletableFuture from(ServletRequest servletRequest) { - return from(request, ServletContextHandler.DEFAULT_MAX_FORM_KEYS); + return from(servletRequest, servletRequest.getContentType()); } /** - *

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.

- * - * @param request the HTTP request with multipart content - * @return a {@link Parts} object to access the individual {@link Part}s - * @throws IOException if reading the request content fails - * @see org.eclipse.jetty.server.handler.DelayedHandler + * 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 Parts from(ServletApiRequest request, int maxParts) throws IOException + public static CompletableFuture from(ServletRequest servletRequest, String contentType) { - try + // Look for an existing future (we use the future here rather than the parts as it can remember any failure). + @SuppressWarnings("unchecked") + CompletableFuture futureServletParts = (CompletableFuture)servletRequest.getAttribute(ServletMultiPartFormData.class.getName()); + if (futureServletParts == null) { - // Look for a previously read and parsed MultiPartFormData from the DelayedHandler. - MultiPartFormData.Parts parts = (MultiPartFormData.Parts)request.getAttribute(MultiPartFormData.Parts.class.getName()); - if (parts != null) - return new Parts(parts); + // No existing parts, so we need to try to read them ourselves - // TODO set the files directory - return new ServletMultiPartFormData().parse(request, maxParts); - } - catch (Throwable x) - { - throw IO.rethrow(x); - } - } + // Is this servlet a valid target for Multipart? + MultipartConfigElement config = (MultipartConfigElement)servletRequest.getAttribute(ServletContextRequest.MULTIPART_CONFIG_ELEMENT); + if (config == null) + return CompletableFuture.failedFuture(new IllegalStateException("No multipart configuration element")); - private Parts parse(ServletApiRequest request, int maxParts) throws IOException - { - MultipartConfigElement config = (MultipartConfigElement)request.getAttribute(ServletContextRequest.MULTIPART_CONFIG_ELEMENT); - if (config == null) - throw new IllegalStateException("No multipart configuration element"); + // Are we the right content type to produce our own parts? + if (contentType == null || !MimeTypes.Type.MULTIPART_FORM_DATA.is(HttpField.valueParameters(contentType, null))) + return CompletableFuture.failedFuture(new IllegalStateException("Not multipart Content-Type")); - String boundary = MultiPart.extractBoundary(request.getContentType()); - if (boundary == null) - throw new IllegalStateException("No multipart boundary parameter in Content-Type"); + // Do we have a boundary? + String boundary = MultiPart.extractBoundary(servletRequest.getContentType()); + if (boundary == null) + return CompletableFuture.failedFuture(new IllegalStateException("No multipart boundary parameter in Content-Type")); - // Store MultiPartFormData as attribute on request so it is released by the HttpChannel. - MultiPartFormData formData = new MultiPartFormData(boundary); - formData.setMaxParts(maxParts); + // Can we access the core request, needed for components (eg buffer pools, temp directory, etc.) as well + // as IO optimization + ServletContextRequest servletContextRequest = ServletContextRequest.getServletContextRequest(servletRequest); + if (servletContextRequest == null) + return CompletableFuture.failedFuture(new IllegalStateException("No core request")); - File tmpDirFile = (File)request.getServletContext().getAttribute(ServletContext.TEMPDIR); - if (tmpDirFile == null) - tmpDirFile = new File(System.getProperty("java.io.tmpdir")); - String fileLocation = config.getLocation(); - if (!StringUtil.isBlank(fileLocation)) - tmpDirFile = new File(fileLocation); + // Get a temporary directory for larger parts. + File filesDirectory = StringUtil.isBlank(config.getLocation()) + ? servletContextRequest.getContext().getTempDirectory() + : new File(config.getLocation()); - formData.setFilesDirectory(tmpDirFile.toPath()); - formData.setMaxMemoryFileSize(config.getFileSizeThreshold()); - formData.setMaxFileSize(config.getMaxFileSize()); - formData.setMaxLength(config.getMaxRequestSize()); - ConnectionMetaData connectionMetaData = request.getRequest().getConnectionMetaData(); - formData.setPartHeadersMaxLength(connectionMetaData.getHttpConfiguration().getRequestHeaderSize()); - - ByteBufferPool byteBufferPool = request.getRequest().getComponents().getByteBufferPool(); - Connection connection = connectionMetaData.getConnection(); - int bufferSize = connection instanceof AbstractConnection c ? c.getInputBufferSize() : 2048; - InputStream input = request.getInputStream(); - while (!formData.isDone()) - { - RetainableByteBuffer retainable = byteBufferPool.acquire(bufferSize, false); - boolean readEof = false; - ByteBuffer buffer = retainable.getByteBuffer(); - while (BufferUtil.space(buffer) > bufferSize / 2) + // Look for an existing future MultiPartFormData.Parts + CompletableFuture futureFormData = MultiPartFormData.from(servletContextRequest, boundary, parser -> { - int read = BufferUtil.readFrom(input, buffer); - if (read < 0) + try { - readEof = true; - break; + // No existing core parts, so we need to configure the parser. + ServletContextHandler contextHandler = servletContextRequest.getServletContext().getServletContextHandler(); + ByteBufferPool byteBufferPool = servletContextRequest.getComponents().getByteBufferPool(); + ConnectionMetaData connectionMetaData = servletContextRequest.getConnectionMetaData(); + Connection connection = connectionMetaData.getConnection(); + + Content.Source source; + if (servletRequest instanceof ServletApiRequest servletApiRequest) + { + source = servletApiRequest.getRequest(); + } + else + { + int bufferSize = connection instanceof AbstractConnection c ? c.getInputBufferSize() : 2048; + InputStreamContentSource iscs = new InputStreamContentSource(servletRequest.getInputStream(), byteBufferPool); + iscs.setBufferSize(bufferSize); + source = iscs; + } + + parser.setMaxParts(contextHandler.getMaxFormKeys()); + parser.setFilesDirectory(filesDirectory.toPath()); + parser.setMaxMemoryFileSize(config.getFileSizeThreshold()); + parser.setMaxFileSize(config.getMaxFileSize()); + parser.setMaxLength(config.getMaxRequestSize()); + parser.setPartHeadersMaxLength(connectionMetaData.getHttpConfiguration().getRequestHeaderSize()); + + // parse the core parts. + return parser.parse(source); } - } + catch (Throwable failure) + { + return CompletableFuture.failedFuture(failure); + } + }); - formData.parse(Content.Chunk.from(buffer, false, retainable::release)); - if (readEof) - { - formData.parse(Content.Chunk.EOF); - break; - } + // When available, convert the core parts to servlet parts + futureServletParts = futureFormData.thenApply(formDataParts -> new Parts(filesDirectory.toPath(), formDataParts)); + + // cache the result in attributes. + servletRequest.setAttribute(ServletMultiPartFormData.class.getName(), futureServletParts); } - - Parts parts = new Parts(formData.join()); - request.setAttribute(Parts.class.getName(), parts); - return parts; + return futureServletParts; } /** @@ -158,9 +156,9 @@ public class ServletMultiPartFormData { private final List parts = new ArrayList<>(); - public Parts(MultiPartFormData.Parts parts) + public Parts(Path directory, MultiPartFormData.Parts parts) { - parts.forEach(part -> this.parts.add(new ServletPart(parts.getMultiPartFormData(), part))); + parts.forEach(part -> this.parts.add(new ServletPart(directory, part))); } public Part getPart(String name) @@ -179,12 +177,12 @@ public class ServletMultiPartFormData private static class ServletPart implements Part { - private final MultiPartFormData _formData; + private final Path _directory; private final MultiPart.Part _part; - private ServletPart(MultiPartFormData formData, MultiPart.Part part) + private ServletPart(Path directory, MultiPart.Part part) { - _formData = formData; + _directory = directory; _part = part; } @@ -222,8 +220,8 @@ public class ServletMultiPartFormData public void write(String fileName) throws IOException { Path filePath = Path.of(fileName); - if (!filePath.isAbsolute()) - filePath = _formData.getFilesDirectory().resolve(filePath).normalize(); + if (!filePath.isAbsolute() && Files.isDirectory(_directory)) + filePath = _directory.resolve(filePath).normalize(); _part.writeTo(filePath); } 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 2ccbbde45b9..9e46e56f678 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 @@ -17,7 +17,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.net.Socket; -import java.nio.ByteBuffer; import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; @@ -51,9 +50,9 @@ import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpTester; 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.Server; import org.eclipse.jetty.server.ServerConnector; @@ -62,7 +61,8 @@ 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; @@ -93,32 +93,27 @@ public class MultiPartServletTest tmpDirString = tmpDir.toAbsolutePath().toString(); } - private void start(HttpServlet servlet) throws Exception + private void start(HttpServlet servlet, MultipartConfigElement config, boolean eager) 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); - 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); + + if (eager) + gzipHandler.setHandler(new EagerFormHandler()); + + servletContextHandler.insertHandler(gzipHandler); server.start(); @@ -134,17 +129,18 @@ public class MultiPartServletTest IO.delete(tmpDir.toFile()); } - @Test - public void testLargePart() throws Exception + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testLargePart(boolean eager) 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), eager); OutputStreamRequestContent content = new OutputStreamRequestContent(); MultiPartRequestContent multiPart = new MultiPartRequestContent(); @@ -170,22 +166,23 @@ public class MultiPartServletTest assert400orEof(listener, responseContent -> { - assertThat(responseContent, containsString("Unable to parse form content")); - assertThat(responseContent, containsString("Form is larger than max length")); + assertThat(responseContent, containsString("400: bad")); + assertThat(responseContent, containsString("Form is larger than max length")); }); } - @Test - public void testManyParts() throws Exception + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testManyParts(boolean eager) 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), eager); byte[] byteArray = new byte[1024]; Arrays.fill(byteArray, (byte)1); @@ -208,13 +205,14 @@ public class MultiPartServletTest assert400orEof(listener, responseContent -> { - assertThat(responseContent, containsString("Unable to parse form content")); + assertThat(responseContent, containsString("400: bad")); assertThat(responseContent, containsString("Form with too many keys")); }); } - @Test - public void testMaxRequestSize() throws Exception + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testMaxRequestSize(boolean eager) throws Exception { start(new HttpServlet() { @@ -223,7 +221,7 @@ public class MultiPartServletTest { req.getParameterMap(); } - }, new MultipartConfigElement(tmpDirString, -1, 1024, 1024 * 1024 * 8)); + }, new MultipartConfigElement(tmpDirString, -1, 1024, 1024 * 1024 * 8), eager); OutputStreamRequestContent content = new OutputStreamRequestContent(); MultiPartRequestContent multiPart = new MultiPartRequestContent(); @@ -282,13 +280,14 @@ public class MultiPartServletTest checkbody.accept(responseContent); } - @Test - public void testSimpleMultiPart() throws Exception + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testSimpleMultiPart(boolean eager) 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 +297,10 @@ public class MultiPartServletTest 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, eager); try (Socket socket = new Socket("localhost", connector.getLocalPort())) { @@ -333,21 +332,23 @@ public class MultiPartServletTest } } - @Test - public void testTempFilesDeletedOnError() throws Exception + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testTempFilesDeletedOnError(boolean eager) 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, eager); MultiPartRequestContent multiPart = new MultiPartRequestContent(); multiPart.addPart(new MultiPart.ContentSourcePart("largePart", "largeFile.bin", HttpFields.EMPTY, new BytesRequestContent(bytes))); @@ -361,7 +362,7 @@ public class MultiPartServletTest .body(multiPart) .send(); - assertEquals(500, response.getStatus()); + assertEquals(400, response.getStatus()); assertThat(response.getContentAsString(), containsString("max file size exceeded")); } @@ -370,18 +371,34 @@ public class MultiPartServletTest assertThat(fileList.length, is(0)); } - @Test - public void testMultiPartGzip() throws Exception + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testMultiPartGzip(boolean eager) 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, eager); + // Do not automatically handle gzip. client.getContentDecoderFactories().clear(); @@ -409,19 +426,18 @@ public class MultiPartServletTest String contentType = headers.get(HttpHeader.CONTENT_TYPE); String boundary = MultiPart.extractBoundary(contentType); - MultiPartFormData formData = new MultiPartFormData(boundary); - formData.setMaxParts(1); - InputStream inputStream = new GZIPInputStream(responseStream.getInputStream()); - formData.parse(Content.Chunk.from(ByteBuffer.wrap(IO.readBytes(inputStream)), true)); - MultiPartFormData.Parts parts = formData.join(); + MultiPartFormData.Parser formData = new MultiPartFormData.Parser(boundary); + formData.setMaxParts(1); + MultiPartFormData.Parts parts = formData.parse(new InputStreamContentSource(inputStream)).join(); assertThat(parts.size(), is(1)); assertThat(parts.get(0).getContentAsString(UTF_8), is(contentString)); } - @Test - public void testDoubleReadFromPart() throws Exception + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testDoubleReadFromPart(boolean eager) throws Exception { start(new HttpServlet() { @@ -435,7 +451,7 @@ public class MultiPartServletTest resp.getWriter().println("Part: name=" + part.getName() + ", size=" + part.getSize() + ", content=" + IO.toString(part.getInputStream())); } } - }); + }, null, eager); String contentString = "the quick brown fox jumps over the lazy dog, " + "the quick brown fox jumps over the lazy dog"; @@ -455,8 +471,9 @@ public class MultiPartServletTest "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 eager) throws Exception { start(new HttpServlet() { @@ -471,7 +488,7 @@ public class MultiPartServletTest resp.getWriter().println("Parameter: " + entry.getKey() + "=" + entry.getValue()[0]); } } - }); + }, 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 0e4e9f8d2b0..75a8ec27097 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 class StandardDescriptorProcessor extends IterativeDescriptorProcessor 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());