FrankChen021 commented on code in PR #19818:
URL: https://github.com/apache/druid/pull/19818#discussion_r3686781995
##########
processing/src/main/java/org/apache/druid/segment/data/ImmutableRTreeObjectStrategy.java:
##########
@@ -65,7 +65,7 @@ public ImmutableRTree fromByteBuffer(ByteBuffer buffer, int
numBytes)
{
// always create the duplicate buffer for creating the objects as original
buffer may have mutations somewhere else which can corrupt objects
ByteBuffer duplicateBuf = buffer.duplicate();
- duplicateBuf.limit(duplicateBuf.position() + numBytes);
+ duplicateBuf.limit(Math.addExact(duplicateBuf.position(), numBytes));
return new ImmutableRTree(duplicateBuf, bitmapFactory);
Review Comment:
Addressed in ae6d91a223 with an explicit bounds check before setting the
duplicate buffer limit. Regression coverage now exercises negative and
oversized serialized lengths.
##########
processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java:
##########
@@ -115,7 +115,7 @@ public Class<ByteBuffer> getClazz()
public ByteBuffer fromByteBuffer(final ByteBuffer buffer, final int
numBytes)
{
final ByteBuffer dup = buffer.asReadOnlyBuffer();
- dup.limit(buffer.position() + numBytes);
+ dup.limit(Math.addExact(buffer.position(), numBytes));
return dup;
Review Comment:
Addressed in ae6d91a223 with an explicit nonnegative/remaining-bytes check
before changing the duplicate buffer limit. The new object-strategy bounds test
covers both invalid cases and verifies the source buffer is unchanged.
##########
multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java:
##########
@@ -976,15 +976,15 @@ public void workerWarning(List<MSQErrorReport>
errorReports)
{
// This check safeguards that the controller doesn't run out of memory.
Workers apply their own limiting to
// protect their own memory, and to conserve worker -> controller
bandwidth.
- long numReportsToAddCheck = Math.min(
+ final int numReportsToAddCheck = Math.min(
errorReports.size(),
- Limits.MAX_WORKERS * Limits.MAX_VERBOSE_WARNINGS -
workerWarnings.size()
+ Math.toIntExact(Limits.MAX_WORKERS * Limits.MAX_VERBOSE_WARNINGS -
workerWarnings.size())
);
Review Comment:
No code change is needed here: is declared as a , so Java binary numeric
promotion widens both multiplications to before is called. The overflow check
is therefore effective. Resolving as not applicable.
##########
processing/src/main/java/org/apache/druid/segment/data/CompressionStrategy.java:
##########
@@ -241,9 +241,10 @@ public static class UncompressedDecompressor implements
Decompressor
public void decompress(ByteBuffer in, int numBytes, ByteBuffer out)
{
final ByteBuffer copyBuffer = in.duplicate();
- copyBuffer.limit(copyBuffer.position() + numBytes);
+ final int newPosition = Math.addExact(copyBuffer.position(), numBytes);
+ copyBuffer.limit(newPosition);
out.put(copyBuffer).flip();
- in.position(in.position() + numBytes);
+ in.position(newPosition);
Review Comment:
Addressed in ae6d91a223 by validating against both input and output
remaining capacity before either buffer can be mutated. Added focused
regression coverage for negative and oversized lengths.
##########
processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongStringComplexMetricSerde.java:
##########
@@ -156,7 +156,7 @@ public SerializablePairLongString fromByteBuffer(ByteBuffer
buffer, int numBytes
{
ByteBuffer readOnlyByteBuffer =
buffer.asReadOnlyBuffer().order(buffer.order());
- readOnlyByteBuffer.limit(buffer.position() + numBytes);
+ readOnlyByteBuffer.limit(Math.addExact(buffer.position(), numBytes));
Review Comment:
Addressed in ae6d91a223 with an explicit nonnegative/remaining-bytes check
before setting the read-only buffer limit. Regression coverage includes
malformed negative and oversized lengths.
--
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]
