Skip to content

GH-1417: Validate decimal value length before raw writes in DecimalVector and Decimal256Vector #1418

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jbonofre merged 2 commits into from
Nov 1, 2026
Merged

GH-1417: Validate decimal value length before raw writes in DecimalVector and Decimal256Vector #1418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5892f8f
fix(vector): validate decimal value length before raw writes
andygrove Oct 31, 2026
67cd552
refactor(vector): tidy decimal length validation and tests
andygrove Oct 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 27 additions & 18 deletions vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.arrow.memory.ArrowBuf;
import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.memory.util.MemoryUtil;
import org.apache.arrow.util.Preconditions;
import org.apache.arrow.vector.complex.impl.Decimal256ReaderImpl;
import org.apache.arrow.vector.complex.reader.FieldReader;
import org.apache.arrow.vector.holders.Decimal256Holder;
Expand Down Expand Up @@ -205,8 +206,9 @@ 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;
checkValueLength(length);
BitVectorHelper.setBit(validityBuffer, index);

// do the bound check.
valueBuffer.checkBytes((long) index * TYPE_WIDTH, (long) (index + 1) * TYPE_WIDTH);
Expand All @@ -222,29 +224,26 @@ public void setBigEndian(int index, byte[] value) {
MemoryUtil.putByte(outAddress + byteIdx, value[length - 1 - byteIdx]);
}

if (length == TYPE_WIDTH) {
return;
}

if (length < TYPE_WIDTH) {
// sign extend
final byte pad = (byte) (value[0] < 0 ? 0xFF : 0x00);
MemoryUtil.setMemory(outAddress + length, Decimal256Vector.TYPE_WIDTH - length, pad);
return;
}
} else {
if (length <= TYPE_WIDTH) {
// copy data from value to outAddress
MemoryUtil.copyToMemory(
value, 0, outAddress + Decimal256Vector.TYPE_WIDTH - length, length);
// sign extend
final byte pad = (byte) (value[0] < 0 ? 0xFF : 0x00);
MemoryUtil.setMemory(outAddress, Decimal256Vector.TYPE_WIDTH - length, pad);
return;
}
// copy data from value to outAddress
MemoryUtil.copyToMemory(value, 0, outAddress + Decimal256Vector.TYPE_WIDTH - length, length);
// sign extend
final byte pad = (byte) (value[0] < 0 ? 0xFF : 0x00);
MemoryUtil.setMemory(outAddress, Decimal256Vector.TYPE_WIDTH - length, pad);
}
throw new IllegalArgumentException(
"Invalid decimal value length. Valid length in [1 - 32], got " + length);
}

private static void checkValueLength(int length) {
Preconditions.checkArgument(
length >= 0 && length <= TYPE_WIDTH,
"Invalid decimal value length. Valid length in [0 - %s], got %s",
TYPE_WIDTH,
length);
}

