Skip to content

Fix size_hint underflow on empty-string sentence iterators - #181

Merged
Manishearth merged 1 commit into
unicode-rs:masterfrom
lenamonj:fix-sentence-size-hint-underflow
Sep 1, 2026
Merged

Fix size_hint underflow on empty-string sentence iterators#181
Manishearth merged 1 commit into
unicode-rs:masterfrom
lenamonj:fix-sentence-size-hint-underflow

Conversation

@lenamonj

@lenamonj lenamonj commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

size_hint() on any of the three public sentence iterators panics on the empty string in debug builds:

"".split_sentence_bounds().size_hint();       // attempt to subtract with overflow
"".split_sentence_bound_indices().size_hint(); // same
"".unicode_sentences().size_hint();            // same

In release builds the same calls return (18446744073709551615, Some(0)) - a lower bound above the upper bound, which the Iterator::size_hint contract forbids. A consumer that passes the lower bound to Vec::with_capacity gets a capacity-overflow panic; that appears to be exactly what the reporter of #146 ran into in a comment on that thread ("something in that logic managed to summon a value that made Vec::with_capacity() panic with a Capacity overflow by just passing the lower bound unaltered") - the issue was closed on the min misreading, but this underflow was live underneath it.

Cause: USentenceBounds::size_hint subtracts 1 from the inner iterator's lower bound, and that bound is 0 for an empty string. The subtraction is on usize, and the cmp::max(0, ...) wrappers around both bounds are no-ops on an unsigned type that read as saturation while the underflow sat inside them.

Fix: both bounds use saturating_sub(1); the wrappers and the now-unused top-level use core::cmp are removed. For every non-empty input the arithmetic is unchanged, since cmp::max(0, x - 1) and x.saturating_sub(1) agree wherever x >= 1; only the empty-string case moves, from a panic (debug) or a wrapped bound (release) to (0, Some(0)).

Test: the new test_size_hint_is_a_valid_bound drives size_hint on all ten public iterators over a corpus led by the empty string plus the crate's own test tables, asserting lower <= yielded <= upper. On master it fails with the debug panic above; with the fix, cargo test, cargo fmt --check, cargo clippy --all-targets --all and cargo +1.85.0 test (the MSRV job's command) all pass.

One related observation, kept out of this PR to keep it minimal: the inner SentenceBreaks::size_hint derives its bounds from the whole string's length rather than the unconsumed remainder, so the outer hint never shrinks as items are yielded (a lower bound of 1 remains after the last item). Happy to follow up separately if that is of interest.


Provenance, for transparency: this defect was found by an automated audit loop I run against open-source projects; the patch and its verification were reviewed by me before filing, and I am happy to answer any questions during review.

USentenceBounds::size_hint subtracted 1 from the inner iterator's lower
bound, which is 0 for an empty string, so size_hint() on any of the
three public sentence iterators panicked with 'attempt to subtract with
overflow' in debug builds and returned a lower bound of usize::MAX in
release builds. The cmp::max(0, ...) wrappers around the expression are
no-ops on usize and hid the underflow.

Both bounds now use saturating_sub(1). The regression test drives
size_hint on every public iterator of the crate over a corpus led by
the empty string and asserts lower <= yielded <= upper in both
profiles.
@Manishearth
Manishearth merged commit 68c127e into unicode-rs:master Sep 1, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants