Merge pull request #4871 from jeddenlea/rm

rm: support non-UTF-8 paths
uutils · May 18, 2023 · b75688b · b75688b
2 parents d458f3e + db5696f
commit b75688b
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 5 deletions.
diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs
@@ -7,8 +7,9 @@
 
 // spell-checker:ignore (path) eacces
 
-use clap::{crate_version, parser::ValueSource, Arg, ArgAction, Command};
+use clap::{builder::ValueParser, crate_version, parser::ValueSource, Arg, ArgAction, Command};
 use std::collections::VecDeque;
+use std::ffi::{OsStr, OsString};
 use std::fs::{self, File, Metadata};
 use std::io::ErrorKind;
 use std::ops::BitOr;
@@ -59,9 +60,9 @@ static ARG_FILES: &str = "files";
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
  let matches = uu_app().after_help(AFTER_HELP).try_get_matches_from(args)?;
 
- let files: Vec<String> = matches
- .get_many::<String>(ARG_FILES)
- .map(|v| v.map(ToString::to_string).collect())
+ let files: Vec<&OsStr> = matches
+ .get_many::<OsString>(ARG_FILES)
+ .map(|v| v.map(OsString::as_os_str).collect())
  .unwrap_or_default();
 
  let force_flag = matches.get_flag(OPT_FORCE);
@@ -231,13 +232,14 @@ pub fn uu_app() -> Command {
  .arg(
  Arg::new(ARG_FILES)
  .action(ArgAction::Append)
+ .value_parser(ValueParser::os_string())
  .num_args(1..)
  .value_hint(clap::ValueHint::AnyPath),
  )
 }
 
 // TODO: implement one-file-system (this may get partially implemented in walkdir)
-fn remove(files: &[String], options: &Options) -> bool {
+fn remove(files: &[&OsStr], options: &Options) -> bool {
  let mut had_err = false;
 
  for filename in files {

diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs
@@ -54,6 +54,8 @@ fn test_rm_interactive() {
 
  at.touch(file_a);
  at.touch(file_b);
+ assert!(at.file_exists(file_a));
+ assert!(at.file_exists(file_b));
 
  scene
  .ucmd()
@@ -84,6 +86,11 @@ fn test_rm_force() {
  let file_a = "test_rm_force_a";
  let file_b = "test_rm_force_b";
 
+ at.touch(file_a);
+ at.touch(file_b);
+ assert!(at.file_exists(file_a));
+ assert!(at.file_exists(file_b));
+
  ucmd.arg("-f")
  .arg(file_a)
  .arg(file_b)
@@ -100,6 +107,11 @@ fn test_rm_force_multiple() {
  let file_a = "test_rm_force_a";
  let file_b = "test_rm_force_b";
 
+ at.touch(file_a);
+ at.touch(file_b);
+ assert!(at.file_exists(file_a));
+ assert!(at.file_exists(file_b));
+
  ucmd.arg("-f")
  .arg("-f")
  .arg("-f")
@@ -578,3 +590,24 @@ fn test_fifo_removal() {
  .timeout(Duration::from_secs(2))
  .succeeds();
 }
+
+#[test]
+#[cfg(any(unix, target_os = "wasi"))]
+#[cfg(not(target_os = "macos"))]
+fn test_non_utf8() {
+ use std::ffi::OsStr;
+ #[cfg(unix)]
+ use std::os::unix::ffi::OsStrExt;
+ #[cfg(target_os = "wasi")]
+ use std::os::wasi::ffi::OsStrExt;
+
+ let file = OsStr::from_bytes(b"not\xffutf8"); // spell-checker:disable-line
+
+ let (at, mut ucmd) = at_and_ucmd!();
+
+ at.touch(file);
+ assert!(at.file_exists(file));
+
+ ucmd.arg(file).succeeds();
+ assert!(!at.file_exists(file));
+}