GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — diffutils: fix panic on non-UTF-8 argument ending in --width=N by SAY-5 · Pull Request #250 · uutils/diffutils

Conversation

@SAY-5

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

Copy link
Copy Markdown
Contributor

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@SAY-5
diffutils: fix panic on non-UTF-8 argument ending in --width=N
8fdc882 
Signed-off-by: Sai Asish Y <[email protected]>
@codspeed

codspeed Bot commented Jun 20, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@codecov

codecov Bot commented Jun 22, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
sylvestre
sylvestre reviewed Sep 24, 2026
View reviewed changes
Comment thread src/params.rs
.cloned()
.peekable()
)
);

@sylvestre sylvestre Sep 24, 2026

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
sylvestre
sylvestre reviewed Sep 24, 2026
View reviewed changes
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

@sylvestre sylvestre Sep 24, 2026

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

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 :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
sylvestre
sylvestre reviewed Sep 24, 2026
View reviewed changes
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(

@sylvestre sylvestre Sep 24, 2026

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@SAY-5
diffutils: assert width-like operands are kept as filenames in tests
8c9a0b3 
Signed-off-by: Sai Asish Y <[email protected]>
@SAY-5

SAY-5 commented Sep 25, 2026

Copy link
Copy Markdown
Contributor 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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sylvestre
sylvestre merged commit 5cdd3a4 into uutils:main Oct 3, 2026
30 of 31 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@ARMeeru ARMeeru mentioned this pull request Oct 11, 2026
diff: don't panic on an operand ending in --width=N #279
Closed
@BrewTestBot BrewTestBot mentioned this pull request Oct 23, 2026
uutils-diffutils 0.6.0 Homebrew/homebrew-core#320660
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sylvestre sylvestre sylvestre left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

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

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@SAY-5 @sylvestre
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.