diff --git a/api/src/main/java/org/apache/iceberg/variants/SerializedArray.java b/api/src/main/java/org/apache/iceberg/variants/SerializedArray.java index d215e018449e..e842146e9e7d 100644 --- a/api/src/main/java/org/apache/iceberg/variants/SerializedArray.java +++ b/api/src/main/java/org/apache/iceberg/variants/SerializedArray.java @@ -36,12 +36,16 @@ static SerializedArray from(VariantMetadata metadata, byte[] bytes) { } static SerializedArray from(VariantMetadata metadata, ByteBuffer value, int header) { + return from(metadata, value, header, 0); + } + + static SerializedArray from(VariantMetadata metadata, ByteBuffer value, int header, int depth) { Preconditions.checkArgument( value.order() == ByteOrder.LITTLE_ENDIAN, "Unsupported byte order: big endian"); BasicType basicType = VariantUtil.basicType(header); Preconditions.checkArgument( basicType == BasicType.ARRAY, "Invalid array, basic type: " + basicType); - return new SerializedArray(metadata, value, header); + return new SerializedArray(metadata, value, header, depth); } private final VariantMetadata metadata; @@ -49,16 +53,33 @@ static SerializedArray from(VariantMetadata metadata, ByteBuffer value, int head private final int offsetSize; private final int offsetListOffset; private final int dataOffset; + private final int depth; private final VariantValue[] array; - private SerializedArray(VariantMetadata metadata, ByteBuffer value, int header) { + private SerializedArray(VariantMetadata metadata, ByteBuffer value, int header, int depth) { this.metadata = metadata; this.value = value; + this.depth = depth; this.offsetSize = 1 + ((header & OFFSET_SIZE_MASK) >> OFFSET_SIZE_SHIFT); int numElementsSize = ((header & IS_LARGE) == IS_LARGE) ? 4 : 1; + Preconditions.checkArgument( + value.remaining() >= HEADER_SIZE + numElementsSize, + "Invalid variant array: buffer too small for element count field"); int numElements = ByteBuffers.readLittleEndianUnsigned(value, HEADER_SIZE, numElementsSize); + Preconditions.checkArgument( + numElements >= 0, "Invalid variant array: negative element count %s", numElements); + Preconditions.checkArgument( + numElements <= VariantUtil.MAX_ELEMENTS, + "Invalid variant array: element count %s exceeds maximum %s", + numElements, + VariantUtil.MAX_ELEMENTS); this.offsetListOffset = HEADER_SIZE + numElementsSize; - this.dataOffset = offsetListOffset + ((1 + numElements) * offsetSize); + long offsetTableEnd = (long) offsetListOffset + ((long) numElements + 1L) * offsetSize; + Preconditions.checkArgument( + offsetTableEnd <= value.remaining(), + "Invalid variant array: element count %s exceeds buffer", + numElements); + this.dataOffset = Math.toIntExact(offsetTableEnd); this.array = new VariantValue[numElements]; } @@ -76,8 +97,16 @@ public VariantValue get(int index) { int next = ByteBuffers.readLittleEndianUnsigned( value, offsetListOffset + (offsetSize * (1 + index)), offsetSize); + long dataLen = value.remaining() - (long) dataOffset; + Preconditions.checkArgument( + offset >= 0 && next >= offset && next <= dataLen, + "Invalid variant array: offset range [%s, %s] out of data region [0, %s]", + offset, + next, + dataLen); array[index] = - VariantValue.from(metadata, VariantUtil.slice(value, dataOffset + offset, next - offset)); + VariantUtil.fromBuffer( + metadata, VariantUtil.slice(value, dataOffset + offset, next - offset), depth + 1); } return array[index]; } diff --git a/api/src/main/java/org/apache/iceberg/variants/SerializedMetadata.java b/api/src/main/java/org/apache/iceberg/variants/SerializedMetadata.java index 9eff21536de6..3eb5bcd264c8 100644 --- a/api/src/main/java/org/apache/iceberg/variants/SerializedMetadata.java +++ b/api/src/main/java/org/apache/iceberg/variants/SerializedMetadata.java @@ -59,15 +59,37 @@ static SerializedMetadata from(ByteBuffer metadata) { private SerializedMetadata(ByteBuffer metadata, int header) { this.isSorted = (header & SORTED_STRINGS) == SORTED_STRINGS; this.offsetSize = 1 + ((header & OFFSET_SIZE_MASK) >> OFFSET_SIZE_SHIFT); + Preconditions.checkArgument( + metadata.remaining() >= HEADER_SIZE + offsetSize, + "Invalid variant metadata: buffer too small for dictionary size field"); int dictSize = ByteBuffers.readLittleEndianUnsigned(metadata, HEADER_SIZE, offsetSize); - this.dict = new String[dictSize]; + Preconditions.checkArgument( + dictSize >= 0, "Invalid variant metadata: negative dictionary size %s", dictSize); + Preconditions.checkArgument( + dictSize <= VariantUtil.MAX_ELEMENTS, + "Invalid variant metadata: dictionary size %s exceeds maximum %s", + dictSize, + VariantUtil.MAX_ELEMENTS); this.offsetListOffset = HEADER_SIZE + offsetSize; - this.dataOffset = offsetListOffset + ((1 + dictSize) * offsetSize); - int endOffset = - dataOffset - + ByteBuffers.readLittleEndianUnsigned( - metadata, offsetListOffset + (offsetSize * dictSize), offsetSize); - if (endOffset < metadata.limit()) { + long offsetTableEnd = (long) offsetListOffset + ((long) dictSize + 1L) * offsetSize; + Preconditions.checkArgument( + offsetTableEnd <= metadata.remaining(), + "Invalid variant metadata: dictionary size %s exceeds buffer", + dictSize); + this.dict = new String[dictSize]; + this.dataOffset = Math.toIntExact(offsetTableEnd); + int lastOffset = + ByteBuffers.readLittleEndianUnsigned( + metadata, offsetListOffset + (offsetSize * dictSize), offsetSize); + Preconditions.checkArgument( + lastOffset >= 0, "Invalid variant metadata: negative end offset %s", lastOffset); + long endOffsetLong = (long) dataOffset + lastOffset; + Preconditions.checkArgument( + endOffsetLong <= metadata.remaining(), + "Invalid variant metadata: end offset %s exceeds buffer", + endOffsetLong); + int endOffset = (int) endOffsetLong; + if (endOffset < metadata.remaining()) { this.metadata = VariantUtil.slice(metadata, 0, endOffset); } else { this.metadata = metadata; @@ -112,6 +134,12 @@ public String get(int index) { int next = ByteBuffers.readLittleEndianUnsigned( metadata, offsetListOffset + (offsetSize * (1 + index)), offsetSize); + Preconditions.checkArgument( + offset >= 0 && next >= offset && (long) dataOffset + next <= metadata.remaining(), + "Invalid variant metadata: dict entry %s offset range [%s, %s] invalid", + index, + offset, + next); dict[index] = VariantUtil.readString(metadata, dataOffset + offset, next - offset); } return dict[index]; diff --git a/api/src/main/java/org/apache/iceberg/variants/SerializedObject.java b/api/src/main/java/org/apache/iceberg/variants/SerializedObject.java index bd9510c9a9b2..b7ddf0a59d7d 100644 --- a/api/src/main/java/org/apache/iceberg/variants/SerializedObject.java +++ b/api/src/main/java/org/apache/iceberg/variants/SerializedObject.java @@ -42,12 +42,16 @@ static SerializedObject from(VariantMetadata metadata, byte[] bytes) { } static SerializedObject from(VariantMetadata metadata, ByteBuffer value, int header) { + return from(metadata, value, header, 0); + } + + static SerializedObject from(VariantMetadata metadata, ByteBuffer value, int header, int depth) { Preconditions.checkArgument( value.order() == ByteOrder.LITTLE_ENDIAN, "Unsupported byte order: big endian"); BasicType basicType = VariantUtil.basicType(header); Preconditions.checkArgument( basicType == BasicType.OBJECT, "Invalid object, basic type: " + basicType); - return new SerializedObject(metadata, value, header); + return new SerializedObject(metadata, value, header, depth); } private final VariantMetadata metadata; @@ -60,21 +64,42 @@ static SerializedObject from(VariantMetadata metadata, ByteBuffer value, int hea private final int[] offsets; private final int[] lengths; private final int dataOffset; + private final int depth; private final VariantValue[] values; - private SerializedObject(VariantMetadata metadata, ByteBuffer value, int header) { + private SerializedObject(VariantMetadata metadata, ByteBuffer value, int header, int depth) { this.metadata = metadata; this.value = value; + this.depth = depth; this.offsetSize = 1 + ((header & OFFSET_SIZE_MASK) >> OFFSET_SIZE_SHIFT); this.fieldIdSize = 1 + ((header & FIELD_ID_SIZE_MASK) >> FIELD_ID_SIZE_SHIFT); int numElementsSize = ((header & IS_LARGE) == IS_LARGE) ? 4 : 1; + Preconditions.checkArgument( + value.remaining() >= HEADER_SIZE + numElementsSize, + "Invalid variant object: buffer too small for element count field"); int numElements = ByteBuffers.readLittleEndianUnsigned(value, HEADER_SIZE, numElementsSize); + Preconditions.checkArgument( + numElements >= 0, "Invalid variant object: negative element count %s", numElements); + Preconditions.checkArgument( + numElements <= VariantUtil.MAX_ELEMENTS, + "Invalid variant object: element count %s exceeds maximum %s", + numElements, + VariantUtil.MAX_ELEMENTS); this.fieldIdListOffset = HEADER_SIZE + numElementsSize; + long dataStart = + (long) fieldIdListOffset + + (long) numElements * fieldIdSize + + ((long) numElements + 1L) * offsetSize; + Preconditions.checkArgument( + dataStart <= value.remaining(), + "Invalid variant object: element count %s exceeds buffer", + numElements); + this.offsetListOffset = + Math.toIntExact((long) fieldIdListOffset + (long) numElements * fieldIdSize); this.fieldIds = new Integer[numElements]; - this.offsetListOffset = fieldIdListOffset + (numElements * fieldIdSize); this.offsets = new int[numElements]; this.lengths = new int[numElements]; - this.dataOffset = offsetListOffset + ((1 + numElements) * offsetSize); + this.dataOffset = Math.toIntExact(dataStart); this.values = new VariantValue[numElements]; if (numElements > 0) { @@ -96,14 +121,28 @@ private void initOffsetsAndLengths(int numElements) { int dataLength = ByteBuffers.readLittleEndianUnsigned( value, offsetListOffset + (numElements * offsetSize), offsetSize); + long dataLen = value.remaining() - (long) dataOffset; + Preconditions.checkArgument( + dataLength >= 0 && dataLength <= dataLen, + "Invalid variant object: data length %s out of data region [0, %s]", + dataLength, + dataLen); + for (int index = 0; index < numElements; index += 1) { + Preconditions.checkArgument( + offsets[index] >= 0 && offsets[index] <= dataLength, + "Invalid variant object: offset %s out of declared data length %s", + offsets[index], + dataLength); + } offsetToLength.put(dataLength, 0); // populate lengths list by sorting offsets List sortedOffsets = offsetToLength.keySet().stream().sorted().collect(Collectors.toList()); - for (int index = 0; index < numElements; index += 1) { - int offset = sortedOffsets.get(index); - int length = sortedOffsets.get(index + 1) - offset; + // shared offsets are spec-legal (zero-length spans); do not require uniqueness + for (int i = 0; i < sortedOffsets.size() - 1; i += 1) { + int offset = sortedOffsets.get(i); + int length = sortedOffsets.get(i + 1) - offset; offsetToLength.put(offset, length); } @@ -161,11 +200,19 @@ public String next() { }; } + // Field id range is validated lazily on first access, not at construction. private int id(int index) { if (null == fieldIds[index]) { - fieldIds[index] = + int dictSize = metadata.dictionarySize(); + int id = ByteBuffers.readLittleEndianUnsigned( value, fieldIdListOffset + (index * fieldIdSize), fieldIdSize); + Preconditions.checkArgument( + id >= 0 && id < dictSize, + "Invalid variant object: field id %s out of range [0, %s)", + id, + dictSize); + fieldIds[index] = id; } return fieldIds[index]; @@ -182,8 +229,10 @@ public VariantValue get(String name) { if (null == values[index]) { values[index] = - VariantValue.from( - metadata, VariantUtil.slice(value, dataOffset + offsets[index], lengths[index])); + VariantUtil.fromBuffer( + metadata, + VariantUtil.slice(value, dataOffset + offsets[index], lengths[index]), + depth + 1); } return values[index]; diff --git a/api/src/main/java/org/apache/iceberg/variants/SerializedPrimitive.java b/api/src/main/java/org/apache/iceberg/variants/SerializedPrimitive.java index 8ea35312737d..3cbd668444b8 100644 --- a/api/src/main/java/org/apache/iceberg/variants/SerializedPrimitive.java +++ b/api/src/main/java/org/apache/iceberg/variants/SerializedPrimitive.java @@ -51,6 +51,56 @@ static SerializedPrimitive from(ByteBuffer value, int header) { private SerializedPrimitive(ByteBuffer value, int header) { this.value = value; this.type = PhysicalType.from(header >> PRIMITIVE_TYPE_SHIFT); + long requiredBytes = PRIMITIVE_OFFSET + payloadSize(type, value); + Preconditions.checkArgument( + requiredBytes <= value.remaining(), + "Invalid variant primitive: %s payload extends past buffer", + type); + } + + private static long payloadSize(PhysicalType type, ByteBuffer value) { + switch (type) { + case NULL: + case BOOLEAN_TRUE: + case BOOLEAN_FALSE: + return 0; + case INT8: + return 1; + case INT16: + return 2; + case INT32: + case DATE: + case FLOAT: + return 4; + case INT64: + case TIMESTAMPTZ: + case TIMESTAMPNTZ: + case TIME: + case TIMESTAMPTZ_NANOS: + case TIMESTAMPNTZ_NANOS: + case DOUBLE: + return 8; + case DECIMAL4: + return 5; + case DECIMAL8: + return 9; + case DECIMAL16: + return 17; + case UUID: + return 16; + case BINARY: + case STRING: + Preconditions.checkArgument( + PRIMITIVE_OFFSET + 4 <= value.remaining(), + "Invalid variant primitive: %s size field extends past buffer", + type); + int size = ByteBuffers.readLittleEndianInt32(value, PRIMITIVE_OFFSET); + Preconditions.checkArgument( + size >= 0, "Invalid variant primitive: negative %s size %s", type, size); + return 4L + size; + } + + throw new UnsupportedOperationException("Unsupported primitive type: " + type); } private Object read() { diff --git a/api/src/main/java/org/apache/iceberg/variants/SerializedShortString.java b/api/src/main/java/org/apache/iceberg/variants/SerializedShortString.java index 90fb54f3b13e..15124b363d9a 100644 --- a/api/src/main/java/org/apache/iceberg/variants/SerializedShortString.java +++ b/api/src/main/java/org/apache/iceberg/variants/SerializedShortString.java @@ -47,6 +47,10 @@ static SerializedShortString from(ByteBuffer value, int header) { private SerializedShortString(ByteBuffer value, int header) { this.value = value; this.length = ((header & LENGTH_MASK) >> LENGTH_SHIFT); + Preconditions.checkArgument( + HEADER_SIZE + length <= value.remaining(), + "Invalid variant short string: length %s exceeds buffer", + length); } @Override diff --git a/api/src/main/java/org/apache/iceberg/variants/VariantUtil.java b/api/src/main/java/org/apache/iceberg/variants/VariantUtil.java index 62c83b51a58d..3c88e38157a5 100644 --- a/api/src/main/java/org/apache/iceberg/variants/VariantUtil.java +++ b/api/src/main/java/org/apache/iceberg/variants/VariantUtil.java @@ -22,6 +22,8 @@ import java.nio.ByteOrder; import java.nio.charset.StandardCharsets; import java.util.function.Function; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; +import org.apache.iceberg.util.ByteBuffers; class VariantUtil { private static final int BASIC_TYPE_MASK = 0b11; @@ -30,8 +32,45 @@ class VariantUtil { private static final int BASIC_TYPE_OBJECT = 2; private static final int BASIC_TYPE_ARRAY = 3; + /** + * Maximum nesting depth in a Variant (permitted depths 0..MAX_VARIANT_DEPTH). Safety limit, not a + * spec bound. Matches parquet-java (apache/parquet-java#3562). + */ + static final int MAX_VARIANT_DEPTH = 1000; + + /** + * Maximum element count for Variant containers and metadata dictionaries. Safety limit against + * buffer-to-heap allocation amplification. + */ + static final int MAX_ELEMENTS = 16_777_216; + private VariantUtil() {} + /** Parses a variant value; validates input and enforces {@link #MAX_VARIANT_DEPTH}. */ + static VariantValue fromBuffer(VariantMetadata metadata, ByteBuffer value, int depth) { + Preconditions.checkArgument(depth >= 0, "Invalid variant: negative depth %s", depth); + Preconditions.checkArgument( + depth <= MAX_VARIANT_DEPTH, + "Invalid variant: nesting depth %s exceeds maximum %s", + depth, + MAX_VARIANT_DEPTH); + Preconditions.checkArgument(value.remaining() >= 1, "Invalid variant: empty value buffer"); + int header = ByteBuffers.readByte(value, 0); + BasicType basicType = basicType(header); + switch (basicType) { + case PRIMITIVE: + return SerializedPrimitive.from(value, header); + case SHORT_STRING: + return SerializedShortString.from(value, header); + case OBJECT: + return SerializedObject.from(metadata, value, header, depth); + case ARRAY: + return SerializedArray.from(metadata, value, header, depth); + } + + throw new UnsupportedOperationException("Unsupported basic type: " + basicType); + } + static float readFloat(ByteBuffer buffer, int offset) { return buffer.getFloat(buffer.position() + offset); } diff --git a/api/src/main/java/org/apache/iceberg/variants/VariantValue.java b/api/src/main/java/org/apache/iceberg/variants/VariantValue.java index 4a477d360ca8..ea27857e2908 100644 --- a/api/src/main/java/org/apache/iceberg/variants/VariantValue.java +++ b/api/src/main/java/org/apache/iceberg/variants/VariantValue.java @@ -19,7 +19,6 @@ package org.apache.iceberg.variants; import java.nio.ByteBuffer; -import org.apache.iceberg.util.ByteBuffers; /** A variant value. */ public interface VariantValue { @@ -62,19 +61,6 @@ default VariantArray asArray() { } static VariantValue from(VariantMetadata metadata, ByteBuffer value) { - int header = ByteBuffers.readByte(value, 0); - BasicType basicType = VariantUtil.basicType(header); - switch (basicType) { - case PRIMITIVE: - return SerializedPrimitive.from(value, header); - case SHORT_STRING: - return SerializedShortString.from(value, header); - case OBJECT: - return SerializedObject.from(metadata, value, header); - case ARRAY: - return SerializedArray.from(metadata, value, header); - } - - throw new UnsupportedOperationException("Unsupported basic type: " + basicType); + return VariantUtil.fromBuffer(metadata, value, 0); } } diff --git a/api/src/test/java/org/apache/iceberg/variants/TestMalformedVariant.java b/api/src/test/java/org/apache/iceberg/variants/TestMalformedVariant.java new file mode 100644 index 000000000000..89638c3fe974 --- /dev/null +++ b/api/src/test/java/org/apache/iceberg/variants/TestMalformedVariant.java @@ -0,0 +1,508 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.variants; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +import org.junit.jupiter.api.Test; + +public class TestMalformedVariant { + + private static final ByteBuffer EMPTY_METADATA = + ByteBuffer.wrap(new byte[] {0x01, 0x00, 0x00}).order(ByteOrder.LITTLE_ENDIAN); + + @Test + public void testOversizedMetadataDictSize() { + // metadata: [0x01 header - v1, offsetSize=1] [0xFF dictSize=255] - buffer stops before offsets + byte[] bytes = new byte[] {0x01, (byte) 0xFF}; + + assertThatThrownBy(() -> SerializedMetadata.from(bytes)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("dictionary size"); + } + + @Test + public void testUnsupportedMetadataVersionRejected() { + // header low 4 bits = version; spec version is 1. 0x02 = version 2, must be rejected. + byte[] bytes = new byte[] {0x02, 0x00, 0x00}; + + assertThatThrownBy(() -> SerializedMetadata.from(bytes)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Unsupported version"); + } + + @Test + public void testOversizedArrayNumElements() { + // array: [0b10011 header - large, offsetSize=1] [4-byte numElements=0x00100000 (1M)] + // buffer stops before the offset table can be read + ByteBuffer value = + ByteBuffer.wrap(new byte[] {(byte) 0b10011, 0x00, 0x00, 0x00, 0x10, 0x00}) + .order(ByteOrder.LITTLE_ENDIAN); + + assertThatThrownBy(() -> VariantValue.from(SerializedMetadata.from(EMPTY_METADATA), value)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("element count"); + } + + @Test + public void testOversizedObjectNumElements() { + // [0b1000010 hdr - large object] [4-byte numElements=0x00100000] - buffer stops before tables + ByteBuffer value = + ByteBuffer.wrap(new byte[] {(byte) 0b1000010, 0x00, 0x00, 0x00, 0x10, 0x00}) + .order(ByteOrder.LITTLE_ENDIAN); + + assertThatThrownBy(() -> VariantValue.from(SerializedMetadata.from(EMPTY_METADATA), value)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("element count"); + } + + @Test + public void testFieldIdOutOfRange() { + // [0x02 hdr] [0x01 numEl] [0x05 fieldId - out of range in empty dict] [0x00,0x01 offsets] + // [0x00 data]; range check is lazy so descend via get() to trigger + ByteBuffer value = + ByteBuffer.wrap(new byte[] {0x02, 0x01, 0x05, 0x00, 0x01, 0x00}) + .order(ByteOrder.LITTLE_ENDIAN); + + VariantValue top = VariantValue.from(SerializedMetadata.from(EMPTY_METADATA), value); + + assertThatThrownBy(() -> top.asObject().get("anything")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("field id"); + } + + @Test + public void testOutOfRangeChildOffsetInArray() { + // [0b0011 hdr - small array, offsetSize=1] [0x01 numEl] [0xFF,0x01 offsets - end top.asArray().get(0)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("out of data region"); + } + + @Test + public void testMalformedChildOffsetCaughtOnDescent() { + // outer [0b0011 hdr] [0x01 numEl] [0x00,0x06 offsets]; inner large-array at byte 4 + // has numElements=0x00100000 - rejected only on descent, not at outer parse + ByteBuffer value = + ByteBuffer.wrap( + new byte[] {0b0011, 0x01, 0x00, 0x06, (byte) 0b10011, 0x00, 0x00, 0x00, 0x10, 0x00}) + .order(ByteOrder.LITTLE_ENDIAN); + + VariantValue top = VariantValue.from(SerializedMetadata.from(EMPTY_METADATA), value); + + assertThatThrownBy(() -> top.asArray().get(0)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("element count"); + } + + @Test + public void testNegativeMetadataEndOffset() { + // metadata: [0b11000001 header - v1, offsetSize=4] [4-byte dictSize=0] + // [0xFFFFFFFF end offset as int32 = -1] + byte[] metadata = { + (byte) 0b11000001, 0x00, 0x00, 0x00, 0x00, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF + }; + + assertThatThrownBy(() -> SerializedMetadata.from(metadata)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("negative end offset"); + } + + @Test + public void testNegativeDictOffsetInGet() { + // metadata: [0b11000001 header - v1, offsetSize=4] [4-byte dictSize=1] + // [4-byte offset[0]=0xFFFFFFFF (= -1)] [4-byte offset[1]=0] + byte[] metadata = { + (byte) 0b11000001, + 0x01, + 0x00, + 0x00, + 0x00, + (byte) 0xFF, + (byte) 0xFF, + (byte) 0xFF, + (byte) 0xFF, + 0x00, + 0x00, + 0x00, + 0x00 + }; + + SerializedMetadata parsed = SerializedMetadata.from(metadata); + + assertThatThrownBy(() -> parsed.get(0)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("dict entry"); + } + + @Test + public void testOversizedPrimitiveStringSize() { + // primitive: [0b1000000 header - STRING primitive] + // [0x7FFFFFFF as 4-byte size = 2GB claimed payload] + ByteBuffer value = + ByteBuffer.wrap(new byte[] {(byte) 0b1000000, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, 0x7F}) + .order(ByteOrder.LITTLE_ENDIAN); + + assertThatThrownBy(() -> VariantValue.from(SerializedMetadata.from(EMPTY_METADATA), value)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("payload"); + } + + @Test + public void testOffsetExceedsDeclaredDataLengthInObject() { + // metadata: [0x01 hdr] [0x01 dictSize] [0x00,0x01 offsets] ['a'] + byte[] metadata = {0x01, 0x01, 0x00, 0x01, 'a'}; + // object: [0x02][0x01][0x00 fieldId][0x32 offset[0]=50 > dataLength=0][0x00][0x00] + ByteBuffer value = + ByteBuffer.wrap(new byte[] {0x02, 0x01, 0x00, 0x32, 0x00, 0x00}) + .order(ByteOrder.LITTLE_ENDIAN); + + assertThatThrownBy(() -> VariantValue.from(SerializedMetadata.from(metadata), value)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("declared data length"); + } + + @Test + public void testOffsetArithmeticOverflowInObject() { + // object: [0x4E header - large, fieldIdSize=1, offsetSize=4] [4-byte numElements=0x33333333] + // numElements * offsetSize overflows signed int; long guard fires first + ByteBuffer value = + ByteBuffer.wrap(new byte[] {0x4E, 0x33, 0x33, 0x33, 0x33, (byte) 0xFF, (byte) 0xFF, 0x3F}) + .order(ByteOrder.LITTLE_ENDIAN); + + assertThatThrownBy(() -> VariantValue.from(SerializedMetadata.from(EMPTY_METADATA), value)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("element count"); + } + + @Test + public void testArrayDataOffsetIntOverflow() { + // array: [0b11111 header - large, offsetSize=4] [4-byte numElements=0x20000000 (~536M)] + // (1+numElements)*offsetSize overflows signed int; long guard must fire first. + ByteBuffer value = + ByteBuffer.wrap(new byte[] {(byte) 0b11111, 0x00, 0x00, 0x00, 0x20, 0x00}) + .order(ByteOrder.LITTLE_ENDIAN); + + assertThatThrownBy(() -> VariantValue.from(SerializedMetadata.from(EMPTY_METADATA), value)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("element count"); + } + + @Test + public void testObjectDataOffsetIntOverflow() { + // [0b1111110 hdr - large object, fieldIdSize=4, offsetSize=4] [4-byte numElements=0x20000000] + // offsetListOffset and dataOffset int expressions overflow; long guard fires first + ByteBuffer value = + ByteBuffer.wrap(new byte[] {(byte) 0b1111110, 0x00, 0x00, 0x00, 0x20, 0x00}) + .order(ByteOrder.LITTLE_ENDIAN); + + assertThatThrownBy(() -> VariantValue.from(SerializedMetadata.from(EMPTY_METADATA), value)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("element count"); + } + + @Test + public void testMetadataDataOffsetIntOverflow() { + // metadata: [0b11000001 header - v1, offsetSize=4] [4-byte dictSize=0x20000000 (~536M)] + // (1+dictSize)*offsetSize overflows signed int; long guard fires first. + byte[] metadata = new byte[] {(byte) 0b11000001, 0x00, 0x00, 0x00, 0x20}; + + assertThatThrownBy(() -> SerializedMetadata.from(metadata)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("dictionary size"); + } + + @Test + public void testDictEntryEndBeforeStart() { + // metadata: [0x01 header] [0x02 dictSize] [0x00,0x05,0x03 offsets - entry 1 ends BEFORE + // it starts] ['A','B','C','D','E' data] + byte[] metadata = {0x01, 0x02, 0x00, 0x05, 0x03, 'A', 'B', 'C', 'D', 'E'}; + + SerializedMetadata parsed = SerializedMetadata.from(metadata); + + assertThatThrownBy(() -> parsed.get(1)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("dict entry"); + } + + @Test + public void testSharedFieldOffsetsInObjectSucceeds() { + // Shared offset is spec-legal (compacting writer dedups). + // metadata: [0x01 hdr] [0x02 dictSize] [0x00,0x01,0x02 offsets] ['a','b'] + byte[] metadata = {0x01, 0x02, 0x00, 0x01, 0x02, 'a', 'b'}; + // object: [0x02 hdr] [0x02 numEl] [0x00,0x01 fieldIds] [0x00,0x00,0x01 offsets] [0x00 NULL] + ByteBuffer value = + ByteBuffer.wrap(new byte[] {0x02, 0x02, 0x00, 0x01, 0x00, 0x00, 0x01, 0x00}) + .order(ByteOrder.LITTLE_ENDIAN); + + VariantValue top = VariantValue.from(SerializedMetadata.from(metadata), value); + VariantObject obj = top.asObject(); + + assertThat(obj.numFields()).isEqualTo(2); + assertThat(obj.get("a").type()).isEqualTo(PhysicalType.NULL); + assertThat(obj.get("b").type()).isEqualTo(PhysicalType.NULL); + } + + @Test + public void testEmptyChildValueBufferInArray() { + // array: [0b0011 header - small, offsetSize=1] [0x01 numElements] + // [0x00,0x00 offsets - offset[0]==offset[1] produces a zero-length child slice] + ByteBuffer value = + ByteBuffer.wrap(new byte[] {0b0011, 0x01, 0x00, 0x00}).order(ByteOrder.LITTLE_ENDIAN); + + VariantValue top = VariantValue.from(SerializedMetadata.from(EMPTY_METADATA), value); + + assertThatThrownBy(() -> top.asArray().get(0)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("empty value buffer"); + } + + @Test + public void testShortStringLengthExceedsBuffer() { + // short string: [0x1D header - length=7 in high 6 bits, basic type=SHORT_STRING] + // header declares length=7, but only the header byte is present. + ByteBuffer value = ByteBuffer.wrap(new byte[] {0x1D}).order(ByteOrder.LITTLE_ENDIAN); + + assertThatThrownBy(() -> VariantValue.from(SerializedMetadata.from(EMPTY_METADATA), value)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("length"); + } + + @Test + public void testNegativeObjectNumElements() { + // object: [0b1000010 header - large, fieldIdSize=1, offsetSize=1] + // [0xFFFFFFFF numElements as int32 = -1] + ByteBuffer value = + ByteBuffer.wrap( + new byte[] {(byte) 0b1000010, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF}) + .order(ByteOrder.LITTLE_ENDIAN); + + assertThatThrownBy(() -> VariantValue.from(SerializedMetadata.from(EMPTY_METADATA), value)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("negative element count"); + } + + @Test + public void testNegativeMetadataDictSize() { + // metadata: [0b11000001 header - v1, offsetSize=4] [0xFFFFFFFF dictSize as int32 = -1] + byte[] metadata = {(byte) 0b11000001, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF}; + + assertThatThrownBy(() -> SerializedMetadata.from(metadata)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("negative dictionary size"); + } + + @Test + public void testOversizedMetadataEndOffset() { + // metadata: [0x01 header - v1, offsetSize=1] [0x00 dictSize=0] + // [0xFF end offset=255 exceeds 3-byte buffer] + byte[] metadata = {0x01, 0x00, (byte) 0xFF}; + + assertThatThrownBy(() -> SerializedMetadata.from(metadata)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("end offset"); + } + + @Test + public void testTruncatedPrimitiveSizeField() { + // primitive: [0b1000000 header - STRING primitive] [0x00 only 1 byte, need 4-byte size field] + ByteBuffer value = + ByteBuffer.wrap(new byte[] {(byte) 0b1000000, 0x00}).order(ByteOrder.LITTLE_ENDIAN); + + assertThatThrownBy(() -> VariantValue.from(SerializedMetadata.from(EMPTY_METADATA), value)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("size field"); + } + + @Test + public void testOverdeepNestingRejected() { + int chainDepth = VariantUtil.MAX_VARIANT_DEPTH + 1; + byte[] bytes = buildNestedArrayChain(chainDepth); + ByteBuffer value = ByteBuffer.wrap(bytes).order(ByteOrder.LITTLE_ENDIAN); + + VariantValue top = VariantValue.from(SerializedMetadata.from(EMPTY_METADATA), value); + VariantArray cursor = top.asArray(); + for (int i = 0; i < VariantUtil.MAX_VARIANT_DEPTH; i += 1) { + cursor = cursor.get(0).asArray(); + } + + VariantArray finalCursor = cursor; + assertThatThrownBy(() -> finalCursor.get(0)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + String.format( + "Invalid variant: nesting depth %d exceeds maximum %d", + VariantUtil.MAX_VARIANT_DEPTH + 1, VariantUtil.MAX_VARIANT_DEPTH)); + } + + @Test + public void testMaxNestingAcceptedArray() { + // Array chain that reaches exactly depth MAX_VARIANT_DEPTH (the deepest allowed). + byte[] bytes = buildNestedArrayChain(VariantUtil.MAX_VARIANT_DEPTH); + ByteBuffer value = ByteBuffer.wrap(bytes).order(ByteOrder.LITTLE_ENDIAN); + + VariantValue top = VariantValue.from(SerializedMetadata.from(EMPTY_METADATA), value); + VariantArray cursor = top.asArray(); + for (int i = 0; i < VariantUtil.MAX_VARIANT_DEPTH - 1; i += 1) { + cursor = cursor.get(0).asArray(); + } + assertThat(cursor.get(0)).isNotNull(); + } + + @Test + public void testMaxNestingAcceptedObject() { + // Object chain that reaches exactly depth MAX_VARIANT_DEPTH. SerializedObject.get() + // routes through the same depth counter as SerializedArray.get() but was not covered. + byte[] metadata = {0x01, 0x01, 0x00, 0x01, 'a'}; + byte[] bytes = buildNestedObjectChain(VariantUtil.MAX_VARIANT_DEPTH); + ByteBuffer value = ByteBuffer.wrap(bytes).order(ByteOrder.LITTLE_ENDIAN); + + VariantValue top = VariantValue.from(SerializedMetadata.from(metadata), value); + VariantObject cursor = top.asObject(); + for (int i = 0; i < VariantUtil.MAX_VARIANT_DEPTH - 1; i += 1) { + cursor = cursor.get("a").asObject(); + } + assertThat(cursor.get("a")).isNotNull(); + } + + @Test + public void testOverdeepNestingRejectedObject() { + byte[] metadata = {0x01, 0x01, 0x00, 0x01, 'a'}; + byte[] bytes = buildNestedObjectChain(VariantUtil.MAX_VARIANT_DEPTH + 1); + ByteBuffer value = ByteBuffer.wrap(bytes).order(ByteOrder.LITTLE_ENDIAN); + + VariantValue top = VariantValue.from(SerializedMetadata.from(metadata), value); + VariantObject cursor = top.asObject(); + for (int i = 0; i < VariantUtil.MAX_VARIANT_DEPTH; i += 1) { + cursor = cursor.get("a").asObject(); + } + + VariantObject finalCursor = cursor; + assertThatThrownBy(() -> finalCursor.get("a")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + String.format( + "Invalid variant: nesting depth %d exceeds maximum %d", + VariantUtil.MAX_VARIANT_DEPTH + 1, VariantUtil.MAX_VARIANT_DEPTH)); + } + + @Test + public void testNegativeDepthRejected() { + // fromBuffer direct-call path; depth < 0 must be rejected before further parsing + ByteBuffer value = ByteBuffer.wrap(new byte[] {0x00}).order(ByteOrder.LITTLE_ENDIAN); + VariantMetadata metadata = SerializedMetadata.from(EMPTY_METADATA); + + assertThatThrownBy(() -> VariantUtil.fromBuffer(metadata, value, -1)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("negative depth"); + } + + @Test + public void testExcessiveArrayNumElementsRejected() { + // [0b10011 hdr - large array, offsetSize=1] [4-byte numElements = MAX_ELEMENTS + 1] + // offsetSize=1 keeps offsetTableEnd within the buffer, so only the absolute cap catches this + ByteBuffer value = ByteBuffer.allocate(5).order(ByteOrder.LITTLE_ENDIAN); + value.put((byte) 0b10011); + value.putInt(VariantUtil.MAX_ELEMENTS + 1); + value.flip(); + + assertThatThrownBy(() -> VariantValue.from(SerializedMetadata.from(EMPTY_METADATA), value)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("exceeds maximum"); + } + + @Test + public void testExcessiveObjectNumElementsRejected() { + // [0b1000010 hdr - large object, fieldIdSize=1, offsetSize=1] + // [4-byte numElements = MAX_ELEMENTS + 1] the absolute cap fires before the bounds check + ByteBuffer value = ByteBuffer.allocate(5).order(ByteOrder.LITTLE_ENDIAN); + value.put((byte) 0b1000010); + value.putInt(VariantUtil.MAX_ELEMENTS + 1); + value.flip(); + + assertThatThrownBy(() -> VariantValue.from(SerializedMetadata.from(EMPTY_METADATA), value)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("exceeds maximum"); + } + + @Test + public void testExcessiveMetadataDictSizeRejected() { + // metadata: [0b11000001 hdr - v1, offsetSize=4] [4-byte dictSize = MAX_ELEMENTS + 1] + // the absolute cap fires before the offset-table bounds check + ByteBuffer metadata = ByteBuffer.allocate(5).order(ByteOrder.LITTLE_ENDIAN); + metadata.put((byte) 0b11000001); + metadata.putInt(VariantUtil.MAX_ELEMENTS + 1); + metadata.flip(); + + assertThatThrownBy(() -> SerializedMetadata.from(metadata)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("exceeds maximum"); + } + + private static byte[] buildNestedArrayChain(int arrayLevels) { + final byte arrayHeader = 0b0111; // small array, offsetSize=2 + final int perLevel = 6; + final int leafSize = 1; + int total = perLevel * arrayLevels + leafSize; + byte[] buf = new byte[total]; + for (int i = 0; i < arrayLevels; i += 1) { + int pos = i * perLevel; + int innerLen = total - pos - perLevel; + buf[pos] = arrayHeader; + buf[pos + 1] = 0x01; + buf[pos + 2] = 0x00; + buf[pos + 3] = 0x00; + buf[pos + 4] = (byte) (innerLen & 0xFF); + buf[pos + 5] = (byte) ((innerLen >> 8) & 0xFF); + } + buf[total - 1] = 0x00; + return buf; + } + + private static byte[] buildNestedObjectChain(int objectLevels) { + // small object, fieldIdSize=1, offsetSize=2, one field with dict id 0 + final byte objectHeader = 0x06; + final int perLevel = 7; + final int leafSize = 1; + int total = perLevel * objectLevels + leafSize; + byte[] buf = new byte[total]; + for (int i = 0; i < objectLevels; i += 1) { + int pos = i * perLevel; + int innerLen = total - pos - perLevel; + buf[pos] = objectHeader; + buf[pos + 1] = 0x01; // numElements + buf[pos + 2] = 0x00; // fieldId = 0 + buf[pos + 3] = 0x00; // offset[0] low byte + buf[pos + 4] = 0x00; // offset[0] high byte + buf[pos + 5] = (byte) (innerLen & 0xFF); // offset[1] = innerLen (data length) + buf[pos + 6] = (byte) ((innerLen >> 8) & 0xFF); + } + buf[total - 1] = 0x00; // leaf NULL primitive + return buf; + } +} diff --git a/api/src/test/java/org/apache/iceberg/variants/TestSerializedArray.java b/api/src/test/java/org/apache/iceberg/variants/TestSerializedArray.java index 5d53c82a317f..67e923c11b78 100644 --- a/api/src/test/java/org/apache/iceberg/variants/TestSerializedArray.java +++ b/api/src/test/java/org/apache/iceberg/variants/TestSerializedArray.java @@ -22,6 +22,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.nio.ByteBuffer; +import java.nio.ByteOrder; import java.util.Random; import org.apache.iceberg.util.RandomUtil; import org.junit.jupiter.api.Test; @@ -55,7 +56,7 @@ public class TestSerializedArray { @Test public void testEmptyArray() { - SerializedArray array = SerializedArray.from(EMPTY_METADATA, new byte[] {0b0011, 0x00}); + SerializedArray array = SerializedArray.from(EMPTY_METADATA, new byte[] {0b0011, 0x00, 0x00}); assertThat(array.type()).isEqualTo(PhysicalType.ARRAY); assertThat(array.numElements()).isEqualTo(0); @@ -64,7 +65,7 @@ public void testEmptyArray() { @Test public void testEmptyLargeArray() { SerializedArray array = - SerializedArray.from(EMPTY_METADATA, new byte[] {0b10011, 0x00, 0x00, 0x00, 0x00}); + SerializedArray.from(EMPTY_METADATA, new byte[] {0b10011, 0x00, 0x00, 0x00, 0x00, 0x00}); assertThat(array.type()).isEqualTo(PhysicalType.ARRAY); assertThat(array.numElements()).isEqualTo(0); @@ -179,12 +180,25 @@ public void testMultiByteOffsets(int multiByteOffset) { @Test public void testLargeArraySize() { - SerializedArray array = - SerializedArray.from( - EMPTY_METADATA, new byte[] {0b10011, (byte) 0xFF, (byte) 0x01, 0x00, 0x00}); - + // largeSize array, offsetSize=2 (offsetSize=1 cannot represent offsets > 255), 511 NULL + // elements + int numElements = 511; + ByteBuffer buf = + ByteBuffer.allocate(1 + 4 + (numElements + 1) * 2 + numElements) + .order(ByteOrder.LITTLE_ENDIAN); + buf.put((byte) 0b10111); + buf.putInt(numElements); + for (int i = 0; i <= numElements; i += 1) { + buf.putShort((short) i); + } + for (int i = 0; i < numElements; i += 1) { + buf.put((byte) 0x00); // NULL primitive header + } + buf.flip(); + + SerializedArray array = SerializedArray.from(EMPTY_METADATA, buf, buf.get(0)); assertThat(array.type()).isEqualTo(PhysicalType.ARRAY); - assertThat(array.numElements()).isEqualTo(511); + assertThat(array.numElements()).isEqualTo(numElements); } @Test @@ -194,7 +208,7 @@ public void testNegativeArraySize() { SerializedArray.from( EMPTY_METADATA, new byte[] {0b10011, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF})) - .isInstanceOf(NegativeArraySizeException.class) - .hasMessage("-1"); + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("negative element count"); } } diff --git a/api/src/test/java/org/apache/iceberg/variants/TestSerializedMetadata.java b/api/src/test/java/org/apache/iceberg/variants/TestSerializedMetadata.java index 70b6a28b946d..3fdd28571741 100644 --- a/api/src/test/java/org/apache/iceberg/variants/TestSerializedMetadata.java +++ b/api/src/test/java/org/apache/iceberg/variants/TestSerializedMetadata.java @@ -235,20 +235,18 @@ public void testInvalidMetadataVersion() { } @Test - @SuppressWarnings("checkstyle:AssertThatThrownByWithMessageCheck") public void testMissingLength() { - // no check on the underlying error msg as it might be missing based on the JDK version assertThatThrownBy(() -> SerializedMetadata.from(new byte[] {0x01})) - .isInstanceOf(IndexOutOfBoundsException.class); + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("buffer too small"); } @Test - @SuppressWarnings("checkstyle:AssertThatThrownByWithMessageCheck") public void testLengthTooShort() { // missing the 4th length byte - // no check on the underlying error msg as it might be missing based on the JDK version assertThatThrownBy( () -> SerializedMetadata.from(new byte[] {(byte) 0b11010001, 0x00, 0x00, 0x00})) - .isInstanceOf(IndexOutOfBoundsException.class); + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("buffer too small"); } } diff --git a/api/src/test/java/org/apache/iceberg/variants/TestSerializedObject.java b/api/src/test/java/org/apache/iceberg/variants/TestSerializedObject.java index 07f7c9a2c5da..0cbd7ceaeede 100644 --- a/api/src/test/java/org/apache/iceberg/variants/TestSerializedObject.java +++ b/api/src/test/java/org/apache/iceberg/variants/TestSerializedObject.java @@ -68,7 +68,7 @@ public class TestSerializedObject { @Test public void testEmptyObject() { - SerializedObject object = SerializedObject.from(EMPTY_METADATA, new byte[] {0b10, 0x00}); + SerializedObject object = SerializedObject.from(EMPTY_METADATA, new byte[] {0b10, 0x00, 0x00}); assertThat(object.type()).isEqualTo(PhysicalType.OBJECT); assertThat(object.numFields()).isEqualTo(0); @@ -77,7 +77,7 @@ public void testEmptyObject() { @Test public void testEmptyLargeObject() { SerializedObject object = - SerializedObject.from(EMPTY_METADATA, new byte[] {0b1000010, 0x00, 0x00, 0x00, 0x00}); + SerializedObject.from(EMPTY_METADATA, new byte[] {0b1000010, 0x00, 0x00, 0x00, 0x00, 0x00}); assertThat(object.type()).isEqualTo(PhysicalType.OBJECT); assertThat(object.numFields()).isEqualTo(0);