From 03b816a5bf711c9fa4dd2a4e5909059ae0a9ca74 Mon Sep 17 00:00:00 2001 From: yangjie01 Date: Tue, 31 Mar 2026 02:42:21 +0800 Subject: [PATCH] fix: Fix always-true precondition check in LargeListVector.setValueCount The precondition in LargeListVector.setValueCount() used `||` (logical OR) instead of `&&` (logical AND), making the check always evaluate to true. This meant childValueCount values exceeding Integer.MAX_VALUE or negative values would silently pass the check, leading to data truncation when cast to int on the next line. Changed `childValueCount <= Integer.MAX_VALUE || childValueCount >= Integer.MIN_VALUE` to `childValueCount <= Integer.MAX_VALUE && childValueCount >= 0`, consistent with the correct implementation in LargeListViewVector (line 888). Added two regression tests to verify the precondition rejects both negative and overflow childValueCount values. --- .../arrow/vector/complex/LargeListVector.java | 2 +- .../arrow/vector/TestLargeListVector.java | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/vector/src/main/java/org/apache/arrow/vector/complex/LargeListVector.java b/vector/src/main/java/org/apache/arrow/vector/complex/LargeListVector.java index 92dd3eaef..7b15b3774 100644 --- a/vector/src/main/java/org/apache/arrow/vector/complex/LargeListVector.java +++ b/vector/src/main/java/org/apache/arrow/vector/complex/LargeListVector.java @@ -1041,7 +1041,7 @@ public void setValueCount(int valueCount) { * TODO: revisit when 64-bit vectors are supported */ Preconditions.checkArgument( - childValueCount <= Integer.MAX_VALUE || childValueCount >= Integer.MIN_VALUE, + childValueCount <= Integer.MAX_VALUE && childValueCount >= 0, "LargeListVector doesn't yet support 64-bit allocations: %s", childValueCount); vector.setValueCount((int) childValueCount); diff --git a/vector/src/test/java/org/apache/arrow/vector/TestLargeListVector.java b/vector/src/test/java/org/apache/arrow/vector/TestLargeListVector.java index bf9bba9c7..0773394cf 100644 --- a/vector/src/test/java/org/apache/arrow/vector/TestLargeListVector.java +++ b/vector/src/test/java/org/apache/arrow/vector/TestLargeListVector.java @@ -21,6 +21,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.ArrayList; @@ -1127,4 +1128,39 @@ private void writeIntValues(UnionLargeListWriter writer, int[] values) { } writer.endList(); } + + @Test + public void testSetValueCountRejectsNegativeChildValueCount() { + try (final LargeListVector vector = LargeListVector.empty("list", allocator)) { + vector.addOrGetVector(FieldType.nullable(MinorType.INT.getType())); + vector.allocateNew(); + + // Write a negative value into the offset buffer to simulate a corrupted offset + // that exceeds Integer.MAX_VALUE (which wraps to negative when read as long offset). + vector.getOffsetBuffer().setLong(LargeListVector.OFFSET_WIDTH, -1L); + vector.setLastSet(0); + + assertThrows( + IllegalArgumentException.class, + () -> vector.setValueCount(1), + "setValueCount should reject negative childValueCount"); + } + } + + @Test + public void testSetValueCountRejectsOverflowChildValueCount() { + try (final LargeListVector vector = LargeListVector.empty("list", allocator)) { + vector.addOrGetVector(FieldType.nullable(MinorType.INT.getType())); + vector.allocateNew(); + + // Write a value exceeding Integer.MAX_VALUE into the offset buffer + vector.getOffsetBuffer().setLong(LargeListVector.OFFSET_WIDTH, (long) Integer.MAX_VALUE + 1L); + vector.setLastSet(0); + + assertThrows( + IllegalArgumentException.class, + () -> vector.setValueCount(1), + "setValueCount should reject childValueCount exceeding Integer.MAX_VALUE"); + } + } }