Conversation
A huge --tabsize was accepted and then used to size the tab expansion buffer, so the first tab in the input made diff overflow or abort on allocation failure. Reject values above isize::MAX - 3 with "invalid tabsize" and exit status 2, as GNU diff does.
sylvestre
reviewed
Sep 28, 2026
| use regex::Regex; | ||
|
|
||
| /// Largest value accepted for `--tabsize`, matching GNU diff. | ||
| const MAX_TABSIZE: usize = isize::MAX as usize - 3; |
Collaborator
There was a problem hiding this comment.
how did you get - 3?
it's not in the manual, so where does this value come from?
sylvestre
reviewed
Sep 28, 2026
| num | ||
| } | ||
| Err(_) => return Err(format!("invalid tabsize «{tabsize_str}»")), | ||
| Ok(num) if num != 0 && num <= MAX_TABSIZE => num, |
Collaborator
There was a problem hiding this comment.
this doesn't fix the crash, it just moves it
-t --tabsize=9223372036854775804 with a tab in the input still aborts in do_expand_tabs (with_capacity + ntabs * (tabsize - 1) overflow)
could you please fix do_expand_tabs instead (no huge preallocation, write the spaces as a stream)?
sylvestre
reviewed
Sep 28, 2026
| if !option.is_empty() { | ||
| cmd.arg(option); | ||
| } | ||
| cmd.arg("-t") |
sylvestre
reviewed
Sep 28, 2026
| file1.write_all("\tx\n".as_bytes())?; | ||
| let mut file2 = NamedTempFile::new()?; | ||
| file2.write_all("y\n".as_bytes())?; | ||
| for option in ["", "-u", "-c"] { |
Collaborator
There was a problem hiding this comment.
the value is rejected while parsing, so looping over the formats doesn't test anything more, no?
please add a test with a value below the limit and a tab in the input, that's the one that crashes
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
2 participants
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.
Fixed an issue where passing an extremely large value to --tabsize could cause diff to crash.
Added an upper limit matching GNU diff.
Values above the limit now return a normal invalid tabsize error.
Added tests for the boundary values and a large --tabsize case.
This keeps the behavior consistent with GNU diff and prevents the crash.
Huge values below the limit (like 2^62) are still accepted, same as GNU, and can still run out of memory. Handling that would need the diff output to be fallible. Let me know if you want that too.
Closes #286