dfengliu commented on code in PR #20315:
URL: https://github.com/apache/druid/pull/20315#discussion_r3987667981
##########
processing/src/main/java/org/apache/druid/segment/data/FixedIndexed.java:
##########
@@ -63,6 +63,14 @@ public static <T> Supplier<FixedIndexed<T>> read(ByteBuffer
bb, TypeStrategy<T>
final boolean isSorted = (flags & IS_SORTED_MASK) == IS_SORTED_MASK ? true
: false;
Preconditions.checkState(!(hasNull && !isSorted), "cannot have null values
if not sorted");
final int size = buffer.getInt() + (hasNull ? 1 : 0);
+ Preconditions.checkArgument(size >= 0, "size[%s] must be non-negative",
size);
+ final int valuesCount = hasNull ? size - 1 : size;
Review Comment:
@FrankChen021 you are right. When the null flag is set the serialized count
is incremented before validation, so a raw count of `-1` produced `size == 0` /
`valuesCount == -1`: it passed both checks and moved the buffer position
backwards. Fixed in 9c7e0983 — the check now validates `valuesCount` (the
post-adjustment derived value), and I added
`testNegativeCountWithNullFlagRejected` covering the null-flag case. I verified
the new test fails without the fix (reproducing `newPosition < 0`) and passes
with it.
--
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]
