From 581a38725b42f00e0f776a7647e9e3b31df4ccf7 Mon Sep 17 00:00:00 2001 From: Adel-Ayoub Date: Sat, 4 Jul 2026 19:06:33 +0100 Subject: [PATCH] uucore: fall back to '~' for an empty backup suffix GNU coreutils treats an empty backup suffix as unset and falls back to the default '~'. determine_backup_suffix returned the empty string instead, so --backup combined with an empty --suffix= (or SIMPLE_BACKUP_SUFFIX=) produced a backup path equal to the destination: cp, install and mv silently skipped the backup, and ln failed outright with "Already exists". Fall back to the default suffix when the resolved suffix is empty, the same way a suffix containing a slash is already handled. This is the single shared spot used by cp, install, ln and mv. Fixes #13168 --- src/uucore/src/lib/features/backup_control.rs | 14 +++++++++-- tests/by-util/test_cp.rs | 17 +++++++++++++ tests/by-util/test_install.rs | 24 +++++++++++++++++++ tests/by-util/test_ln.rs | 19 +++++++++++++++ 4 files changed, 72 insertions(+), 2 deletions(-) diff --git a/src/uucore/src/lib/features/backup_control.rs b/src/uucore/src/lib/features/backup_control.rs index 34f73521061..a47f3292b05 100644 --- a/src/uucore/src/lib/features/backup_control.rs +++ b/src/uucore/src/lib/features/backup_control.rs @@ -245,7 +245,8 @@ pub mod arguments { /// /// 1. From the '-S' or '--suffix' CLI argument, if present /// 2. From the "SIMPLE_BACKUP_SUFFIX" environment variable, if present -/// 3. By using the default '~' if none of the others apply, or if they contained slashes +/// 3. By using the default '~' if none of the others apply, or if the +/// resolved suffix is empty or contains slashes /// /// This function directly takes [`ArgMatches`] as argument and looks for /// the '-S' and '--suffix' arguments itself. @@ -256,7 +257,7 @@ pub fn determine_backup_suffix(matches: &ArgMatches) -> String { } else { env::var("SIMPLE_BACKUP_SUFFIX").unwrap_or_else(|_| DEFAULT_BACKUP_SUFFIX.to_owned()) }; - if suffix.contains('/') { + if suffix.is_empty() || suffix.contains('/') { DEFAULT_BACKUP_SUFFIX.to_owned() } else { suffix @@ -710,6 +711,15 @@ mod tests { assert_eq!(result, DEFAULT_BACKUP_SUFFIX); } + #[test] + fn test_suffix_empty_defaults_to_tilde() { + let _dummy = TEST_MUTEX.lock().unwrap(); + let matches = make_app().get_matches_from(vec!["command", "-b", "--suffix", ""]); + + let result = determine_backup_suffix(&matches); + assert_eq!(result, DEFAULT_BACKUP_SUFFIX); + } + #[test] fn test_numbered_backup_path() { assert_eq!(numbered_backup_path(Path::new("")), PathBuf::from(".~1~")); diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 7261ef51c9d..8ab4c6c0dca 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -1156,6 +1156,23 @@ fn test_cp_arg_suffix_hyphen_value() { ); } +#[test] +fn test_cp_arg_suffix_empty_defaults_to_tilde() { + let (at, mut ucmd) = at_and_ucmd!(); + + ucmd.arg(TEST_HELLO_WORLD_SOURCE) + .arg("-b") + .arg("--suffix=") + .arg(TEST_HOW_ARE_YOU_SOURCE) + .succeeds(); + + assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "Hello, World!\n"); + assert_eq!( + at.read(&format!("{TEST_HOW_ARE_YOU_SOURCE}~")), + "How are you?\n" + ); +} + #[test] fn test_cp_custom_backup_suffix_via_env() { let (at, mut ucmd) = at_and_ucmd!(); diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index 2454cb5a320..75895f0f0ab 100644 --- a/tests/by-util/test_install.rs +++ b/tests/by-util/test_install.rs @@ -1309,6 +1309,30 @@ fn test_install_backup_short_custom_suffix() { assert!(at.file_exists(format!("{file_b}{suffix}"))); } +#[test] +fn test_install_backup_empty_suffix_defaults_to_tilde() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file_a = "test_install_backup_empty_suffix_file_a"; + let file_b = "test_install_backup_empty_suffix_file_b"; + + at.touch(file_a); + at.touch(file_b); + scene + .ucmd() + .arg("-b") + .arg("--suffix=") + .arg(file_a) + .arg(file_b) + .succeeds() + .no_stderr(); + + assert!(at.file_exists(file_a)); + assert!(at.file_exists(file_b)); + assert!(at.file_exists(format!("{file_b}~"))); +} + #[test] fn test_install_suffix_without_backup_option() { let scene = TestScenario::new(util_name!()); diff --git a/tests/by-util/test_ln.rs b/tests/by-util/test_ln.rs index 7702b8b9de1..412292fbbf0 100644 --- a/tests/by-util/test_ln.rs +++ b/tests/by-util/test_ln.rs @@ -244,6 +244,25 @@ fn test_symlink_custom_backup_suffix() { assert_eq!(at.resolve_link(backup), file); } +#[test] +fn test_ln_backup_empty_suffix_defaults_to_tilde() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.write("a", "a\n"); + at.write("b", "b2\n"); + + scene + .ucmd() + .args(&["-b", "--suffix=", "a", "b"]) + .succeeds() + .no_stderr(); + + // b is now a hard link to a, and the original b was backed up to b~ + assert_eq!(at.read("b"), "a\n"); + assert_eq!(at.read("b~"), "b2\n"); +} + #[test] fn test_symlink_suffix_without_backup_option() { let scene = TestScenario::new(util_name!());