diff --git a/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java b/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java index f9d7e5cb9e..4fd484c66b 100644 --- a/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java +++ b/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java @@ -205,9 +205,19 @@ public void set(int index, ArrowBuf buffer) { * @param value array of bytes containing decimal in big endian byte order. */ public void setBigEndian(int index, byte[] value) { - BitVectorHelper.setBit(validityBuffer, index); final int length = value.length; + // Reject an oversized value before touching the vector. The little-endian path below + // copies all `length` bytes into the fixed TYPE_WIDTH slot with unchecked native writes, + // so validating after the copy would leave adjacent memory already corrupted. Checking + // before setBit also keeps the slot null when the call is rejected. + if (length > TYPE_WIDTH) { + throw new IllegalArgumentException( + "Invalid decimal value length. Valid length in [1 - 32], got " + length); + } + + BitVectorHelper.setBit(validityBuffer, index); + // do the bound check. valueBuffer.checkBytes((long) index * TYPE_WIDTH, (long) (index + 1) * TYPE_WIDTH); @@ -243,8 +253,6 @@ public void setBigEndian(int index, byte[] value) { return; } } - throw new IllegalArgumentException( - "Invalid decimal value length. Valid length in [1 - 32], got " + length); } /** @@ -305,6 +313,13 @@ public void setSafe(int index, long start, ArrowBuf buffer, int length) { * @param length length of the value in the buffer */ public void setBigEndianSafe(int index, long start, ArrowBuf buffer, int length) { + // Validate before handleSafe/setBit so a rejected call has no side effects: it neither + // grows the vector nor marks the slot valid. + if (length > TYPE_WIDTH) { + throw new IllegalArgumentException( + "Invalid decimal value length. Valid length in [1 - 32], got " + length); + } + handleSafe(index); BitVectorHelper.setBit(validityBuffer, index); diff --git a/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java b/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java index 9bf1812cc6..7c4000d99a 100644 --- a/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java +++ b/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java @@ -204,9 +204,19 @@ public void set(int index, ArrowBuf buffer) { * @param value array of bytes containing decimal in big endian byte order. */ public void setBigEndian(int index, byte[] value) { - BitVectorHelper.setBit(validityBuffer, index); final int length = value.length; + // Reject an oversized value before touching the vector. The little-endian path below + // copies all `length` bytes into the fixed TYPE_WIDTH slot with unchecked native writes, + // so validating after the copy would leave adjacent memory already corrupted. Checking + // before setBit also keeps the slot null when the call is rejected. + if (length > TYPE_WIDTH) { + throw new IllegalArgumentException( + "Invalid decimal value length. Valid length in [1 - 16], got " + length); + } + + BitVectorHelper.setBit(validityBuffer, index); + // do the bound check. valueBuffer.checkBytes((long) index * TYPE_WIDTH, (long) (index + 1) * TYPE_WIDTH); @@ -241,8 +251,6 @@ public void setBigEndian(int index, byte[] value) { return; } } - throw new IllegalArgumentException( - "Invalid decimal value length. Valid length in [1 - 16], got " + length); } /** @@ -303,6 +311,13 @@ public void setSafe(int index, long start, ArrowBuf buffer, int length) { * @param length length of the value in the buffer */ public void setBigEndianSafe(int index, long start, ArrowBuf buffer, int length) { + // Validate before handleSafe/setBit so a rejected call has no side effects: it neither + // grows the vector nor marks the slot valid. + if (length > TYPE_WIDTH) { + throw new IllegalArgumentException( + "Invalid decimal value length. Valid length in [1 - 16], got " + length); + } + handleSafe(index); BitVectorHelper.setBit(validityBuffer, index); diff --git a/vector/src/test/java/org/apache/arrow/vector/TestDecimal256Vector.java b/vector/src/test/java/org/apache/arrow/vector/TestDecimal256Vector.java index b995dc5d92..42f58f36be 100644 --- a/vector/src/test/java/org/apache/arrow/vector/TestDecimal256Vector.java +++ b/vector/src/test/java/org/apache/arrow/vector/TestDecimal256Vector.java @@ -283,6 +283,45 @@ public void decimalBE2NE() { } } + @Test + public void setBigEndianOversizedDoesNotCorruptNeighbor() { + try (Decimal256Vector decimalVector = + TestUtils.newVector( + Decimal256Vector.class, "decimal", new ArrowType.Decimal(60, 0, 256), allocator)) { + decimalVector.allocateNew(2); + + // A value whose low bytes are all non-zero, stored in the slot right after the target. + final BigInteger neighbor = new BigInteger("305419896"); // 0x12345678 + decimalVector.setBigEndian(1, neighbor.toByteArray()); + + // A value longer than the 32-byte decimal must be rejected before anything is written. + assertThrows( + IllegalArgumentException.class, () -> decimalVector.setBigEndian(0, new byte[40])); + + // The rejected call must leave its own slot untouched (still null) and the neighbor intact. + assertTrue(decimalVector.isNull(0)); + assertEquals(neighbor, decimalVector.getObject(1).unscaledValue()); + } + } + + @Test + public void setBigEndianSafeOversizedHasNoSideEffects() { + try (Decimal256Vector decimalVector = + TestUtils.newVector( + Decimal256Vector.class, "decimal", new ArrowType.Decimal(60, 0, 256), allocator); + ArrowBuf buf = allocator.buffer(40)) { + decimalVector.allocateNew(1); + final int capacityBefore = decimalVector.getValueCapacity(); + + // Oversize length must be rejected before handleSafe grows the vector or setBit runs. + assertThrows( + IllegalArgumentException.class, () -> decimalVector.setBigEndianSafe(5, 0, buf, 40)); + + assertEquals(capacityBefore, decimalVector.getValueCapacity()); + assertTrue(decimalVector.isNull(0)); + } + } + @Test public void setUsingArrowBufOfLEInts() { try (Decimal256Vector decimalVector = diff --git a/vector/src/test/java/org/apache/arrow/vector/TestDecimalVector.java b/vector/src/test/java/org/apache/arrow/vector/TestDecimalVector.java index 85c11e8f3d..28b6143a24 100644 --- a/vector/src/test/java/org/apache/arrow/vector/TestDecimalVector.java +++ b/vector/src/test/java/org/apache/arrow/vector/TestDecimalVector.java @@ -18,6 +18,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; 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 static org.junit.jupiter.api.Assertions.fail; @@ -279,6 +280,45 @@ public void decimalBE2NE() { } } + @Test + public void setBigEndianOversizedDoesNotCorruptNeighbor() { + try (DecimalVector decimalVector = + TestUtils.newVector( + DecimalVector.class, "decimal", new ArrowType.Decimal(38, 0, 128), allocator)) { + decimalVector.allocateNew(2); + + // A value whose low bytes are all non-zero, stored in the slot right after the target. + final BigInteger neighbor = new BigInteger("305419896"); // 0x12345678 + decimalVector.setBigEndian(1, neighbor.toByteArray()); + + // A value longer than the 16-byte decimal must be rejected before anything is written. + assertThrows( + IllegalArgumentException.class, () -> decimalVector.setBigEndian(0, new byte[24])); + + // The rejected call must leave its own slot untouched (still null) and the neighbor intact. + assertTrue(decimalVector.isNull(0)); + assertEquals(neighbor, decimalVector.getObject(1).unscaledValue()); + } + } + + @Test + public void setBigEndianSafeOversizedHasNoSideEffects() { + try (DecimalVector decimalVector = + TestUtils.newVector( + DecimalVector.class, "decimal", new ArrowType.Decimal(38, 0, 128), allocator); + ArrowBuf buf = allocator.buffer(24)) { + decimalVector.allocateNew(1); + final int capacityBefore = decimalVector.getValueCapacity(); + + // Oversize length must be rejected before handleSafe grows the vector or setBit runs. + assertThrows( + IllegalArgumentException.class, () -> decimalVector.setBigEndianSafe(5, 0, buf, 24)); + + assertEquals(capacityBefore, decimalVector.getValueCapacity()); + assertTrue(decimalVector.isNull(0)); + } + } + @Test public void setUsingArrowBufOfInts() { try (DecimalVector decimalVector =