dfengliu opened a new pull request, #20174:
URL: https://github.com/apache/druid/pull/20174
### Description
`CompressedVSizeColumnarIntsSupplier.fromByteBuffer()` trusts the `numBytes`
byte in the serialized column header. Writers only ever emit values 1..4
(`VSizeColumnarInts.getNumBytesForMax()`), but the read path never verifies it:
the byte directly drives the shift/mask constants (`bigEndianShift`,
`littleEndianMask`) and the per-value absolute reads
(`buffer.getInt(bufferIndex * numBytes)`).
A segment carrying any other value reads back silently corrupted data. Using
a serialized supplier with the `numBytes` byte tampered to 100, `get(0)`
returns 50462976 instead of 0 and `get(4)` returns 117835012 instead of 4, with
no error raised. With `numBytes=0` every value reads as 0. This reader backs
every dictionary-encoded column, multi-value columns, and nested variant
columns, so malformed storage can silently poison query results. JVM bounds
checks contain reads to the decompressed buffer, so this is an
integrity/robustness issue rather than a memory-safety one.
#### Fixed the bug ...
Validate `numBytes` in the `CompressedVSizeColumnarIntsSupplier` constructor
with `Preconditions.checkArgument` (range `[1, Integer.BYTES]`), so malformed
headers fail fast with a descriptive message on both the read
(`fromByteBuffer`) and construction (`fromList`) paths, before any shift/mask
constants or offsets are computed.
Added `testInvalidNumBytesRejected` to
`CompressedVSizeColumnarIntsSupplierTest`: it serializes a real supplier,
tampers the `numBytes` header byte with 0, 5, 100 and -1, and asserts
rejection. I verified the test fails without the fix and passes with it; the
control round-trip still reads all values correctly.
#### Release note
- Fix a potential silent data corruption when reading a column whose
serialized header carries an invalid `numBytes` value. Druid now rejects such
columns with a descriptive `IllegalArgumentException` instead of returning
wrong values.
<hr>
##### Key changed/added classes in this PR
* `CompressedVSizeColumnarIntsSupplier`
* `CompressedVSizeColumnarIntsSupplierTest`
<hr>
This PR has:
- [x] been self-reviewed.
- [ ] added documentation for new or modified features or behaviors.
- [x] a release note entry in the PR description.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked
related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in
[licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [ ] added comments explaining the "why" and the intent of the code
wherever would not be obvious for an unfamiliar reader.
- [x] added unit tests or modified existing tests to cover new code paths,
ensuring the threshold for [code
coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md)
is met.
- [ ] added integration tests.
- [ ] been tested in a test Druid cluster.
--
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]
