tr: stop an absurd [c*N] repeat count from crashing or hanging - #14317
Open
arbelonson-source wants to merge 1 commit into
Open
tr: stop an absurd [c*N] repeat count from crashing or hanging#14317arbelonson-source wants to merge 1 commit into
arbelonson-source wants to merge 1 commit into
Conversation
An explicit repeat count in a [c*N] construct (as opposed to the open-ended [c*], which is already bounded by the other set's real length) was taken completely literally: Sequence::flatten's lazy std::iter::repeat_n(c, n) got collected into an N-byte Vec<u8> with no upper bound, both when solving set1 and when solving set2. tr '[a*999999999999]' x aborts the process outright (a ~1 TB allocation request); a merely huge rather than astronomical N -- still far more than any real command-line argument could need -- hangs for seconds to minutes instead, since materializing and then iterating N bytes has a real cost even when the allocation itself succeeds. Every position past the point where the other set's real (bounded, by construction, to whatever a single shell argument can hold) length runs out collapses to the same result regardless of exactly how much further N reaches, since that's tr's own "pad the shorter set with its last element" rule -- confirmed empirically: GNU tr's output for [a*N] x is identical for N from 1 up to the largest N it can complete in reasonable time (50000), and for a matched pair of one-million character sets, this fix's output is byte-identical to GNU's own. GNU itself is not immune to this class of input, though: [a*999999999999] x hangs GNU tr too (confirmed directly, contradicting an initial, unverified report that it handled the value gracefully) -- a [x*999999999999] does not, which is the more common practical case (padding one set to match a long literal replacement) and completes instantly on both GNU and, after this fix, on this implementation. Capping the parsed repeat count at 2 MiB -- far more headroom than any single shell argument can realistically contain, confirmed directly: attempting to even construct the equivalent real arguments to test at exactly that boundary hit the OS's own ARG_MAX first -- keeps every realistic use working exactly as before while turning both the crash and the hang into an instant, correct result. AI-assisted-by: Claude Opus 5, via Claude Code
oech3
reviewed
Aug 31, 2026
| // it N actually reaches; not capping it means materializing N literal | ||
| // bytes downstream, which is either an out-of-memory abort or, for a | ||
| // merely huge rather than astronomical N, a multi-second hang. | ||
| const MAX_CHAR_REPEAT: usize = 2 * 1024 * 1024; |
Contributor
There was a problem hiding this comment.
You should catch actual allocation's error instead of adding such bound. Or it panics when RAM is actually lacking.
sylvestre
reviewed
Aug 31, 2026
| .map(|(l, a)| (l, Ok(Self::CharStar(a)))) | ||
| } | ||
|
|
||
| // `[c*N]` only ever needs to be as long as the other set it is padding |
Contributor
There was a problem hiding this comment.
we don't need 9 lines of comments for this
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
An explicit repeat count in a
[c*N]construct (as opposed to theopen-ended
[c*], which is already bounded by the other set's reallength) was taken completely literally:
Sequence::flatten's lazystd::iter::repeat_n(c, n)got collected into anN-byteVec<u8>with no upper bound, both when solving set1 and when solving set2.
A merely huge rather than astronomical
N-- still far more than anyreal command-line argument could ever need -- hangs for seconds to
minutes instead of aborting outright, since materializing and then
iterating
Nbytes has a real cost even when the allocation itselfsucceeds.
Fix
Every position past the point where the other set's real length
runs out collapses to the same result regardless of exactly how much
further
Nreaches, since that'str's own "pad the shorter set withits last element" rule. Confirmed empirically before assuming it: GNU
tr's output for
[a*N] xis identical for everyNfrom 1 up to thelargest it can complete in reasonable time (50000), and for a matched
pair of one-million-character sets, this fix's output is byte-for-byte
identical to GNU's own.
GNU itself is not immune to this class of input, though, so this
isn't a "fix the wording to match GNU" PR like most of the rest of
this series --
[a*999999999999] xhangs GNU tr too (confirmeddirectly with a real 15s timeout; an earlier, unverified pass at this
had reported GNU handled the value gracefully, which turned out to be
wrong).
a [x*999999999999]does not hang GNU, though, and is themore common practical case in the first place (padding one set to
match a long literal replacement) -- and completes instantly on both
GNU and, after this fix, on this implementation.
Capping the parsed repeat count at 2 MiB keeps every realistic use
working exactly as before (confirmed with a real matched-pair test at
100,000 characters, byte-identical to GNU) while turning both the
crash and the hang into an instant, correct result. 2 MiB is far more
headroom than any single shell argument can realistically contain --
confirmed directly, since even attempting to construct the equivalent
real command-line arguments to test right at that boundary hit the
OS's own
ARG_MAXbefore this cap ever could.Testing
cargo test -p uu_tr/ fulltests/by-util/test_tr.rssuite: 169 passed, 0 failed.-d), all completing in well under a second.[a*N] xfor N = 1, 5, 50, 500, 5000, 50000 against GNU tr 9.11 -- byte-identical output throughout.md5sum-compared) against GNU -- identical.-d,-s, and-cmodes too, not just plain translation.cargo clippy -p uu_tr --all-targets -- -D warningsandcargo fmt --check: clean.This PR was written with AI assistance (Claude Opus 5, via Claude Code). I've tested the changes but please review the code carefully.