fix: harden arithmetic and bounds checks

[FrankChen021]

Summary

This PR is intentionally limited to test changes related to the arithmetic and bounds-checking review.

• Align test arithmetic, loop/index types, and fixtures with the wider values used by the tests.
• Add and refine test-side validation and diagnostics for malformed serialized lengths.
• Remove obsolete test-only coverage that depends on production hardening not present on master.
• Restore JUnit discovery for the concurrency test.

No production/source files are changed in this PR. Production hardening and source-level bounds checks are intentionally out of scope and should be handled separately.

Validation

• Focused tests for the affected processing code pass.
• Targeted affected-module tests pass.
• git diff --check passes.

[Copilot]

Pull request overview

This PR hardens arithmetic and bounds handling across Druid’s core processing, MSQ, and several extensions by widening operands before multiplication/addition, using checked arithmetic (Math.*Exact, Ints.checkedCast, Math.toIntExact), and adding explicit range validations when interpreting serialized lengths and indexes.

Changes:

• Replaced overflow-prone arithmetic with checked arithmetic and widened intermediates (e.g., int → long) where size/offset computations are security-sensitive.
• Added explicit validations for serialized numBytes, row ranges, enum ordinals, and buffer bounds before slicing/allocating/looping.
• Added narrow CodeQL suppressions and comments where overflow/wrapping semantics are intentional (hashing, expression arithmetic, concise set cardinality).

Reviewed changes

