TODOs from reviews

Author: gregw

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<Byte
         @Override
         public Content.Source newContentSource(ByteBufferPool.Sized bufferPool, long offset, long length)
         {
+            // TODO: support offset and length
             return new ByteBufferContentSource(content);
         }
 
@@ -538,6 +540,7 @@ public ChunksPart(String name, String fileName, HttpFields fields, List<Content.
         @Override
         public Content.Source newContentSource(ByteBufferPool.Sized bufferPool, long offset, long length)
         {
+            // TODO support offset and length
             try (AutoLock ignored = lock.lock())
             {
                 if (closed)

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java

@@ -229,6 +229,7 @@ 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);
         }
 
@@ -253,6 +254,7 @@ 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);
         }
 
@@ -282,6 +284,7 @@ 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);
         }
 

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java

@@ -56,11 +56,13 @@ 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)
@@ -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;
         }
 

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();