Skip to content

Prevent repeat slice length overflow#9819

Merged
alamb merged 1 commit intoapache:mainfrom
alamb:codex/repeat-slice-length-overflow
Apr 25, 2026
Merged

Prevent repeat slice length overflow#9819
alamb merged 1 commit intoapache:mainfrom
alamb:codex/repeat-slice-length-overflow

Conversation

@alamb
Copy link
Copy Markdown
Contributor

@alamb alamb commented Apr 25, 2026

Which issue does this PR close?

  • None.

Rationale for this change

MutableBuffer repeated slices used unchecked usize arithmetic when calculating the number of bytes to reserve. In optimized builds, very large repeat counts could wrap the capacity calculation before copying repeated bytes.

What changes are included in this PR?

This adds checked arithmetic for repeated slice byte length validation before reserving capacity and copying repeated data.

Are these changes tested?

Yes. This adds regression coverage for overflowing repeated slice length calculations.

Are there any user-facing changes?

Invalid repeat counts whose requested byte length cannot be represented without overflow now panic consistently. There are no API changes.

@github-actions github-actions Bot added the arrow Changes to the arrow crate label Apr 25, 2026
@alamb alamb marked this pull request as ready for review April 25, 2026 03:48
@alamb
Copy link
Copy Markdown
Contributor Author

alamb commented Apr 25, 2026

Thanks (again) @Dandandan

@alamb alamb merged commit a8e8261 into apache:main Apr 25, 2026
26 checks passed
.expect("repeated slice byte length overflow");
self.len
.checked_add(repeated_bytes)
.expect("mutable buffer length overflow");
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.

I don't love introducing panics (tho clearly that's better than UB).

Any possibility to make this API more robust?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think it would be fine to add another function like try_repeat_slice_n_times... that returned a Result for those that wanted it

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.

3 participants