From bc7837812003883270761ae693bdec7a20b5bbcc Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 10 Oct 2025 12:14:16 +1100 Subject: [PATCH 01/26] Fix copying of zero length resources Fix #14685 by handling zero length resources --- .../io/internal/ByteChannelContentSource.java | 9 +++++- .../org/eclipse/jetty/io/IOResourcesTest.java | 31 +++++++++++++------ jetty-core/jetty-io/src/test/resources/one | 1 + jetty-core/jetty-io/src/test/resources/zero | 0 4 files changed, 30 insertions(+), 11 deletions(-) create mode 100644 jetty-core/jetty-io/src/test/resources/one create mode 100644 jetty-core/jetty-io/src/test/resources/zero diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java index 244d4d525ea7..39c84b66ee6b 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java @@ -155,6 +155,13 @@ public Content.Chunk read() try (AutoLock ignored = lock.lock()) { lockedEnsureOpenOrTerminal(); + + if (_length == 0) + { + lockedSetTerminal(Content.Chunk.EOF); + return Content.Chunk.EOF; + } + if (_terminal != null) return _terminal; @@ -172,7 +179,7 @@ else if (_buffer.isRetained()) { ByteBuffer byteBuffer = _buffer.getByteBuffer(); BufferUtil.clearToFill(byteBuffer); - if (_length >= 0) + if (_length > 0) byteBuffer.limit((int)Math.min(_buffer.capacity(), _length - _totalRead)); int read = _byteChannel.read(byteBuffer); BufferUtil.flipToFlush(byteBuffer, 0); diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/IOResourcesTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/IOResourcesTest.java index 28dfe20e0388..6984a9a7111a 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/IOResourcesTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/IOResourcesTest.java @@ -142,10 +142,13 @@ public Content.Source newContentSource(ByteBufferPool.Sized bufferPool, long off public static Stream all() throws Exception { Path testResourcePath = MavenTestingUtils.getTestResourcePath("keystore.p12"); + URI resourceUri = testResourcePath.toUri(); return Stream.of( ResourceFactory.root().newResource(resourceUri), ResourceFactory.root().newMemoryResource(resourceUri.toURL()), + ResourceFactory.root().newResource(MavenTestingUtils.getTestResourcePath("zero")), + ResourceFactory.root().newResource(MavenTestingUtils.getTestResourcePath("one")), new URLResourceFactory().newResource(resourceUri), new TestContentSourceFactoryResource(resourceUri, Files.readAllBytes(testResourcePath)) ); @@ -181,12 +184,13 @@ public void testAsContentSourceWithFirst(Resource resource) throws Exception { TestSink sink = new TestSink(); Callback.Completable callback = new Callback.Completable(); + Content.Source contentSource = IOResources.asContentSource(resource, bufferPool, 100, -1); Content.copy(contentSource, sink, callback); callback.get(); List chunks = sink.takeAccumulatedChunks(); long sum = chunks.stream().mapToLong(Content.Chunk::remaining).sum(); - assertThat(sum, is(resource.length() - 100L)); + assertThat(sum, is(Math.max(0L, resource.length() - 100L))); assertThat(chunks.get(chunks.size() - 1).isLast(), is(true)); } @@ -201,7 +205,7 @@ public void testAsContentSourceWithLength(Resource resource) throws Exception callback.get(); List chunks = sink.takeAccumulatedChunks(); long sum = chunks.stream().mapToLong(Content.Chunk::remaining).sum(); - assertThat(sum, is(500L)); + assertThat(sum, is(Math.min(resource.length(), 500L))); assertThat(chunks.get(chunks.size() - 1).isLast(), is(true)); } @@ -211,12 +215,15 @@ public void testAsContentSourceWithFirstAndLength(Resource resource) throws Exce { TestSink sink = new TestSink(); Callback.Completable callback = new Callback.Completable(); - Content.Source contentSource = IOResources.asContentSource(resource, bufferPool, 100, 500); + + long offset = Math.min(resource.length(), 100); + long length = Math.min(resource.length() - offset, 500); + Content.Source contentSource = IOResources.asContentSource(resource, bufferPool, offset, length); Content.copy(contentSource, sink, callback); callback.get(); List chunks = sink.takeAccumulatedChunks(); long sum = chunks.stream().mapToLong(Content.Chunk::remaining).sum(); - assertThat(sum, is(500L)); + assertThat(sum, is(length)); assertThat(chunks.get(chunks.size() - 1).isLast(), is(true)); } @@ -240,11 +247,12 @@ public void testCopyWithFirst(Resource resource) throws Exception { TestSink sink = new TestSink(); Callback.Completable callback = new Callback.Completable(); - IOResources.copy(resource, sink, bufferPool, 100, -1, callback); + long offset = Math.min(resource.length(), 100); + IOResources.copy(resource, sink, bufferPool, offset, -1, callback); callback.get(); List chunks = sink.takeAccumulatedChunks(); long sum = chunks.stream().mapToLong(Content.Chunk::remaining).sum(); - assertThat(sum, is(resource.length() - 100L)); + assertThat(sum, is(Math.max(0L, resource.length() - 100L))); assertThat(chunks.get(chunks.size() - 1).isLast(), is(true)); } @@ -254,11 +262,12 @@ public void testCopyWithLength(Resource resource) throws Exception { TestSink sink = new TestSink(); Callback.Completable callback = new Callback.Completable(); - IOResources.copy(resource, sink, bufferPool, 0, 500, callback); + long length = resource.length() >= 0 ? Math.min(resource.length(), 500) : 500; + IOResources.copy(resource, sink, bufferPool, 0, length, callback); callback.get(); List chunks = sink.takeAccumulatedChunks(); long sum = chunks.stream().mapToLong(Content.Chunk::remaining).sum(); - assertThat(sum, is(500L)); + assertThat(sum, is(length)); assertThat(chunks.get(chunks.size() - 1).isLast(), is(true)); } @@ -268,11 +277,13 @@ public void testCopyWithFirstAndLength(Resource resource) throws Exception { TestSink sink = new TestSink(); Callback.Completable callback = new Callback.Completable(); - IOResources.copy(resource, sink, bufferPool, 100, 500, callback); + long offset = Math.min(resource.length(), 100); + long length = Math.min(resource.length() - offset, 500); + IOResources.copy(resource, sink, bufferPool, offset, length, callback); callback.get(); List chunks = sink.takeAccumulatedChunks(); long sum = chunks.stream().mapToLong(Content.Chunk::remaining).sum(); - assertThat(sum, is(500L)); + assertThat(sum, is(length)); assertThat(chunks.get(chunks.size() - 1).isLast(), is(true)); } diff --git a/jetty-core/jetty-io/src/test/resources/one b/jetty-core/jetty-io/src/test/resources/one new file mode 100644 index 000000000000..56a6051ca2b0 --- /dev/null +++ b/jetty-core/jetty-io/src/test/resources/one @@ -0,0 +1 @@ +1 \ No newline at end of file diff --git a/jetty-core/jetty-io/src/test/resources/zero b/jetty-core/jetty-io/src/test/resources/zero new file mode 100644 index 000000000000..e69de29bb2d1 From 1abce9d2b08b3963d5a9d19753338f6294ea2133 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 10 Oct 2025 21:35:45 +1100 Subject: [PATCH 02/26] Fix copying of zero length resources Fix #14685 by handling zero length resources --- .../eclipse/jetty/io/internal/ByteChannelContentSource.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java index 39c84b66ee6b..391b8408c20b 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java @@ -156,15 +156,15 @@ public Content.Chunk read() { lockedEnsureOpenOrTerminal(); + if (_terminal != null) + return _terminal; + if (_length == 0) { lockedSetTerminal(Content.Chunk.EOF); return Content.Chunk.EOF; } - if (_terminal != null) - return _terminal; - if (_buffer == null) { _buffer = _byteBufferPool.acquire(); From c30d2fa571b273d41f6a9b82ec33008b9f4765ab Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Sun, 12 Oct 2025 08:35:36 +1100 Subject: [PATCH 03/26] TODOs from reviews --- .../main/java/org/eclipse/jetty/http/MultiPart.java | 3 +++ .../src/main/java/org/eclipse/jetty/io/Content.java | 3 +++ .../jetty/io/internal/ByteChannelContentSource.java | 13 ++++++++++++- .../java/org/eclipse/jetty/io/IOResourcesTest.java | 8 ++++---- 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java index 4bb2dcc6a37e..f6d5fd9a9ee3 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 @@ -354,6 +354,7 @@ public final Content.Source createContentSource() @Override public Content.Source newContentSource(ByteBufferPool.Sized bufferPool, long offset, long length) { + // TODO support offset and length return newContentSource(); } @@ -502,6 +503,7 @@ public ByteBufferPart(String name, String fileName, HttpFields fields, List= 0 && seekableByteChannel != null) { try { + // TODO negative offset is an IAE, but a too large offset is corrected, but length is not checked. seekableByteChannel.position(offset); } catch (IOException e) @@ -83,6 +85,7 @@ public ByteChannelContentSource(ByteBufferPool.Sized byteBufferPool, ByteChannel private ByteChannelContentSource(ByteBufferPool.Sized byteBufferPool, ByteChannel byteChannel, long offset, long length) { + // TODO Is this the contract we want for offset/length? We are correcting negative offset, but not checking actual size. _byteBufferPool = Objects.requireNonNullElse(byteBufferPool, ByteBufferPool.SIZED_NON_POOLING); _byteChannel = byteChannel; _offset = offset < 0 ? 0 : offset; @@ -270,7 +273,15 @@ public PathContentSource(ByteBufferPool.Sized byteBufferPool, Path path) public PathContentSource(ByteBufferPool.Sized byteBufferPool, Path path, long offset, long length) { - super(byteBufferPool, null, offset, length < 0L ? size(path) : length); + this (byteBufferPool, path, size(path), offset, length); + } + + private PathContentSource(ByteBufferPool.Sized byteBufferPool, Path path, long size, long offset, long length) + { + // TODO Is this the contract we want for offset/length? auto correcting it? Validity can be checked in super. + super(byteBufferPool, null, + Math.min(offset, size), + Math.min(length, size - offset)); _path = path; } diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/IOResourcesTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/IOResourcesTest.java index 6984a9a7111a..6a8e2d136aef 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/IOResourcesTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/IOResourcesTest.java @@ -180,7 +180,7 @@ public void testAsContentSource(Resource resource) throws Exception @ParameterizedTest @MethodSource("all") - public void testAsContentSourceWithFirst(Resource resource) throws Exception + public void testAsContentSourceWithOffset(Resource resource) throws Exception { TestSink sink = new TestSink(); Callback.Completable callback = new Callback.Completable(); @@ -211,7 +211,7 @@ public void testAsContentSourceWithLength(Resource resource) throws Exception @ParameterizedTest @MethodSource("all") - public void testAsContentSourceWithFirstAndLength(Resource resource) throws Exception + public void testAsContentSourceWithOffsetAndLength(Resource resource) throws Exception { TestSink sink = new TestSink(); Callback.Completable callback = new Callback.Completable(); @@ -243,7 +243,7 @@ public void testCopy(Resource resource) throws Exception @ParameterizedTest @MethodSource("all") - public void testCopyWithFirst(Resource resource) throws Exception + public void testCopyWithOffset(Resource resource) throws Exception { TestSink sink = new TestSink(); Callback.Completable callback = new Callback.Completable(); @@ -273,7 +273,7 @@ public void testCopyWithLength(Resource resource) throws Exception @ParameterizedTest @MethodSource("all") - public void testCopyWithFirstAndLength(Resource resource) throws Exception + public void testCopyWithOffsetAndLength(Resource resource) throws Exception { TestSink sink = new TestSink(); Callback.Completable callback = new Callback.Completable(); From 01d9696d97e7776f339446b9444ce3bec7c19b0c Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Sun, 12 Oct 2025 08:36:49 +1100 Subject: [PATCH 04/26] updates from reviews --- .../org/eclipse/jetty/io/internal/ByteChannelContentSource.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java index 2e1eef8814d7..cac6db5f6263 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java @@ -123,6 +123,7 @@ private void invokeDemandCallback() protected void lockedSetTerminal(Content.Chunk terminal) { + assert lock.isHeldByCurrentThread(); if (_terminal == null) _terminal = Objects.requireNonNull(terminal); else @@ -135,6 +136,7 @@ protected void lockedSetTerminal(Content.Chunk terminal) private void lockedEnsureOpenOrTerminal() { + assert lock.isHeldByCurrentThread(); if (_terminal == null && (_byteChannel == null || !_byteChannel.isOpen())) { try From a3002095aeb61edffd65b39eae0abe3925511a61 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 13 Oct 2025 08:44:32 +1100 Subject: [PATCH 05/26] Propose the checkOffsetLengthSize contract --- .../org/eclipse/jetty/http/MultiPart.java | 13 ++++- .../java/org/eclipse/jetty/io/Content.java | 47 ++++++++++++++----- .../java/org/eclipse/jetty/util/TypeUtil.java | 27 +++++++++++ 3 files changed, 73 insertions(+), 14 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java index f6d5fd9a9ee3..e0220d9f2fc1 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 @@ -500,10 +500,21 @@ public ByteBufferPart(String name, String fileName, HttpFields fields, List= 0) ? size - offset : length; + } } From ce185bf86f3f3c98dc6ef6ed7d34edfa68351e38 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 13 Oct 2025 09:28:59 +1100 Subject: [PATCH 06/26] Propose the checkOffsetLengthSize contract WIP --- .../org/eclipse/jetty/http/MultiPart.java | 35 +++++++-- .../org/eclipse/jetty/io/IOResources.java | 16 +++- .../io/content/ByteBufferContentSource.java | 16 +++- .../org/eclipse/jetty/util/BufferUtil.java | 78 +++++++++++++++++++ 4 files changed, 135 insertions(+), 10 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java index e0220d9f2fc1..a3eab4580fcb 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 @@ -512,10 +512,7 @@ public long getLength() @Override public Content.Source newContentSource(ByteBufferPool.Sized bufferPool, long offset, long length) { - long size = getLength(); - length = TypeUtil.checkOffsetLengthSize(offset, length, size); - // TODO slice the content ByteBuffers according to offset and length - return new ByteBufferContentSource(content); + return new ByteBufferContentSource(content, offset, length); } @Override @@ -548,10 +545,30 @@ public ChunksPart(String name, String fileName, HttpFields fields, List byteBuffers) { - this.byteBuffers = byteBuffers; - this.length = byteBuffers.stream().mapToLong(Buffer::remaining).sum(); + this(byteBuffers, 0, -1); + } + + public ByteBufferContentSource(Collection byteBuffers, long offset, long length) + { + long size = byteBuffers.stream().mapToLong(Buffer::remaining).sum(); + length = TypeUtil.checkOffsetLengthSize(offset, length, size); + if (offset == 0 && size == length) + this.byteBuffers = byteBuffers; + else + this.byteBuffers = BufferUtil.sliceByteBuffers(byteBuffers, offset, length); + this.length = length; } public Collection getByteBuffers() 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 2fb397157c29..6027b7d3203e 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 @@ -32,7 +32,11 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Objects; import org.eclipse.jetty.util.resource.Resource; import org.slf4j.Logger; @@ -312,6 +316,80 @@ public static ByteBuffer slice(ByteBuffer buffer, int offset, int length) return slice; } + /** + * Slice a collection of ByteBuffers given an offset and a length, with checks similar to + * {@link TypeUtil#checkOffsetLengthSize(long, long, long)}. + * + * @param byteBuffers The buffers to slice (positions/limits respected; no mutation) + * @param offset The offset, relative to the current position of the buffers, must be non-negative + * @param length The length; -1 means "to the end" of the buffers + * @return the sliced buffers in a new collection, which may be immutable and may contain the original buffers or slices of them + * @throws IndexOutOfBoundsException if the offset or length are invalid + */ + public static Collection sliceByteBuffers(Collection byteBuffers, long offset, long length) + { + Objects.requireNonNull(byteBuffers); + if (offset < 0) + throw new IndexOutOfBoundsException("offset < 0: " + offset); + if (length < -1) + throw new IndexOutOfBoundsException("length < -1: " + length); + + if (length == 0) + return Collections.emptyList(); + + boolean allAvailable = (length == -1); + ArrayList sliced = new ArrayList<>(byteBuffers.size()); + + for (ByteBuffer buffer : byteBuffers) + { + int remaining = BufferUtil.length(buffer); + if (remaining <= 0) + continue; + + if (offset >= remaining) + { + offset -= remaining; + continue; + } + + if (allAvailable) + { + if (offset == 0) + { + sliced.add(buffer); + } + else + { + sliced.add(slice(buffer, (int)offset, -1)); + offset = 0; + } + continue; + } + + if (length == 0) + break; + + long bytes = (long)remaining - offset; + if (bytes > length) + { + sliced.add(slice(buffer, (int)offset, (int)length)); + length = 0; + offset = 0; + break; + } + sliced.add(slice(buffer, (int)offset, (int)bytes)); + length -= bytes; + offset = 0; + } + + if (offset > 0) + throw new IndexOutOfBoundsException("offset too large; leftover=" + offset); + if (!allAvailable && length > 0) + throw new IndexOutOfBoundsException("length too large; leftover=" + length); + + return sliced; + } + /** * Convert a ByteBuffer to a byte array. * From e8cd8ddd10eb2588a381adc02be3bd2edba7ec71 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 13 Oct 2025 15:02:49 +1100 Subject: [PATCH 07/26] Propose the checkOffsetLengthSize contract unit test --- .../io/content/ByteBufferContentSource.java | 2 +- .../org/eclipse/jetty/util/BufferUtil.java | 10 +- .../eclipse/jetty/util/BufferUtilTest.java | 124 ++++++++++++++---- 3 files changed, 102 insertions(+), 34 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ByteBufferContentSource.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ByteBufferContentSource.java index 66d42b228670..15037d172fdf 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ByteBufferContentSource.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ByteBufferContentSource.java @@ -57,7 +57,7 @@ public ByteBufferContentSource(Collection byteBuffers, long offset, if (offset == 0 && size == length) this.byteBuffers = byteBuffers; else - this.byteBuffers = BufferUtil.sliceByteBuffers(byteBuffers, offset, length); + this.byteBuffers = BufferUtil.slice(byteBuffers, offset, length); this.length = length; } 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 6027b7d3203e..1b03128653a5 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 @@ -35,7 +35,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.Objects; import org.eclipse.jetty.util.resource.Resource; @@ -326,7 +325,7 @@ public static ByteBuffer slice(ByteBuffer buffer, int offset, int length) * @return the sliced buffers in a new collection, which may be immutable and may contain the original buffers or slices of them * @throws IndexOutOfBoundsException if the offset or length are invalid */ - public static Collection sliceByteBuffers(Collection byteBuffers, long offset, long length) + public static Collection slice(Collection byteBuffers, long offset, long length) { Objects.requireNonNull(byteBuffers); if (offset < 0) @@ -334,9 +333,6 @@ public static Collection sliceByteBuffers(Collection byt if (length < -1) throw new IndexOutOfBoundsException("length < -1: " + length); - if (length == 0) - return Collections.emptyList(); - boolean allAvailable = (length == -1); ArrayList sliced = new ArrayList<>(byteBuffers.size()); @@ -383,9 +379,9 @@ public static Collection sliceByteBuffers(Collection byt } if (offset > 0) - throw new IndexOutOfBoundsException("offset too large; leftover=" + offset); + throw new IndexOutOfBoundsException("offset too large by " + offset); if (!allAvailable && length > 0) - throw new IndexOutOfBoundsException("length too large; leftover=" + length); + throw new IndexOutOfBoundsException("length too large by" + length); return sliced; } diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/BufferUtilTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/BufferUtilTest.java index 22351fe7e758..c6d2518768bc 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/BufferUtilTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/BufferUtilTest.java @@ -15,12 +15,13 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.net.URI; import java.nio.BufferOverflowException; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.util.Arrays; +import java.util.Collection; +import java.util.List; import java.util.concurrent.ThreadLocalRandom; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; @@ -31,8 +32,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.parallel.Isolated; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; @@ -43,6 +42,8 @@ import static org.hamcrest.Matchers.sameInstance; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -80,7 +81,83 @@ public void testSlice() } @Test - public void testToInt() throws Exception + public void testSliceCollection() + { + ByteBuffer empty = ByteBuffer.allocate(0); + ByteBuffer one = ByteBuffer.wrap("1".getBytes(StandardCharsets.UTF_8)); + ByteBuffer digits = ByteBuffer.wrap("0123456789".getBytes(StandardCharsets.UTF_8)); + ByteBuffer alpha = ByteBuffer.wrap("abcdefghijklmnopqrstuvwxyz".getBytes(StandardCharsets.UTF_8)); + + Collection slices = BufferUtil.slice(List.of(), 0, 0); + assertThat(slices.size(), is(0)); + + slices = BufferUtil.slice(List.of(empty), 0, 0); + assertThat(slices.size(), is(0)); + + slices = BufferUtil.slice(List.of(one), 0, 0); + assertThat(slices.size(), is(0)); + + slices = BufferUtil.slice(List.of(empty, one, digits), 0, 0); + assertThat(slices.size(), is(0)); + + Throwable thrown = assertThrows(IndexOutOfBoundsException.class, () -> BufferUtil.slice(List.of(), 1, 0)); + assertThat(thrown.getMessage(), containsString("offset too large")); + thrown = assertThrows(IndexOutOfBoundsException.class, () -> BufferUtil.slice(List.of(empty), 1, 0)); + assertThat(thrown.getMessage(), containsString("offset too large")); + thrown = assertThrows(IndexOutOfBoundsException.class, () -> BufferUtil.slice(List.of(empty, empty), 1, 0)); + assertThat(thrown.getMessage(), containsString("offset too large")); + thrown = assertThrows(IndexOutOfBoundsException.class, () -> BufferUtil.slice(List.of(empty, one, empty), 2, 0)); + assertThat(thrown.getMessage(), containsString("offset too large")); + thrown = assertThrows(IndexOutOfBoundsException.class, () -> BufferUtil.slice(List.of(empty, one, alpha), 28, 0)); + assertThat(thrown.getMessage(), containsString("offset too large")); + + thrown = assertThrows(IndexOutOfBoundsException.class, () -> BufferUtil.slice(List.of(), 0, 1)); + assertThat(thrown.getMessage(), containsString("length too large")); + thrown = assertThrows(IndexOutOfBoundsException.class, () -> BufferUtil.slice(List.of(empty), 0, 1)); + assertThat(thrown.getMessage(), containsString("length too large")); + thrown = assertThrows(IndexOutOfBoundsException.class, () -> BufferUtil.slice(List.of(empty, empty), 0, 1)); + assertThat(thrown.getMessage(), containsString("length too large")); + thrown = assertThrows(IndexOutOfBoundsException.class, () -> BufferUtil.slice(List.of(empty, one, empty), 0, 2)); + assertThat(thrown.getMessage(), containsString("length too large")); + thrown = assertThrows(IndexOutOfBoundsException.class, () -> BufferUtil.slice(List.of(empty, one, alpha), 0, 28)); + assertThat(thrown.getMessage(), containsString("length too large")); + + thrown = assertThrows(IndexOutOfBoundsException.class, () -> BufferUtil.slice(List.of(one), 1, 1)); + assertThat(thrown.getMessage(), containsString("length too large")); + thrown = assertThrows(IndexOutOfBoundsException.class, () -> BufferUtil.slice(List.of(one), 1, 1)); + assertThat(thrown.getMessage(), containsString("length too large")); + thrown = assertThrows(IndexOutOfBoundsException.class, () -> BufferUtil.slice(List.of(empty, one, empty), 1, 1)); + assertThat(thrown.getMessage(), containsString("length too large")); + thrown = assertThrows(IndexOutOfBoundsException.class, () -> BufferUtil.slice(List.of(empty, one, digits), 1, 11)); + assertThat(thrown.getMessage(), containsString("length too large")); + thrown = assertThrows(IndexOutOfBoundsException.class, () -> BufferUtil.slice(List.of(empty, digits, alpha), 10, 27)); + assertThat(thrown.getMessage(), containsString("length too large")); + + String sliced = BufferUtil.slice(List.of(empty, one), 0, 1).stream().map(BufferUtil::toString).reduce("", (a, b) -> a + b); + assertThat(sliced, is("1")); + sliced = BufferUtil.slice(List.of(empty, one, digits, empty, alpha), 0, 10).stream().map(BufferUtil::toString).reduce("", (a, b) -> a + b); + assertThat(sliced, is("1012345678")); + sliced = BufferUtil.slice(List.of(empty, one, digits, empty, alpha), 1, 10).stream().map(BufferUtil::toString).reduce("", (a, b) -> a + b); + assertThat(sliced, is("0123456789")); + sliced = BufferUtil.slice(List.of(empty, one, digits, empty, alpha), 1, 11).stream().map(BufferUtil::toString).reduce("", (a, b) -> a + b); + assertThat(sliced, is("0123456789a")); + + sliced = BufferUtil.slice(List.of(empty, one, digits, empty, alpha), 0, -1).stream().map(BufferUtil::toString).reduce("", (a, b) -> a + b); + assertThat(sliced, is("10123456789abcdefghijklmnopqrstuvwxyz")); + sliced = BufferUtil.slice(List.of(empty, one, digits, empty, alpha), 1, -1).stream().map(BufferUtil::toString).reduce("", (a, b) -> a + b); + assertThat(sliced, is("0123456789abcdefghijklmnopqrstuvwxyz")); + sliced = BufferUtil.slice(List.of(empty, one, digits, empty, alpha), 11, -1).stream().map(BufferUtil::toString).reduce("", (a, b) -> a + b); + assertThat(sliced, is("abcdefghijklmnopqrstuvwxyz")); + sliced = BufferUtil.slice(List.of(empty, one, digits, empty, alpha), 12, -1).stream().map(BufferUtil::toString).reduce("", (a, b) -> a + b); + assertThat(sliced, is("bcdefghijklmnopqrstuvwxyz")); + sliced = BufferUtil.slice(List.of(empty, one, digits, empty, alpha), 36, -1).stream().map(BufferUtil::toString).reduce("", (a, b) -> a + b); + assertThat(sliced, is("z")); + sliced = BufferUtil.slice(List.of(empty, one, digits, empty, alpha), 37, -1).stream().map(BufferUtil::toString).reduce("", (a, b) -> a + b); + assertThat(sliced, is("")); + } + + @Test + public void testToInt() { ByteBuffer[] buf = { @@ -105,7 +182,7 @@ public void testToInt() throws Exception } @Test - public void testPutInt() throws Exception + public void testPutInt() { int[] val = { @@ -129,7 +206,7 @@ public void testPutInt() throws Exception } @Test - public void testPutLong() throws Exception + public void testPutLong() { long[] val = { @@ -153,7 +230,7 @@ public void testPutLong() throws Exception } @Test - public void testPutHexInt() throws Exception + public void testPutHexInt() { int[] val = { @@ -177,7 +254,7 @@ public void testPutHexInt() throws Exception } @Test - public void testPut() throws Exception + public void testPut() { ByteBuffer to = BufferUtil.allocate(10); ByteBuffer from = BufferUtil.toBuffer("12345"); @@ -196,7 +273,7 @@ public void testPut() throws Exception } @Test - public void testAppend() throws Exception + public void testAppend() { ByteBuffer to = BufferUtil.allocate(8); ByteBuffer from = BufferUtil.toBuffer("12345"); @@ -206,14 +283,11 @@ public void testAppend() throws Exception BufferUtil.append(to, from.array(), 3, 2); assertEquals("12345", BufferUtil.toString(to)); - assertThrows(BufferOverflowException.class, () -> - { - BufferUtil.append(to, from.array(), 0, 5); - }); + assertThrows(BufferOverflowException.class, () -> BufferUtil.append(to, from.array(), 0, 5)); } @Test - public void testPutDirect() throws Exception + public void testPutDirect() { ByteBuffer to = BufferUtil.allocateDirect(10); ByteBuffer from = BufferUtil.toBuffer("12345"); @@ -242,7 +316,7 @@ public void testToBufferArray() while (buf.remaining() > 0) { byte b = buf.get(); - assertEquals(b, 0x44); + assertEquals(0x44, b); count++; } @@ -253,7 +327,7 @@ public void testToBufferArray() public void testToBufferArrayOffsetLength() { byte[] arr = new byte[128]; - Arrays.fill(arr, (byte)0xFF); // fill whole thing with FF + Arrays.fill(arr, (byte)0xFF); // fill the whole thing with FF int offset = 10; int length = 100; Arrays.fill(arr, offset, offset + length, (byte)0x77); // fill partial with 0x77 @@ -263,15 +337,13 @@ public void testToBufferArrayOffsetLength() while (buf.remaining() > 0) { byte b = buf.get(); - assertEquals(b, 0x77); + assertEquals(0x77, b); count++; } assertEquals(length, count, "Count of bytes"); } - private static final Logger LOG = LoggerFactory.getLogger(BufferUtilTest.class); - @Test public void testWriteToWithBufferThatDoesNotExposeArrayAndSmallContent() throws IOException { @@ -297,15 +369,15 @@ public void testWriteToWithBufferThatDoesNotExposeArrayAndContentSlightlyBiggerT @Test @SuppressWarnings("ReferenceEquality") - public void testEnsureCapacity() throws Exception + public void testEnsureCapacity() { ByteBuffer b = BufferUtil.toBuffer("Goodbye Cruel World"); - assertTrue(b == BufferUtil.ensureCapacity(b, 0)); - assertTrue(b == BufferUtil.ensureCapacity(b, 10)); - assertTrue(b == BufferUtil.ensureCapacity(b, b.capacity())); + assertSame(b, BufferUtil.ensureCapacity(b, 0)); + assertSame(b, BufferUtil.ensureCapacity(b, 10)); + assertSame(b, BufferUtil.ensureCapacity(b, b.capacity())); ByteBuffer b1 = BufferUtil.ensureCapacity(b, 64); - assertTrue(b != b1); + assertNotSame(b, b1); assertEquals(64, b1.capacity()); assertEquals("Goodbye Cruel World", BufferUtil.toString(b1)); @@ -318,10 +390,10 @@ public void testEnsureCapacity() throws Exception assertEquals(8, b2.arrayOffset()); assertEquals(5, b2.capacity()); - assertTrue(b2 == BufferUtil.ensureCapacity(b2, 5)); + assertSame(b2, BufferUtil.ensureCapacity(b2, 5)); ByteBuffer b3 = BufferUtil.ensureCapacity(b2, 64); - assertTrue(b2 != b3); + assertNotSame(b2, b3); assertEquals(64, b3.capacity()); assertEquals("Cruel", BufferUtil.toString(b3)); assertEquals(0, b3.arrayOffset()); From 4cca3168098b24c4623b66f326608acd7f2b0547 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 13 Oct 2025 15:17:33 +1100 Subject: [PATCH 08/26] Propose the checkOffsetLengthSize contract WIP --- .../jetty/io/internal/ByteChannelContentSource.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java index cac6db5f6263..b0b1f216f42c 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java @@ -29,6 +29,7 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.ExceptionUtil; import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.SerializedInvoker; @@ -275,15 +276,12 @@ public PathContentSource(ByteBufferPool.Sized byteBufferPool, Path path) public PathContentSource(ByteBufferPool.Sized byteBufferPool, Path path, long offset, long length) { - this (byteBufferPool, path, size(path), offset, length); + this(byteBufferPool, path, size(path), offset, length); } private PathContentSource(ByteBufferPool.Sized byteBufferPool, Path path, long size, long offset, long length) { - // TODO Is this the contract we want for offset/length? auto correcting it? Validity can be checked in super. - super(byteBufferPool, null, - Math.min(offset, size), - Math.min(length, size - offset)); + super(byteBufferPool, null, offset, TypeUtil.checkOffsetLengthSize(offset, length, size)); _path = path; } From 7ed9ed722fed3ea846e96c699abc778f59150e16 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 13 Oct 2025 15:28:55 +1100 Subject: [PATCH 09/26] Propose the checkOffsetLengthSize contract WIP --- .../src/main/java/org/eclipse/jetty/http/MultiPart.java | 4 +++- .../jetty-io/src/main/java/org/eclipse/jetty/io/Content.java | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java index a3eab4580fcb..1dcaef100bb5 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 @@ -354,7 +354,9 @@ public final Content.Source createContentSource() @Override public Content.Source newContentSource(ByteBufferPool.Sized bufferPool, long offset, long length) { - // TODO support offset and length + // TODO support offset and length, or document why we are keeping this + // or wrap the returned Content.Source to support offset and length. + // or document that we should wrap if somebody needs it. return newContentSource(); } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java index 24c643dc6b68..6756b5d7f851 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java @@ -175,7 +175,7 @@ interface Factory * otherwise must be greater than 0 and less than or equal to the resource length (if known) minus the offset. * @return a {@link Content.Source}. * @throws IndexOutOfBoundsException if the offset or length are out of range. - * @see Objects#checkFromIndexSize(long, long, long) + * @see TypeUtil#checkOffsetLengthSize(long, long, long) */ Content.Source newContentSource(ByteBufferPool.Sized bufferPool, long offset, long length); } From ccb80c0cd61c801741e160d46c95c10b481360a6 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 13 Oct 2025 15:32:48 +1100 Subject: [PATCH 10/26] Propose the checkOffsetLengthSize contract WIP --- .../jetty-io/src/main/java/org/eclipse/jetty/io/Content.java | 1 - .../org/eclipse/jetty/io/content/InputStreamContentSource.java | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java index 6756b5d7f851..ee777ff4860e 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java @@ -305,7 +305,6 @@ static Content.Source from(ByteBufferPool.Sized byteBufferPool, InputStream inpu */ static Content.Source from(ByteBufferPool.Sized byteBufferPool, InputStream inputStream, long offset, long length) { - // TODO define contract for offset/lengths outside of bounds return new InputStreamContentSource(inputStream, byteBufferPool, offset, length); } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/InputStreamContentSource.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/InputStreamContentSource.java index f96954279544..35cf058aee93 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/InputStreamContentSource.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/InputStreamContentSource.java @@ -24,6 +24,7 @@ import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.util.ExceptionUtil; import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.SerializedInvoker; @@ -63,6 +64,7 @@ public InputStreamContentSource(InputStream inputStream, ByteBufferPool.Sized bu public InputStreamContentSource(InputStream inputStream, ByteBufferPool.Sized bufferPool, long offset, long length) { + length = TypeUtil.checkOffsetLengthSize(offset, length, -1); this.inputStream = Objects.requireNonNull(inputStream); this.bufferPool = Objects.requireNonNullElse(bufferPool, ByteBufferPool.SIZED_NON_POOLING); if (length != 0) From a61e7f382b7abce6f19bd1418032efee9eb2289f Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 13 Oct 2025 16:49:44 +1100 Subject: [PATCH 11/26] More forgiving offsetLengthSize contract --- .../java/org/eclipse/jetty/io/Content.java | 38 ++++++------ .../org/eclipse/jetty/io/IOResourcesTest.java | 9 ++- .../java/org/eclipse/jetty/util/TypeUtil.java | 51 ++++++++-------- .../org/eclipse/jetty/util/TypeUtilTest.java | 61 +++++++++++++++---- 4 files changed, 100 insertions(+), 59 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java index ee777ff4860e..9ff9fc1d8dd1 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java @@ -169,10 +169,10 @@ interface Factory * Creates a new {@link Content.Source}. * * @param bufferPool the {@link ByteBufferPool.Sized} to get buffers from. {@code null} means allocate new buffers as needed. - * @param offset the offset byte of the resource to start from. - * Must be greater than or equal to 0 and less than the resource length (if known). - * @param length the length of the content to make available, -1 for the full length, - * otherwise must be greater than 0 and less than or equal to the resource length (if known) minus the offset. + * @param offset the offset byte of the content to start from. + * Must be greater than or equal to 0 and less than the content length (if known). + * @param length the length of the content to make available, -1 for the full length. + * If the size of the content is known, the length may be truncated to the content size minus the offset. * @return a {@link Content.Source}. * @throws IndexOutOfBoundsException if the offset or length are out of range. * @see TypeUtil#checkOffsetLengthSize(long, long, long) @@ -204,10 +204,10 @@ static Content.Source from(Path path) * Create a {@code Content.Source} from a {@link Path}. * * @param path The {@link Path}s to use as the source. - * @param offset the offset byte of the resource to start from. - * Must be greater than or equal to 0 and less than the resource size (if known). - * @param length the length of the content to make available, -1 for the full length available, - * otherwise must be greater than 0 and less than or equal to the resource size (if known) minus the offset. + * @param offset the offset byte of the content to start from. + * Must be greater than or equal to 0 and less than the content length (if known). + * @param length the length of the content to make available, -1 for the full length. + * If the size of the content is known, the length may be truncated to the content size minus the offset. * @return a {@link Content.Source}. * @throws IndexOutOfBoundsException if the offset or length are out of range. * @see TypeUtil#checkOffsetLengthSize(long, long, long) @@ -232,10 +232,10 @@ static Content.Source from(ByteBufferPool.Sized byteBufferPool, Path path) * Create a {@code Content.Source} from a {@link Path}. * @param byteBufferPool The {@link org.eclipse.jetty.io.ByteBufferPool.Sized} to use for any internal buffers. * @param path The {@link Path}s to use as the source. - * @param offset the offset byte of the resource to start from. - * Must be greater than or equal to 0 and less than the resource size (if known). - * @param length the length of the content to make available, -1 for the full length available, - * otherwise must be greater than 0 and less than or equal to the resource size (if known) minus the offset. + * @param offset the offset byte of the content to start from. + * Must be greater than or equal to 0 and less than the content length (if known). + * @param length the length of the content to make available, -1 for the full length. + * If the size of the content is known, the length may be truncated to the content size minus the offset. * @return a {@link Content.Source}. * @throws IndexOutOfBoundsException if the offset or length are out of range. * @see TypeUtil#checkOffsetLengthSize(long, long, long) @@ -261,10 +261,10 @@ static Content.Source from(ByteBufferPool.Sized byteBufferPool, ByteChannel byte * Create a {@code Content.Source} from a {@link ByteChannel}. * @param byteBufferPool The {@link org.eclipse.jetty.io.ByteBufferPool.Sized} to use for any internal buffers. * @param seekableByteChannel The {@link ByteChannel}s to use as the source. - * @param offset the offset byte of the resource to start from. - * Must be greater than or equal to 0 and less than the resource size (if known). - * @param length the length of the content to make available, -1 for the full length available, - * otherwise must be greater than 0 and less than or equal to the resource size (if known) minus the offset. + * @param offset the offset byte of the content to start from. + * Must be greater than or equal to 0 and less than the content length (if known). + * @param length the length of the content to make available, -1 for the full length. + * If the size of the content is known, the length may be truncated to the content size minus the offset. * @return a {@link Content.Source}. * @throws IndexOutOfBoundsException if the offset or length are out of range. * @see TypeUtil#checkOffsetLengthSize(long, long, long) @@ -296,9 +296,9 @@ static Content.Source from(ByteBufferPool.Sized byteBufferPool, InputStream inpu * @param byteBufferPool The {@link org.eclipse.jetty.io.ByteBufferPool.Sized} to use for any internal buffers. * @param inputStream The {@link InputStream}s to use as the source. * @param offset the offset byte of the resource to start from. - * Must be greater than or equal to 0 and less than the resource size (if known). - * @param length the length of the content to make available, -1 for the full length available, - * otherwise must be greater than 0 and less than or equal to the resource size (if known) minus the offset. + * Must be greater than or equal to 0 and less than the resource size (if known). + * @param length the length of the content to make available, or -1 for the full length available. + * The length may be truncated if the stream ends sooner. * @return a {@link Content.Source}. * @throws IndexOutOfBoundsException if the offset or length are out of range. * @see TypeUtil#checkOffsetLengthSize(long, long, long) diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/IOResourcesTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/IOResourcesTest.java index 6a8e2d136aef..4f6dfa5183ba 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/IOResourcesTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/IOResourcesTest.java @@ -26,6 +26,7 @@ import org.eclipse.jetty.util.Blocker; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceFactory; import org.eclipse.jetty.util.resource.URLResourceFactory; @@ -135,6 +136,7 @@ public Resource resolve(String subUriPath) @Override public Content.Source newContentSource(ByteBufferPool.Sized bufferPool, long offset, long length) { + length = TypeUtil.checkOffsetLengthSize(offset, length, buffer.remaining()); return Content.Source.from(BufferUtil.slice(buffer, Math.toIntExact(offset), Math.toIntExact(length))); } } @@ -185,6 +187,11 @@ public void testAsContentSourceWithOffset(Resource resource) throws Exception TestSink sink = new TestSink(); Callback.Completable callback = new Callback.Completable(); + if (resource.length() < 100) + { + assertThrows(IndexOutOfBoundsException.class, () -> IOResources.asContentSource(resource, bufferPool, 100, -1)); + return; + } Content.Source contentSource = IOResources.asContentSource(resource, bufferPool, 100, -1); Content.copy(contentSource, sink, callback); callback.get(); @@ -294,7 +301,7 @@ public void testOutOfRangeOffset(Resource resource) TestSink sink = new TestSink(); Blocker.Callback callback = Blocker.callback(); IOResources.copy(resource, sink, bufferPool, Integer.MAX_VALUE, 1, callback); - assertThrows(IllegalArgumentException.class, callback::block); + assertThrows(IndexOutOfBoundsException.class, callback::block); } @ParameterizedTest diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java index 743f72563ac5..065d9a5e784d 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java @@ -409,17 +409,7 @@ public static String toString(byte[] bytes, int base) { StringBuilder buf = new StringBuilder(); for (byte b : bytes) - { - int bi = 0xff & b; - int c = '0' + (bi / base) % base; - if (c > '9') - c = 'a' + (c - '0' - 10); - buf.append((char)c); - c = '0' + bi % base; - if (c > '9') - c = 'a' + (c - '0' - 10); - buf.append((char)c); - } + StringUtil.append(buf, b, base); return buf.toString(); } @@ -781,7 +771,7 @@ public static Stream> serviceProviderStream(Servic */ public static Predicate truePredicate() { - return new Predicate() + return new Predicate<>() { @Override public boolean test(T t) @@ -817,7 +807,7 @@ public Predicate or(Predicate other) */ public static Predicate falsePredicate() { - return new Predicate() + return new Predicate<>() { @Override public boolean test(T t) @@ -909,28 +899,35 @@ else if (value.getClass().isArray()) /** * Check that the offset and length of a subrange are within the size of a range. * @param offset an offset of a subrange within a range of 0 to {code size} to start from. - * Must be greater than or equal to 0 and less than the range size (if known). + * Must be greater than or equal to 0 and less than the range size (if known). * @param length the length of the subrange, -1 for the full length available from the offset, - * otherwise must be greater than or equal to 0 and less than or equal to the range size (if known) minus the offset. + * otherwise must be greater than or equal to 0. + * If the size is known, then the length will be reduced if necessary to fit within the size-offset. * @param size the size of the range, or -1 if unknown. - * @return the length of the subrange, which if the passed length was -1 and the size is known, will be size-offset. + * @return the length of the subrange, which may be calculated if length was less than 0 or if the length is limited by + * a known size. * @throws IndexOutOfBoundsException if the offset or length are out of range. - * @see Objects#checkFromIndexSize(long, long, long) + * @throws ArithmeticException if the length is not -1 and offset+length overflows. */ public static long checkOffsetLengthSize(long offset, long length, long size) { - if (size < 0) - { - if (offset < 0) - throw new IndexOutOfBoundsException("Negative offset: " + offset); - if (length < -1) - throw new IndexOutOfBoundsException("Length less than -1: " + length); - } - else + if (length == 0) + return 0; + + if (offset < 0) + throw new IndexOutOfBoundsException("Negative offset: " + offset); + if (length < -1) + throw new IndexOutOfBoundsException("Length less than -1: " + length); + + if (size >= 0) { - Objects.checkFromIndexSize(offset, length < 0 ? size - offset : length, size); + if (offset > size) + throw new IndexOutOfBoundsException("Offset > Size: " + offset + " > " + size); + + if (length == -1 || Math.addExact(offset, length) > size) + length = size - offset; } - return (length < 0 && size >= 0) ? size - offset : length; + return length; } } diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/TypeUtilTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/TypeUtilTest.java index a11d0e995707..8e07724e3052 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/TypeUtilTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/TypeUtilTest.java @@ -15,12 +15,14 @@ import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Objects; import java.util.stream.Stream; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceFactory; import org.eclipse.jetty.util.test10.ExampleClass; import org.hamcrest.Matchers; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -31,11 +33,13 @@ import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; public class TypeUtilTest { + @Test public void convertHexDigitTest() { @@ -46,12 +50,12 @@ public void convertHexDigitTest() assertEquals((byte)15, TypeUtil.convertHexDigit((byte)'f')); assertEquals((byte)15, TypeUtil.convertHexDigit((byte)'F')); - assertEquals((int)0, TypeUtil.convertHexDigit((int)'0')); - assertEquals((int)9, TypeUtil.convertHexDigit((int)'9')); - assertEquals((int)10, TypeUtil.convertHexDigit((int)'a')); - assertEquals((int)10, TypeUtil.convertHexDigit((int)'A')); - assertEquals((int)15, TypeUtil.convertHexDigit((int)'f')); - assertEquals((int)15, TypeUtil.convertHexDigit((int)'F')); + assertEquals(0, TypeUtil.convertHexDigit((int)'0')); + assertEquals(9, TypeUtil.convertHexDigit((int)'9')); + assertEquals(10, TypeUtil.convertHexDigit((int)'a')); + assertEquals(10, TypeUtil.convertHexDigit((int)'A')); + assertEquals(15, TypeUtil.convertHexDigit((int)'f')); + assertEquals(15, TypeUtil.convertHexDigit((int)'F')); } @Test @@ -60,7 +64,7 @@ public void testToHexInt() throws Exception StringBuilder b = new StringBuilder(); b.setLength(0); - TypeUtil.toHex((int)0, b); + TypeUtil.toHex(0, b); assertEquals("00000000", b.toString()); b.setLength(0); @@ -202,7 +206,7 @@ public String toString() } @Test - public void testGetLocationOfClassFromMavenRepo() throws Exception + public void testGetLocationOfClassFromMavenRepo() { String mavenRepoPathProperty = System.getProperty("mavenRepoPath"); assumeTrue(mavenRepoPathProperty != null); @@ -211,7 +215,7 @@ public void testGetLocationOfClassFromMavenRepo() throws Exception // Classes from maven dependencies try (ResourceFactory.Closeable resourceFactory = ResourceFactory.closeable()) { - Resource resource = resourceFactory.newResource(TypeUtil.getLocationOfClass(org.junit.jupiter.api.Assertions.class).toASCIIString()); + Resource resource = resourceFactory.newResource(Objects.requireNonNull(TypeUtil.getLocationOfClass(Assertions.class)).toASCIIString()); assertThat(resource.getPath().toString(), Matchers.startsWith(mavenRepoPath.toString())); } } @@ -220,7 +224,7 @@ public void testGetLocationOfClassFromMavenRepo() throws Exception public void getLocationOfClassClassDirectory() { // Class from project dependencies - assertThat(TypeUtil.getLocationOfClass(TypeUtil.class).toASCIIString(), containsString("/classes/")); + assertThat(Objects.requireNonNull(TypeUtil.getLocationOfClass(TypeUtil.class)).toASCIIString(), containsString("/classes/")); } @Test @@ -228,7 +232,7 @@ public void testGetLocationJvmCoreJPMS() { // Class from JVM core String expectedJavaBase = "/java.base"; - assertThat(TypeUtil.getLocationOfClass(String.class).toASCIIString(), containsString(expectedJavaBase)); + assertThat(Objects.requireNonNull(TypeUtil.getLocationOfClass(String.class)).toASCIIString(), containsString(expectedJavaBase)); } @Test @@ -236,7 +240,7 @@ public void testGetLocationJavaLangThreadDeathJPMS() { // Class from JVM core String expectedJavaBase = "/java.base"; - assertThat(TypeUtil.getLocationOfClass(java.lang.ThreadDeath.class).toASCIIString(), containsString(expectedJavaBase)); + assertThat(Objects.requireNonNull(TypeUtil.getLocationOfClass(ThreadDeath.class)).toASCIIString(), containsString(expectedJavaBase)); } public static Stream shortNames() @@ -285,4 +289,37 @@ public void testIsMethodDeclaredOn() assertFalse(TypeUtil.isDeclaredMethodOn(example, "methodA", String.class)); assertTrue(TypeUtil.isDeclaredMethodOn(example, "methodB", String.class)); } + + public static Stream offsetLengthSizeExpected() + { + return Stream.of( + Arguments.of(0L, 0L, 0L, 0L), + Arguments.of(0L, 0L, 10L, 0L), + Arguments.of(0L, 10L, 10L, 10L), + Arguments.of(5L, 5L, 10L, 5L), + Arguments.of(5L, 5L, 15L, 5L), + Arguments.of(5L, 10L, 15L, 10L), + Arguments.of(0L, 10L, -1L, 10L), + Arguments.of(5L, 10L, -1L, 10L), + Arguments.of(5L, -1L, 15L, 10L), + Arguments.of(0L, -1L, -1L, -1L), + Arguments.of(5L, -1L, -1L, -1L), + Arguments.of(1L, 0L, 0L, 0L), + Arguments.of(0L, 1L, 0L, 0L), + Arguments.of(1L, 1L, 0L, -2L), + Arguments.of(10L, 0L, 10L, 0L), + Arguments.of(0L, 11L, 10L, 10L), + Arguments.of(5L, 6L, 10L, 5L) + ); + } + + @ParameterizedTest + @MethodSource("offsetLengthSizeExpected") + public void testCheckOffsetLengthSize(long offset, long length, long size, long expected) + { + if (expected < -1) + assertThrows(IndexOutOfBoundsException.class, () -> TypeUtil.checkOffsetLengthSize(offset, length, size)); + else + assertThat(TypeUtil.checkOffsetLengthSize(offset, length, size), is(expected)); + } } From bab1fddd55df24cf13084bae17b14207696aea74 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 15 Oct 2025 08:46:57 +1100 Subject: [PATCH 12/26] Updates from review --- .../jetty/io/internal/ByteChannelContentSource.java | 7 +------ .../src/main/java/org/eclipse/jetty/util/BufferUtil.java | 5 ++--- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java index b0b1f216f42c..036be08c380b 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java @@ -276,12 +276,7 @@ public PathContentSource(ByteBufferPool.Sized byteBufferPool, Path path) public PathContentSource(ByteBufferPool.Sized byteBufferPool, Path path, long offset, long length) { - this(byteBufferPool, path, size(path), offset, length); - } - - private PathContentSource(ByteBufferPool.Sized byteBufferPool, Path path, long size, long offset, long length) - { - super(byteBufferPool, null, offset, TypeUtil.checkOffsetLengthSize(offset, length, size)); + super(byteBufferPool, null, offset, TypeUtil.checkOffsetLengthSize(offset, length, size(path))); _path = path; } 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 1b03128653a5..a380c4f881b1 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 @@ -333,7 +333,6 @@ public static Collection slice(Collection byteBuffers, l if (length < -1) throw new IndexOutOfBoundsException("length < -1: " + length); - boolean allAvailable = (length == -1); ArrayList sliced = new ArrayList<>(byteBuffers.size()); for (ByteBuffer buffer : byteBuffers) @@ -348,7 +347,7 @@ public static Collection slice(Collection byteBuffers, l continue; } - if (allAvailable) + if (length == -1) { if (offset == 0) { @@ -380,7 +379,7 @@ public static Collection slice(Collection byteBuffers, l if (offset > 0) throw new IndexOutOfBoundsException("offset too large by " + offset); - if (!allAvailable && length > 0) + if (length > 0) throw new IndexOutOfBoundsException("length too large by" + length); return sliced; From c1d181d984680476386fe78930805b2b1e61e840 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 15 Oct 2025 17:41:56 +1100 Subject: [PATCH 13/26] Implementing TODOs; added ContentSourceRange class and improved testing Signed-off-by: Lachlan Roberts --- .../org/eclipse/jetty/http/MultiPart.java | 14 +- .../java/org/eclipse/jetty/io/Content.java | 23 ++- .../io/internal/ByteChannelContentSource.java | 75 ++++--- .../jetty/io/internal/ContentSourceRange.java | 140 +++++++++++++ .../jetty/io/ContentSourceRangeTest.java | 193 ++++++++++++++++++ 5 files changed, 405 insertions(+), 40 deletions(-) create mode 100644 jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java create mode 100644 jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceRangeTest.java 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 1dcaef100bb5..6762d9f72aef 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 @@ -354,10 +354,9 @@ public final Content.Source createContentSource() @Override public Content.Source newContentSource(ByteBufferPool.Sized bufferPool, long offset, long length) { - // TODO support offset and length, or document why we are keeping this - // or wrap the returned Content.Source to support offset and length. - // or document that we should wrap if somebody needs it. - return newContentSource(); + // We call the deprecated newContentSource() to support existing subclasses. + // All current implementations of Part do override newContentSource(ByteBufferPool.Sized, long, long). + return Content.Source.from(newContentSource(), offset, length); } public long getLength() @@ -569,8 +568,6 @@ public Content.Source newContentSource(ByteBufferPool.Sized bufferPool, long off long size = getLength(); length = TypeUtil.checkOffsetLengthSize(offset, length, size); - // TODO implement offset and length support! - try (AutoLock ignored = lock.lock()) { if (closed) @@ -589,7 +586,7 @@ public Content.Source newContentSource(ByteBufferPool.Sized bufferPool, long off ChunksContentSource newContentSource = new ChunksContentSource(chunks); chunks.forEach(Content.Chunk::release); contentSources.add(newContentSource); - return newContentSource; + return Content.Source.from(newContentSource, offset, length); } } @@ -693,10 +690,9 @@ public long getLength() public Content.Source newContentSource(ByteBufferPool.Sized bufferPool, long offset, long length) { length = TypeUtil.checkOffsetLengthSize(offset, length, content.getLength()); - // TODO implement offset and length support! Content.Source c = content; content = null; - return c; + return Content.Source.from(c, offset, length); } @Override diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java index 9ff9fc1d8dd1..b3552d511ad3 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java @@ -42,6 +42,7 @@ import org.eclipse.jetty.io.internal.ContentCopier; import org.eclipse.jetty.io.internal.ContentSourceByteBuffer; import org.eclipse.jetty.io.internal.ContentSourceConsumer; +import org.eclipse.jetty.io.internal.ContentSourceRange; import org.eclipse.jetty.io.internal.ContentSourceRetainableByteBuffer; import org.eclipse.jetty.io.internal.ContentSourceString; import org.eclipse.jetty.util.Blocker; @@ -217,6 +218,27 @@ static Content.Source from(Path path, long offset, long length) return from(null, path, offset, length); } + /** + * Wrap a {@link Content.Source} to make it appear as a sub-range of the original. + * + * @param source The {@link Content.Source} to wrap. + * @param offset the offset byte of the content to start from. + * Must be greater than or equal to 0 and less than the content length (if known). + * @param length the length of the content to make available, -1 for the full length. + * If the size of the content is known, the length may be truncated to the content size minus the offset. + * @return a {@link Content.Source}. + * @throws IndexOutOfBoundsException if the offset or length are out of range. + * @see TypeUtil#checkOffsetLengthSize(long, long, long) + */ + static Content.Source from(Content.Source source, long offset, long length) + { + // If the offset and length include the full content, then do not wrap. + if (offset == 0 && (length == -1 || length == source.getLength())) + return source; + + return new ContentSourceRange(source, offset, length); + } + /** * Create a {@code Content.Source} from a {@link Path}. * @param byteBufferPool The {@link org.eclipse.jetty.io.ByteBufferPool.Sized} to use for any internal buffers. @@ -271,7 +293,6 @@ static Content.Source from(ByteBufferPool.Sized byteBufferPool, ByteChannel byte */ static Content.Source from(ByteBufferPool.Sized byteBufferPool, SeekableByteChannel seekableByteChannel, long offset, long length) { - // TODO define contract for offset/lengths outside of bounds return new ByteChannelContentSource(byteBufferPool, seekableByteChannel, offset, length); } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java index 036be08c380b..cff615e319ca 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java @@ -46,6 +46,7 @@ public class ByteChannelContentSource implements Content.Source private final long _offset; private final long _length; private RetainableByteBuffer _buffer; + private long _offsetRemaining; private long _totalRead; private Runnable demandCallback; private Content.Chunk _terminal; @@ -57,21 +58,7 @@ public ByteChannelContentSource(SeekableByteChannel seekableByteChannel, long of public ByteChannelContentSource(ByteBufferPool.Sized byteBufferPool, SeekableByteChannel seekableByteChannel, long offset, long length) { - // TODO define contract for offset/lengths outside of bounds this(byteBufferPool, (ByteChannel)seekableByteChannel, offset, length); - if (offset >= 0 && seekableByteChannel != null) - { - try - { - // TODO negative offset is an IAE, but a too large offset is corrected, but length is not checked. - seekableByteChannel.position(offset); - } - catch (IOException e) - { - // lock not needed in constructor - lockedSetTerminal(Content.Chunk.from(e, true)); - } - } } public ByteChannelContentSource(ByteChannel byteChannel) @@ -86,11 +73,11 @@ public ByteChannelContentSource(ByteBufferPool.Sized byteBufferPool, ByteChannel private ByteChannelContentSource(ByteBufferPool.Sized byteBufferPool, ByteChannel byteChannel, long offset, long length) { - // TODO Is this the contract we want for offset/length? We are correcting negative offset, but not checking actual size. _byteBufferPool = Objects.requireNonNullElse(byteBufferPool, ByteBufferPool.SIZED_NON_POOLING); _byteChannel = byteChannel; - _offset = offset < 0 ? 0 : offset; - _length = length; + _offset = offset; + _length = TypeUtil.checkOffsetLengthSize(offset, length, -1L); + _offsetRemaining = offset; } protected ByteChannel open() throws IOException @@ -144,9 +131,14 @@ private void lockedEnsureOpenOrTerminal() { _byteChannel = open(); if (_byteChannel == null || !_byteChannel.isOpen()) + { lockedSetTerminal(Content.Chunk.from(new ClosedChannelException(), true)); - else if (_offset >= 0 && _byteChannel instanceof SeekableByteChannel seekableByteChannel) + } + else if (_byteChannel instanceof SeekableByteChannel seekableByteChannel) + { seekableByteChannel.position(_offset); + _offsetRemaining = 0; + } } catch (IOException e) { @@ -184,6 +176,26 @@ else if (_buffer.isRetained()) try { ByteBuffer byteBuffer = _buffer.getByteBuffer(); + if (_offsetRemaining > 0) + { + // Discard all bytes read until we reach the staring offset. + while (true) + { + BufferUtil.clearToFill(byteBuffer); + byteBuffer.limit((int)Math.min(_buffer.capacity(), _offsetRemaining)); + int read = _byteChannel.read(byteBuffer); + if (read < 0) + { + lockedSetTerminal(Content.Chunk.EOF); + return _terminal; + } + if (read == 0) + return null; + + _offsetRemaining -= read; + } + } + BufferUtil.clearToFill(byteBuffer); if (_length > 0) byteBuffer.limit((int)Math.min(_buffer.capacity(), _length - _totalRead)); @@ -232,6 +244,10 @@ public boolean rewind() { try (AutoLock ignored = lock.lock()) { + // We can only rewind if we have a SeekableByteChannel. + if (!(_byteChannel instanceof SeekableByteChannel seekableByteChannel)) + return false; + // We can remove terminal condition for a rewind that is likely to occur if (_terminal != null && !Content.Chunk.isFailure(_terminal) && (_byteChannel == null || _byteChannel instanceof SeekableByteChannel)) _terminal = null; @@ -240,20 +256,19 @@ public boolean rewind() if (_terminal != null || _byteChannel == null || !_byteChannel.isOpen()) return false; - if (_offset >= 0 && _byteChannel instanceof SeekableByteChannel seekableByteChannel) + try { - try - { - seekableByteChannel.position(_offset); - _totalRead = 0; - return true; - } - catch (Throwable t) - { - lockedSetTerminal(Content.Chunk.from(t, true)); - } + seekableByteChannel.position(_offset); + _offsetRemaining = 0; + _totalRead = 0; + return true; } - return false; + catch (Throwable t) + { + lockedSetTerminal(Content.Chunk.from(t, true)); + } + + return true; } } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java new file mode 100644 index 000000000000..80e9f4832194 --- /dev/null +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java @@ -0,0 +1,140 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.io.internal; + +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.util.TypeUtil; + +public class ContentSourceRange implements Content.Source +{ + private final long _offset; + private final long _length; + private final Content.Source _source; + long _offsetRemaining; + long _lengthRemaining; + + public ContentSourceRange(Content.Source source, long offset, long length) + { + _source = source; + _offset = offset; + _length = TypeUtil.checkOffsetLengthSize(offset, length, source.getLength()); + _offsetRemaining = _offset; + _lengthRemaining = _length; + } + + @Override + public long getLength() + { + return _length; + } + + @Override + public Content.Chunk read() + { + while (true) + { + Content.Chunk chunk = _source.read(); + if (chunk == null) + return null; + + if (_offsetRemaining > 0) + { + if (_offsetRemaining >= chunk.remaining()) + { + // We can skip this whole chunk. + _offsetRemaining -= chunk.remaining(); + if (chunk.isLast()) + { + chunk.clear(); + return chunk; + } + + chunk.release(); + continue; + } + else + { + // Advance position to the correct offset. + chunk.skip(_offsetRemaining); + _offsetRemaining = 0; + } + } + + // We can start processing the limited length if we have reached the starting offset. + if (_offsetRemaining == 0 && _lengthRemaining >= 0) + { + if (_lengthRemaining == 0) + { + // Release the chunk and continue until we find a last chunk. + if (!chunk.isLast()) + { + chunk.release(); + continue; + } + + chunk.clear(); + } + else if (_lengthRemaining >= chunk.remaining()) + { + // We can take the whole chunk. + _lengthRemaining -= chunk.remaining(); + } + else if (_lengthRemaining < chunk.remaining()) + { + // We must limit the size of the chunk to the remaining length. + chunk.limit(_lengthRemaining); + _lengthRemaining = 0; + } + } + + return chunk; + } + } + + @Override + public void demand(Runnable demandCallback) + { + _source.demand(demandCallback); + } + + @Override + public void fail(Throwable failure) + { + _source.fail(failure); + } + + @Override + public void fail(Throwable failure, boolean last) + { + _source.fail(failure, last); + } + + @Override + public boolean rewind() + { + boolean rewound = _source.rewind(); + if (rewound) + { + _offsetRemaining = _offset; + _lengthRemaining = _length; + } + return rewound; + } + + @Override + public String toString() + { + return String.format("%s{off=%s, len=%s, source=%s}", getClass().getSimpleName(), _offset, _length, _source); + } +} diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceRangeTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceRangeTest.java new file mode 100644 index 000000000000..5f6cdbc494b4 --- /dev/null +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceRangeTest.java @@ -0,0 +1,193 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.io; + +import org.eclipse.jetty.io.content.AsyncContent; +import org.eclipse.jetty.io.content.ByteBufferContentSource; +import org.eclipse.jetty.io.internal.ContentSourceRange; +import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.Callback; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class ContentSourceRangeTest +{ + @Test + public void testNullReturn() + { + Content.Source source = new ContentSourceRange(new AsyncContent(), 0, -1); + assertNull(source.read()); + } + + @Test + public void testInvalidArguments() + { + // Throws if the offset is negative. + assertThrows(IndexOutOfBoundsException.class, () -> new ContentSourceRange(new AsyncContent(), -1, -1)); + + // Throws if the offset is beyond the length of the source. + ByteBufferContentSource source = new ByteBufferContentSource(BufferUtil.toBuffer("hello".getBytes())); + assertThrows(IndexOutOfBoundsException.class, () -> new ContentSourceRange(source, 6, -1)); + } + + @Test + public void testFullLength() + { + AsyncContent asyncContent = new AsyncContent(); + Content.Source source = new ContentSourceRange(asyncContent, 0, -1); + + asyncContent.write(false, BufferUtil.toBuffer("A"), Callback.NOOP); + asyncContent.write(false, BufferUtil.toBuffer("B"), Callback.NOOP); + asyncContent.write(true, BufferUtil.EMPTY_BUFFER, Callback.NOOP); + + Content.Chunk c1 = source.read(); + assertThat(c1.getByteBuffer(), equalTo(BufferUtil.toBuffer("A"))); + assertFalse(c1.isLast()); + c1.release(); + + Content.Chunk c2 = source.read(); + assertThat(c2.getByteBuffer(), equalTo(BufferUtil.toBuffer("B"))); + assertFalse(c2.isLast()); + c2.release(); + + Content.Chunk last = source.read(); + assertThat(last.remaining(), equalTo(0)); + assertTrue(last.isLast()); + last.release(); + } + + @Test + public void testOffset() + { + AsyncContent asyncContent = new AsyncContent(); + Content.Source source = new ContentSourceRange(asyncContent, 5, -1); + + asyncContent.write(false, BufferUtil.toBuffer("12"), Callback.NOOP); + asyncContent.write(false, BufferUtil.toBuffer("345"), Callback.NOOP); + asyncContent.write(false, BufferUtil.toBuffer("XYZ"), Callback.NOOP); + asyncContent.write(true, BufferUtil.EMPTY_BUFFER, Callback.NOOP); + + Content.Chunk c1 = source.read(); + assertThat(c1.getByteBuffer(), equalTo(BufferUtil.toBuffer("XYZ"))); + assertFalse(c1.isLast()); + c1.release(); + + Content.Chunk c2 = source.read(); + assertTrue(c2.isLast()); + assertThat(c2.remaining(), equalTo(0)); + c2.release(); + } + + @Test + public void testOffsetMidChunk() + { + AsyncContent asyncContent = new AsyncContent(); + Content.Source source = new ContentSourceRange(asyncContent, 3, 2); + + asyncContent.write(true, BufferUtil.toBuffer("abcdef"), Callback.NOOP); + + Content.Chunk c1 = source.read(); + assertTrue(c1.isLast()); + assertThat(c1.getByteBuffer(), equalTo(BufferUtil.toBuffer("de"))); + c1.release(); + } + + @Test + public void testSpanMultipleChunks() + { + AsyncContent asyncContent = new AsyncContent(); + Content.Source source = new ContentSourceRange(asyncContent, 2, 8); + + asyncContent.write(false, BufferUtil.toBuffer("AAAA"), Callback.NOOP); + asyncContent.write(false, BufferUtil.toBuffer("BBBB"), Callback.NOOP); + asyncContent.write(true, BufferUtil.toBuffer("CCCC"), Callback.NOOP); + + Content.Chunk c1 = source.read(); + assertThat(c1.getByteBuffer(), equalTo(BufferUtil.toBuffer("AA"))); + assertFalse(c1.isLast()); + c1.release(); + + Content.Chunk c2 = source.read(); + assertThat(c2.getByteBuffer(), equalTo(BufferUtil.toBuffer("BBBB"))); + assertFalse(c2.isLast()); + c2.release(); + + Content.Chunk c3 = source.read(); + assertThat(c3.getByteBuffer(), equalTo(BufferUtil.toBuffer("CC"))); + assertTrue(c3.isLast()); + c3.release(); + } + + @Test + public void testZeroLengthRange() + { + AsyncContent asyncContent = new AsyncContent(); + Content.Source source = new ContentSourceRange(asyncContent, 0, 0); + + asyncContent.write(false, BufferUtil.toBuffer("foo"), Callback.NOOP); + asyncContent.write(false, BufferUtil.toBuffer("bar"), Callback.NOOP); + asyncContent.write(true, BufferUtil.EMPTY_BUFFER, Callback.NOOP); + + Content.Chunk c1 = source.read(); + assertTrue(c1.isLast()); + assertThat(c1.remaining(), equalTo(0)); + c1.release(); + } + + @Test + public void testOffsetBeyondLength() + { + AsyncContent asyncContent = new AsyncContent(); + Content.Source source = new ContentSourceRange(asyncContent, 200, -1); + + asyncContent.write(false, BufferUtil.toBuffer("hello"), Callback.NOOP); + asyncContent.write(false, BufferUtil.toBuffer("world"), Callback.NOOP); + asyncContent.write(true, BufferUtil.EMPTY_BUFFER, Callback.NOOP); + + Content.Chunk c1 = source.read(); + assertTrue(c1.isLast()); + assertThat(c1.remaining(), equalTo(0)); + c1.release(); + } + + @Test + public void testFailureChunkBeforeEOF() + { + AsyncContent asyncContent = new AsyncContent(); + Content.Source source = new ContentSourceRange(asyncContent, 5, 5); + + asyncContent.write(false, BufferUtil.toBuffer("hello"), Callback.NOOP); + asyncContent.write(false, BufferUtil.toBuffer("world"), Callback.NOOP); + + Content.Chunk c1 = source.read(); + assertFalse(c1.isLast()); + assertThat(c1.getByteBuffer(), equalTo(BufferUtil.toBuffer("world"))); + c1.release(); + + // We have read the full range, but trying to read to EOF we can still get a failure. + asyncContent.fail(new RuntimeException("test exception")); + + Content.Chunk c2 = source.read(); + assertTrue(c2.isLast()); + assertThat(c2.getFailure(), instanceOf(RuntimeException.class)); + c2.release(); + } +} \ No newline at end of file From 4147ef3621b78071bc940f4622ead8191f1feaae Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 15 Oct 2025 23:40:47 +1100 Subject: [PATCH 14/26] PR #13690 - fixes for test failures Signed-off-by: Lachlan Roberts --- .../src/main/java/org/eclipse/jetty/io/Content.java | 1 - .../eclipse/jetty/io/internal/ByteChannelContentSource.java | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java index b3552d511ad3..a2575eebacff 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java @@ -264,7 +264,6 @@ static Content.Source from(ByteBufferPool.Sized byteBufferPool, Path path) */ static Content.Source from(ByteBufferPool.Sized byteBufferPool, Path path, long offset, long length) { - // TODO define contract for offset/lengths outside of bounds return new ByteChannelContentSource.PathContentSource(byteBufferPool, path, offset, length); } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java index cff615e319ca..f36f316290c4 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java @@ -179,7 +179,7 @@ else if (_buffer.isRetained()) if (_offsetRemaining > 0) { // Discard all bytes read until we reach the staring offset. - while (true) + while (_offsetRemaining > 0) { BufferUtil.clearToFill(byteBuffer); byteBuffer.limit((int)Math.min(_buffer.capacity(), _offsetRemaining)); @@ -245,7 +245,7 @@ public boolean rewind() try (AutoLock ignored = lock.lock()) { // We can only rewind if we have a SeekableByteChannel. - if (!(_byteChannel instanceof SeekableByteChannel seekableByteChannel)) + if (!(_byteChannel instanceof SeekableByteChannel)) return false; // We can remove terminal condition for a rewind that is likely to occur @@ -258,7 +258,7 @@ public boolean rewind() try { - seekableByteChannel.position(_offset); + ((SeekableByteChannel)_byteChannel).position(_offset); _offsetRemaining = 0; _totalRead = 0; return true; From 25a685c8a76efff2a59f67ae7acb0b6bee545f57 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 20 Oct 2025 08:49:12 +1100 Subject: [PATCH 15/26] Updates from review --- .../java/org/eclipse/jetty/io/internal/ContentSourceRange.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java index 80e9f4832194..a3f6e91b9ae2 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java @@ -48,6 +48,9 @@ public Content.Chunk read() if (chunk == null) return null; + if (Content.Chunk.isFailure(chunk)) + return chunk; + if (_offsetRemaining > 0) { if (_offsetRemaining >= chunk.remaining()) From 0706abaad79e098a469442f3a14aa80d6deb0ac9 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 20 Oct 2025 13:37:31 +1100 Subject: [PATCH 16/26] PR #13690 - changes from review Signed-off-by: Lachlan Roberts --- .../io/internal/ByteChannelContentSource.java | 35 ++++++++++--------- .../jetty/io/internal/ContentSourceRange.java | 33 +++++++++++++++++ 2 files changed, 52 insertions(+), 16 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java index f36f316290c4..152d2e2259bd 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java @@ -51,27 +51,30 @@ public class ByteChannelContentSource implements Content.Source private Runnable demandCallback; private Content.Chunk _terminal; - public ByteChannelContentSource(SeekableByteChannel seekableByteChannel, long offset, long length) - { - this(null, seekableByteChannel, offset, length); - } - - public ByteChannelContentSource(ByteBufferPool.Sized byteBufferPool, SeekableByteChannel seekableByteChannel, long offset, long length) - { - this(byteBufferPool, (ByteChannel)seekableByteChannel, offset, length); - } - - public ByteChannelContentSource(ByteChannel byteChannel) - { - this(null, byteChannel, 0L, -1L); - } - + /** + * Create a {@link ByteChannelContentSource} which reads from a {@link ByteChannel}. + * @param byteBufferPool The {@link org.eclipse.jetty.io.ByteBufferPool.Sized} to use for any internal buffers. + * @param byteChannel The {@link ByteChannel}s to use as the source. + */ public ByteChannelContentSource(ByteBufferPool.Sized byteBufferPool, ByteChannel byteChannel) { this(byteBufferPool, byteChannel, 0L, -1L); } - private ByteChannelContentSource(ByteBufferPool.Sized byteBufferPool, ByteChannel byteChannel, long offset, long length) + /** + * Create a {@link ByteChannelContentSource} which reads from a {@link ByteChannel}. + * If the {@link ByteChannel} is an instance of {@link SeekableByteChannel} the implementation will use + * {@link SeekableByteChannel#position(long)} to navigate to the starting offset. + * @param byteBufferPool The {@link org.eclipse.jetty.io.ByteBufferPool.Sized} to use for any internal buffers. + * @param byteChannel The {@link ByteChannel}s to use as the source. + * @param offset the offset byte of the content to start from. + * Must be greater than or equal to 0 and less than the content length (if known). + * @param length the length of the content to make available, -1 for the full length. + * If the size of the content is known, the length may be truncated to the content size minus the offset. + * @throws IndexOutOfBoundsException if the offset or length are out of range. + * @see TypeUtil#checkOffsetLengthSize(long, long, long) + */ + public ByteChannelContentSource(ByteBufferPool.Sized byteBufferPool, ByteChannel byteChannel, long offset, long length) { _byteBufferPool = Objects.requireNonNullElse(byteBufferPool, ByteBufferPool.SIZED_NON_POOLING); _byteChannel = byteChannel; diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java index a3f6e91b9ae2..906bc68c599e 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java @@ -13,17 +13,35 @@ package org.eclipse.jetty.io.internal; +import java.nio.channels.ClosedChannelException; + import org.eclipse.jetty.io.Content; import org.eclipse.jetty.util.TypeUtil; +/** + * A {@link Content.Source} that provides a range of content from another {@link Content.Source}. + */ public class ContentSourceRange implements Content.Source { private final long _offset; private final long _length; private final Content.Source _source; + boolean _readToEof = false; long _offsetRemaining; long _lengthRemaining; + /** + * Create a {@link ContentSourceRange} which wraps another {@link Content.Source} to appear + * as a sub-range of the original. + * + * @param source The {@link Content.Source} to wrap. + * @param offset the offset byte of the content to start from. + * Must be greater than or equal to 0 and less than the content length (if known). + * @param length the length of the content to make available, -1 for the full length. + * If the size of the content is known, the length may be truncated to the content size minus the offset. + * @throws IndexOutOfBoundsException if the offset or length are out of range. + * @see TypeUtil#checkOffsetLengthSize(long, long, long) + */ public ContentSourceRange(Content.Source source, long offset, long length) { _source = source; @@ -33,6 +51,14 @@ public ContentSourceRange(Content.Source source, long offset, long length) _lengthRemaining = _length; } + /** + * @param readToEof - if true, read until EOF of the source after the range is read. + */ + public void setReadToEof(boolean readToEof) + { + _readToEof = readToEof; + } + @Override public long getLength() { @@ -79,6 +105,13 @@ public Content.Chunk read() { if (_lengthRemaining == 0) { + if (!_readToEof) + { + // We do not have to read until EOF of the source, so we can return EOF now and fail the source. + _source.fail(new ClosedChannelException()); + return Content.Chunk.EOF; + } + // Release the chunk and continue until we find a last chunk. if (!chunk.isLast()) { From a00d1261f4a0ca8ae561de8e052a9b88043f519a Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 20 Oct 2025 13:50:27 +1100 Subject: [PATCH 17/26] PR #13690 - extra test for failure case Signed-off-by: Lachlan Roberts --- .../jetty/io/ContentSourceRangeTest.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceRangeTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceRangeTest.java index 5f6cdbc494b4..be32ae1c82dc 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceRangeTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceRangeTest.java @@ -190,4 +190,23 @@ public void testFailureChunkBeforeEOF() assertThat(c2.getFailure(), instanceOf(RuntimeException.class)); c2.release(); } + + @Test + public void testFailureBeforeStartingOffset() + { + AsyncContent asyncContent = new AsyncContent(); + Content.Source source = new ContentSourceRange(asyncContent, 20, 5); + + asyncContent.write(false, BufferUtil.toBuffer("hello"), Callback.NOOP); + asyncContent.write(false, BufferUtil.toBuffer("world"), Callback.NOOP); + asyncContent.fail(new RuntimeException("test exception")); + + Content.Chunk c1 = source.read(); + assertTrue(c1.isLast()); + assertTrue(c1.isEmpty()); + assertThat(c1.getFailure(), instanceOf(RuntimeException.class)); + assertThat(c1.getFailure().getMessage(), equalTo("test exception")); + + c1.release(); + } } \ No newline at end of file From c4c6d6a2faebf83b87bd518d9cfaee6844e5e9ce Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 21 Oct 2025 10:56:49 +1100 Subject: [PATCH 18/26] PR #13690 - changes from review Signed-off-by: Lachlan Roberts --- .../jetty/io/internal/ContentSourceRange.java | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java index 906bc68c599e..f465d1198923 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java @@ -26,9 +26,9 @@ public class ContentSourceRange implements Content.Source private final long _offset; private final long _length; private final Content.Source _source; - boolean _readToEof = false; - long _offsetRemaining; - long _lengthRemaining; + private final boolean _readToEof; + private long _offsetRemaining; + private long _lengthRemaining; /** * Create a {@link ContentSourceRange} which wraps another {@link Content.Source} to appear @@ -44,18 +44,29 @@ public class ContentSourceRange implements Content.Source */ public ContentSourceRange(Content.Source source, long offset, long length) { - _source = source; - _offset = offset; - _length = TypeUtil.checkOffsetLengthSize(offset, length, source.getLength()); - _offsetRemaining = _offset; - _lengthRemaining = _length; + this(source, offset, length, false); } /** + * Create a {@link ContentSourceRange} which wraps another {@link Content.Source} to appear + * as a sub-range of the original. + * + * @param source The {@link Content.Source} to wrap. + * @param offset the offset byte of the content to start from. + * Must be greater than or equal to 0 and less than the content length (if known). + * @param length the length of the content to make available, -1 for the full length. + * If the size of the content is known, the length may be truncated to the content size minus the offset. * @param readToEof - if true, read until EOF of the source after the range is read. + * @throws IndexOutOfBoundsException if the offset or length are out of range. + * @see TypeUtil#checkOffsetLengthSize(long, long, long) */ - public void setReadToEof(boolean readToEof) + public ContentSourceRange(Content.Source source, long offset, long length, boolean readToEof) { + _source = source; + _offset = offset; + _length = TypeUtil.checkOffsetLengthSize(offset, length, source.getLength()); + _offsetRemaining = _offset; + _lengthRemaining = _length; _readToEof = readToEof; } From 00856b428d6ead3b5d31beab6116d04d89bace62 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 24 Oct 2025 13:07:07 +1100 Subject: [PATCH 19/26] update from review --- .../org/eclipse/jetty/http/MultiPart.java | 7 ------ .../org/eclipse/jetty/io/IOResourcesTest.java | 2 +- .../java/org/eclipse/jetty/util/TypeUtil.java | 23 ++++++++++--------- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java index 35fc37b0acdc..b3d292279dfc 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 @@ -552,13 +552,6 @@ public long getLength() long length = 0; for (Content.Chunk c : content) length += c.size(); - for (Content.Source s : contentSources) - { - long l = s.getLength(); - if (l < 0) - return -1; - length += l; - } return length; } diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/IOResourcesTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/IOResourcesTest.java index 4f6dfa5183ba..37460ddf2662 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/IOResourcesTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/IOResourcesTest.java @@ -187,7 +187,7 @@ public void testAsContentSourceWithOffset(Resource resource) throws Exception TestSink sink = new TestSink(); Callback.Completable callback = new Callback.Completable(); - if (resource.length() < 100) + if (resource.length() >= 0 && resource.length() < 100) { assertThrows(IndexOutOfBoundsException.class, () -> IOResources.asContentSource(resource, bufferPool, 100, -1)); return; diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java index 065d9a5e784d..5376148a45b7 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java @@ -898,33 +898,34 @@ else if (value.getClass().isArray()) /** * Check that the offset and length of a subrange are within the size of a range. + * * @param offset an offset of a subrange within a range of 0 to {code size} to start from. - * Must be greater than or equal to 0 and less than the range size (if known). + * Must be greater than or equal to 0 and less than the range size (if known). * @param length the length of the subrange, -1 for the full length available from the offset, - * otherwise must be greater than or equal to 0. - * If the size is known, then the length will be reduced if necessary to fit within the size-offset. + * otherwise must be greater than or equal to 0. + * If the size is known, then the length will be reduced if necessary to fit within the size-offset. * @param size the size of the range, or -1 if unknown. * @return the length of the subrange, which may be calculated if length was less than 0 or if the length is limited by - * a known size. + * a known size. * @throws IndexOutOfBoundsException if the offset or length are out of range. * @throws ArithmeticException if the length is not -1 and offset+length overflows. */ - public static long checkOffsetLengthSize(long offset, long length, long size) + public static long checkOffsetLengthSize(long offset, long length, long size) throws IndexOutOfBoundsException, ArithmeticException { - if (length == 0) - return 0; + if (length == 0L) + return 0L; - if (offset < 0) + if (offset < 0L) throw new IndexOutOfBoundsException("Negative offset: " + offset); - if (length < -1) + if (length < -1L) throw new IndexOutOfBoundsException("Length less than -1: " + length); - if (size >= 0) + if (size >= 0L) { if (offset > size) throw new IndexOutOfBoundsException("Offset > Size: " + offset + " > " + size); - if (length == -1 || Math.addExact(offset, length) > size) + if (length == -1L || Math.addExact(offset, length) > size) length = size - offset; } From 6c725c410aa310b738810e66859f5f70fd8406b8 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 28 Oct 2025 15:52:33 +1100 Subject: [PATCH 20/26] Updates from review --- .../jetty/io/internal/ContentSourceRange.java | 73 ++++++++++--------- .../org/eclipse/jetty/util/BufferUtil.java | 3 +- 2 files changed, 42 insertions(+), 34 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java index f465d1198923..7249cd5be903 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java @@ -16,6 +16,7 @@ import java.nio.channels.ClosedChannelException; import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.util.TypeUtil; /** @@ -29,6 +30,7 @@ public class ContentSourceRange implements Content.Source private final boolean _readToEof; private long _offsetRemaining; private long _lengthRemaining; + private Content.Chunk _terminal; /** * Create a {@link ContentSourceRange} which wraps another {@link Content.Source} to appear @@ -79,6 +81,13 @@ public long getLength() @Override public Content.Chunk read() { + if (_terminal != null) + { + Content.Chunk chunk = _terminal; + _terminal = Content.Chunk.next(_terminal); + return chunk; + } + while (true) { Content.Chunk chunk = _source.read(); @@ -86,7 +95,10 @@ public Content.Chunk read() return null; if (Content.Chunk.isFailure(chunk)) + { + _terminal = Content.Chunk.next(chunk); return chunk; + } if (_offsetRemaining > 0) { @@ -94,13 +106,10 @@ public Content.Chunk read() { // We can skip this whole chunk. _offsetRemaining -= chunk.remaining(); - if (chunk.isLast()) - { - chunk.clear(); - return chunk; - } - + chunk.clear(); chunk.release(); + if (chunk.isLast()) + return _terminal = Content.Chunk.EOF; continue; } else @@ -112,39 +121,37 @@ public Content.Chunk read() } // We can start processing the limited length if we have reached the starting offset. - if (_offsetRemaining == 0 && _lengthRemaining >= 0) + + if (_lengthRemaining == 0) { - if (_lengthRemaining == 0) + // We have read all we need to + if (_readToEof) { - if (!_readToEof) - { - // We do not have to read until EOF of the source, so we can return EOF now and fail the source. - _source.fail(new ClosedChannelException()); - return Content.Chunk.EOF; - } - - // Release the chunk and continue until we find a last chunk. - if (!chunk.isLast()) - { - chunk.release(); - continue; - } - + // Release the chunk and continue until we find the last chunk. chunk.clear(); + chunk.release(); + if (chunk.isLast()) + return _terminal = Content.Chunk.EOF; + continue; } - else if (_lengthRemaining >= chunk.remaining()) - { - // We can take the whole chunk. - _lengthRemaining -= chunk.remaining(); - } - else if (_lengthRemaining < chunk.remaining()) - { - // We must limit the size of the chunk to the remaining length. - chunk.limit(_lengthRemaining); - _lengthRemaining = 0; - } + + // We do not have to read until EOF of the source, so we can return EOF now and fail the source. + fail(new ClosedChannelException()); + return _terminal = Content.Chunk.EOF; + } + + if (_lengthRemaining > 0 && _lengthRemaining < chunk.remaining()) + { + // We must limit the size of the chunk to the remaining length. + RetainableByteBuffer slice = chunk.slice(_lengthRemaining); + _lengthRemaining = 0; + chunk.clear(); + chunk.release(); + return Content.Chunk.from(slice.getByteBuffer(), chunk.isLast(), slice::release); } + // We can return the whole chunk. + _lengthRemaining -= chunk.remaining(); return chunk; } } 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 a380c4f881b1..5672fd824475 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 @@ -35,6 +35,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.List; import java.util.Objects; import org.eclipse.jetty.util.resource.Resource; @@ -325,7 +326,7 @@ public static ByteBuffer slice(ByteBuffer buffer, int offset, int length) * @return the sliced buffers in a new collection, which may be immutable and may contain the original buffers or slices of them * @throws IndexOutOfBoundsException if the offset or length are invalid */ - public static Collection slice(Collection byteBuffers, long offset, long length) + public static List slice(Collection byteBuffers, long offset, long length) { Objects.requireNonNull(byteBuffers); if (offset < 0) From 2fe1d8fa3531805f1b97ba6e58b28fb8ed9ee086 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 28 Oct 2025 22:11:48 +1100 Subject: [PATCH 21/26] Updates from review --- .../java/org/eclipse/jetty/io/Content.java | 5 ++++ .../jetty/io/content/AsyncContent.java | 6 +++++ .../jetty/io/internal/ByteBufferChunk.java | 27 +++++++++++++++++++ .../jetty/io/internal/ContentSourceRange.java | 15 ++++++----- 4 files changed, 46 insertions(+), 7 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java index a2575eebacff..a00d48dea528 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java @@ -1004,6 +1004,11 @@ static Chunk from(ByteBuffer byteBuffer, boolean last) return last ? EOF : EMPTY; } + static Chunk from(RetainableByteBuffer buffer, boolean last) + { + return new ByteBufferChunk.WithRetainableByteBuffer(buffer, last); + } + /** *

