Skip to content

diff: don't panic on an oversized context count - #274

Open
MsfPablo wants to merge 1 commit into
uutils:mainfrom
MsfPablo:fix-oversized-context
Open

diff: don't panic on an oversized context count#274
MsfPablo wants to merge 1 commit into
uutils:mainfrom
MsfPablo:fix-oversized-context

Conversation

@MsfPablo

Copy link
Copy Markdown

Fixes #245.

Both panics in the issue come from the context count being used unchecked:

  • -u99999999999999999999 and friends reach numvalue.as_str().parse::<usize>().unwrap() in params.rs, and PosOverflow aborts the process.
  • A count that does fit in usize but is large enough that N * 16 > isize::MAX gets past the parser and aborts in VecDeque::with_capacity instead, which is the second case @leeewee added in the comments.

The parse now saturates, and make_diff clamps context_size to the combined length of the two inputs. Neither file can have more lines than it has bytes, so a request above that already meant "the whole file" — the clamp only removes values that were never reachable, and it keeps both the preallocation and the context_size + 1 arithmetic below it in range. That covers the unified and context paths in one place rather than guarding each site.

All six affected spellings now produce the same output as a plain -u/-c run and exit 1.

Tests: a unit test in params.rs for the saturating parse, and an integration test running each spelling end to end.

One thing I left alone: the separate-argument forms (-C 99999999999999999999) already return invalid context length rather than panicking. GNU accepts those, so the behaviour still differs, but since it is not a crash I did not want to fold a compatibility change into this fix. Happy to do it separately if you want them aligned.

@github-actions

Copy link
Copy Markdown

GNU diffutils testsuite comparison:

Test results comparison:
  Current:   TOTAL: 33 / PASSED: 0 / FAILED: 33 / SKIPPED: 0
  Reference: TOTAL: 33 / PASSED: 8 / FAILED: 21 / SKIPPED: 4

Changes from main branch:
  TOTAL: +0
  PASSED: -8
  FAILED: +12

New test failures (12):
  - basic
  - bignum
  - brief-vs-stat-zero-kernel-lies
  - bug-64316
  - cmp
  - diff3
  - help-version
  - large-subopt
  - strcoll-0-names
  - strip-trailing-cr
  - timezone
  - y2038-vs-32bit

An inline count like -u99999999999999999999 reached
`parse::<usize>().unwrap()` and aborted with a PosOverflow. A count that
fits in usize but is large enough that N * 16 exceeds isize::MAX aborted
later instead, in VecDeque::with_capacity.

Saturate the parse, and clamp context_size to the combined input length
in both make_diff implementations. A file cannot have more lines than
bytes, so anything larger already meant the whole file; the clamp keeps
the surrounding arithmetic in range and stops the preallocation from
scaling with a number the caller picked.

Fixes uutils#245
@sylvestre
sylvestre force-pushed the fix-oversized-context branch from e852df3 to 8428c8f Compare August 18, 2026 06:55
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.

diff panics on an oversized inline context count (-c/-u/-C/-U): parse overflow and capacity overflow

1 participant