Skip to content

[56_maintenance] Prevent FixedSizeBinaryArray i32 offset overflows (#9872) - #9917

Merged
alamb merged 1 commit into
apache:56_maintenancefrom
alamb:alamb/backport_9872
May 6, 2026
Merged

[56_maintenance] Prevent FixedSizeBinaryArray i32 offset overflows (#9872)#9917
alamb merged 1 commit into
apache:56_maintenancefrom
alamb:alamb/backport_9872

Conversation

@alamb

@alamb alamb commented May 5, 2026

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot added the arrow Changes to the arrow crate label May 5, 2026
…#9872)

- Closes apache#9850

`FixedSizeBinaryArray::value_offset_at` works use `i32` arithmetic which
can overflow. For offsets beyond `i32::MAX`, that can be bad

1. Prevent any FixedSizedBinaryArrays from being constructed where the
offset calculation could overflow
2. Add some other overflow checks

As @adamreeve [pointed
out](apache#9850 (comment))
on apache#9850 there are several places
where the `i32` arithmetic is problematic in `FixedSizeBinaryArray`. I
will fix them for real in a different, follow on PR, by switching to
entirely `usize` based arithmetic for offset calculations

However, since I hope to backport this PR to older releases, I would
like something that is easy to review and has the least potential for
unintended consequences.

I added unit tests. However, I can't find any way to fully trigger the
actual paths short of trying to allocate very large arrays, which I
don't think is appropriate for unit tests.

Better limit checking
@alamb
alamb force-pushed the alamb/backport_9872 branch from 7a714c5 to 280cd37 Compare May 5, 2026 19:41
@alamb
alamb marked this pull request as ready for review May 5, 2026 20:30

@etseidl etseidl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name change threw me at first, but looks correct. 😄

@alamb
alamb merged commit 1e7097f into apache:56_maintenance May 6, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrow Changes to the arrow crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants