This is an automated email from the ASF dual-hosted git repository.
Jefffrey pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new 22327cb27e fix(arrow-string): reject FixedSizeBinary concat widths
that overflow i32 (#10981)
22327cb27e is described below
commit 22327cb27ead135e8bb0c30a774e24b299b2b8e4
Author: pawan <[email protected]>
AuthorDate: Fri Sep 4 17:08:22 2026 +0530
fix(arrow-string): reject FixedSizeBinary concat widths that overflow i32
(#10981)
# Which issue does this PR close?
- Closes #10972.
# Rationale for this change
`concat_elements_fixed_size_binary` adds the two input widths together
as a usize, then casts
the sum to i32 to size the builder. two arrays of width 0x70000000 come
out at 3758096384,
which wraps to -536870912, and the builder asserts the value length is
not negative. that
width is accepted at array construction and can come from a user
controlled schema, so nothing
unusual on the caller's side is needed to reach it
it comes through `concat_elements_dyn` as well, since that dispatches
here for fixed size
binary inputs
# What changes are included in this PR?
use i32::try_from on the combined width and return an invalid argument
error when it does not
fit, rather than casting
the byte view builder in this same file already guards exactly this,
checking data_size
against i32::MAX before it builds, so this is that guard applied to the
fixed size binary path
instead of a new mechanism. widths that already fit behave the same as
before
i went through the rest of the file for the same shape while i was in
there. that cast was the
only unchecked one, so this is a single site rather than a family the
way #10437 and #10575
were
one thing i left out on purpose. the next line still reserves the
combined width through
`MutableBuffer::with_capacity`, so a sum just under i32::MAX asks for
roughly 2 GB before a
single row is written. that looked like #10973 rather than this one, but
say the word and i
will fold it in
# Are these changes tested?
yes. `test_fixed_size_binary_concat_width_overflow` uses the widths from
the issue and checks
the call comes back as an error instead of panicking. with only the test
applied to current
main it fails inside `fixed_size_binary_builder.rs` at line 64, which is
the panic site in the
report. fmt and clippy with -D warnings are both clean on arrow-string
# Are there any user-facing changes?
concatenating two fixed size binary arrays whose widths sum past
i32::MAX returns an error now
instead of panicking. no API changes
---
arrow-string/src/concat_elements.rs | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/arrow-string/src/concat_elements.rs
b/arrow-string/src/concat_elements.rs
index 8934f62987..0df4216c66 100644
--- a/arrow-string/src/concat_elements.rs
+++ b/arrow-string/src/concat_elements.rs
@@ -219,11 +219,16 @@ pub fn concat_elements_fixed_size_binary(
))
})?;
let output_size = left_size + right_size;
+ let output_value_length = i32::try_from(output_size).map_err(|_| {
+ ArrowError::InvalidArgumentError(format!(
+ "Concatenated FixedSizeBinary value length {output_size} exceeds
i32"
+ ))
+ })?;
// Pre-compute combined null bitmap so the per-row NULL check is efficient
let nulls = NullBuffer::union(left.nulls(), right.nulls());
- let mut result = FixedSizeBinaryBuilder::with_capacity(left.len(),
output_size as i32);
+ let mut result = FixedSizeBinaryBuilder::with_capacity(left.len(),
output_value_length);
let mut buffer = MutableBuffer::with_capacity(output_size);
for i in 0..left.len() {
if nulls.as_ref().is_some_and(|n| n.is_null(i)) {
@@ -691,6 +696,21 @@ mod tests {
);
}
+ #[test]
+ fn test_fixed_size_binary_concat_width_overflow() {
+ let width = 0x7000_0000_i32;
+ let left =
+ FixedSizeBinaryArray::try_new(width,
Buffer::from(Vec::<u8>::new()), None).unwrap();
+ let right =
+ FixedSizeBinaryArray::try_new(width,
Buffer::from(Vec::<u8>::new()), None).unwrap();
+
+ let output = concat_elements_fixed_size_binary(&left, &right);
+ assert_eq!(
+ output.unwrap_err().to_string(),
+ "Invalid argument error: Concatenated FixedSizeBinary value length
3758096384 exceeds i32".to_string()
+ );
+ }
+
#[test]
fn test_fixed_size_binary_concat_empty() {
let left = FixedSizeBinaryArray::new(0, Buffer::from(&[]), None);