Creates a Chunk with the given ByteBuffer.

*

The returned Chunk must be {@link #release() released}.

diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/AsyncContent.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/AsyncContent.java index 93833eaa609b..2d3cac7ba5ca 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/AsyncContent.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/AsyncContent.java @@ -310,6 +310,12 @@ public boolean isRetained() return canRetain() && referenceCounter.isRetained(); } + @Override + public int getRetained() + { + return referenceCounter.getRetained(); + } + @Override public void retain() { diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteBufferChunk.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteBufferChunk.java index 3d0927a3c20e..9742e7d94be9 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteBufferChunk.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteBufferChunk.java @@ -179,4 +179,31 @@ public String toString() return "%s[%s]".formatted(super.toString(), retainable); } } + + public static class WithRetainableByteBuffer extends RetainableByteBuffer.Wrapper implements Content.Chunk + { + private final boolean last; + + public WithRetainableByteBuffer(RetainableByteBuffer wrapped, boolean last) + { + super(wrapped); + this.last = last; + } + + @Override + public boolean isLast() + { + return last; + } + + @Override + public String toString() + { + return "%s@%x[l=%b,b=%s]".formatted( + TypeUtil.toShortName(getClass()), + hashCode(), + isLast(), + getWrapped().toDetailString()); + } + } } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java index 7249cd5be903..0aa8d6df8183 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRange.java @@ -84,7 +84,7 @@ public Content.Chunk read() if (_terminal != null) { Content.Chunk chunk = _terminal; - _terminal = Content.Chunk.next(_terminal); + _terminal = Content.Chunk.next(chunk); return chunk; } @@ -106,7 +106,6 @@ public Content.Chunk read() { // We can skip this whole chunk. _offsetRemaining -= chunk.remaining(); - chunk.clear(); chunk.release(); if (chunk.isLast()) return _terminal = Content.Chunk.EOF; @@ -115,8 +114,12 @@ public Content.Chunk read() else { // Advance position to the correct offset. - chunk.skip(_offsetRemaining); - _offsetRemaining = 0; + RetainableByteBuffer slice = chunk.slice(); + _offsetRemaining -= slice.skip(_offsetRemaining); + chunk.release(); + if (_offsetRemaining > 0) + continue; + chunk = Content.Chunk.from(slice, chunk.isLast()); } } @@ -128,7 +131,6 @@ public Content.Chunk read() if (_readToEof) { // Release the chunk and continue until we find the last chunk. - chunk.clear(); chunk.release(); if (chunk.isLast()) return _terminal = Content.Chunk.EOF; @@ -145,9 +147,8 @@ public Content.Chunk read() // We must limit the size of the chunk to the remaining length. RetainableByteBuffer slice = chunk.slice(_lengthRemaining); _lengthRemaining = 0; - chunk.clear(); chunk.release(); - return Content.Chunk.from(slice.getByteBuffer(), chunk.isLast(), slice::release); + return Content.Chunk.from(slice, chunk.isLast()); } // We can return the whole chunk. From cc50b3706cd5373e59120097b5ba8922708e1bb3 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Tue, 28 Oct 2025 18:16:30 +0100 Subject: [PATCH 22/26] add missing checks in HttpContent.writeTo() implementations Signed-off-by: Ludovic Orban --- .../jetty/http/content/CachingHttpContentFactory.java | 2 ++ .../jetty/http/content/FileMappingHttpContentFactory.java | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java index d73601fc5beb..c640925a33f7 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java @@ -36,6 +36,7 @@ import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.NanoTime; import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.resource.Resource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -367,6 +368,7 @@ public void writeTo(Content.Sink sink, long offset, long length, Callback callba boolean retained = false; try { + TypeUtil.checkOffsetLengthSize(offset, length, _buffer.remaining()); retained = tryRetain(); if (retained) sink.write(true, BufferUtil.slice(_buffer.getByteBuffer(), Math.toIntExact(offset), Math.toIntExact(length)), Callback.from(this::release, callback)); diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactory.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactory.java index ae748951a2fd..ead71938a7bf 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactory.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactory.java @@ -25,6 +25,7 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IteratingNestedCallback; +import org.eclipse.jetty.util.TypeUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -115,7 +116,8 @@ public void writeTo(Content.Sink sink, long offset, long length, Callback callba { try { - sink.write(true, BufferUtil.slice(_buffer, (int)offset, (int)length), callback); + TypeUtil.checkOffsetLengthSize(offset, length, _buffer.remaining()); + sink.write(true, BufferUtil.slice(_buffer, Math.toIntExact(offset), Math.toIntExact(length)), callback); } catch (Throwable x) { @@ -196,6 +198,7 @@ public void writeTo(Content.Sink sink, long offset, long length, Callback callba { try { + TypeUtil.checkOffsetLengthSize(offset, length, _contentLengthValue); if (offset > getContentLengthValue()) throw new IllegalArgumentException("Offset outside of mapped file range"); if (length > -1 && length + offset > getContentLengthValue()) From e36049cdfa9e5f8283c7f9ed5e6949cfd8f5d4db Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 29 Oct 2025 07:38:13 +1100 Subject: [PATCH 23/26] Updates from review --- .../eclipse/jetty/http/content/CachingHttpContentFactory.java | 2 +- .../jetty/http/content/FileMappingHttpContentFactory.java | 4 ++-- .../src/main/java/org/eclipse/jetty/util/TypeUtil.java | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java index c640925a33f7..84600ba4011b 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java @@ -368,7 +368,7 @@ public void writeTo(Content.Sink sink, long offset, long length, Callback callba boolean retained = false; try { - TypeUtil.checkOffsetLengthSize(offset, length, _buffer.remaining()); + length = TypeUtil.checkOffsetLengthSize(offset, length, _buffer.remaining()); retained = tryRetain(); if (retained) sink.write(true, BufferUtil.slice(_buffer.getByteBuffer(), Math.toIntExact(offset), Math.toIntExact(length)), Callback.from(this::release, callback)); diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactory.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactory.java index ead71938a7bf..90d23a4259ea 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactory.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactory.java @@ -116,7 +116,7 @@ public void writeTo(Content.Sink sink, long offset, long length, Callback callba { try { - TypeUtil.checkOffsetLengthSize(offset, length, _buffer.remaining()); + length = TypeUtil.checkOffsetLengthSize(offset, length, _buffer.remaining()); sink.write(true, BufferUtil.slice(_buffer, Math.toIntExact(offset), Math.toIntExact(length)), callback); } catch (Throwable x) @@ -198,7 +198,7 @@ public void writeTo(Content.Sink sink, long offset, long length, Callback callba { try { - TypeUtil.checkOffsetLengthSize(offset, length, _contentLengthValue); + length = TypeUtil.checkOffsetLengthSize(offset, length, _contentLengthValue); if (offset > getContentLengthValue()) throw new IllegalArgumentException("Offset outside of mapped file range"); if (length > -1 && length + offset > getContentLengthValue()) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java index 5376148a45b7..aadf243e63de 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java @@ -905,7 +905,7 @@ else if (value.getClass().isArray()) * otherwise must be greater than or equal to 0. * If the size is known, then the length will be reduced if necessary to fit within the size-offset. * @param size the size of the range, or -1 if unknown. - * @return the length of the subrange, which may be calculated if length was less than 0 or if the length is limited by + * @return the length of the subrange, which may be calculated if the length was less than 0 or if the length is limited by * a known size. * @throws IndexOutOfBoundsException if the offset or length are out of range. * @throws ArithmeticException if the length is not -1 and offset+length overflows. From a6ca6a783a459de28faf476be8efaec2b2435572 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 29 Oct 2025 09:08:51 +1100 Subject: [PATCH 24/26] Updates from review --- .../content/CachingHttpContentFactory.java | 27 ++++++++++++++----- .../FileMappingHttpContentFactory.java | 4 --- .../FileMappingHttpContentFactoryTest.java | 8 +++--- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java index 84600ba4011b..3f0491be5465 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java @@ -22,6 +22,7 @@ import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; import org.eclipse.jetty.http.CompressedContentFormat; import org.eclipse.jetty.http.HttpField; @@ -301,7 +302,7 @@ protected interface CachingHttpContent extends HttpContent protected class CachedHttpContent extends HttpContent.Wrapper implements CachingHttpContent { - private final RetainableByteBuffer _buffer; + private final AtomicReference _buffer = new AtomicReference<>(); private final String _cacheKey; private final HttpField _etagField; private volatile long _lastAccessed; @@ -334,7 +335,7 @@ public CachedHttpContent(String key, HttpContent httpContent) throw new IllegalArgumentException("Resource is too large: length " + contentLengthValue + " > " + _maxCachedFileSize); // Read the content into memory - _buffer = IOResources.toRetainableByteBuffer(httpContent.getResource(), _bufferPool); + _buffer.set(IOResources.toRetainableByteBuffer(httpContent.getResource(), _bufferPool)); _characterEncoding = httpContent.getCharacterEncoding(); _compressedFormats = httpContent.getPreCompressedContentFormats(); @@ -365,13 +366,20 @@ public String getKey() @Override public void writeTo(Content.Sink sink, long offset, long length, Callback callback) { + RetainableByteBuffer buffer = _buffer.get(); + if (buffer == null) + { + super.writeTo(sink, offset, length, callback); + return; + } + boolean retained = false; try { - length = TypeUtil.checkOffsetLengthSize(offset, length, _buffer.remaining()); + length = TypeUtil.checkOffsetLengthSize(offset, length, buffer.remaining()); retained = tryRetain(); if (retained) - sink.write(true, BufferUtil.slice(_buffer.getByteBuffer(), Math.toIntExact(offset), Math.toIntExact(length)), Callback.from(this::release, callback)); + sink.write(true, BufferUtil.slice(buffer.getByteBuffer(), Math.toIntExact(offset), Math.toIntExact(length)), Callback.from(this::release, callback)); else getWrapped().writeTo(sink, offset, length, callback); } @@ -394,7 +402,10 @@ private boolean tryRetain() { return _cache.computeIfPresent(_cacheKey, (s, cachingHttpContent) -> { - _buffer.retain(); + RetainableByteBuffer buffer = _buffer.get(); + if (buffer == null) + return null; + buffer.retain(); return cachingHttpContent; }) != null; } @@ -402,7 +413,9 @@ private boolean tryRetain() @Override public void release() { - _buffer.release(); + RetainableByteBuffer buffer = _buffer.get(); + if (buffer != null && buffer.release()) + _buffer.set(null); } @Override @@ -438,7 +451,7 @@ public HttpField getContentLength() @Override public long getContentLengthValue() { - return _buffer.remaining(); + return _contentLength.getLongValue(); } @Override diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactory.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactory.java index 90d23a4259ea..eda1f435b7fa 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactory.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactory.java @@ -199,10 +199,6 @@ public void writeTo(Content.Sink sink, long offset, long length, Callback callba try { length = TypeUtil.checkOffsetLengthSize(offset, length, _contentLengthValue); - if (offset > getContentLengthValue()) - throw new IllegalArgumentException("Offset outside of mapped file range"); - if (length > -1 && length + offset > getContentLengthValue()) - throw new IllegalArgumentException("Offset / length outside of mapped file range"); int beginIndex = Math.toIntExact(offset / maxBufferSize); int firstOffset = Math.toIntExact(offset % maxBufferSize); diff --git a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactoryTest.java b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactoryTest.java index 32d936c82d15..e5562be1702c 100644 --- a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactoryTest.java +++ b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactoryTest.java @@ -54,12 +54,14 @@ public void testMultiBufferFileMappedOffsetAndLength() throws Exception HttpContent content = fileMappingHttpContentFactory.getContent("file.txt"); - assertThrows(IllegalArgumentException.class, () -> writeToString(content, 0, 31)); - assertThrows(IllegalArgumentException.class, () -> writeToString(content, 30, 1)); - assertThrows(IllegalArgumentException.class, () -> writeToString(content, 31, 0)); + assertThrows(IllegalArgumentException.class, () -> writeToString(content, -1, 0)); + assertThrows(IndexOutOfBoundsException.class, () -> writeToString(content, 31, 1)); + assertThat(writeToString(content, 0, 100), is("0123456789abcdefghijABCDEFGHIJ")); + assertThat(writeToString(content, 0, 31), is("0123456789abcdefghijABCDEFGHIJ")); assertThat(writeToString(content, 0, 30), is("0123456789abcdefghijABCDEFGHIJ")); assertThat(writeToString(content, 29, 1), is("J")); + assertThat(writeToString(content, 30, 1), is("")); assertThat(writeToString(content, 0, 0), is("")); assertThat(writeToString(content, 10, 0), is("")); assertThat(writeToString(content, 15, 0), is("")); From ccfc4b9ea6c005201bc0d4f97776883dbbd480aa Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 29 Oct 2025 09:10:45 +1100 Subject: [PATCH 25/26] Updates from review --- .../main/java/org/eclipse/jetty/http/content/HttpContent.java | 1 + 1 file changed, 1 insertion(+) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/HttpContent.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/HttpContent.java index 0b638b81492e..3be14e754bcf 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/HttpContent.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/HttpContent.java @@ -118,6 +118,7 @@ public interface HttpContent * @param sink the sink to write to. * @param offset the offset byte of the resource to start from. * @param length the length of the resource's contents to copy, -1 for the full length. + * If the length is longer than the available content, the write is truncated to the available length. * @param callback the callback to notify when writing is done. */ void writeTo(Content.Sink sink, long offset, long length, Callback callback); From b2ebc77c1ffc3231ae2d036d69408355242c72a1 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 30 Oct 2025 07:44:33 +1100 Subject: [PATCH 26/26] Updates from review --- .../eclipse/jetty/http/content/CachingHttpContentFactory.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java index 3f0491be5465..e9e6da865008 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java @@ -413,9 +413,7 @@ private boolean tryRetain() @Override public void release() { - RetainableByteBuffer buffer = _buffer.get(); - if (buffer != null && buffer.release()) - _buffer.set(null); + _buffer.getAndUpdate(buffer -> (buffer != null && buffer.release()) ? null : buffer); } @Override