From 6b32c30d57cf60a09179bc5b294f430a864804a2 Mon Sep 17 00:00:00 2001 From: hamflx Date: Sun, 24 Mar 2024 17:15:34 +0800 Subject: [PATCH] mv: fix invalid numbered backup path --- src/uucore/src/lib/features/backup_control.rs | 58 ++++++++++++++++--- tests/by-util/test_mv.rs | 24 ++++++++ 2 files changed, 74 insertions(+), 8 deletions(-) diff --git a/src/uucore/src/lib/features/backup_control.rs b/src/uucore/src/lib/features/backup_control.rs index 9086acb19..4b4f7aa93 100644 --- a/src/uucore/src/lib/features/backup_control.rs +++ b/src/uucore/src/lib/features/backup_control.rs @@ -421,25 +421,29 @@ pub fn get_backup_path( } fn simple_backup_path(path: &Path, suffix: &str) -> PathBuf { - let mut p = path.to_string_lossy().into_owned(); - p.push_str(suffix); - PathBuf::from(p) + let mut file_name = path.file_name().unwrap_or_default().to_os_string(); + file_name.push(suffix); + path.with_file_name(file_name) } fn numbered_backup_path(path: &Path) -> PathBuf { + let file_name = path.file_name().unwrap_or_default(); for i in 1_u64.. { - let path_str = &format!("{}.~{}~", path.to_string_lossy(), i); - let path = Path::new(path_str); + let mut numbered_file_name = file_name.to_os_string(); + numbered_file_name.push(format!(".~{}~", i)); + let path = path.with_file_name(numbered_file_name); if !path.exists() { - return path.to_path_buf(); + return path; } } panic!("cannot create backup") } fn existing_backup_path(path: &Path, suffix: &str) -> PathBuf { - let test_path_str = &format!("{}.~1~", path.to_string_lossy()); - let test_path = Path::new(test_path_str); + let file_name = path.file_name().unwrap_or_default(); + let mut numbered_file_name = file_name.to_os_string(); + numbered_file_name.push(".~1~"); + let test_path = path.with_file_name(numbered_file_name); if test_path.exists() { numbered_backup_path(path) } else { @@ -660,6 +664,44 @@ mod tests { let result = determine_backup_suffix(&matches); assert_eq!(result, "-v"); } + + #[test] + fn test_numbered_backup_path() { + assert_eq!(numbered_backup_path(&Path::new("")), PathBuf::from(".~1~")); + assert_eq!( + numbered_backup_path(&Path::new("/")), + PathBuf::from("/.~1~") + ); + assert_eq!( + numbered_backup_path(&Path::new("/hello/world")), + PathBuf::from("/hello/world.~1~") + ); + assert_eq!( + numbered_backup_path(&Path::new("/hello/world/")), + PathBuf::from("/hello/world.~1~") + ); + } + + #[test] + fn test_simple_backup_path() { + assert_eq!( + simple_backup_path(&Path::new(""), ".bak"), + PathBuf::from(".bak") + ); + assert_eq!( + simple_backup_path(&Path::new("/"), ".bak"), + PathBuf::from("/.bak") + ); + assert_eq!( + simple_backup_path(&Path::new("/hello/world"), ".bak"), + PathBuf::from("/hello/world.bak") + ); + assert_eq!( + simple_backup_path(&Path::new("/hello/world/"), ".bak"), + PathBuf::from("/hello/world.bak") + ); + } + #[test] fn test_source_is_target_backup() { let source = Path::new("data.txt.bak"); diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 6ab989ee4..ac64fae7e 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -571,6 +571,30 @@ fn test_mv_simple_backup() { assert!(at.file_exists(format!("{file_b}~"))); } +#[test] +fn test_mv_simple_backup_for_directory() { + let (at, mut ucmd) = at_and_ucmd!(); + let dir_a = "test_mv_simple_backup_dir_a"; + let dir_b = "test_mv_simple_backup_dir_b"; + + at.mkdir(dir_a); + at.mkdir(dir_b); + at.touch(format!("{dir_a}/file_a")); + at.touch(format!("{dir_b}/file_b")); + ucmd.arg("-T") + .arg("-b") + .arg(dir_a) + .arg(dir_b) + .succeeds() + .no_stderr(); + + assert!(!at.dir_exists(dir_a)); + assert!(at.dir_exists(dir_b)); + assert!(at.dir_exists(&format!("{dir_b}~"))); + assert!(at.file_exists(format!("{dir_b}/file_a"))); + assert!(at.file_exists(format!("{dir_b}~/file_b"))); +} + #[test] fn test_mv_simple_backup_with_file_extension() { let (at, mut ucmd) = at_and_ucmd!();