Copilot reviewed 61 out of 61 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
server/src/test/java/org/apache/druid/segment/loading/StorageLocationTest.java	Use long loop index to match maxSize type in assertions.
processing/src/test/java/org/apache/druid/segment/serde/HyperUniquesSerdeForTest.java	Add explicit numBytes vs buffer-remaining validation in test serde.
processing/src/test/java/org/apache/druid/segment/data/V3CompressedVSizeColumnarMultiIntsSerializerTest.java	Convert numRows safely to int and iterate using the narrowed value.
processing/src/test/java/org/apache/druid/segment/data/SafeWritableMemoryTest.java	Widen multiplications/loop indices and avoid narrowing capacity computations.
processing/src/test/java/org/apache/druid/query/scan/ScanQueryRunnerFactoryTest.java	Align comparisons/loops to long scan-row limit.
processing/src/test/java/org/apache/druid/query/aggregation/SerializablePairLongStringBufferStoreTest.java	Remove unused narrowing accumulator.
processing/src/test/java/org/apache/druid/java/util/common/CompressionUtilsTest.java	Widen length * 3 expectations to long to avoid overflow.
processing/src/test/java/org/apache/druid/collections/bitmap/BitmapOperationAgainstConsecutiveRunsTest.java	Safely narrow minIntersection for loop bounds.
processing/src/main/java/org/apache/druid/timeline/SegmentId.java	Document intentional hash overflow and add targeted CodeQL suppression.
processing/src/main/java/org/apache/druid/segment/StringDimensionIndexer.java	Widen multiplication to long before assigning to long.
processing/src/main/java/org/apache/druid/segment/nested/VariantColumn.java	Use Math.addExact for dictionary id offset computations.
processing/src/main/java/org/apache/druid/segment/nested/NestedPathArrayElement.java	Harden negative indexing logic with widened arithmetic and bounds checks.
processing/src/main/java/org/apache/druid/segment/data/VSizeLongSerde.java	Avoid byte overflow by using int accumulator for bit counting.
processing/src/main/java/org/apache/druid/segment/data/SafeWritableMemory.java	Avoid int loop overflow by checked-casting lengthBytes once.
processing/src/main/java/org/apache/druid/segment/data/ImmutableRTreeObjectStrategy.java	Use checked addition when setting duplicate buffer limit.
processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java	Use checked addition when slicing returned ByteBuffer strategy.
processing/src/main/java/org/apache/druid/segment/data/FixedIndexed.java	Add width/size validation and use checked arithmetic for offsets and positioning.
processing/src/main/java/org/apache/druid/segment/data/CompressionStrategy.java	Use checked arithmetic when advancing positions/limits in uncompressed decompressor.
processing/src/main/java/org/apache/druid/segment/data/CompressedColumnarIntsSupplier.java	Validate constructor inputs and use checked subtraction.
processing/src/main/java/org/apache/druid/segment/data/CompressedBlockReader.java	Validate block counts/sizes and use checked arithmetic for offsets/limits.
processing/src/main/java/org/apache/druid/query/rowsandcols/RearrangedRowsAndColumns.java	Strengthen row-range validation and use checked subtraction for row count.
processing/src/main/java/org/apache/druid/query/rowsandcols/LimitedRowsAndColumns.java	Validate row range (start/end) and use checked subtraction for row count.
processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java	Validate offsets against list size and use checked subtraction for row count.
processing/src/main/java/org/apache/druid/query/groupby/orderby/DefaultLimitSpec.java	Use Math.addExact with overflow-to-ISE translation for limit+offset.
processing/src/main/java/org/apache/druid/query/cache/CacheKeyBuilder.java	Use checked addition for byte-size calculation.
processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongStringComplexMetricSerde.java	Use checked addition when setting read-only buffer limit.
processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongLongComplexMetricSerde.java	Use checked addition when setting read-only buffer limit.
processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongFloatComplexMetricSerde.java	Use checked addition when setting read-only buffer limit.
processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongDoubleComplexMetricSerde.java	Use checked addition when setting read-only buffer limit.
processing/src/main/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesSerde.java	Use checked addition when setting read-only buffer limit.
processing/src/main/java/org/apache/druid/math/expr/BinaryMathOperatorExpr.java	Document intentional wrapping/IEEE semantics and add targeted CodeQL suppressions.
processing/src/main/java/org/apache/druid/java/util/http/client/io/AppendableByteArrayInputStream.java	Use checked arithmetic and checked casts when updating counters/indexes.
processing/src/main/java/org/apache/druid/java/util/common/io/smoosh/FileSmoosher.java	Checked-cast bytes written and use checked addition for offsets.
processing/src/main/java/org/apache/druid/java/util/common/granularity/PeriodGranularity.java	Use checked add/subtract when computing truncate offsets.
processing/src/main/java/org/apache/druid/frame/read/FrameReaderUtils.java	Use checked addition before computing region offsets for ByteBuffer slicing.
processing/src/main/java/org/apache/druid/frame/read/columnar/StringFrameColumnReader.java	Use checked arithmetic for string offset calculations and lengths.
processing/src/main/java/org/apache/druid/frame/read/columnar/ComplexFrameColumnReader.java	Use checked arithmetic for payload bounds and offset calculations.
processing/src/main/java/org/apache/druid/frame/key/FrameComparisonWidgetImpl.java	Narrow key-length computation to int safely and use checked allocation sizing.
processing/src/main/java/org/apache/druid/extendedset/intset/ConciseSet.java	Document intentional arithmetic semantics for size/cardinality paths with CodeQL suppressions.
processing/src/main/java/org/apache/druid/crypto/CryptoService.java	Use checked addition when computing encrypted payload buffer allocation size.
multi-stage-query/src/main/java/org/apache/druid/msq/statistics/QuantilesSketchKeyCollector.java	Use long loop index to avoid int overflow in weighted updates.
multi-stage-query/src/main/java/org/apache/druid/msq/statistics/ClusterByStatisticsCollectorImpl.java	Change retained-bytes tracking from double to long.
multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java	Narrow report-add counts to int and add checked conversions.
indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorStateManagerTest.java	Widen arithmetic in assertion to avoid overflow.
extensions-core/stats/src/main/java/org/apache/druid/query/aggregation/variance/VarianceSerde.java	Validate numBytes and use checked addition when setting buffer limit.
extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3TaskLogsTest.java	Avoid overflow-prone negation by using 1L - length expression.
extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/output/RetryableS3OutputStreamTest.java	Widen byte-count arithmetic to long in assertions.
extensions-core/lookups-cached-global/src/test/java/org/apache/druid/data/input/MapPopulatorTest.java	Widen string length * bytes arithmetic to long in assertion.
extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramSerde.java	Validate numBytes and use checked addition when setting buffer limit.
extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogram.java	Replace ordinal indexing with explicit byte→enum decoding + validation.
extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogramFoldingSerde.java	Validate numBytes and use checked addition when setting buffer limit.
extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java	Use long loop indices for bin repeats and avoid byte overflow on negation.
extensions-core/google-extensions/src/test/java/org/apache/druid/data/input/google/GoogleCloudStorageInputSourceTest.java	Widen processed-bytes expectations to long.
extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/TDigestSketchObjectStrategy.java	Validate numBytes and use checked addition for buffer limit.
extensions-contrib/spectator-histogram/src/test/java/org/apache/druid/spectator/histogram/SpectatorHistogramTest.java	Widen expected serialized-size arithmetic to long.
extensions-contrib/momentsketch/src/main/java/org/apache/druid/query/aggregation/momentsketch/MomentSketchObjectStrategy.java	Validate numBytes and use checked addition for buffer limit.
extensions-contrib/ddsketch/src/main/java/org/apache/druid/query/aggregation/ddsketch/DDSketchObjectStrategy.java	Validate numBytes, use checked addition, and make locals final.
extensions-contrib/compressed-bigdecimal/src/main/java/org/apache/druid/compressedbigdecimal/ByteBufferCompressedBigDecimal.java	Centralize index→buffer-position math with bounds checking and checked arithmetic.
extensions-contrib/compressed-bigdecimal/src/main/java/org/apache/druid/compressedbigdecimal/aggregator/CompressedBigDecimalAggregatorFactoryBase.java	Make locals final and use checked addition for cache-key sizing.
extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/storage/aliyun/OssTaskLogsTest.java	Avoid overflow-prone negation by using 1L - length expression.
extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/data/input/aliyun/OssInputSourceTest.java	Widen processed-bytes expectations to long.

