Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
cp: Add test for recursively copying a source directory with an inval…
…id UTF-8 path.
  • Loading branch information
aweinstock committed Feb 27, 2026
commit 40d7ba70095ff4b996acfe1c8633aa9a5f3ce553
23 changes: 12 additions & 11 deletions src/uu/cp/src/copydir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,17 +122,18 @@ impl<'a> Context<'a> {
let current_dir = env::current_dir()?;
let root_path = current_dir.join(root);
let target_is_file = target.is_file();
let root_parent = if target.exists() && !root.as_os_str().as_encoded_bytes().ends_with(b"/.") {
root_path.parent().map(ToOwned::to_owned)
} else if root == Path::new(".") && target.is_dir() {
// Special case: when copying current directory (.) to an existing directory,
// we don't want to use the parent path as root_parent because we want to
// copy the contents of the current directory directly into the target directory,
// not create a subdirectory with the current directory's name.
None
} else {
Some(root_path)
};
let root_parent =
if target.exists() && !root.as_os_str().as_encoded_bytes().ends_with(b"/.") {
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

root_path.parent().map(ToOwned::to_owned)
} else if root == Path::new(".") && target.is_dir() {
// Special case: when copying current directory (.) to an existing directory,
// we don't want to use the parent path as root_parent because we want to
// copy the contents of the current directory directly into the target directory,
// not create a subdirectory with the current directory's name.
None
} else {
Some(root_path)
};
Ok(Self {
current_dir,
root_parent,
Expand Down
19 changes: 19 additions & 0 deletions tests/by-util/test_cp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7708,3 +7708,22 @@ fn test_cp_preserve_context_with_z_fails() {
.fails()
.stderr_contains("cannot combine");
}

#[test]
#[cfg(all(unix, not(target_os = "macos")))]
fn test_cp_recursive_non_utf8_source() {
use std::{ffi::OsStr, os::unix::ffi::OsStrExt};
let (at, mut ucmd) = at_and_ucmd!();

at.mkdir(OsStr::from_bytes(b"dir\x80"));
at.mkdir("dir2");
at.touch(OsStr::from_bytes(b"dir\x80/a"));

ucmd.arg("-r")
.arg(OsStr::from_bytes(b"dir\x80/."))
.arg("dir2")
.succeeds()
.no_output();

assert!(at.plus("dir2").join("a").exists());
}
Loading