Skip to content

diffutils: fix panic on non-UTF-8 argument ending in --width=N - #250

Open
SAY-5 wants to merge 2 commits into
uutils:mainfrom
SAY-5:fix-width-non-utf8-panic
Open

SAY-5 wants to merge 2 commits into
uutils:mainfrom
SAY-5:fix-width-non-utf8-panic

Conversation

@SAY-5

@SAY-5 SAY-5 commented Jun 20, 2026

Copy link
Copy Markdown

diff panicked with exit 101 on a non-UTF-8 argument whose lossy form ends in --width=N, e.g. diff $'\xff--width=5' A B. The --width regex was missing the leading ^ anchor that --tabsize already has, so the lossy (U+FFFD) form matched and the following into_string().unwrap() then failed on the real bytes.

Anchoring the regex (^--width=...$, matching tabsize_re) makes such an argument fall through to the operand path instead of being parsed as a width option, so it no longer panics. As a side effect diff xyz--width=5 is now also treated as a filename rather than silently accepting a width.

Added a width test and a #[cfg(unix)] regression test for the non-UTF-8 case.

Fixes #247.

Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
@codspeed

codspeed Bot commented Jun 20, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will degrade performance by 3.06%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
❌ 4 regressed benchmarks
✅ 21 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ cmd_cmp_gnu_equal[1000] 23.2 µs 24.9 µs -6.67%
❌ cmd_cmp_release_equal[25000] 24.8 µs 26.2 µs -5.32%
❌ cmd_cmp_release_equal[100] 25.2 µs 26.5 µs -4.86%
❌ cmd_cmp_release_equal[10000] 24.8 µs 25.6 µs -3.28%
⚡ cmd_diff_release_equal[1000] 25.8 µs 24.5 µs +5.26%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing SAY-5:fix-width-non-utf8-panic (8c9a0b3) with main (860f6f4)

Open in CodSpeed

@codecov

codecov Bot commented Jun 22, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (dc9ca17) to head (8fdc882).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@     Coverage Diff     @@
##   main   #250   +/-   ##
===========================
===========================

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread src/params.rs
.cloned()
.peekable()
)
);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

please also add a case for xyz--width=5 being an operand, since the PR changes that behavior

Comment thread src/params.rs Outdated
#[test]
fn width_non_utf8_is_not_an_option() {
use std::os::unix::ffi::OsStringExt;
// A non-UTF-8 argument whose lossy form ends in `--width=N` must be

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

comment could be shorter, the test name already says most of it :)

Comment thread src/params.rs Outdated
// treated as an operand, not parsed as the width option (which used to
// panic in `into_string().unwrap()`).
let bad = OsString::from_vec(b"\xff--width=5".to_vec());
assert!(parse_params(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is_err() only works here because of the extra operand.
could you please pass bad and foo only and check that from == bad? that shows it is really treated as an operand

Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
@SAY-5

SAY-5 commented Sep 25, 2026

Copy link
Copy Markdown
Author

all three done in 8c9a0b3: added the xyz--width=5 operand case, trimmed the comment, and the non-utf8 test now passes just bad and foo and asserts from == bad.

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 (into_string().unwrap()) on a non-UTF-8 argument ending in --width=N

2 participants