/**
Expand All @@ -268,6 +267,7 @@ public void set(int index, long start, ArrowBuf buffer) {
* @param length length of the value in the buffer
*/
public void setSafe(int index, long start, ArrowBuf buffer, int length) {
checkValueLength(length);
handleSafe(index);
BitVectorHelper.setBit(validityBuffer, index);

Expand All @@ -277,6 +277,10 @@ public void setSafe(int index, long start, ArrowBuf buffer, int length) {

long inAddress = buffer.memoryAddress() + start;
long outAddress = valueBuffer.memoryAddress() + (long) index * TYPE_WIDTH;
if (length == 0) {
MemoryUtil.setMemory(outAddress, Decimal256Vector.TYPE_WIDTH, (byte) 0);
return;
}
if (LITTLE_ENDIAN) {
MemoryUtil.copyMemory(inAddress, outAddress, length);
// sign extend
Expand Down Expand Up @@ -305,6 +309,7 @@ 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) {
checkValueLength(length);
handleSafe(index);
BitVectorHelper.setBit(validityBuffer, index);

Expand All @@ -315,14 +320,18 @@ public void setBigEndianSafe(int index, long start, ArrowBuf buffer, int length)
// not using buffer.getByte() to avoid boundary checks for every byte.
long inAddress = buffer.memoryAddress() + start;
long outAddress = valueBuffer.memoryAddress() + (long) index * TYPE_WIDTH;
if (length == 0) {
MemoryUtil.setMemory(outAddress, Decimal256Vector.TYPE_WIDTH, (byte) 0);
return;
}
if (LITTLE_ENDIAN) {
// swap bytes to convert BE to LE
for (int byteIdx = 0; byteIdx < length; ++byteIdx) {
byte val = MemoryUtil.getByte((inAddress + length - 1) - byteIdx);
MemoryUtil.putByte(outAddress + byteIdx, val);
}
// sign extend
if (length < 32) {
if (length < TYPE_WIDTH) {
byte msb = MemoryUtil.getByte(inAddress);
final byte pad = (byte) (msb < 0 ? 0xFF : 0x00);
MemoryUtil.setMemory(outAddress + length, Decimal256Vector.TYPE_WIDTH - length, pad);
Expand Down
42 changes: 26 additions & 16 deletions vector/src/main/java/org/apache/arrow/vector/DecimalVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.arrow.memory.ArrowBuf;
import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.memory.util.MemoryUtil;
import org.apache.arrow.util.Preconditions;
import org.apache.arrow.vector.complex.impl.DecimalReaderImpl;
import org.apache.arrow.vector.complex.reader.FieldReader;
import org.apache.arrow.vector.holders.DecimalHolder;
Expand Down Expand Up @@ -204,8 +205,9 @@ 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;
checkValueLength(length);
BitVectorHelper.setBit(validityBuffer, index);

// do the bound check.
valueBuffer.checkBytes((long) index * TYPE_WIDTH, (long) (index + 1) * TYPE_WIDTH);
Expand All @@ -221,28 +223,26 @@ public void setBigEndian(int index, byte[] value) {
MemoryUtil.putByte(outAddress + byteIdx, value[length - 1 - byteIdx]);
}

if (length == TYPE_WIDTH) {
return;
}

if (length < TYPE_WIDTH) {
// sign extend
final byte pad = (byte) (value[0] < 0 ? 0xFF : 0x00);
MemoryUtil.setMemory(outAddress + length, DecimalVector.TYPE_WIDTH - length, pad);
return;
}
} else {
if (length <= TYPE_WIDTH) {
// copy data from value to outAddress
MemoryUtil.copyToMemory(value, 0, outAddress + DecimalVector.TYPE_WIDTH - length, length);
// sign extend
final byte pad = (byte) (value[0] < 0 ? 0xFF : 0x00);
MemoryUtil.setMemory(outAddress, DecimalVector.TYPE_WIDTH - length, pad);
return;
}
// copy data from value to outAddress
MemoryUtil.copyToMemory(value, 0, outAddress + DecimalVector.TYPE_WIDTH - length, length);
// sign extend
final byte pad = (byte) (value[0] < 0 ? 0xFF : 0x00);
MemoryUtil.setMemory(outAddress, DecimalVector.TYPE_WIDTH - length, pad);
}
throw new IllegalArgumentException(
"Invalid decimal value length. Valid length in [1 - 16], got " + length);
}

private static void checkValueLength(int length) {
Preconditions.checkArgument(
length >= 0 && length <= TYPE_WIDTH,
"Invalid decimal value length. Valid length in [0 - %s], got %s",
TYPE_WIDTH,
length);
}

/**
Expand All @@ -266,6 +266,7 @@ public void set(int index, long start, ArrowBuf buffer) {
* @param length length of the value in the buffer
*/
public void setSafe(int index, long start, ArrowBuf buffer, int length) {
checkValueLength(length);
handleSafe(index);
BitVectorHelper.setBit(validityBuffer, index);

Expand All @@ -275,6 +276,10 @@ public void setSafe(int index, long start, ArrowBuf buffer, int length) {

long inAddress = buffer.memoryAddress() + start;
long outAddress = valueBuffer.memoryAddress() + (long) index * TYPE_WIDTH;
if (length == 0) {
MemoryUtil.setMemory(outAddress, DecimalVector.TYPE_WIDTH, (byte) 0);
return;
}
if (LITTLE_ENDIAN) {
MemoryUtil.copyMemory(inAddress, outAddress, length);
// sign extend
Expand Down Expand Up @@ -303,6 +308,7 @@ 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) {
checkValueLength(length);
handleSafe(index);
BitVectorHelper.setBit(validityBuffer, index);

Expand All @@ -313,6 +319,10 @@ public void setBigEndianSafe(int index, long start, ArrowBuf buffer, int length)
// not using buffer.getByte() to avoid boundary checks for every byte.
long inAddress = buffer.memoryAddress() + start;
long outAddress = valueBuffer.memoryAddress() + (long) index * TYPE_WIDTH;
if (length == 0) {
MemoryUtil.setMemory(outAddress, DecimalVector.TYPE_WIDTH, (byte) 0);
return;
}
if (LITTLE_ENDIAN) {
// swap bytes to convert BE to LE
for (int byteIdx = 0; byteIdx < length; ++byteIdx) {
Expand Down
77 changes: 76 additions & 1 deletion vector/src/test/java/org/apache/arrow/vector/TestDecimal256Vector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@
package org.apache.arrow.vector;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
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.math.BigDecimal;
import java.math.BigInteger;
import java.util.function.BiConsumer;
import org.apache.arrow.memory.ArrowBuf;
import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.vector.types.pojo.ArrowType;
Expand Down Expand Up @@ -268,7 +270,7 @@ public void decimalBE2NE() {
IllegalArgumentException.class,
() -> decimalVector.setBigEndian(insertionIdxCapture, new byte[33]));
assertTrue(
ex.getMessage().equals("Invalid decimal value length. Valid length in [1 - 32], got 33"));
ex.getMessage().equals("Invalid decimal value length. Valid length in [0 - 32], got 33"));
decimalVector.setValueCount(insertionIdx);

// retrieve values and check if they are correct
Expand Down Expand Up @@ -419,4 +421,77 @@ private void verifyWritingArrowBufWithBigEndianBytes(
assertEquals(expectedValues[i], value);
}
}

private static final int OVERSIZED_LENGTH = Decimal256Vector.TYPE_WIDTH + 1;

private Decimal256Vector newVectorWithFirstEightValid() {
Decimal256Vector vector = new Decimal256Vector("decimal", allocator, 38, 0);
vector.allocateNew(64);
for (int i = 0; i < 8; i++) {
vector.setSafe(i, BigDecimal.valueOf(i));
}
return vector;
}

// Validity bits sit right after the value buffer, so an overrun of the last slot clobbers them.
private static void assertFirstEightValid(Decimal256Vector vector) {
for (int i = 0; i < 8; i++) {
assertFalse(vector.isNull(i), "validity bit " + i + " was overwritten by an oversized write");
assertEquals(BigDecimal.valueOf(i), vector.getObject(i));
}
}

private void assertOversizedWriteRejected(BiConsumer<Decimal256Vector, ArrowBuf> write) {
try (Decimal256Vector vector = newVectorWithFirstEightValid();
ArrowBuf src = allocator.buffer(OVERSIZED_LENGTH)) {
src.setZero(0, OVERSIZED_LENGTH);
assertThrows(IllegalArgumentException.class, () -> write.accept(vector, src));
assertFirstEightValid(vector);
}
}

@Test
public void setBigEndianSafeRejectsOversizedByteArray() {
assertOversizedWriteRejected(
(vector, src) ->
vector.setBigEndianSafe(vector.getValueCapacity() - 1, new byte[OVERSIZED_LENGTH]));
}

@Test
public void setSafeFromArrowBufRejectsOversizedLength() {
assertOversizedWriteRejected(
(vector, src) -> vector.setSafe(vector.getValueCapacity() - 1, 0, src, OVERSIZED_LENGTH));
}

@Test
public void setBigEndianSafeFromArrowBufRejectsOversizedLength() {
assertOversizedWriteRejected(
(vector, src) ->
vector.setBigEndianSafe(vector.getValueCapacity() - 1, 0, src, OVERSIZED_LENGTH));
}

@Test
public void setSafeFromArrowBufWithZeroLengthDoesNotReadBeforeStart() {
try (Decimal256Vector vector = new Decimal256Vector("decimal", allocator, 38, 0);
ArrowBuf src = allocator.buffer(2)) {
vector.allocateNew(1);
// A negative byte immediately before `start` must not be used for sign extension.
src.setByte(0, (byte) 0xFF);
src.setByte(1, (byte) 0x00);
vector.setSafe(0, 1, src, 0);
assertEquals(BigDecimal.ZERO, vector.getObject(0));
}
}

@Test
public void setBigEndianSafeFromArrowBufWithZeroLengthDoesNotReadAtStart() {
try (Decimal256Vector vector = new Decimal256Vector("decimal", allocator, 38, 0);
ArrowBuf src = allocator.buffer(1)) {
vector.allocateNew(1);
// A negative byte at `start` must not be used for sign extension of an empty value.
src.setByte(0, (byte) 0xFF);
vector.setBigEndianSafe(0, 0, src, 0);
assertEquals(BigDecimal.ZERO, vector.getObject(0));
}
}
}
Loading
Loading