viirya opened a new pull request, #12179:
URL: https://github.com/apache/arrow-rs/pull/12179
# Which issue does this PR close?
- Closes #12178
# Rationale for this change
`ByteArrayDecoderPlain` can panic on corrupt PLAIN `BYTE_ARRAY` pages, and
both panics can be reached from `ParquetRecordBatchReader`.
- `skip` moves past the end of the page without an error. The next `read` on
that page then underflows `buf.len() - offset`.
- `read` divides by `max_remaining_values`, which is 0 on a v2 page whose
header claims every value is null.
The issue has repro steps.
# What changes are included in this PR?
- `ByteArrayDecoderPlain::skip` checks each value's end offset against the
page, using `checked_add`, and returns the same `ParquetError::EOF("eof
decoding byte array")` that `read` returns. On error the offset stays at the
start of the bad value.
- `ByteArrayDecoderPlain::read` returns `Ok(0)` when there is nothing to
read (`to_read == 0`), before it estimates the reservation. It already returned
`Ok(0)` for that case. The column reader compares the count with the number of
values the levels asked for, so a page that claims no values while the levels
want one now fails with "insufficient values read from column" instead of
panicking.
- `ByteViewArrayDecoderPlain::skip` has the identical loop and gets the same
check. It didn't panic, because the view `read` bounds-checks first, but on a
corrupt page `skip` returned `Ok`.
# Are these changes tested?
Yes.
- Unit tests cover:
- `skip` past the end of the page, for both plain decoders
- `read` with no remaining values
- a truncation table for `read` and `skip`: a value longer than the page,
a `u32::MAX` length, and a length prefix cut to 1-3 bytes. Each case checks for
`EOF`, that earlier values are kept, and that the offset stays at the bad value.
- Two reader tests in `arrow_reader/tests/column_reader.rs` patch bytes of a
written file and read it with `ParquetRecordBatchReader`. Both panicked before
this change.
# Are there any significant user-facing changes?
No.
---
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]
