viirya opened a new pull request, #59087: URL: https://github.com/apache/spark/pull/59087
### What changes were proposed in this pull request? Similar to SPARK-59352, this validates lengths read from the file in two more vectorized Parquet readers, mainly on their skip paths. `VectorizedPlainValuesReader`: - Add `readLength()`, which rejects a negative binary length, and use it in `readBinary`, `skipBinary` and `readGeoData`. - Add `skipFully(long)`, which throws a `ParquetDecodingException` when the page has fewer bytes than requested, and use it in `skipBinary` and in all fixed-width skips (`skipIntegers`, `skipLongs`, `skipFloats`, `skipDoubles`, `skipBytes`, `skipShorts`, `skipFixedLenByteArray`). The fixed-width skips still do a single call per batch. `VectorizedDeltaLengthByteArrayReader`: - Add `checkLength(int)`, which rejects a negative value length, and use it in `readBinary`, `readGeoData` and `getBytes` (`getBytes` is also how the DELTA_BYTE_ARRAY reader reads suffixes). - Rewrite `skipBinary` to validate and sum the lengths of the batch, then skip them with a single `skipFully`, replacing the `remaining -= in.skip(remaining)` loop. ### Why are the changes needed? The skip paths are used when column-index filtering produces row ranges, and they did not validate what they read from the file, while the read paths mostly did. In Parquet 1.18.1, `SingleBufferInputStream.skip(n)` (what Spark gets for a data page) returns -1 at the end of the buffer, skips fewer bytes when the buffer is short, and moves the position backwards for a negative `n`. As a result, on a corrupt page: - `VectorizedDeltaLengthByteArrayReader.skipBinary` with a length past the end of the page: `in.skip` returns -1, so `remaining` grows until the int overflows (about 2^31 iterations per value), then the read silently continues. A negative length is silently skipped. - `VectorizedPlainValuesReader.skipBinary` with a negative length moves the stream position backwards, so the following values in the page are decoded from the wrong offset and the query returns wrong results without an error. For example, for a page `[4]"abcd"[-12]"zzzz"`, alternating read and skip returns `"abcd"` repeatedly. - The fixed-width skips and `skipBinary` in the PLAIN reader silently skip fewer bytes than requested when the page is too short. - On the read paths, a negative length was not rejected either. Slicing a negative length also moves the position backwards, or fails with an `ArrayIndexOutOfBoundsException` instead of a `ParquetDecodingException`. Note that `ByteBufferInputStream.skipFully` alone does not reject a negative `n` on `SingleBufferInputStream` (`skip(n)` returns `n`, so the `skipped < n` check passes), so negative lengths need an explicit check. ### Does this PR introduce _any_ user-facing change? Only for corrupt Parquet files: reading a page with an invalid value length or too few bytes for the values being skipped now fails with a `ParquetDecodingException` instead of silently returning wrong values or spinning. Valid files are not affected. ### How was this patch tested? New unit tests: - `VectorizedPlainValuesReaderSuite` (new): negative binary length on read and skip, binary skip past the end of the page, every fixed-width skip past the end of the page, and skips within the page landing on the next value. - `ParquetDeltaLengthByteArrayEncodingSuite`: skip past the end of the page, and negative value length on `readBinary`, `skipBinary` and `getBytes`. All the new negative tests fail without this change. Existing suites also pass: `ParquetDeltaByteArrayEncodingSuite`, `ParquetEncodingSuite`, `ParquetVectorizedSuite`, `ParquetColumnIndexSuite`, `ParquetVectorUpdaterSuite`, `ParquetIOSuite`, `ParquetDeltaEncodingInteger`, `ParquetDeltaEncodingLong` and `ParquetByteStreamSplitEncoding*`. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5.5) This pull request and its description were written by Isaac. -- 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]
