NopeCharon opened a new pull request, #3888: URL: https://github.com/apache/parquet-java/pull/3888
### Rationale for this change A run header that encodes zero values is not valid RLE/bit-packing hybrid data, but `RunLengthBitPackingHybridDecoder` accepts it: - **RLE run with repeat count 0:** `readInt()` decrements `currentCount` from 0 to -1 and returns the run's value. The count never reaches 0 again, so every later value on the page is that same value and the rest of the stream is never read. A corrupt page turns into silently wrong data. - **Bit-packed run with 0 groups:** `readInt()` reads from an empty buffer and fails with an `ArrayIndexOutOfBoundsException`. - **`skipInts()`:** it simply moves past such runs, so the same page can decode differently depending on whether values are read or skipped. ### What changes are included in this PR? `readNext()` now throws a `ParquetDecodingException` naming the invalid run when a header has a repeat count or group count of 0. This matches the Arrow C++ decoder, which also rejects both cases as an "illegal number of encoded values". parquet-java's own encoder never writes zero-length runs: RLE runs are only written for at least 8 repeats, and a bit-packed header is only finalised after at least one group. So files that are valid today are unaffected. ### Are these changes tested? Yes. The new `TestRunLengthBitPackingHybridDecoder` builds streams by hand that start with a zero-length run followed by a valid run. It checks that `readInt()` rejects a zero-length RLE run and a zero-group bit-packed run, and that `skipInts()` rejects a zero-length RLE run. All three tests fail without the fix: two cases return or skip silently, and the bit-packed case fails with the `ArrayIndexOutOfBoundsException`. With the fix they pass. Other checks: - `./mvnw verify -pl parquet-column` passes: 720 tests, Spotless, RAT and japicmp. - In parquet-hadoop, these pass: the read/write suites (`TestParquetWriter`, `TestParquetReader`, `TestColumnIndexFiltering`, `TestParquetFileWriter`, ...) and the interop tests that read `parquet-testing` files (`TestInterOp*`, `TestInterop*`, `TestEncryptionOptions`, `TestReadInvalidTypeCombination`). ### Are there any user-facing changes? Yes. Reading a page that contains a zero-length run now fails with a `ParquetDecodingException` that describes the problem, instead of returning wrong values or failing with an `ArrayIndexOutOfBoundsException`. There are no API changes. Closes #2148 -- 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]
