Cintu07 opened a new pull request, #10981:
URL: https://github.com/apache/arrow-rs/pull/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
   


-- 
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]

Reply via email to