From e563f54d4b900a5b61102decb789a9b550501ce2 Mon Sep 17 00:00:00 2001 From: charliechen Date: Fri, 4 Sep 2026 16:38:17 -0700 Subject: [PATCH 1/2] Core: Reject negative ByteBuffer stream seeks Validate seek positions before resetting buffer state so invalid offsets fail consistently without changing the current position. Generated-by: Codex --- .../org/apache/iceberg/io/MultiBufferInputStream.java | 2 ++ .../apache/iceberg/io/SingleBufferInputStream.java | 2 ++ .../apache/iceberg/io/TestByteBufferInputStreams.java | 11 +++++++++++ 3 files changed, 15 insertions(+) diff --git a/core/src/main/java/org/apache/iceberg/io/MultiBufferInputStream.java b/core/src/main/java/org/apache/iceberg/io/MultiBufferInputStream.java index fae2e615ef8e..44872b620500 100644 --- a/core/src/main/java/org/apache/iceberg/io/MultiBufferInputStream.java +++ b/core/src/main/java/org/apache/iceberg/io/MultiBufferInputStream.java @@ -24,6 +24,7 @@ import java.util.Collections; import java.util.Iterator; import java.util.List; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.Iterators; import org.apache.iceberg.relocated.com.google.common.collect.Lists; @@ -68,6 +69,7 @@ public long getPos() { @Override public void seek(long newPosition) throws IOException { + Preconditions.checkArgument(newPosition >= 0, "Position is negative: %s", newPosition); if (newPosition > length) { throw new EOFException( String.format("Cannot seek to position after end of file: %s", newPosition)); diff --git a/core/src/main/java/org/apache/iceberg/io/SingleBufferInputStream.java b/core/src/main/java/org/apache/iceberg/io/SingleBufferInputStream.java index 50431faf7e95..2c70cb5a09d6 100644 --- a/core/src/main/java/org/apache/iceberg/io/SingleBufferInputStream.java +++ b/core/src/main/java/org/apache/iceberg/io/SingleBufferInputStream.java @@ -23,6 +23,7 @@ import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; /** * This ByteBufferInputStream does not consume the ByteBuffer being passed in, but will create a @@ -82,6 +83,7 @@ public int read(byte[] bytes, int off, int len) throws IOException { @Override public void seek(long newPosition) throws IOException { + Preconditions.checkArgument(newPosition >= 0, "Position is negative: %s", newPosition); if (newPosition > length) { throw new EOFException( String.format("Cannot seek to position after end of file: %s", newPosition)); diff --git a/core/src/test/java/org/apache/iceberg/io/TestByteBufferInputStreams.java b/core/src/test/java/org/apache/iceberg/io/TestByteBufferInputStreams.java index fd2bd6a9d3a5..2af1b0a06ef4 100644 --- a/core/src/test/java/org/apache/iceberg/io/TestByteBufferInputStreams.java +++ b/core/src/test/java/org/apache/iceberg/io/TestByteBufferInputStreams.java @@ -362,6 +362,17 @@ public void testSkipFully() throws Exception { .hasMessageStartingWith("Not enough bytes to skip"); } + @Test + void seekRejectsNegativePositionWithoutChangingState() throws Exception { + ByteBufferInputStream stream = newStream(); + assertThat(stream.read()).isEqualTo(0); + + assertThatThrownBy(() -> stream.seek(-1)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Position is negative: -1"); + assertThat(stream.getPos()).isEqualTo(1); + } + @Test public void testMark() throws Exception { ByteBufferInputStream stream = newStream(); From 7510dfb5ac971b0edbced1f78c4c16606e890212 Mon Sep 17 00:00:00 2001 From: charliechen Date: Fri, 11 Sep 2026 18:40:32 -0700 Subject: [PATCH 2/2] Core: Strengthen negative seek regression coverage Cover rejection at the initial position and after reading, verify that subsequent reads remain usable, and match the shared test naming style. Generated-by: Codex --- .../io/TestByteBufferInputStreams.java | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/core/src/test/java/org/apache/iceberg/io/TestByteBufferInputStreams.java b/core/src/test/java/org/apache/iceberg/io/TestByteBufferInputStreams.java index 2af1b0a06ef4..4521a2a419ea 100644 --- a/core/src/test/java/org/apache/iceberg/io/TestByteBufferInputStreams.java +++ b/core/src/test/java/org/apache/iceberg/io/TestByteBufferInputStreams.java @@ -363,16 +363,31 @@ public void testSkipFully() throws Exception { } @Test - void seekRejectsNegativePositionWithoutChangingState() throws Exception { + public void testSeekRejectsNegativePosition() throws Exception { ByteBufferInputStream stream = newStream(); - assertThat(stream.read()).isEqualTo(0); + assertThat(stream.getPos()).isEqualTo(0); assertThatThrownBy(() -> stream.seek(-1)) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Position is negative: -1"); + assertThat(stream.getPos()).isEqualTo(0); + assertThat(stream.read()).isGreaterThanOrEqualTo(0); assertThat(stream.getPos()).isEqualTo(1); } + @Test + public void testSeekRejectsNegativePositionWithoutChangingState() throws Exception { + ByteBufferInputStream stream = newStream(); + assertThat(stream.read()).isGreaterThanOrEqualTo(0); + + assertThatThrownBy(() -> stream.seek(-1)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Position is negative: -1"); + assertThat(stream.getPos()).isEqualTo(1); + assertThat(stream.read()).isGreaterThanOrEqualTo(0); + assertThat(stream.getPos()).isEqualTo(2); + } + @Test public void testMark() throws Exception { ByteBufferInputStream stream = newStream();