Skip to content

cp: Fix panic when recursively copying a directory with a non-UTF8 name#11148

Merged
sylvestre merged 2 commits intouutils:mainfrom
Zellic:cp-fix-nonutf8-panic
Feb 28, 2026
Merged

cp: Fix panic when recursively copying a directory with a non-UTF8 name#11148
sylvestre merged 2 commits intouutils:mainfrom
Zellic:cp-fix-nonutf8-panic

Conversation

@aweinstock314
Copy link
Contributor

@aweinstock314 aweinstock314 commented Feb 27, 2026

Comparative output from GNU cp and uu cp prior to the fix:

root@b082d601e194:/tmp# cp --version
cp (GNU coreutils) 9.7
Packaged by Debian (9.7-3)
Copyright (C) 2025 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by Torbjorn Granlund, David MacKenzie, and Jim Meyering.
root@b082d601e194:/tmp# mkdir dir$'\x80' dir2
root@b082d601e194:/tmp# touch dir$'\x80'/a
root@b082d601e194:/tmp# cp -r dir$'\x80'/. dir2
root@b082d601e194:/tmp# ls -l dir2/
total 0
-rw-r--r-- 1 root root 0 Feb 25 23:46 a
root@03081849be59:/tmp# cp --version
cp (uutils coreutils) 0.6.0
root@03081849be59:/tmp# mkdir dir$'\x80' dir2
root@03081849be59:/tmp# touch dir$'\x80'/a
root@03081849be59:/tmp# cp -r dir$'\x80'/. dir2

thread 'main' (1659) panicked at src/uu/cp/src/copydir.rs:113:64:
called `Option::unwrap()` on a `None` value
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Aborted

@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/cut/bounded-memory (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/pr/bounded-memory (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/tail/tail-n0f is now passing!

@codspeed-hq
Copy link

codspeed-hq bot commented Feb 27, 2026

Merging this PR will improve performance by 6.49%

⚡ 2 improved benchmarks
✅ 292 untouched benchmarks
⏩ 42 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation shuf_input_range[1000000] 89.7 ms 84.2 ms +6.49%
Simulation cp_large_file[16] 391.3 µs 379 µs +3.25%

Comparing Zellic:cp-fix-nonutf8-panic (40d7ba7) with main (a611f98)

Open in CodSpeed

Footnotes

  1. 42 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Some(root_path)
};
let root_parent =
if target.exists() && !root.as_os_str().as_encoded_bytes().ends_with(b"/.") {
Copy link
Contributor

@xtqqczze xtqqczze Feb 27, 2026

Choose a reason for hiding this comment

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

Suggested change
if target.exists() && !root.as_os_str().as_encoded_bytes().ends_with(b"/.") {
if target.exists() && !root.ends_with("/.") {

I don't know whether this suggestion works, but converting to bytes might not be correct. Maybe the os_str_bytes crate would be helpful.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are querying for a two byte sequence at the end of it, what do you foresee could be the problem? Encoding issues should be out of the question at this point

Copy link
Contributor

Choose a reason for hiding this comment

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

It could be more complicated than that, maybe some platform uses multi-byte encoding?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, that suggestion compiled but didn't pass tests.

sylvestre added a commit to sylvestre/coreutils-1 that referenced this pull request Feb 28, 2026
Missing test identified here:
 uutils/coreutils#11148

* tests/cp/non-utf8-name.sh: Add a new test to cover this case
@sylvestre sylvestre merged commit 2a81c11 into uutils:main Feb 28, 2026
159 checks passed
@xtqqczze
Copy link
Contributor

@oech3 why is shuf_input_range[1000000] so noisy?

@oech3
Copy link
Contributor

oech3 commented Feb 28, 2026

I don't know. But I think the test is not using frozen seed at least for.

hubot pushed a commit to coreutils/coreutils that referenced this pull request Mar 2, 2026
Missing test identified here:
 uutils/coreutils#11148

* tests/cp/non-utf8-name.sh: Add a new test to cover this case.
#207
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.

5 participants