This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-11944-44f6371167979c83d634fbe04f6a5c60d79a4bbb in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
commit 7168fa813aea5a0d9b4ba4115ed30c726376609c Author: vntoinekaio <[email protected]> AuthorDate: Fri Oct 30 19:23:48 2026 +0000 Accept buffers with trailing bytes in `ArrayData` validation instead of panicking (#11944) # Which issue does this PR close? - Closes #11943. # Rationale for this change `validate` only requires a minimum size for fixed-width buffers, but `typed_buffer` typed the whole buffer with `Buffer::typed_data`, which asserts that the length is a multiple of the element width. A buffer with a few trailing bytes, for example from a corrupted IPC message, therefore panicked during validation. # What changes are included in this PR? `typed_buffer` types only the `required_len` bytes it reads, and returns an error instead of asserting when they are misaligned. `validate` already checks alignment before reaching `typed_buffer`, so that error is for `validate_values` called on unchecked data. # Are these changes tested? Yes: an `ArrayData` with a 14-byte `Utf8` offsets buffer now validates, and the IPC reader reads a hand-built batch with such a buffer. Both tests panic without the change. # Are there any significant user-facing changes? No. AI usage: the bug was found and reproduced with Claude Code, which also drafted the change, the tests and this description. I reviewed the change, ran both tests with and without the fix, and checked it against the original reproduction: the two byte positions that panicked are read without error after the change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <[email protected]> --- arrow-data/src/data.rs | 31 ++++++++++++++++++++++++++++++- arrow-ipc/src/reader.rs | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/arrow-data/src/data.rs b/arrow-data/src/data.rs index c8063c6cc5..3ccaa9101f 100644 --- a/arrow-data/src/data.rs +++ b/arrow-data/src/data.rs @@ -1139,7 +1139,21 @@ impl ArrayData { ))); } - Ok(&buffer.typed_data::<T>()[self.offset..required_elements]) + // Only the bytes in use are typed: the buffer may be longer than + // `required_len` by a number of bytes that is not a multiple of + // `byte_width`, which `Buffer::typed_data` would assert on. + let bytes = &buffer.as_slice()[..required_len]; + // SAFETY: ArrowNativeType is trivially transmutable and sealed (as in + // `Buffer::typed_data`); alignment is checked below + let (prefix, values, suffix) = unsafe { bytes.align_to::<T>() }; + if !prefix.is_empty() || !suffix.is_empty() { + return Err(ArrowError::InvalidArgumentError(format!( + "Buffer {idx} of {} is not aligned to {} bytes", + self.data_type, + mem::align_of::<T>() + ))); + } + Ok(&values[self.offset..]) } /// Does a cheap sanity check that the `self.len` values in `buffer` are valid @@ -3813,6 +3827,21 @@ mod tests { ); } + /// `validate` only requires a minimum buffer size, so an offsets buffer + /// may carry trailing bytes that are not a whole number of offsets. + #[test] + fn test_validate_offsets_buffer_with_trailing_bytes() { + let mut offsets = Buffer::from_slice_ref([0_i32, 2, 5]).as_slice().to_vec(); + offsets.extend_from_slice(&[0, 0]); + let data = ArrayData::builder(DataType::Utf8) + .len(2) + .add_buffer(Buffer::from_slice_ref(&offsets)) + .add_buffer(Buffer::from_slice_ref(b"hello")) + .build() + .unwrap(); + data.validate_full().unwrap(); + } + #[test] fn should_fail_validation_when_having_map_field_type_is_not_struct() { let map_field = Field::new("key", DataType::Int32, false); diff --git a/arrow-ipc/src/reader.rs b/arrow-ipc/src/reader.rs index 9a2ce3d720..3701a685fc 100644 --- a/arrow-ipc/src/reader.rs +++ b/arrow-ipc/src/reader.rs @@ -2405,6 +2405,52 @@ mod tests { } } + #[test] + fn test_offsets_buffer_with_trailing_bytes() { + use crate::r#gen::Message::*; + use flatbuffers::FlatBufferBuilder; + + let schema = Arc::new(Schema::new(vec![Field::new("col", DataType::Utf8, false)])); + + // The offsets buffer is 14 bytes: three i32 offsets and two trailing + // bytes, which is more than the array needs but not a whole number + // of offsets. + let mut fbb = FlatBufferBuilder::new(); + let nodes = fbb.create_vector(&[FieldNode::new(2, 0)]); + let buffers = fbb.create_vector(&[ + crate::Buffer::new(0, 0), + crate::Buffer::new(0, 14), + crate::Buffer::new(16, 5), + ]); + let batch_offset = RecordBatch::create( + &mut fbb, + &RecordBatchArgs { + length: 2, + nodes: Some(nodes), + buffers: Some(buffers), + compression: None, + variadicBufferCounts: None, + }, + ); + fbb.finish_minimal(batch_offset); + let batch_bytes = fbb.finished_data().to_vec(); + let batch = flatbuffers::root::<RecordBatch>(&batch_bytes).unwrap(); + + let mut body = Buffer::from_slice_ref([0_i32, 2, 5]).as_slice().to_vec(); + body.extend_from_slice(&[0; 4]); + body.extend_from_slice(b"hello\0\0\0"); + let data_buffer = Buffer::from_slice_ref(&body); + let dictionaries: HashMap<i64, ArrayRef> = HashMap::new(); + let metadata = MetadataVersion::V5; + + let decoder = + RecordBatchDecoder::try_new(&data_buffer, batch, schema, &dictionaries, &metadata) + .unwrap(); + let batch = decoder.read_record_batch().unwrap(); + let col = batch.column(0).as_any().downcast_ref::<StringArray>(); + assert_eq!(col.unwrap(), &StringArray::from(vec!["he", "llo"])); + } + /// A `Utf8View` batch whose variadic buffer count is `count`, with two buffers /// in the message. Returns the error from reading it with the given projection. fn read_batch_with_variadic_count(count: i64, projection: Option<&[usize]>) -> ArrowError {
