FrankChen021 commented on code in PR #20447:
URL: https://github.com/apache/druid/pull/20447#discussion_r4133377895
##########
processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java:
##########
@@ -602,6 +618,14 @@ public T get(final int index)
ByteBuffer bufferedIndexedGetByteBuffer(ByteBuffer copyValueBuffer, int
startOffset, int endOffset)
{
int size = endOffset - startOffset;
+ if (startOffset < Integer.BYTES || endOffset < startOffset || endOffset
> copyValueBuffer.limit()) {
Review Comment:
[P1] Validate against a stable bound before reusing the buffer
**Finding:** The limit of copyValueBuffer is mutable: this method sets it to
endOffset, and the UTF8_STRATEGY branch of BufferIndexed.indexOf calls
getByteBuffer directly without restoring it. Consequently, a valid sorted
dictionary containing a, b, c throws while searching for c: the first probe
reads b and lowers the limit to its end, then the next probe for c fails this
new check. StringEncodingStrategies supplies exactly this singleThreaded UTF-8
view for ordinary string dictionaries, so valid dictionary lookups and queries
can fail; the shared helper affects both V1 and V2. The added tests only
exercise the non-buffered get path and miss this regression.
**Suggestion:** Validate against a stable boundary captured from the
original value region rather than the reused buffer's current limit, and add
singleThreaded UTF-8 tests covering rightward binary-search probes and
successive lookups.
--
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]
