diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/InputStreamResponseListener.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/InputStreamResponseListener.java index 77cdd8508d0..e0dc2c2b28f 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/InputStreamResponseListener.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/InputStreamResponseListener.java @@ -33,6 +33,7 @@ import java.util.function.Consumer; import org.eclipse.jetty.client.Response.Listener; import org.eclipse.jetty.io.Content; import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.StaticException; import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -301,7 +302,7 @@ public class InputStreamResponseListener extends Listener.Adapter break; if (failure != null) - throw toIOException(failure); + throw new IOException(failure); if (closed) throw new AsynchronousCloseException(); @@ -327,14 +328,6 @@ public class InputStreamResponseListener extends Listener.Adapter } } - private IOException toIOException(Throwable failure) - { - if (failure instanceof IOException) - return (IOException)failure; - else - return new IOException(failure); - } - @Override public void close() throws IOException { diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/MultiPartRequestContent.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/MultiPartRequestContent.java index 88ea11d45dc..b9105fcc487 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/MultiPartRequestContent.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/MultiPartRequestContent.java @@ -69,7 +69,7 @@ public class MultiPartRequestContent extends MultiPartFormData.ContentSource imp if (headers.contains(HttpHeader.CONTENT_TYPE)) return headers; - Content.Source partContent = part.getContent(); + Content.Source partContent = part.getContentSource(); if (partContent instanceof Request.Content requestContent) { String contentType = requestContent.getContentType(); 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 21bb2adfab4..23635f63804 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 @@ -135,7 +135,7 @@ public class MultiPartRequestContentTest extends AbstractHttpClientServerTest int equal = contentType.lastIndexOf('='); Charset charset = Charset.forName(contentType.substring(equal + 1)); assertEquals(encoding, charset); - assertEquals(value, Content.Source.asString(part.getContent(), charset)); + assertEquals(value, Content.Source.asString(part.getContentSource(), charset)); } }); @@ -169,7 +169,7 @@ public class MultiPartRequestContentTest extends AbstractHttpClientServerTest MultiPart.Part part = parts.iterator().next(); assertEquals(name, part.getName()); assertEquals("text/plain", part.getHeaders().get(HttpHeader.CONTENT_TYPE)); - assertArrayEquals(data, Content.Source.asByteBuffer(part.getContent()).array()); + assertArrayEquals(data, Content.Source.asByteBuffer(part.getContentSource()).array()); } }); @@ -221,8 +221,8 @@ public class MultiPartRequestContentTest extends AbstractHttpClientServerTest assertEquals(name, part.getName()); assertEquals(contentType, part.getHeaders().get(HttpHeader.CONTENT_TYPE)); assertEquals(fileName, part.getFileName()); - assertEquals(data.length, part.getContent().getLength()); - assertArrayEquals(data, Content.Source.asByteBuffer(part.getContent()).array()); + assertEquals(data.length, part.getContentSource().getLength()); + assertArrayEquals(data, Content.Source.asByteBuffer(part.getContentSource()).array()); } }); @@ -277,8 +277,8 @@ public class MultiPartRequestContentTest extends AbstractHttpClientServerTest assertEquals(name, part.getName()); assertEquals(contentType, part.getHeaders().get(HttpHeader.CONTENT_TYPE)); assertEquals(tmpPath.getFileName().toString(), part.getFileName()); - assertEquals(Files.size(tmpPath), part.getContent().getLength()); - assertEquals(data, Content.Source.asString(part.getContent(), encoding)); + assertEquals(Files.size(tmpPath), part.getContentSource().getLength()); + assertEquals(data, Content.Source.asString(part.getContentSource(), encoding)); } }); @@ -329,14 +329,14 @@ public class MultiPartRequestContentTest extends AbstractHttpClientServerTest assertEquals(field, fieldPart.getName()); assertEquals(contentType, fieldPart.getHeaders().get(HttpHeader.CONTENT_TYPE)); - assertEquals(value, Content.Source.asString(fieldPart.getContent(), encoding)); + assertEquals(value, Content.Source.asString(fieldPart.getContentSource(), encoding)); assertEquals(headerValue, fieldPart.getHeaders().get(headerName)); assertEquals(fileField, filePart.getName()); assertEquals("application/octet-stream", filePart.getHeaders().get(HttpHeader.CONTENT_TYPE)); assertEquals(tmpPath.getFileName().toString(), filePart.getFileName()); - assertEquals(Files.size(tmpPath), filePart.getContent().getLength()); - assertArrayEquals(data, Content.Source.asByteBuffer(filePart.getContent()).array()); + assertEquals(Files.size(tmpPath), filePart.getContentSource().getLength()); + assertArrayEquals(data, Content.Source.asByteBuffer(filePart.getContentSource()).array()); } }); @@ -373,11 +373,11 @@ public class MultiPartRequestContentTest extends AbstractHttpClientServerTest MultiPart.Part fieldPart = parts.get(0); MultiPart.Part filePart = parts.get(1); - assertEquals(value, Content.Source.asString(fieldPart.getContent(), encoding)); + assertEquals(value, Content.Source.asString(fieldPart.getContentSource(), encoding)); assertEquals("file", filePart.getName()); assertEquals("application/octet-stream", filePart.getHeaders().get(HttpHeader.CONTENT_TYPE)); assertEquals("fileName", filePart.getFileName()); - assertArrayEquals(fileData, Content.Source.asByteBuffer(filePart.getContent()).array()); + assertArrayEquals(fileData, Content.Source.asByteBuffer(filePart.getContentSource()).array()); } }); diff --git a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/HttpStreamOverFCGI.java b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/HttpStreamOverFCGI.java index b4a185e774b..163bda1e6b9 100644 --- a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/HttpStreamOverFCGI.java +++ b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/HttpStreamOverFCGI.java @@ -304,6 +304,12 @@ public class HttpStreamOverFCGI implements HttpStream return _committed; } + @Override + public Throwable consumeAvailable() + { + return HttpStream.consumeAvailable(this, _httpChannel.getConnectionMetaData().getHttpConfiguration()); + } + @Override public void succeeded() { diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java index 7da150ff0ee..6ef5b7d9243 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java @@ -18,7 +18,6 @@ import java.io.EOFException; import java.io.IOException; import java.io.OutputStream; import java.io.UncheckedIOException; -import java.nio.Buffer; import java.nio.ByteBuffer; import java.nio.charset.Charset; import java.nio.file.Files; @@ -40,6 +39,7 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.QuotedStringTokenizer; import org.eclipse.jetty.util.SearchPattern; +import org.eclipse.jetty.util.StaticException; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.Utf8StringBuilder; import org.eclipse.jetty.util.thread.AutoLock; @@ -65,6 +65,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; */ public class MultiPart { + private static final Logger LOG = LoggerFactory.getLogger(MultiPart.class); private static final int MAX_BOUNDARY_LENGTH = 70; private MultiPart() @@ -117,17 +118,33 @@ public class MultiPart *

A part has an optional name, an optional fileName, * optional headers and an optional content.

*/ - public abstract static class Part + public abstract static class Part implements Closeable { + private static final Throwable CLOSE_EXCEPTION = new StaticException("Closed"); + private final String name; private final String fileName; private final HttpFields fields; + private Content.Source contentSource; + private Path path; + private boolean temporary = true; public Part(String name, String fileName, HttpFields fields) + { + this(name, fileName, fields, null); + } + + private Part(String name, String fileName, HttpFields fields, Path path) { this.name = name; this.fileName = fileName; this.fields = fields != null ? fields : HttpFields.EMPTY; + this.path = path; + } + + private Path getPath() + { + return path; } /** @@ -158,7 +175,8 @@ public class MultiPart } /** - *

Returns the content of this part.

+ *

Returns the content of this part as a {@link Content.Source}.

+ *

Calling this method multiple times will return the same instance, which can only be consumed once.

*

The content type and content encoding are specified in this part's * {@link #getHeaders() headers}.

*

The content encoding may be specified by the part named {@code _charset_}, @@ -166,8 +184,34 @@ public class MultiPart * RFC 7578, section 4.6.

* * @return the content of this part + * @see #newContentSource() */ - public abstract Content.Source getContent(); + public Content.Source getContentSource() + { + if (contentSource == null) + contentSource = newContentSource(); + return contentSource; + } + + /** + *

Returns the content of this part as a new {@link Content.Source}

+ *

If the content is reproducible, invoking this method multiple times will return + * a different independent instance for every invocation.

+ *

If the content is not reproducible, subsequent calls to this method will return null.

+ *

The content type and content encoding are specified in this part's {@link #getHeaders() headers}.

+ *

The content encoding may be specified by the part named {@code _charset_}, + * as specified in + * RFC 7578, section 4.6.

+ * + * @return the content of this part as a new {@link Content.Source} or null if the content cannot be consumed multiple times. + * @see #getContentSource() + */ + public abstract Content.Source newContentSource(); + + public long getLength() + { + return getContentSource().getLength(); + } /** *

Returns the content of this part as a string.

@@ -191,7 +235,7 @@ public class MultiPart Charset charset = defaultCharset != null ? defaultCharset : UTF_8; if (charsetName != null) charset = Charset.forName(charsetName); - return Content.Source.asString(getContent(), charset); + return Content.Source.asString(newContentSource(), charset); } catch (IOException x) { @@ -215,9 +259,46 @@ public class MultiPart */ public void writeTo(Path path) throws IOException { - try (OutputStream out = Files.newOutputStream(path)) + if (this.path == null) { - IO.copy(Content.Source.asInputStream(getContent()), out); + try (OutputStream out = Files.newOutputStream(path)) + { + IO.copy(Content.Source.asInputStream(newContentSource()), out); + } + this.path = path; + this.temporary = false; + } + else + { + this.path = Files.move(this.path, path, StandardCopyOption.REPLACE_EXISTING); + this.temporary = false; + } + } + + public void delete() throws IOException + { + if (this.path != null) + Files.delete(this.path); + } + + @Override + public void close() + { + fail(CLOSE_EXCEPTION); + } + + public void fail(Throwable t) + { + try + { + getContentSource().fail(t); + if (temporary) + delete(); + } + catch (Throwable x) + { + if (LOG.isDebugEnabled()) + LOG.debug("Error closing part {}", this, x); } } } @@ -228,8 +309,7 @@ public class MultiPart */ public static class ByteBufferPart extends Part { - private final Content.Source content; - private final long length; + private final List content; public ByteBufferPart(String name, String fileName, HttpFields fields, ByteBuffer... buffers) { @@ -239,14 +319,13 @@ public class MultiPart public ByteBufferPart(String name, String fileName, HttpFields fields, List content) { super(name, fileName, fields); - this.content = new ByteBufferContentSource(content); - this.length = content.stream().mapToLong(Buffer::remaining).sum(); + this.content = content; } @Override - public Content.Source getContent() + public Content.Source newContentSource() { - return content; + return new ByteBufferContentSource(content); } @Override @@ -257,7 +336,7 @@ public class MultiPart hashCode(), getName(), getFileName(), - length + getLength() ); } } @@ -267,20 +346,29 @@ public class MultiPart */ public static class ChunksPart extends Part { - private final Content.Source content; - private final long length; + private final List content; public ChunksPart(String name, String fileName, HttpFields fields, List content) { super(name, fileName, fields); - this.content = new ChunksContentSource(content); - this.length = content.stream().mapToLong(c -> c.getByteBuffer().remaining()).sum(); + this.content = Objects.requireNonNull(content); + content.forEach(Content.Chunk::retain); } @Override - public Content.Source getContent() + public Content.Source newContentSource() { - return content; + List newChunks = content.stream() + .map(chunk -> Content.Chunk.from(chunk.getByteBuffer().slice(), chunk.isLast())) + .toList(); + return new ChunksContentSource(newChunks); + } + + @Override + public void close() + { + super.close(); + content.forEach(Content.Chunk::release); } @Override @@ -291,7 +379,7 @@ public class MultiPart hashCode(), getName(), getFileName(), - length + getLength() ); } } @@ -301,41 +389,20 @@ public class MultiPart */ public static class PathPart extends Part { - private final PathContentSource content; - public PathPart(String name, String fileName, HttpFields fields, Path path) { - super(name, fileName, fields); - this.content = new PathContentSource(path); + super(name, fileName, fields, path); } public Path getPath() { - return content.getPath(); + return super.getPath(); } @Override - public Content.Source getContent() + public Content.Source newContentSource() { - return content; - } - - @Override - public void writeTo(Path path) throws IOException - { - Files.move(getPath(), path, StandardCopyOption.REPLACE_EXISTING); - } - - public void delete() - { - try - { - Files.delete(getPath()); - } - catch (IOException x) - { - throw new UncheckedIOException(x); - } + return new PathContentSource(getPath()); } @Override @@ -356,18 +423,20 @@ public class MultiPart */ public static class ContentSourcePart extends Part { - private final Content.Source content; + private Content.Source content; public ContentSourcePart(String name, String fileName, HttpFields fields, Content.Source content) { super(name, fileName, fields); - this.content = content; + this.content = Objects.requireNonNull(content); } @Override - public Content.Source getContent() + public Content.Source newContentSource() { - return content; + Content.Source c = content; + content = null; + return c; } @Override @@ -378,7 +447,7 @@ public class MultiPart hashCode(), getName(), getFileName(), - content.getLength() + getLength() ); } } @@ -628,7 +697,7 @@ public class MultiPart } case CONTENT -> { - Content.Chunk chunk = part.getContent().read(); + Content.Chunk chunk = part.getContentSource().read(); if (chunk == null || chunk instanceof Content.Chunk.Error) yield chunk; if (!chunk.isLast()) @@ -667,7 +736,7 @@ public class MultiPart if (state == State.CONTENT) { - part.getContent().demand(() -> + part.getContentSource().demand(() -> { try (AutoLock ignoredAgain = lock.lock()) { @@ -688,18 +757,21 @@ public class MultiPart @Override public void fail(Throwable failure) { + Part part; List drained; try (AutoLock ignored = lock.lock()) { - if (closed && parts.isEmpty()) - return; if (errorChunk != null) return; errorChunk = Content.Chunk.from(failure); drained = List.copyOf(parts); parts.clear(); + part = this.part; + this.part = null; } - drained.forEach(part -> part.getContent().fail(failure)); + if (part != null) + part.fail(failure); + drained.forEach(p -> p.fail(failure)); invoker.run(this::invokeDemandCallback); } @@ -763,6 +835,8 @@ public class MultiPart private int trailingWhiteSpaces; private String fieldName; private String fieldValue; + private long maxParts = 1000; + private int numParts; public Parser(String boundary, Listener listener) { @@ -794,6 +868,22 @@ public class MultiPart this.partHeadersMaxLength = partHeadersMaxLength; } + /** + * @return the maximum number of parts that can be parsed from the multipart content (0 for no parts allowed, -1 for unlimited parts). + */ + public long getMaxParts() + { + return maxParts; + } + + /** + * @param maxParts the maximum number of parts that can be parsed from the multipart content (0 for no parts allowed, -1 for unlimited parts). + */ + public void setMaxParts(long maxParts) + { + this.maxParts = maxParts; + } + /** *

Resets this parser to make it ready to parse again a multipart/form-data content.

*/ @@ -852,6 +942,10 @@ public class MultiPart } else if (type == HttpTokens.Type.LF) { + numParts++; + if (maxParts >= 0 && numParts > maxParts) + throw new IllegalStateException(String.format("Form with too many keys [%d > %d]", numParts, maxParts)); + notifyPartBegin(); state = State.HEADER_START; trailingWhiteSpaces = 0; 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 0f9f0a4eb24..a0918c9bd22 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 @@ -244,7 +244,8 @@ public class MultiPartByteRanges extends CompletableFuture toFail; + List partsToFail; + List partChunksToFail; try (AutoLock ignored = lock.lock()) { if (failure != null) return; failure = cause; - toFail = new ArrayList<>(parts); + partsToFail = new ArrayList<>(parts); parts.clear(); + partChunksToFail = new ArrayList<>(partChunks); + partChunks.clear(); } - toFail.forEach(part -> part.getContent().fail(cause)); + partsToFail.forEach(p -> p.fail(cause)); + partChunksToFail.forEach(Content.Chunk::release); } } } 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 e370da19488..0806f446419 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 @@ -28,6 +28,8 @@ import java.util.Objects; import java.util.concurrent.CompletableFuture; import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.Retainable; +import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.QuotedStringTokenizer; import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; @@ -273,6 +275,22 @@ public class MultiPartFormData extends CompletableFutureAn ordered list of {@link MultiPart.Part}s that can * be accessed by index or by name, or iterated over.

*/ - public static class Parts implements Iterable + public class Parts implements Iterable, Closeable { - private final String boundary; private final List parts; - private Parts(String boundary, List parts) + private Parts(List parts) { - this.boundary = boundary; this.parts = parts; } - /** - * @return the boundary string - */ - public String getBoundary() + public MultiPartFormData getMultiPartFormData() { - return boundary; + return MultiPartFormData.this; } /** @@ -364,6 +377,15 @@ public class MultiPartFormData extends CompletableFuture maxMemoryFileSize) { - // Must save to disk. - if (ensureFileChannel()) + try { - // Write existing memory chunks. - for (Content.Chunk c : partChunks) + // Must save to disk. + if (ensureFileChannel()) { - if (!write(c.getByteBuffer())) - return; + // Write existing memory chunks. + for (Content.Chunk c : partChunks) + { + write(c.getByteBuffer()); + } } + write(buffer); + if (chunk.isLast()) + close(); } - write(buffer); - if (chunk.isLast()) - close(); + catch (Throwable x) + { + onFailure(x); + } + + partChunks.forEach(Content.Chunk::release); return; } } @@ -448,24 +478,15 @@ public class MultiPartFormData extends CompletableFuture 0) { - int remaining = buffer.remaining(); - while (remaining > 0) - { - int written = fileChannel.write(buffer); - if (written == 0) - throw new NonWritableChannelException(); - remaining -= written; - } - return true; - } - catch (Throwable x) - { - onFailure(x); - return false; + int written = fileChannel.write(buffer); + if (written == 0) + throw new NonWritableChannelException(); + remaining -= written; } } @@ -496,6 +517,7 @@ public class MultiPartFormData extends CompletableFuture getParts() @@ -528,22 +550,20 @@ public class MultiPartFormData extends CompletableFuture toFail; + List partsToFail; + List partChunksToFail; try (AutoLock ignored = lock.lock()) { if (failure != null) return; failure = cause; - toFail = new ArrayList<>(parts); + partsToFail = new ArrayList<>(parts); parts.clear(); + partChunksToFail = new ArrayList<>(partChunks); + partChunks.clear(); } - for (MultiPart.Part part : toFail) - { - if (part instanceof MultiPart.PathPart pathPart) - pathPart.delete(); - else - part.getContent().fail(cause); - } + partsToFail.forEach(p -> p.fail(cause)); + partChunksToFail.forEach(Retainable::release); close(); delete(); } diff --git a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartCaptureTest.java b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartCaptureTest.java index 146bb555c4f..8dabc7d7f11 100644 --- a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartCaptureTest.java +++ b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartCaptureTest.java @@ -51,7 +51,6 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; -import static org.junit.jupiter.api.Assertions.assertTrue; public class MultiPartCaptureTest { @@ -245,7 +244,7 @@ public class MultiPartCaptureTest List charSetParts = allParts.get("_charset_"); if (charSetParts != null) { - defaultCharset = Promise.Completable.with(p -> Content.Source.asString(charSetParts.get(0).getContent(), StandardCharsets.US_ASCII, p)) + defaultCharset = Promise.Completable.with(p -> Content.Source.asString(charSetParts.get(0).getContentSource(), StandardCharsets.US_ASCII, p)) .get(); } @@ -255,8 +254,7 @@ public class MultiPartCaptureTest assertThat("Part[" + expected.name + "]", parts, is(notNullValue())); MultiPart.Part part = parts.get(0); String charset = getCharsetFromContentType(part.getHeaders().get(HttpHeader.CONTENT_TYPE), defaultCharset); - assertTrue(part.getContent().rewind()); - String partContent = Content.Source.asString(part.getContent(), Charset.forName(charset)); + String partContent = Content.Source.asString(part.newContentSource(), Charset.forName(charset)); assertThat("Part[" + expected.name + "].contents", partContent, containsString(expected.value)); } @@ -276,8 +274,7 @@ public class MultiPartCaptureTest assertThat("Part[" + expected.name + "]", parts, is(notNullValue())); MultiPart.Part part = parts.get(0); MessageDigest digest = MessageDigest.getInstance("SHA1"); - assertTrue(part.getContent().rewind()); - try (InputStream partInputStream = Content.Source.asInputStream(part.getContent()); + try (InputStream partInputStream = Content.Source.asInputStream(part.newContentSource()); DigestOutputStream digester = new DigestOutputStream(OutputStream.nullOutputStream(), digest)) { IO.copy(partInputStream, digester); 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 7b30c229326..2fb00ea3b94 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 @@ -189,25 +189,25 @@ public class MultiPartFormDataTest MultiPart.Part fileName = parts.getFirst("fileName"); assertThat(fileName, notNullValue()); - Content.Source partContent = fileName.getContent(); + Content.Source partContent = fileName.getContentSource(); assertThat(partContent.getLength(), is(3L)); assertThat(Content.Source.asString(partContent), is("abc")); MultiPart.Part desc = parts.getFirst("desc"); assertThat(desc, notNullValue()); - partContent = desc.getContent(); + partContent = desc.getContentSource(); assertThat(partContent.getLength(), is(3L)); assertThat(Content.Source.asString(partContent), is("123")); MultiPart.Part title = parts.getFirst("title"); assertThat(title, notNullValue()); - partContent = title.getContent(); + partContent = title.getContentSource(); assertThat(partContent.getLength(), is(3L)); assertThat(Content.Source.asString(partContent), is("ttt")); MultiPart.Part datafile = parts.getFirst("datafile5239138112980980385.txt"); assertThat(datafile, notNullValue()); - partContent = datafile.getContent(); + partContent = datafile.getContentSource(); assertThat(partContent.getLength(), is(3L)); assertThat(Content.Source.asString(partContent), is("000")); } @@ -275,11 +275,11 @@ public class MultiPartFormDataTest assertThat(parts.size(), is(2)); MultiPart.Part part1 = parts.getFirst("field1"); assertThat(part1, notNullValue()); - Content.Source partContent = part1.getContent(); + Content.Source partContent = part1.getContentSource(); assertThat(Content.Source.asString(partContent), is("Joe Blow")); MultiPart.Part part2 = parts.getFirst("stuff"); assertThat(part2, notNullValue()); - partContent = part2.getContent(); + partContent = part2.getContentSource(); assertThat(Content.Source.asString(partContent), is("aaaabbbbb")); } @@ -312,7 +312,7 @@ public class MultiPartFormDataTest assertThat(parts.size(), is(1)); MultiPart.Part part2 = parts.getFirst("stuff"); assertThat(part2, notNullValue()); - Content.Source partContent = part2.getContent(); + Content.Source partContent = part2.getContentSource(); assertThat(Content.Source.asString(partContent), is("aaaabbbbb")); } @@ -340,7 +340,7 @@ public class MultiPartFormDataTest assertThat(part, instanceOf(MultiPart.PathPart.class)); MultiPart.PathPart pathPart = (MultiPart.PathPart)part; assertTrue(Files.exists(pathPart.getPath())); - assertEquals("ABCDEFGHIJKLMNOPQRSTUVWXYZ", Content.Source.asString(part.getContent())); + assertEquals("ABCDEFGHIJKLMNOPQRSTUVWXYZ", Content.Source.asString(part.getContentSource())); } @Test @@ -422,13 +422,13 @@ public class MultiPartFormDataTest MultiPart.Part part1 = parts.get(0); assertThat(part1, instanceOf(MultiPart.ChunksPart.class)); - assertEquals(chunk, Content.Source.asString(part1.getContent())); + assertEquals(chunk, Content.Source.asString(part1.getContentSource())); MultiPart.Part part2 = parts.get(1); assertThat(part2, instanceOf(MultiPart.PathPart.class)); MultiPart.PathPart pathPart2 = (MultiPart.PathPart)part2; assertTrue(Files.exists(pathPart2.getPath())); - assertEquals(chunk.repeat(4), Content.Source.asString(part2.getContent())); + assertEquals(chunk.repeat(4), Content.Source.asString(part2.getContentSource())); } @Test diff --git a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartTest.java b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartTest.java index 88cc21ba741..d1d4a3524c8 100644 --- a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartTest.java +++ b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartTest.java @@ -361,11 +361,11 @@ public class MultiPartTest MultiPart.Part part1 = listener.parts.get(0); assertEquals("value", part1.getHeaders().get("name")); - assertEquals("Hello", Content.Source.asString(part1.getContent())); + assertEquals("Hello", Content.Source.asString(part1.getContentSource())); MultiPart.Part part2 = listener.parts.get(1); assertEquals("9001", part2.getHeaders().get("powerLevel")); - assertEquals("secondary\r\ncontent", Content.Source.asString(part2.getContent())); + assertEquals("secondary\r\ncontent", Content.Source.asString(part2.getContentSource())); assertEquals(0, data.remaining()); } @@ -397,11 +397,11 @@ public class MultiPartTest MultiPart.Part part1 = listener.parts.get(0); assertEquals("value", part1.getHeaders().get("name")); - assertEquals("Hello", Content.Source.asString(part1.getContent())); + assertEquals("Hello", Content.Source.asString(part1.getContentSource())); MultiPart.Part part2 = listener.parts.get(1); assertEquals("9001", part2.getHeaders().get("powerLevel")); - assertEquals("secondary\ncontent", Content.Source.asString(part2.getContent())); + assertEquals("secondary\ncontent", Content.Source.asString(part2.getContentSource())); assertEquals(0, data.remaining()); } @@ -457,7 +457,7 @@ public class MultiPartTest assertEquals(1, listener.parts.size()); MultiPart.Part part = listener.parts.get(0); assertEquals("value", part.getHeaders().get("name")); - assertEquals("", Content.Source.asString(part.getContent())); + assertEquals("", Content.Source.asString(part.getContentSource())); } @Test @@ -477,7 +477,7 @@ public class MultiPartTest assertEquals(1, listener.parts.size()); MultiPart.Part part = listener.parts.get(0); assertEquals("value", part.getHeaders().get("name")); - assertEquals("", Content.Source.asString(part.getContent())); + assertEquals("", Content.Source.asString(part.getContentSource())); } @Test @@ -508,7 +508,7 @@ public class MultiPartTest assertEquals(1, listener.parts.size()); MultiPart.Part part = listener.parts.get(0); assertEquals("value", part.getHeaders().get("name")); - assertThat(Content.Source.asString(part.getContent()), is(""" + assertThat(Content.Source.asString(part.getContentSource()), is(""" Hello\r this is not a --BOUNDARY\r that's a boundary""")); @@ -532,7 +532,7 @@ public class MultiPartTest assertThat(epilogueBuffer.remaining(), is(0)); assertEquals(1, listener.parts.size()); MultiPart.Part part = listener.parts.get(0); - assertThat(Content.Source.asByteBuffer(part.getContent()), is(ByteBuffer.wrap(random))); + assertThat(Content.Source.asByteBuffer(part.getContentSource()), is(ByteBuffer.wrap(random))); } @Test @@ -556,7 +556,7 @@ public class MultiPartTest assertEquals(1, listener.parts.size()); MultiPart.Part part = listener.parts.get(0); assertEquals("value", part.getHeaders().get("name")); - assertEquals("Hello", Content.Source.asString(part.getContent())); + assertEquals("Hello", Content.Source.asString(part.getContentSource())); } @Test diff --git a/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HttpStreamOverHTTP2.java b/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HttpStreamOverHTTP2.java index 4e82907054c..2e82d6f2549 100644 --- a/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HttpStreamOverHTTP2.java +++ b/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HttpStreamOverHTTP2.java @@ -564,7 +564,7 @@ public class HttpStreamOverHTTP2 implements HttpStream, HTTP2Channel.Server { if (tunnelSupport != null) return null; - return HttpStream.super.consumeAvailable(); + return HttpStream.consumeAvailable(this, _httpChannel.getConnectionMetaData().getHttpConfiguration()); } @Override diff --git a/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/HttpStreamOverHTTP3.java b/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/HttpStreamOverHTTP3.java index c55b86c886f..67b048d58f5 100644 --- a/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/HttpStreamOverHTTP3.java +++ b/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/HttpStreamOverHTTP3.java @@ -487,6 +487,14 @@ public class HttpStreamOverHTTP3 implements HttpStream return committed; } + @Override + public Throwable consumeAvailable() + { + if (getTunnelSupport() != null) + return null; + return HttpStream.consumeAvailable(this, httpChannel.getConnectionMetaData().getHttpConfiguration()); + } + public boolean isIdle() { // TODO: is this necessary? diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ChunksContentSource.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ChunksContentSource.java index f4433f56488..8b88e4ddce8 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ChunksContentSource.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ChunksContentSource.java @@ -40,6 +40,7 @@ public class ChunksContentSource implements Content.Source public ChunksContentSource(Collection chunks) { + chunks.forEach(Content.Chunk::retain); this.chunks = chunks; this.length = chunks.stream().mapToLong(c -> c.getByteBuffer().remaining()).sum(); } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java index dfebdb3e6e1..6797c807809 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java @@ -79,6 +79,7 @@ public class HttpConfiguration implements Dumpable private boolean _relativeRedirectAllowed; private HostPort _serverAuthority; private SocketAddress _localAddress; + private int _maxUnconsumedRequestContentReads = 16; /** *

An interface that allows a request object to be customized @@ -716,6 +717,28 @@ public class HttpConfiguration implements Dumpable _serverAuthority = authority; } + /** + * Sets the maximum amount of {@link HttpStream#read()}s that can be done by the {@link HttpStream} if the content is not + * fully consumed by the application. If this is unable to consume to EOF then the connection will be made non-persistent. + * + * @param maxUnconsumedRequestContentReads the maximum amount of reads for unconsumed content or -1 for unlimited. + */ + public void setMaxUnconsumedRequestContentReads(int maxUnconsumedRequestContentReads) + { + _maxUnconsumedRequestContentReads = maxUnconsumedRequestContentReads; + } + + /** + * Gets the maximum amount of {@link HttpStream#read()}s that can be done by the {@link HttpStream} if the content is not + * fully consumed by the application. If this is unable to consume to EOF then the connection will be made non-persistent. + * + * @return the maximum amount of reads for unconsumed content or -1 for unlimited. + */ + public int getMaxUnconsumedRequestContentReads() + { + return _maxUnconsumedRequestContentReads; + } + @Override public String dump() { diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpStream.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpStream.java index 6c228c2be04..2d784b71e51 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpStream.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpStream.java @@ -13,7 +13,6 @@ package org.eclipse.jetty.server; -import java.io.IOException; import java.nio.ByteBuffer; import org.eclipse.jetty.http.HttpFields; @@ -22,6 +21,7 @@ import org.eclipse.jetty.io.Connection; import org.eclipse.jetty.io.Content; import org.eclipse.jetty.io.Content.Chunk; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.StaticException; /** * A HttpStream is an abstraction that together with {@link MetaData.Request}, represents the @@ -31,6 +31,8 @@ import org.eclipse.jetty.util.Callback; */ public interface HttpStream extends Callback { + Exception CONTENT_NOT_CONSUMED = new StaticException("Content not consumed"); + /** *

Attribute name to be used as a {@link Request} attribute to store/retrieve * the {@link Connection} created during the HTTP/1.1 upgrade mechanism or the @@ -104,27 +106,34 @@ public interface HttpStream extends Callback return null; } - default Throwable consumeAvailable() + Throwable consumeAvailable(); + + static Throwable consumeAvailable(HttpStream stream, HttpConfiguration httpConfig) { - while (true) + int numReads = 0; + int maxReads = httpConfig.getMaxUnconsumedRequestContentReads(); + while (maxReads < 0 || numReads < maxReads) { // We can always just read again here as EOF and Error content will be persistently returned. - Content.Chunk content = read(); + Chunk content = stream.read(); + numReads++; // if we cannot read to EOF then fail the stream rather than wait for unconsumed content if (content == null) - return new IOException("Content not consumed"); + return CONTENT_NOT_CONSUMED; // Always release any returned content. This is a noop for EOF and Error content. content.release(); // if the input failed, then fail the stream for same reason - if (content instanceof Content.Chunk.Error error) + if (content instanceof Chunk.Error error) return error.getCause(); if (content.isLast()) return null; } + + return CONTENT_NOT_CONSUMED; } class Wrapper implements HttpStream diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 24bf2a9c061..0f62e099deb 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -228,6 +228,16 @@ public interface Request extends Attributes, Content.Source @Override Content.Chunk read(); + /** + * Consume any available content. This bypasses any request wrappers to process the content in + * {@link Request#read()} and reads directly from the {@link HttpStream}. This reads until + * there is no content currently available or it reaches EOF. + * The {@link HttpConfiguration#setMaxUnconsumedRequestContentReads(int)} configuration can be used + * to configure how many reads will be attempted by this method. + * @return true if the content was fully consumed. + */ + boolean consumeAvailable(); + /** *

Pushes the given {@code resource} to the client.

* @@ -616,6 +626,12 @@ public interface Request extends Attributes, Content.Source return getWrapped().read(); } + @Override + public boolean consumeAvailable() + { + return getWrapped().consumeAvailable(); + } + @Override public void demand(Runnable demandCallback) { diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java index 3df2552217e..bb980f6dc68 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java @@ -316,13 +316,17 @@ public interface Response extends Content.Sink } static void ensureConsumeAvailableOrNotPersistent(Request request, Response response) + { + if (request.consumeAvailable()) + return; + ensureNotPersistent(request, response); + } + + static void ensureNotPersistent(Request request, Response response) { switch (request.getConnectionMetaData().getHttpVersion()) { case HTTP_1_0: - if (consumeAvailable(request)) - return; - // Remove any keep-alive value in Connection headers response.getHeaders().computeField(HttpHeader.CONNECTION, (h, fields) -> { @@ -339,9 +343,6 @@ public interface Response extends Content.Sink break; case HTTP_1_1: - if (consumeAvailable(request)) - return; - // Add close value to Connection headers response.getHeaders().computeField(HttpHeader.CONNECTION, (h, fields) -> { @@ -375,19 +376,6 @@ public interface Response extends Content.Sink } } - static boolean consumeAvailable(Request request) - { - while (true) - { - Content.Chunk chunk = request.read(); - if (chunk == null) - return false; - chunk.release(); - if (chunk.isLast()) - return true; - } - } - class Wrapper implements Response { private final Request _request; 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 73f9cb5c9c2..c695f867bcb 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 @@ -270,13 +270,13 @@ public class DelayedHandler extends Handler.Wrapper super(handler, wrapped, response, callback); String boundary = MultiPart.extractBoundary(contentType); _formData = boundary == null ? null : new MultiPartFormData(boundary); - getRequest().setAttribute(MultiPartFormData.class.getName(), _formData); } private void process(MultiPartFormData.Parts parts, Throwable x) { if (x == null) { + getRequest().setAttribute(MultiPartFormData.Parts.class.getName(), parts); super.process(); } else @@ -291,7 +291,7 @@ public class DelayedHandler extends Handler.Wrapper { // 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(super::process); + getRequest().getContext().execute(() -> process(parts, x)); } else { @@ -304,7 +304,7 @@ public class DelayedHandler extends Handler.Wrapper { if (_formData == null) { - super.process(); + this.process(); } else { @@ -313,13 +313,28 @@ public class DelayedHandler extends Handler.Wrapper // 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. - _formData.whenComplete(_formData.isDone() ? this::process : this::executeProcess); + 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 (true) + while (!_formData.isDone()) { Content.Chunk chunk = getRequest().read(); if (chunk == null) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java index 05aad2237c4..93664d28cc1 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java @@ -34,6 +34,7 @@ import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.http.MultiPartFormData.Parts; import org.eclipse.jetty.http.PreEncodedHttpField; import org.eclipse.jetty.http.Trailers; import org.eclipse.jetty.http.UriCompliance; @@ -647,6 +648,11 @@ public class HttpChannelState implements HttpChannel, Components requestLog.log(_request.getLoggedRequest(), _request._response); } + + // Clean up any multipart tmp files and release any associated resources. + Parts parts = (Parts)_request.getAttribute(Parts.class.getName()); + if (parts != null) + parts.close(); } finally { @@ -874,6 +880,19 @@ public class HttpChannelState implements HttpChannel, Components return chunk; } + @Override + public boolean consumeAvailable() + { + HttpStream stream; + try (AutoLock ignored = _lock.lock()) + { + HttpChannelState httpChannel = lockedGetHttpChannel(); + stream = httpChannel._stream; + } + + return stream.consumeAvailable() == null; + } + @Override public void demand(Runnable demandCallback) { diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java index 6c942d8cd45..0d8beb431d0 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java @@ -52,6 +52,7 @@ import org.eclipse.jetty.io.Content; import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.io.RetainableByteBuffer; +import org.eclipse.jetty.io.RuntimeIOException; import org.eclipse.jetty.io.WriteFlusher; import org.eclipse.jetty.io.ssl.SslConnection; import org.eclipse.jetty.server.ConnectionFactory; @@ -594,6 +595,8 @@ public class HttpConnection extends AbstractConnection implements Runnable, Writ if (LOG.isDebugEnabled()) LOG.debug("{} parse {}", this, _retainableByteBuffer); + if (_parser.isTerminated()) + throw new RuntimeIOException("Parser is terminated"); boolean handle = _parser.parseNext(_retainableByteBuffer == null ? BufferUtil.EMPTY_BUFFER : _retainableByteBuffer.getByteBuffer()); if (LOG.isDebugEnabled()) @@ -1136,6 +1139,15 @@ public class HttpConnection extends AbstractConnection implements Runnable, Writ _uri.path("/"); } + @Override + public Throwable consumeAvailable() + { + Throwable result = HttpStream.consumeAvailable(this, getHttpConfiguration()); + if (result != null) + _generator.setPersistent(false); + return result; + } + public void parsedHeader(HttpField field) { HttpHeader header = field.getHeader(); diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/MockHttpStream.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/MockHttpStream.java index 526d0129ae1..d183dbde981 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/MockHttpStream.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/MockHttpStream.java @@ -225,6 +225,12 @@ public class MockHttpStream implements HttpStream return response != null && response.getStatus() >= 200; } + @Override + public Throwable consumeAvailable() + { + return HttpStream.consumeAvailable(this, new HttpConfiguration()); + } + public boolean isComplete() { return _completed.getCount() == 0; 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 5aafb96a91a..96b07fecfb0 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,11 +119,11 @@ public class MultiPartByteRangesTest assertEquals(3, parts.size()); MultiPart.Part part1 = parts.get(0); - assertEquals("12", Content.Source.asString(part1.getContent())); + assertEquals("12", Content.Source.asString(part1.getContentSource())); MultiPart.Part part2 = parts.get(1); - assertEquals("456", Content.Source.asString(part2.getContent())); + assertEquals("456", Content.Source.asString(part2.getContentSource())); MultiPart.Part part3 = parts.get(2); - assertEquals("CDEF", Content.Source.asString(part3.getContent())); + assertEquals("CDEF", Content.Source.asString(part3.getContentSource())); } } } 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 2c449efe687..e7619610811 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 @@ -80,7 +80,7 @@ public class MultiPartFormDataHandlerTest .whenComplete((parts, failure) -> { if (parts != null) - Content.copy(parts.get(0).getContent(), response, callback); + Content.copy(parts.get(0).getContentSource(), response, callback); else Response.writeError(request, response, callback, failure); }); @@ -126,10 +126,10 @@ public class MultiPartFormDataHandlerTest public boolean process(Request request, Response response, Callback callback) throws Exception { processLatch.countDown(); - MultiPartFormData formData = (MultiPartFormData)request.getAttribute(MultiPartFormData.class.getName()); - assertNotNull(formData); - MultiPart.Part part = formData.get().get(0); - Content.copy(part.getContent(), response, callback); + 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; } }); @@ -195,8 +195,8 @@ public class MultiPartFormDataHandlerTest { if (parts != null) { - response.getHeaders().put(HttpHeader.CONTENT_TYPE, "multipart/form-data; boundary=\"%s\"".formatted(parts.getBoundary())); - MultiPartFormData.ContentSource source = new MultiPartFormData.ContentSource(parts.getBoundary()); + response.getHeaders().put(HttpHeader.CONTENT_TYPE, "multipart/form-data; boundary=\"%s\"".formatted(parts.getMultiPartFormData().getBoundary())); + MultiPartFormData.ContentSource source = new MultiPartFormData.ContentSource(parts.getMultiPartFormData().getBoundary()); source.setPartHeadersMaxLength(1024); parts.forEach(source::addPart); source.close(); @@ -321,7 +321,7 @@ public class MultiPartFormDataHandlerTest HttpFields headers2 = part2.getHeaders(); assertEquals(2, headers2.size()); assertEquals("application/octet-stream", headers2.get(HttpHeader.CONTENT_TYPE)); - assertEquals(32, part2.getContent().getLength()); + assertEquals(32, part2.getContentSource().getLength()); } } } 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 05e803ed1b6..b4ec605b7cc 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 @@ -181,10 +181,10 @@ public class ResourceHandlerByteRangesTest assertEquals(2, parts.size()); MultiPart.Part part1 = parts.get(0); assertEquals("text/plain", part1.getHeaders().get(HttpHeader.CONTENT_TYPE)); - assertEquals("234", Content.Source.asString(part1.getContent())); + assertEquals("234", Content.Source.asString(part1.getContentSource())); MultiPart.Part part2 = parts.get(1); assertEquals("text/plain", part2.getHeaders().get(HttpHeader.CONTENT_TYPE)); - assertEquals("xyz", Content.Source.asString(part2.getContent())); + assertEquals("xyz", Content.Source.asString(part2.getContentSource())); } } } diff --git a/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/TestableRequest.java b/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/TestableRequest.java index 4ab9a592f6b..b5b759724f2 100644 --- a/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/TestableRequest.java +++ b/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/TestableRequest.java @@ -144,6 +144,12 @@ public class TestableRequest implements Request return null; } + @Override + public boolean consumeAvailable() + { + return false; + } + @Override public void demand(Runnable demandCallback) { diff --git a/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/HttpClientStreamTest.java b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/HttpClientStreamTest.java index c6470f0bfd3..cbb8274bb6a 100644 --- a/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/HttpClientStreamTest.java +++ b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/HttpClientStreamTest.java @@ -286,7 +286,7 @@ public class HttpClientStreamTest extends AbstractTest Response response = listener.get(5, TimeUnit.SECONDS); assertEquals(200, response.getStatus()); - assertThrows(AsynchronousCloseException.class, stream::read); + assertThrows(IOException.class, stream::read); } @ParameterizedTest @@ -329,7 +329,7 @@ public class HttpClientStreamTest extends AbstractTest assertTrue(latch.await(5, TimeUnit.SECONDS)); - assertThrows(AsynchronousCloseException.class, input::read); + assertThrows(IOException.class, input::read); } @ParameterizedTest diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java index 0da50c764ca..df01247f9a7 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java @@ -648,6 +648,43 @@ public class BufferUtil } } + public static int readFrom(InputStream is, ByteBuffer buffer) throws IOException + { + if (buffer.hasArray()) + { + int read = is.read(buffer.array(), buffer.arrayOffset() + buffer.limit(), buffer.capacity() - buffer.limit()); + buffer.limit(buffer.limit() + read); + return read; + } + else + { + int totalRead = 0; + ByteBuffer tmp = allocate(8192); + while (BufferUtil.space(tmp) > 0 && BufferUtil.space(buffer) > 0) + { + int read = is.read(tmp.array(), 0, Math.min(BufferUtil.space(tmp), BufferUtil.space(buffer))); + if (read == 0) + { + break; + } + else if (read < 0) + { + if (totalRead == 0) + return -1; + break; + } + totalRead += read; + tmp.position(0); + tmp.limit(read); + + int pos = BufferUtil.flipToFill(buffer); + BufferUtil.put(tmp, buffer); + BufferUtil.flipToFlush(buffer, pos); + } + return totalRead; + } + } + public static void writeTo(ByteBuffer buffer, OutputStream out) throws IOException { if (buffer.hasArray()) diff --git a/jetty-ee10/jetty-ee10-proxy/src/test/java/org/eclipse/jetty/ee10/proxy/ProxyServletTest.java b/jetty-ee10/jetty-ee10-proxy/src/test/java/org/eclipse/jetty/ee10/proxy/ProxyServletTest.java index 255b09601b7..11e71990c03 100644 --- a/jetty-ee10/jetty-ee10-proxy/src/test/java/org/eclipse/jetty/ee10/proxy/ProxyServletTest.java +++ b/jetty-ee10/jetty-ee10-proxy/src/test/java/org/eclipse/jetty/ee10/proxy/ProxyServletTest.java @@ -13,7 +13,6 @@ package org.eclipse.jetty.ee10.proxy; -import java.io.EOFException; import java.io.IOException; import java.io.InputStream; import java.io.InterruptedIOException; @@ -1360,7 +1359,7 @@ public class ProxyServletTest chunk1Latch.countDown(); - assertThrows(EOFException.class, () -> + assertThrows(IOException.class, () -> { // Make sure the proxy does not receive chunk2. input.read(); diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContentProducer.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContentProducer.java index 39f809480e5..8979484c9a1 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContentProducer.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContentProducer.java @@ -60,11 +60,9 @@ class AsyncContentProducer implements ContentProducer // Make sure that asking this instance for chunks between // recycle() and reopen() will only produce error chunks. - if (_chunk == null) - _chunk = RECYCLED_ERROR_CHUNK; - // The chunk must be fully consumed. - else if (!_chunk.isLast() || _chunk.hasRemaining()) - throw new IllegalStateException("ContentProducer with unconsumed chunk cannot be recycled"); + if (_chunk != null) + _chunk.release(); + _chunk = RECYCLED_ERROR_CHUNK; } @Override @@ -182,18 +180,7 @@ class AsyncContentProducer implements ContentProducer private boolean consumeAvailableChunks() { - ServletContextRequest request = _servletChannel.getServletContextRequest(); - while (true) - { - Content.Chunk chunk = request.read(); - if (chunk == null) - return false; - - chunk.release(); - - if (chunk.isLast()) - return true; - } + return _servletChannel.getServletContextRequest().consumeAvailable(); } @Override diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/MultiPartFormInputStream.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/MultiPartFormInputStream.java deleted file mode 100644 index 3d019334e4a..00000000000 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/MultiPartFormInputStream.java +++ /dev/null @@ -1,32 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2022 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.io.InputStream; -import java.util.List; - -import jakarta.servlet.http.Part; - -public class MultiPartFormInputStream -{ - public MultiPartFormInputStream(InputStream inputStream, String contentType, Object o, Object o1) - { - - } - - public List getParts() - { - return null; - } -} 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 967b4201bd1..f104975ae93 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 @@ -15,9 +15,11 @@ package org.eclipse.jetty.ee10.servlet; import java.io.BufferedReader; import java.io.IOException; +import java.io.InputStream; import java.io.InputStreamReader; import java.io.UnsupportedEncodingException; import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.nio.charset.UnsupportedCharsetException; import java.security.Principal; import java.util.ArrayList; @@ -35,6 +37,7 @@ 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; @@ -65,6 +68,7 @@ import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MimeTypes; +import org.eclipse.jetty.io.RuntimeIOException; import org.eclipse.jetty.server.ConnectionMetaData; import org.eclipse.jetty.server.FormFields; import org.eclipse.jetty.server.HttpCookieUtils; @@ -74,6 +78,7 @@ import org.eclipse.jetty.session.AbstractSessionManager; import org.eclipse.jetty.session.ManagedSession; import org.eclipse.jetty.util.Fields; import org.eclipse.jetty.util.HostPort; +import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.URIUtil; import org.slf4j.Logger; @@ -473,11 +478,76 @@ public class ServletApiRequest implements HttpServletRequest @Override public Collection getParts() throws IOException, ServletException { - 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())); if (_parts == null) - _parts = ServletMultiPartFormData.from(this); + { + 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 = _request.getContext().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 (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. + b. if the request.getCharacterEncoding() returns a value, use that. + (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; + + long formContentSize = 0; + for (Part p : parts) + { + if (p.getSubmittedFileName() == null) + { + 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); + } + } + } + } + return _parts.getParts(); } @@ -780,13 +850,47 @@ public class ServletApiRequest implements HttpServletRequest { try { - int maxKeys = _request.getServletRequestState().getContextHandler().getMaxFormKeys(); - int maxContentSize = _request.getServletRequestState().getContextHandler().getMaxFormContentSize(); - _contentParameters = FormFields.from(getServletContextRequest(), maxKeys, maxContentSize).get(); + int contentLength = getContentLength(); + if (contentLength != 0 && _inputState == ServletContextRequest.INPUT_NONE) + { + String baseType = HttpField.valueParameters(getContentType(), null); + if (MimeTypes.Type.FORM_ENCODED.is(baseType) && + _request.getConnectionMetaData().getHttpConfiguration().isFormEncodedMethod(getMethod())) + { + try + { + int maxKeys = _request.getServletRequestState().getContextHandler().getMaxFormKeys(); + int maxContentSize = _request.getServletRequestState().getContextHandler().getMaxFormContentSize(); + _contentParameters = FormFields.from(getServletContextRequest(), maxKeys, maxContentSize).get(); + } + catch (IllegalStateException | IllegalArgumentException | ExecutionException | + InterruptedException e) + { + LOG.warn(e.toString()); + throw new BadMessageException("Unable to parse form content", e); + } + } + else if (MimeTypes.Type.MULTIPART_FORM_DATA.is(baseType) && + getAttribute(ServletContextRequest.MULTIPART_CONFIG_ELEMENT) != null) + { + try + { + getParts(); + } + catch (IOException | ServletException e) + { + String msg = "Unable to extract content parameters"; + if (LOG.isDebugEnabled()) + LOG.debug(msg, e); + throw new RuntimeIOException(msg, e); + } + } + } + if (_contentParameters == null || _contentParameters.isEmpty()) _contentParameters = ServletContextRequest.NO_PARAMS; } - catch (IllegalStateException | IllegalArgumentException | ExecutionException | InterruptedException e) + catch (IllegalStateException | IllegalArgumentException e) { LOG.warn(e.toString()); throw new BadMessageException("Unable to parse form content", e); diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java index a906c5d5770..6b19907c4de 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java @@ -489,7 +489,8 @@ public class ServletChannel // from the failed dispatch, then we try to consume it here and if we fail we add a // Connection:close. This can't be deferred to COMPLETE as the response will be committed // by then. - Response.ensureConsumeAvailableOrNotPersistent(_servletContextRequest, _servletContextRequest.getResponse()); + if (!_httpInput.consumeAvailable()) + Response.ensureNotPersistent(_servletContextRequest, _servletContextRequest.getResponse()); ContextHandler.ScopedContext context = (ContextHandler.ScopedContext)_servletContextRequest.getAttribute(ErrorHandler.ERROR_CONTEXT); Request.Processor errorProcessor = ErrorHandler.getErrorProcessor(getServer(), context == null ? null : context.getContextHandler()); 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 616c2697fe7..0530b0b984b 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 @@ -44,7 +44,7 @@ import org.slf4j.LoggerFactory; public class ServletContextRequest extends ContextRequest { - public static final String __MULTIPART_CONFIG_ELEMENT = "org.eclipse.jetty.multipartConfig"; + public static final String MULTIPART_CONFIG_ELEMENT = "org.eclipse.jetty.multipartConfig"; private static final Logger LOG = LoggerFactory.getLogger(ServletContextRequest.class); static final int INPUT_NONE = 0; static final int INPUT_STREAM = 1; 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 7b1a797f976..f5d5865aada 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 @@ -715,7 +715,7 @@ public class ServletHolder extends Holder implements ComparableParses 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 + */ + public static Parts from(ServletApiRequest request, int maxParts) throws IOException { try { - // Look for a previously read and parsed MultiPartFormData from the DelayedHandler - MultiPartFormData formData = (MultiPartFormData)request.getAttribute(MultiPartFormData.class.getName()); - if (formData != null) - return new Parts(formData); + // 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); // TODO set the files directory - return new ServletMultiPartFormData().parse(request); + return new ServletMultiPartFormData().parse(request, maxParts); } catch (Throwable x) { @@ -73,9 +91,9 @@ public class ServletMultiPartFormData } } - private Parts parse(ServletApiRequest request) throws IOException + private Parts parse(ServletApiRequest request, int maxParts) throws IOException { - MultipartConfigElement config = (MultipartConfigElement)request.getAttribute(ServletContextRequest.__MULTIPART_CONFIG_ELEMENT); + MultipartConfigElement config = (MultipartConfigElement)request.getAttribute(ServletContextRequest.MULTIPART_CONFIG_ELEMENT); if (config == null) throw new IllegalStateException("No multipart configuration element"); @@ -83,7 +101,9 @@ public class ServletMultiPartFormData if (boundary == null) throw 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); File tmpDirFile = (File)request.getServletContext().getAttribute(ServletContext.TEMPDIR); if (tmpDirFile == null) @@ -99,24 +119,36 @@ public class ServletMultiPartFormData ConnectionMetaData connectionMetaData = request.getServletContextRequest().getConnectionMetaData(); formData.setPartHeadersMaxLength(connectionMetaData.getHttpConfiguration().getRequestHeaderSize()); + ByteBufferPool byteBufferPool = request.getServletContextRequest().getComponents().getByteBufferPool(); Connection connection = connectionMetaData.getConnection(); int bufferSize = connection instanceof AbstractConnection c ? c.getInputBufferSize() : 2048; - byte[] buffer = new byte[bufferSize]; InputStream input = request.getInputStream(); - while (true) + while (!formData.isDone()) { - int read = input.read(buffer); - if (read < 0) + RetainableByteBuffer retainable = byteBufferPool.acquire(bufferSize, false); + boolean readEof = false; + ByteBuffer buffer = retainable.getByteBuffer(); + while (BufferUtil.space(buffer) > bufferSize / 2) + { + int read = BufferUtil.readFrom(input, buffer); + if (read < 0) + { + readEof = true; + break; + } + } + + formData.parse(Content.Chunk.from(buffer, false, retainable::release)); + if (readEof) { formData.parse(Content.Chunk.EOF); break; } - Content.Chunk chunk = Content.Chunk.from(ByteBuffer.wrap(buffer, 0, read), false); - formData.parse(chunk); - chunk.release(); } - return new Parts(formData); + Parts parts = new Parts(formData.join()); + request.setAttribute(Parts.class.getName(), parts); + return parts; } /** @@ -126,9 +158,9 @@ public class ServletMultiPartFormData { private final List parts = new ArrayList<>(); - public Parts(MultiPartFormData formData) + public Parts(MultiPartFormData.Parts parts) { - formData.join().forEach(part -> parts.add(new ServletPart(formData, part))); + parts.forEach(part -> this.parts.add(new ServletPart(parts.getMultiPartFormData(), part))); } public Part getPart(String name) @@ -149,22 +181,17 @@ public class ServletMultiPartFormData { private final MultiPartFormData _formData; private final MultiPart.Part _part; - private final long _length; - private final InputStream _input; private ServletPart(MultiPartFormData formData, MultiPart.Part part) { _formData = formData; _part = part; - Content.Source content = part.getContent(); - _length = content.getLength(); - _input = Content.Source.asInputStream(content); } @Override public InputStream getInputStream() throws IOException { - return _input; + return Content.Source.asInputStream(_part.newContentSource()); } @Override @@ -188,13 +215,12 @@ public class ServletMultiPartFormData @Override public long getSize() { - return _length; + return _part.getLength(); } @Override public void write(String fileName) throws IOException { - // TODO This should simply move a part that is already on the file system. Path filePath = Path.of(fileName); if (!filePath.isAbsolute()) filePath = _formData.getFilesDirectory().resolve(filePath).normalize(); @@ -204,8 +230,7 @@ public class ServletMultiPartFormData @Override public void delete() throws IOException { - if (_part instanceof MultiPart.PathPart pathPart) - pathPart.delete(); + _part.delete(); } @Override diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletRequestState.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletRequestState.java index 42eda0c5b03..ebebb146e4a 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletRequestState.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletRequestState.java @@ -851,7 +851,7 @@ public class ServletRequestState request.setAttribute(ERROR_EXCEPTION_TYPE, th.getClass()); // Set Jetty specific attributes. - request.setAttribute(ErrorProcessor.ERROR_EXCEPTION, null); + request.setAttribute(ErrorProcessor.ERROR_EXCEPTION, th); // Ensure any async lifecycle is ended! _requestState = RequestState.BLOCKING; 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 b9f2b852b8d..2c06e132c17 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/MultiPartServletTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/MultiPartServletTest.java @@ -22,6 +22,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; import java.util.Collection; +import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.zip.GZIPInputStream; @@ -36,6 +37,7 @@ import org.eclipse.jetty.client.ContentResponse; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.InputStreamResponseListener; import org.eclipse.jetty.client.MultiPartRequestContent; +import org.eclipse.jetty.client.OutputStreamRequestContent; import org.eclipse.jetty.client.Response; import org.eclipse.jetty.client.StringRequestContent; import org.eclipse.jetty.http.HttpFields; @@ -46,7 +48,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.logging.StacklessLogging; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; @@ -54,15 +58,20 @@ import org.eclipse.jetty.server.handler.gzip.GzipHandler; 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 static java.nio.charset.StandardCharsets.UTF_8; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.fail; public class MultiPartServletTest { @@ -72,18 +81,31 @@ public class MultiPartServletTest private ServerConnector connector; private HttpClient client; private Path tmpDir; + private String tmpDirString; + + @BeforeEach + public void before() throws Exception + { + tmpDir = Files.createTempDirectory(MultiPartServletTest.class.getSimpleName()); + tmpDirString = tmpDir.toAbsolutePath().toString(); + } private void start(HttpServlet servlet) throws Exception { - tmpDir = Files.createTempDirectory(MultiPartServletTest.class.getSimpleName()); + start(servlet, new MultipartConfigElement(tmpDirString, MAX_FILE_SIZE, -1, 0)); + } - server = new Server(); + 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); connector = new ServerConnector(server); server.addConnector(connector); - MultipartConfigElement config = new MultipartConfigElement(tmpDir.toAbsolutePath().toString(), - MAX_FILE_SIZE, -1, 0); - ServletContextHandler contextHandler = new ServletContextHandler(server, "/"); ServletHolder servletHolder = new ServletHolder(servlet); servletHolder.getRegistration().setMultipartConfig(config); @@ -109,6 +131,136 @@ public class MultiPartServletTest IO.delete(tmpDir.toFile()); } + @Test + public void testLargePart() throws Exception + { + // TODO: Use normal pool when a fix for https://github.com/eclipse/jetty.project/issues/9311 is merged. + ByteBufferPool bufferPool = new ByteBufferPool.NonPooling(); + start(new HttpServlet() + { + @Override + protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + req.getParameterMap(); + } + }, new MultipartConfigElement(tmpDirString), bufferPool); + + OutputStreamRequestContent content = new OutputStreamRequestContent(); + MultiPartRequestContent multiPart = new MultiPartRequestContent(); + multiPart.addPart(new MultiPart.ContentSourcePart("param", null, null, content)); + multiPart.close(); + + InputStreamResponseListener listener = new InputStreamResponseListener(); + client.newRequest("localhost", connector.getLocalPort()) + .path("/defaultConfig") + .scheme(HttpScheme.HTTP.asString()) + .method(HttpMethod.POST) + .body(multiPart) + .send(listener); + + // Write large amount of content to the part. + byte[] byteArray = new byte[1024 * 1024]; + Arrays.fill(byteArray, (byte)1); + for (int i = 0; i < 1024 * 2; i++) + { + content.getOutputStream().write(byteArray); + } + content.close(); + + Response response = listener.get(30, TimeUnit.MINUTES); + assertThat(response.getStatus(), equalTo(HttpStatus.BAD_REQUEST_400)); + String responseContent = IO.toString(listener.getInputStream()); + assertThat(responseContent, containsString("Unable to parse form content")); + assertThat(responseContent, containsString("Form is larger than max length")); + } + + @Test + public void testManyParts() throws Exception + { + start(new HttpServlet() + { + @Override + protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + req.getParameterMap(); + } + }, new MultipartConfigElement(tmpDirString)); + + byte[] byteArray = new byte[1024]; + Arrays.fill(byteArray, (byte)1); + + MultiPartRequestContent multiPart = new MultiPartRequestContent(); + for (int i = 0; i < 1024 * 1024; i++) + { + BytesRequestContent content = new BytesRequestContent(byteArray); + multiPart.addPart(new MultiPart.ContentSourcePart("part" + i, null, null, content)); + } + multiPart.close(); + + InputStreamResponseListener listener = new InputStreamResponseListener(); + client.newRequest("localhost", connector.getLocalPort()) + .path("/defaultConfig") + .scheme(HttpScheme.HTTP.asString()) + .method(HttpMethod.POST) + .body(multiPart) + .send(listener); + + Response response = listener.get(30, TimeUnit.SECONDS); + assertThat(response.getStatus(), equalTo(HttpStatus.BAD_REQUEST_400)); + String responseContent = IO.toString(listener.getInputStream()); + assertThat(responseContent, containsString("Unable to parse form content")); + assertThat(responseContent, containsString("Form with too many keys")); + } + + @Test + public void testMaxRequestSize() throws Exception + { + start(new HttpServlet() + { + @Override + protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + req.getParameterMap(); + } + }, new MultipartConfigElement(tmpDirString, -1, 1024, 1024 * 1024 * 8)); + + OutputStreamRequestContent content = new OutputStreamRequestContent(); + MultiPartRequestContent multiPart = new MultiPartRequestContent(); + multiPart.addPart(new MultiPart.ContentSourcePart("param", null, null, content)); + multiPart.close(); + + InputStreamResponseListener listener = new InputStreamResponseListener(); + client.newRequest("localhost", connector.getLocalPort()) + .path("/requestSizeLimit") + .scheme(HttpScheme.HTTP.asString()) + .method(HttpMethod.POST) + .body(multiPart) + .send(listener); + + Throwable writeError = null; + try + { + // Write large amount of content to the part. + byte[] byteArray = new byte[1024 * 1024]; + Arrays.fill(byteArray, (byte)1); + for (int i = 0; i < 1024 * 1024; i++) + { + content.getOutputStream().write(byteArray); + } + fail("We should never be able to write all the content."); + } + catch (Exception e) + { + writeError = e; + } + + assertThat(writeError, instanceOf(EofException.class)); + + // We should get 400 response, for some reason reading the content throws EofException. + Response response = listener.get(30, TimeUnit.SECONDS); + assertThat(response.getStatus(), equalTo(HttpStatus.BAD_REQUEST_400)); + } + @Test public void testSimpleMultiPart() throws Exception { @@ -237,6 +389,7 @@ 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)); @@ -245,4 +398,80 @@ public class MultiPartServletTest assertThat(parts.size(), is(1)); assertThat(parts.get(0).getContentAsString(UTF_8), is(contentString)); } + + @Test + public void testDoubleReadFromPart() throws Exception + { + start(new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + resp.setContentType("text/plain"); + for (Part part : req.getParts()) + { + resp.getWriter().println("Part: name=" + part.getName() + ", size=" + part.getSize() + ", content=" + IO.toString(part.getInputStream())); + resp.getWriter().println("Part: name=" + part.getName() + ", size=" + part.getSize() + ", content=" + IO.toString(part.getInputStream())); + } + } + }); + + String contentString = "the quick brown fox jumps over the lazy dog, " + + "the quick brown fox jumps over the lazy dog"; + StringRequestContent content = new StringRequestContent(contentString); + MultiPartRequestContent multiPart = new MultiPartRequestContent(); + multiPart.addPart(new MultiPart.ContentSourcePart("myPart", null, HttpFields.EMPTY, content)); + multiPart.close(); + + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .scheme(HttpScheme.HTTP.asString()) + .method(HttpMethod.POST) + .body(multiPart) + .send(); + + assertEquals(200, response.getStatus()); + assertThat(response.getContentAsString(), containsString("Part: name=myPart, size=88, content=the quick brown fox jumps over the lazy dog, the quick brown fox jumps over the lazy dog\n" + + "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 + { + start(new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + resp.setContentType("text/plain"); + Map parameterMap = req.getParameterMap(); + for (Map.Entry entry : parameterMap.entrySet()) + { + assertThat(entry.getValue().length, equalTo(1)); + resp.getWriter().println("Parameter: " + entry.getKey() + "=" + entry.getValue()[0]); + } + } + }); + + String contentString = "the quick brown fox jumps over the lazy dog, " + + "the quick brown fox jumps over the lazy dog"; + MultiPartRequestContent multiPart = new MultiPartRequestContent(); + multiPart.addPart(new MultiPart.ContentSourcePart("part1", null, HttpFields.EMPTY, new StringRequestContent(contentString))); + multiPart.addPart(new MultiPart.ContentSourcePart("part2", null, HttpFields.EMPTY, new StringRequestContent(contentString))); + multiPart.addPart(new MultiPart.ContentSourcePart("part3", null, HttpFields.EMPTY, new StringRequestContent(contentString))); + multiPart.addPart(new MultiPart.ContentSourcePart("partFileName", "myFile", HttpFields.EMPTY, new StringRequestContent(contentString))); + multiPart.close(); + + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .scheme(HttpScheme.HTTP.asString()) + .method(HttpMethod.POST) + .body(multiPart) + .send(); + + assertEquals(200, response.getStatus()); + String responseContent = response.getContentAsString(); + assertThat(responseContent, containsString("Parameter: part1=" + contentString)); + assertThat(responseContent, containsString("Parameter: part2=" + contentString)); + assertThat(responseContent, containsString("Parameter: part3=" + contentString)); + assertThat(responseContent, not(containsString("Parameter: partFileName=" + contentString))); + } } diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/AsyncContentProducer.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/AsyncContentProducer.java index 0107299b0e4..14715eeebae 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/AsyncContentProducer.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/AsyncContentProducer.java @@ -189,18 +189,8 @@ class AsyncContentProducer implements ContentProducer LOG.trace("consumeAll {}", this, x); } failCurrentContent(x); - // A specific HttpChannel mechanism must be used as the following code - // does not guarantee that the channel will synchronously deliver all - // content it already contains: - // while (true) - // { - // HttpInput.Content content = _httpChannel.produceContent(); - // ... - // } - // as the HttpChannel's produceContent() contract makes no such promise; - // for instance the H2 implementation calls Stream.demand() that may - // deliver the content asynchronously. Tests in StreamResetTest cover this. - boolean atEof = _httpChannel.failAllContent(x); + + boolean atEof = _httpChannel.getRequest().getCoreRequest().consumeAvailable(); if (LOG.isDebugEnabled()) LOG.debug("failed all content of http channel EOF={} {}", atEof, this); return atEof; diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartFormInputStream.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartFormInputStream.java index 9c8ba258a38..2c4f2fb52ee 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartFormInputStream.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartFormInputStream.java @@ -47,6 +47,8 @@ import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.eclipse.jetty.ee9.nested.ContextHandler.DEFAULT_MAX_FORM_KEYS; + /** * MultiPartInputStream *

@@ -97,6 +99,8 @@ public class MultiPartFormInputStream private final MultipartConfigElement _config; private final File _contextTmpDir; private final String _contentType; + private final int _maxParts; + private int _numParts = 0; private volatile Throwable _err; private volatile Path _tmpDir; private volatile boolean _deleteOnExit; @@ -380,9 +384,20 @@ public class MultiPartFormInputStream * @param in Request input stream * @param contentType Content-Type header * @param config MultipartConfigElement - * @param contextTmpDir jakarta.servlet.context.tempdir + * @param contextTmpDir javax.servlet.context.tempdir */ public MultiPartFormInputStream(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir) + { + this(in, contentType, config, contextTmpDir, DEFAULT_MAX_FORM_KEYS); + } + + /** + * @param in Request input stream + * @param contentType Content-Type header + * @param config MultipartConfigElement + * @param contextTmpDir javax.servlet.context.tempdir + */ + public MultiPartFormInputStream(InputStream in, String contentType, MultipartConfigElement config, File contextTmpDir, int maxParts) { // Must be a multipart request. _contentType = contentType; @@ -391,6 +406,7 @@ public class MultiPartFormInputStream _contextTmpDir = (contextTmpDir != null) ? contextTmpDir : new File(System.getProperty("java.io.tmpdir")); _config = (config != null) ? config : new MultipartConfigElement(_contextTmpDir.getAbsolutePath()); + _maxParts = maxParts; if (in instanceof ServletInputStream) { @@ -809,6 +825,9 @@ public class MultiPartFormInputStream public void startPart() { reset(); + _numParts++; + if (_maxParts >= 0 && _numParts > _maxParts) + throw new IllegalStateException(String.format("Form with too many keys [%d > %d]", _numParts, _maxParts)); } @Override diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java index d0156c77e36..95afc0d668c 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java @@ -1953,7 +1953,21 @@ public class Request implements HttpServletRequest if (config == null) throw new IllegalStateException("No multipart config for servlet"); - _multiParts = newMultiParts(config); + int maxFormContentSize = ContextHandler.DEFAULT_MAX_FORM_CONTENT_SIZE; + int maxFormKeys = ContextHandler.DEFAULT_MAX_FORM_KEYS; + if (_context != null) + { + ContextHandler contextHandler = _context.getContextHandler(); + maxFormContentSize = contextHandler.getMaxFormContentSize(); + maxFormKeys = contextHandler.getMaxFormKeys(); + } + else + { + maxFormContentSize = lookupServerAttribute(ContextHandler.MAX_FORM_CONTENT_SIZE_KEY, maxFormContentSize); + maxFormKeys = lookupServerAttribute(ContextHandler.MAX_FORM_KEYS_KEY, maxFormKeys); + } + + _multiParts = newMultiParts(config, maxFormKeys); Collection parts = _multiParts.getParts(); setNonComplianceViolationsOnRequest(); @@ -1987,11 +2001,16 @@ public class Request implements HttpServletRequest else defaultCharset = StandardCharsets.UTF_8; + long formContentSize = 0; ByteArrayOutputStream os = null; for (Part p : parts) { if (p.getSubmittedFileName() == null) { + 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) @@ -2032,10 +2051,10 @@ public class Request implements HttpServletRequest setAttribute(HttpCompliance.VIOLATIONS_ATTR, violations); } - private MultiPartFormInputStream newMultiParts(MultipartConfigElement config) throws IOException + private MultiPartFormInputStream newMultiParts(MultipartConfigElement config, int maxParts) throws IOException { return new MultiPartFormInputStream(getInputStream(), getContentType(), config, - (_context != null ? (File)_context.getAttribute("jakarta.servlet.context.tempdir") : null)); + (_context != null ? (File)_context.getAttribute("jakarta.servlet.context.tempdir") : null), maxParts); } @Override diff --git a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java index 8c172e50bb8..0813a230917 100644 --- a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java +++ b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java @@ -2347,6 +2347,12 @@ public class RequestTest return null; } + @Override + public boolean consumeAvailable() + { + return false; + } + @Override public void demand(Runnable demandCallback) { diff --git a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ResponseTest.java b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ResponseTest.java index ae1169d3d87..f5e72a2961f 100644 --- a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ResponseTest.java +++ b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ResponseTest.java @@ -2367,6 +2367,12 @@ public class ResponseTest return Content.Chunk.EOF; } + @Override + public boolean consumeAvailable() + { + return true; + } + @Override public void demand(Runnable demandCallback) { diff --git a/jetty-ee9/jetty-ee9-proxy/src/test/java/org/eclipse/jetty/ee9/proxy/ProxyServletTest.java b/jetty-ee9/jetty-ee9-proxy/src/test/java/org/eclipse/jetty/ee9/proxy/ProxyServletTest.java index c6d41f538c7..db00f9c4f0b 100644 --- a/jetty-ee9/jetty-ee9-proxy/src/test/java/org/eclipse/jetty/ee9/proxy/ProxyServletTest.java +++ b/jetty-ee9/jetty-ee9-proxy/src/test/java/org/eclipse/jetty/ee9/proxy/ProxyServletTest.java @@ -13,7 +13,6 @@ package org.eclipse.jetty.ee9.proxy; -import java.io.EOFException; import java.io.IOException; import java.io.InputStream; import java.io.InterruptedIOException; @@ -1359,7 +1358,7 @@ public class ProxyServletTest chunk1Latch.countDown(); - assertThrows(EOFException.class, () -> + assertThrows(IOException.class, () -> { // Make sure the proxy does not receive chunk2. input.read(); diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java index 863b0232d4e..5f988e44ebf 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java +++ b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/MultiPartServletTest.java @@ -34,6 +34,7 @@ import org.eclipse.jetty.client.ContentResponse; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.InputStreamResponseListener; import org.eclipse.jetty.client.MultiPartRequestContent; +import org.eclipse.jetty.client.OutputStreamRequestContent; import org.eclipse.jetty.client.Response; import org.eclipse.jetty.client.StringRequestContent; import org.eclipse.jetty.ee9.nested.HttpChannel; @@ -42,8 +43,10 @@ import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.HttpScheme; +import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.http.MultiPart; +import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.logging.StacklessLogging; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; @@ -55,10 +58,13 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.fail; public class MultiPartServletTest { @@ -69,6 +75,20 @@ public class MultiPartServletTest private static final int MAX_FILE_SIZE = 512 * 1024; private static final int LARGE_MESSAGE_SIZE = 1024 * 1024; + private static final int MAX_REQUEST_SIZE = 1024 * 1024 * 8; + + public static class RequestParameterServlet extends HttpServlet + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + req.getParameterMap(); + req.getParts(); + resp.setStatus(200); + resp.getWriter().print("success"); + resp.getWriter().close(); + } + } public static class MultiPartServlet extends HttpServlet { @@ -119,11 +139,19 @@ public class MultiPartServletTest MultipartConfigElement config = new MultipartConfigElement(tmpDir.toAbsolutePath().toString(), MAX_FILE_SIZE, -1, 1); + MultipartConfigElement requestSizedConfig = new MultipartConfigElement(tmpDir.toAbsolutePath().toString(), + -1, MAX_REQUEST_SIZE, 1); + MultipartConfigElement defaultConfig = new MultipartConfigElement(tmpDir.toAbsolutePath().toString(), + -1, -1, 1); ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.SESSIONS); contextHandler.setContextPath("/"); ServletHolder servletHolder = contextHandler.addServlet(MultiPartServlet.class, "/"); servletHolder.getRegistration().setMultipartConfig(config); + servletHolder = contextHandler.addServlet(RequestParameterServlet.class, "/defaultConfig"); + servletHolder.getRegistration().setMultipartConfig(defaultConfig); + servletHolder = contextHandler.addServlet(RequestParameterServlet.class, "/requestSizeLimit"); + servletHolder.getRegistration().setMultipartConfig(requestSizedConfig); servletHolder = contextHandler.addServlet(MultiPartEchoServlet.class, "/echo"); servletHolder.getRegistration().setMultipartConfig(config); @@ -149,6 +177,107 @@ public class MultiPartServletTest IO.delete(tmpDir.toFile()); } + @Test + public void testLargePart() throws Exception + { + OutputStreamRequestContent content = new OutputStreamRequestContent(); + MultiPartRequestContent multiPart = new MultiPartRequestContent(); + multiPart.addPart(new MultiPart.ContentSourcePart("param", null, null, content)); + multiPart.close(); + + InputStreamResponseListener listener = new InputStreamResponseListener(); + client.newRequest("localhost", connector.getLocalPort()) + .path("/defaultConfig") + .scheme(HttpScheme.HTTP.asString()) + .method(HttpMethod.POST) + .body(multiPart) + .send(listener); + + // Write large amount of content to the part. + byte[] byteArray = new byte[1024 * 1024]; + Arrays.fill(byteArray, (byte)1); + for (int i = 0; i < 1024 * 2; i++) + { + content.getOutputStream().write(byteArray); + } + content.close(); + + Response response = listener.get(2, TimeUnit.MINUTES); + assertThat(response.getStatus(), equalTo(HttpStatus.BAD_REQUEST_400)); + String responseContent = IO.toString(listener.getInputStream()); + assertThat(responseContent, containsString("Unable to parse form content")); + assertThat(responseContent, containsString("Form is larger than max length")); + } + + @Test + public void testManyParts() throws Exception + { + byte[] byteArray = new byte[1024]; + Arrays.fill(byteArray, (byte)1); + + MultiPartRequestContent multiPart = new MultiPartRequestContent(); + for (int i = 0; i < 1024 * 1024; i++) + { + BytesRequestContent content = new BytesRequestContent(byteArray); + multiPart.addPart(new MultiPart.ContentSourcePart("part" + i, null, null, content)); + } + multiPart.close(); + + InputStreamResponseListener listener = new InputStreamResponseListener(); + client.newRequest("localhost", connector.getLocalPort()) + .path("/defaultConfig") + .scheme(HttpScheme.HTTP.asString()) + .method(HttpMethod.POST) + .body(multiPart) + .send(listener); + + Response response = listener.get(30, TimeUnit.SECONDS); + assertThat(response.getStatus(), equalTo(HttpStatus.BAD_REQUEST_400)); + String responseContent = IO.toString(listener.getInputStream()); + assertThat(responseContent, containsString("Unable to parse form content")); + assertThat(responseContent, containsString("Form with too many keys")); + } + + @Test + public void testMaxRequestSize() throws Exception + { + OutputStreamRequestContent content = new OutputStreamRequestContent(); + MultiPartRequestContent multiPart = new MultiPartRequestContent(); + multiPart.addPart(new MultiPart.ContentSourcePart("param", null, null, content)); + multiPart.close(); + + InputStreamResponseListener listener = new InputStreamResponseListener(); + client.newRequest("localhost", connector.getLocalPort()) + .path("/requestSizeLimit") + .scheme(HttpScheme.HTTP.asString()) + .method(HttpMethod.POST) + .body(multiPart) + .send(listener); + + Throwable writeError = null; + try + { + // Write large amount of content to the part. + byte[] byteArray = new byte[1024 * 1024]; + Arrays.fill(byteArray, (byte)1); + for (int i = 0; i < 1024 * 1024; i++) + { + content.getOutputStream().write(byteArray); + } + fail("We should never be able to write all the content."); + } + catch (Exception e) + { + writeError = e; + } + + assertThat(writeError, instanceOf(EofException.class)); + + // We should get 400 response, for some reason reading the content throws EofException. + Response response = listener.get(30, TimeUnit.SECONDS); + assertThat(response.getStatus(), equalTo(HttpStatus.BAD_REQUEST_400)); + } + @Test public void testTempFilesDeletedOnError() throws Exception {