Comments suppressed due to low confidence (1)

multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java:988

• Same overflow issue as above: Limits.MAX_WORKERS * Limits.MAX_VERBOSE_WARNINGS - workerWarnings.size() is evaluated as int before toIntExact, so it can wrap. Cast to long before multiplying/subtracting.

        final int numReportsToAdd = Math.min(
            errorReports.size(),
            Math.toIntExact(Limits.MAX_WORKERS * Limits.MAX_VERBOSE_WARNINGS - workerWarnings.size())
        );

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

processing/src/main/java/org/apache/druid/segment/data/CompressedBlockReader.java:90

• compressedSize is derived from the offsets table, but it isn't validated against the remaining bytes in buffer before advancing buffer.position(...). If compressedSize is larger than the post-offset remaining data, compressedData.limit(...) will throw after the caller’s buffer has already been partially consumed (advanced by offsetsSize), which makes error handling brittle. Add an explicit compressedSize <= buffer.remaining() - offsetsSize check before mutating buffer.position(...) so invalid serialized data fails fast without partially consuming the source buffer.

[FrankChen021]

Addressed Copilot’s suppressed CompressedBlockReader finding in commit 2fc3590. The parser now verifies both the offsets table and declared compressed payload fit in the remaining buffer before advancing into either region. A regression test confirms an oversized payload is rejected without advancing past the offsets position. The targeted test and full processing Checkstyle/PMD/enforcer validation pass.

[FrankChen021]

I have reviewed the code for correctness, edge cases, concurrency, and integration risks; no issues found.

Reviewed 64 of 64 changed files.

---

This is an automated review by Codex GPT-5.6-Sol

[Copilot]

Pull request overview

Copilot reviewed 64 out of 64 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

processing/src/test/java/org/apache/druid/segment/serde/HyperUniquesSerdeForTest.java:1

