nssalian commented on code in PR #16568:
URL: https://github.com/apache/iceberg/pull/16568#discussion_r3555502542
##########
api/src/main/java/org/apache/iceberg/variants/SerializedObject.java:
##########
@@ -96,11 +116,26 @@ 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<Integer> sortedOffsets =
offsetToLength.keySet().stream().sorted().collect(Collectors.toList());
+ Preconditions.checkArgument(
+ sortedOffsets.size() == numElements + 1, "Invalid variant object:
duplicate field offsets");
Review Comment:
Great catch. I dropped the uniqueness assertion and switched to a
sorted-offset pass that tolerates equal offsets (zero-length spans). The
per-field bounds check just above still guarantees every offset lands in `[0,
dataLength]`, which is what the spec requires.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
