azecevic000 opened a new pull request, #11658:
URL: https://github.com/apache/arrow-rs/pull/11658
# Which issue does this PR close?
- Relates to #7124 and #11044.
# Rationale for this change
#11044 made validated IPC reads return an error, instead of panicking, when
a validity buffer is too short. Its check is in `ArrayDataBuilder::build`. The
IPC reader does not build struct arrays with `ArrayDataBuilder`: when a struct
node has `null_count > 0`, `create_struct_array` calls
`BooleanBuffer::new(null_buffer, 0, len)` directly. A struct validity buffer
shorter than `ceil(len / 8)` bytes therefore still panics with "buffer not
large enough", even with validation enabled. Structs with and without fields
are both affected, because the null buffer is built before the zero-field
branch.
# What changes are included in this PR?
- `create_struct_array` checks the validity buffer length before it builds
the `BooleanBuffer`, and returns the same error as #11044 (`null_bit_buffer
size too small. got {} needed {}`). IPC field nodes have no offset, so the
needed length is `len.div_ceil(8)`.
- The check runs only when validation is enabled; the `with_skip_validation`
path is unchanged. `arrow-ipc` has no `force_validate` feature, so unlike
#11044 there is no condition on it.
- A regression test next to the #11044 one: it writes a `StructArray` with a
short validity buffer (built with `StructArray::new_unchecked`) with
`StreamWriter` and reads it back with `StreamReader`.
An alternative is to build struct arrays with `ArrayDataBuilder` and reuse
the #11044 check. I did not do that because the builder only rejects children
shorter than the struct (longer ones are sliced), while `StructArray::try_new`
rejects any length mismatch. It would also change existing error messages
(`test_invalid_struct_array_ipc_read_errors` would fail) and add per-child
conversion work to the struct decode path, which #8127 and #10206 optimized.
# Are these changes tested?
Yes. `test_stream_reader_rejects_short_struct_validity_buffer` panics with
"buffer not large enough" without the fix and passes with it.
# Are there any significant user-facing changes?
No.
If possible, it would be good to include this and #11044 in 60.1.0 (#10525).
#11044 is not in 60.0.0.
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]