• The exception message is inaccurate for the numBytes < 0 case and doesn’t include any useful context (requested vs remaining). Consider updating it to reflect both invalid conditions and include the values (e.g., numBytes and remaining) to aid debugging.
  extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogram.java:1083
• Hardcoding numeric values duplicates the enum mapping logic and requires manual updates if OutlierHandlingMode adds new values or changes serialization rules. If the serialized value is intended to be the enum ordinal, a more maintainable approach is to bounds-check and index into OutlierHandlingMode.values(); if it’s intentionally a stable wire-value mapping, consider documenting that contract (and possibly defining explicit constants) to prevent accidental divergence.

  private static OutlierHandlingMode readOutlierHandlingMode(final ByteBuffer buf)
  {
    final byte mode = buf.get();
    switch (mode) {
      case 0:
        return OutlierHandlingMode.IGNORE;
      case 1:
        return OutlierHandlingMode.OVERFLOW;
      case 2:
        return OutlierHandlingMode.CLIP;
      default:
        throw new ISE("Invalid outlier handling mode [%s].", mode);
    }
  }

[FrankChen021]

Severity	Findings
P0	0
P1	0
P2	1
P3	0
Total	1

Reviewed 63 of 63 changed files.

Validation: reverse git apply --check against the canonical diff passed. Builds and tests were not run.

---

This is an automated review by Codex GPT-5.6-Luna(max)

[FrankChen021]

Severity	Findings
P0	0
P1	3
P2	3
P3	0
Total	6

Reviewed 16 of 16 changed files in the current PR comparison. The incremental merge delta was inspected separately for regressions against the previous reviewed head.

Findings that could not be attached inline:

• processing/src/main/java/org/apache/druid/segment/data/CompressedBlockReader.java:93 - [P1] Restore compressed-payload bounds validation. The merge result removed the compressedSize >= 0 and compressedSize <= buffer.remaining() - offsetsSize checks that existed at the previously reviewed head. Because compressedSize comes from the serialized offsets table, malformed segment data can reach this limit call with an out-of-range value after buffer.position(...) has already advanced, leaving the source buffer partially consumed when parsing fails. Restore the validation before mutating buffer, together with the regression coverage.
• processing/src/main/java/org/apache/druid/segment/data/CompressionStrategy.java:244 - [P1] Restore uncompressed-length validation. The merge result removed the explicit checks for negative or oversized numBytes and the checked position calculation that were added at the previously reviewed head. Serialized lengths can therefore still flow into unchecked position + numBytes, limit, and out.put operations, so this updated head no longer hardens the uncompressed decompressor as intended. Restore the guard (and its invalid-length regression test) when resolving the merge.
• processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java:118 - [P1] Restore serialized-object bounds checks. The merge result removed the nonnegative/remaining-bytes check and checked position calculation from UTF8_STRATEGY.fromByteBuffer; the same loss is present in ImmutableRTreeObjectStrategy, HyperUniquesSerde, and the SerializablePair long-* serdes. These numBytes values are read from serialized data, so the current head again performs unchecked buffer slicing instead of rejecting malformed lengths. Restore the guards across these strategies.
• processing/src/main/java/org/apache/druid/java/util/http/client/io/AppendableByteArrayInputStream.java:176 - [P2] Keep long skip accounting. This merge regresses the prior review's skip-accounting fix: numPulled is a long, but available is an int and available -= numPulled silently narrows the result. A skip spanning more than Integer.MAX_VALUE bytes can therefore wrap available() after consuming the data. Keep the accounting checked or represent the available count with a wide type.
• processing/src/test/java/org/apache/druid/segment/data/ObjectStrategyBoundsTest.java:1 - [P2] Retain the object-strategy regression test. The incremental merge diff deletes the regression test added at the previously reviewed head, which exercised seven object strategies with negative and oversized serialized lengths and verified that the source buffer was unchanged. Since the corresponding production guards are also absent from the current tree, retaining equivalent coverage is needed to prevent this bounds-hardening regression.

---

This is an automated review by Codex GPT-5.6-Luna(max)