From f15dac138ead8c3ae33cbf86ca87beec485e29b9 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Wed, 13 Aug 2025 20:29:13 +0200 Subject: [PATCH] chmod/readlink/du: improve non-UTF-8 filename handling and code style --- src/uu/du/src/du.rs | 23 ++++++++++++++++++----- src/uu/readlink/src/readlink.rs | 4 ++-- tests/by-util/test_cat.rs | 10 +++++----- tests/by-util/test_head.rs | 12 +++++++----- tests/by-util/test_ln.rs | 20 +++++++++++--------- tests/by-util/test_readlink.rs | 8 ++++---- tests/by-util/test_realpath.rs | 8 ++++---- tests/by-util/test_rm.rs | 25 ++++++++++--------------- tests/by-util/test_touch.rs | 8 ++++---- tests/by-util/test_unlink.rs | 12 +++++------- 10 files changed, 70 insertions(+), 60 deletions(-) diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index 64a7662bb..64c99998a 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -7,9 +7,14 @@ use clap::{Arg, ArgAction, ArgMatches, Command, builder::PossibleValue}; use glob::Pattern; use std::collections::HashSet; use std::env; +#[cfg(unix)] +use std::ffi::OsStr; +use std::ffi::OsString; use std::fs::Metadata; use std::fs::{self, DirEntry, File}; use std::io::{BufRead, BufReader, stdout}; +#[cfg(unix)] +use std::os::unix::ffi::OsStrExt; #[cfg(not(windows))] use std::os::unix::fs::MetadataExt; #[cfg(windows)] @@ -568,6 +573,9 @@ fn read_files_from(file_name: &str) -> Result, std::io::Error> { ); set_exit_code(1); } else { + #[cfg(unix)] + let p = PathBuf::from(OsStr::from_bytes(&path)); + #[cfg(windows)] let p = PathBuf::from(String::from_utf8_lossy(&path).to_string()); if !paths.contains(&p) { paths.push(p); @@ -594,21 +602,24 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { summarize, )?; - let files = if let Some(file_from) = matches.get_one::(options::FILES0_FROM) { - if file_from == "-" && matches.get_one::(options::FILE).is_some() { + let files = if let Some(file_from) = matches.get_one::(options::FILES0_FROM) { + if file_from.to_string_lossy() == "-" + && matches.get_one::(options::FILE).is_some() + { return Err(std::io::Error::other( translate!("du-error-extra-operand-with-files0-from", "file" => matches - .get_one::(options::FILE) + .get_one::(options::FILE) .unwrap() + .to_string_lossy() .quote() ), ) .into()); } - read_files_from(file_from)? - } else if let Some(files) = matches.get_many::(options::FILE) { + read_files_from(&file_from.to_string_lossy())? + } else if let Some(files) = matches.get_many::(options::FILE) { let files = files.map(PathBuf::from); if count_links { files.collect() @@ -984,6 +995,7 @@ pub fn uu_app() -> Command { .long("files0-from") .value_name("FILE") .value_hint(clap::ValueHint::FilePath) + .value_parser(clap::value_parser!(OsString)) .help(translate!("du-help-files0-from")) .action(ArgAction::Append), ) @@ -1010,6 +1022,7 @@ pub fn uu_app() -> Command { Arg::new(options::FILE) .hide(true) .value_hint(clap::ValueHint::AnyPath) + .value_parser(clap::value_parser!(OsString)) .action(ArgAction::Append), ) } diff --git a/src/uu/readlink/src/readlink.rs b/src/uu/readlink/src/readlink.rs index c5e28ca8b..4d5e32f78 100644 --- a/src/uu/readlink/src/readlink.rs +++ b/src/uu/readlink/src/readlink.rs @@ -81,9 +81,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { for f in &files { let p = f; let path_result = if res_mode == ResolveMode::None { - fs::read_link(&p) + fs::read_link(p) } else { - canonicalize(&p, can_mode, res_mode) + canonicalize(p, can_mode, res_mode) }; match path_result { diff --git a/tests/by-util/test_cat.rs b/tests/by-util/test_cat.rs index 647775a3a..c809231c7 100644 --- a/tests/by-util/test_cat.rs +++ b/tests/by-util/test_cat.rs @@ -14,9 +14,9 @@ use std::process::Stdio; use uutests::at_and_ucmd; use uutests::new_ucmd; use uutests::util::TestScenario; -use uutests::util_name; #[cfg(not(windows))] use uutests::util::vec_of_size; +use uutests::util_name; #[test] fn test_output_simple() { @@ -755,17 +755,17 @@ fn test_cat_non_utf8_paths() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; - + // Create a test file with non-UTF-8 bytes in the name let non_utf8_bytes = b"test_\xFF\xFE.txt"; let non_utf8_name = OsStr::from_bytes(non_utf8_bytes); - + // Create the actual file with some content std::fs::write(at.plus(non_utf8_name), "Hello, non-UTF-8 world!\n").unwrap(); - + // Test that cat handles non-UTF-8 file names without crashing let result = scene.ucmd().arg(non_utf8_name).succeeds(); - + // The result should contain the file content let output = result.stdout_str_lossy(); assert_eq!(output, "Hello, non-UTF-8 world!\n"); diff --git a/tests/by-util/test_head.rs b/tests/by-util/test_head.rs index 2cdabdf3d..88c9e5358 100644 --- a/tests/by-util/test_head.rs +++ b/tests/by-util/test_head.rs @@ -867,17 +867,17 @@ fn test_head_non_utf8_paths() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; - + // Create a test file with non-UTF-8 bytes in the name let non_utf8_bytes = b"test_\xFF\xFE.txt"; let non_utf8_name = OsStr::from_bytes(non_utf8_bytes); - + // Create the actual file with some content std::fs::write(at.plus(non_utf8_name), "line1\nline2\nline3\n").unwrap(); - + // Test that head handles non-UTF-8 file names without crashing let result = scene.ucmd().arg(non_utf8_name).succeeds(); - + // The result should contain the file content let output = result.stdout_str_lossy(); assert!(output.contains("line1")); @@ -885,10 +885,12 @@ fn test_head_non_utf8_paths() { assert!(output.contains("line3")); // Test with line count argument - scene.ucmd() + scene + .ucmd() .args(&["-n", "2"]) .arg(non_utf8_name) .succeeds() .stdout_contains("line1") .stdout_contains("line2"); } + // Test that head handles non-UTF-8 file names without crashing diff --git a/tests/by-util/test_ln.rs b/tests/by-util/test_ln.rs index bdcbe20ec..71f9b5716 100644 --- a/tests/by-util/test_ln.rs +++ b/tests/by-util/test_ln.rs @@ -852,36 +852,38 @@ fn test_ln_non_utf8_paths() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; - + // Create a test file with non-UTF-8 bytes in the name let non_utf8_bytes = b"test_\xFF\xFE.txt"; let non_utf8_name = OsStr::from_bytes(non_utf8_bytes); let non_utf8_link_bytes = b"link_\xFF\xFE.txt"; let non_utf8_link_name = OsStr::from_bytes(non_utf8_link_bytes); - + // Create the actual file at.touch(non_utf8_name); - + // Test creating a hard link with non-UTF-8 file names - scene.ucmd() + scene + .ucmd() .arg(non_utf8_name) .arg(non_utf8_link_name) .succeeds(); - + // Both files should exist assert!(at.file_exists(non_utf8_name)); assert!(at.file_exists(non_utf8_link_name)); - + // Test creating a symbolic link with non-UTF-8 file names let symlink_bytes = b"symlink_\xFF\xFE.txt"; let symlink_name = OsStr::from_bytes(symlink_bytes); - - scene.ucmd() + + scene + .ucmd() .args(&["-s"]) .arg(non_utf8_name) .arg(symlink_name) .succeeds(); - + // Check if symlink was created successfully let symlink_path = at.plus(symlink_name); assert!(symlink_path.is_symlink()); diff --git a/tests/by-util/test_readlink.rs b/tests/by-util/test_readlink.rs index 06b92ec2a..c3791a5fb 100644 --- a/tests/by-util/test_readlink.rs +++ b/tests/by-util/test_readlink.rs @@ -382,18 +382,18 @@ fn test_readlink_non_utf8_paths() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; - + // Create a target file and a symlink with non-UTF-8 bytes in the name at.touch("target_file"); let non_utf8_bytes = b"symlink_\xFF\xFE"; let non_utf8_name = OsStr::from_bytes(non_utf8_bytes); - + // Create symlink using std::os::unix::fs::symlink std::os::unix::fs::symlink(at.plus_as_string("target_file"), at.plus(non_utf8_name)).unwrap(); - + // Test that readlink handles non-UTF-8 symlink names without crashing let result = scene.ucmd().arg(non_utf8_name).succeeds(); - + // The result should contain the target path let output = result.stdout_str_lossy(); assert!(output.contains("target_file")); diff --git a/tests/by-util/test_realpath.rs b/tests/by-util/test_realpath.rs index bc1bf9276..a081b163f 100644 --- a/tests/by-util/test_realpath.rs +++ b/tests/by-util/test_realpath.rs @@ -473,17 +473,17 @@ fn test_realpath_non_utf8_paths() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; - + // Create a test file with non-UTF-8 bytes in the name let non_utf8_bytes = b"test_\xFF\xFE.txt"; let non_utf8_name = OsStr::from_bytes(non_utf8_bytes); - + // Create the actual file at.touch(non_utf8_name); - + // Test that realpath handles non-UTF-8 paths without crashing let result = scene.ucmd().arg(non_utf8_name).succeeds(); - + // The result should contain the non-UTF-8 bytes let output = result.stdout_str_lossy(); assert!(output.contains("test_")); diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs index 18dbec8fa..e14268a20 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -1046,34 +1046,29 @@ fn test_rm_non_utf8_paths() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; - + // Create a test file with non-UTF-8 bytes in the name let non_utf8_bytes = b"test_\xFF\xFE.txt"; let non_utf8_name = OsStr::from_bytes(non_utf8_bytes); - + // Create the actual file at.touch(non_utf8_name); assert!(at.file_exists(non_utf8_name)); - + // Test that rm handles non-UTF-8 file names without crashing - scene.ucmd() - .arg(non_utf8_name) - .succeeds(); - + scene.ucmd().arg(non_utf8_name).succeeds(); + // The file should be removed assert!(!at.file_exists(non_utf8_name)); - + // Test with directory let non_utf8_dir_bytes = b"test_dir_\xFF\xFE"; let non_utf8_dir_name = OsStr::from_bytes(non_utf8_dir_bytes); - + at.mkdir(non_utf8_dir_name); assert!(at.dir_exists(non_utf8_dir_name)); - - scene.ucmd() - .args(&["-r"]) - .arg(non_utf8_dir_name) - .succeeds(); - + + scene.ucmd().args(&["-r"]).arg(non_utf8_dir_name).succeeds(); + assert!(!at.dir_exists(non_utf8_dir_name)); } diff --git a/tests/by-util/test_touch.rs b/tests/by-util/test_touch.rs index 88c91ebcb..aa088204a 100644 --- a/tests/by-util/test_touch.rs +++ b/tests/by-util/test_touch.rs @@ -1022,17 +1022,17 @@ fn test_touch_non_utf8_paths() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; - + // Create a test file with non-UTF-8 bytes in the name let non_utf8_bytes = b"test_\xFF\xFE.txt"; let non_utf8_name = OsStr::from_bytes(non_utf8_bytes); - + // Test that touch handles non-UTF-8 file names without crashing let result = scene.ucmd().arg(non_utf8_name).succeeds(); - + // Verify no output and file was created result.no_output(); - + // Check that the file was created (using the raw path) assert!(std::fs::metadata(at.plus(non_utf8_name)).is_ok()); } diff --git a/tests/by-util/test_unlink.rs b/tests/by-util/test_unlink.rs index 002a5929c..b508fef38 100644 --- a/tests/by-util/test_unlink.rs +++ b/tests/by-util/test_unlink.rs @@ -86,20 +86,18 @@ fn test_unlink_non_utf8_paths() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; - + // Create a test file with non-UTF-8 bytes in the name let non_utf8_bytes = b"test_\xFF\xFE.txt"; let non_utf8_name = OsStr::from_bytes(non_utf8_bytes); - + // Create the actual file at.touch(non_utf8_name); assert!(at.file_exists(non_utf8_name)); - + // Test that unlink handles non-UTF-8 file names without crashing - scene.ucmd() - .arg(non_utf8_name) - .succeeds(); - + scene.ucmd().arg(non_utf8_name).succeeds(); + // The file should be removed assert!(!at.file_exists(non_utf8_name)); }