From 24aff229da4adba9e9ab5ed05e536f57d9a7ee3a Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 23 Jun 2023 23:58:38 +0200 Subject: [PATCH 1/2] Add a function to detect if file is likely to be the simple backup file --- src/uucore/src/lib/mods/backup_control.rs | 52 +++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/uucore/src/lib/mods/backup_control.rs b/src/uucore/src/lib/mods/backup_control.rs index 2d161c43fa0..9998c7560d7 100644 --- a/src/uucore/src/lib/mods/backup_control.rs +++ b/src/uucore/src/lib/mods/backup_control.rs @@ -438,6 +438,32 @@ fn existing_backup_path(path: &Path, suffix: &str) -> PathBuf { } } +/// Returns true if the source file is likely to be the simple backup file for the target file. +/// +/// # Arguments +/// +/// * `source` - A Path reference that holds the source (backup) file path. +/// * `target` - A Path reference that holds the target file path. +/// * `suffix` - Str that holds the backup suffix. +/// +/// # Examples +/// +/// ``` +/// use std::path::Path; +/// use uucore::backup_control::source_is_target_backup; +/// let source = Path::new("data.txt~"); +/// let target = Path::new("data.txt"); +/// let suffix = String::from("~"); +/// +/// assert_eq!(source_is_target_backup(&source, &target, &suffix), true); +/// ``` +/// +pub fn source_is_target_backup(source: &Path, target: &Path, suffix: &str) -> bool { + let source_filename = source.to_string_lossy(); + let target_backup_filename = format!("{}{suffix}", target.to_string_lossy()); + source_filename == target_backup_filename +} + // // Tests for this module // @@ -626,4 +652,30 @@ mod tests { let result = determine_backup_suffix(&matches); assert_eq!(result, "-v"); } + #[test] + fn test_source_is_target_backup() { + let source = Path::new("data.txt.bak"); + let target = Path::new("data.txt"); + let suffix = String::from(".bak"); + + assert!(source_is_target_backup(&source, &target, &suffix)); + } + + #[test] + fn test_source_is_not_target_backup() { + let source = Path::new("data.txt"); + let target = Path::new("backup.txt"); + let suffix = String::from(".bak"); + + assert!(!source_is_target_backup(&source, &target, &suffix)); + } + + #[test] + fn test_source_is_target_backup_with_tilde_suffix() { + let source = Path::new("example~"); + let target = Path::new("example"); + let suffix = String::from("~"); + + assert!(source_is_target_backup(&source, &target, &suffix)); + } } From 40c598852b7a3e6f7bca8ac2dbe4e1ffc662c3ca Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 24 Jun 2023 00:06:54 +0200 Subject: [PATCH 2/2] mv: add the check with --b=simple and when the source is a backup --- src/uu/mv/src/mv.rs | 13 ++++++++++++- tests/by-util/test_mv.rs | 16 ++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 6289e79f90a..32214b302ad 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -22,7 +22,7 @@ use std::os::unix; #[cfg(windows)] use std::os::windows; use std::path::{Path, PathBuf}; -use uucore::backup_control::{self, BackupMode}; +use uucore::backup_control::{self, source_is_target_backup, BackupMode}; use uucore::display::Quotable; use uucore::error::{set_exit_code, FromIo, UError, UResult, USimpleError, UUsageError}; use uucore::fs::{are_hardlinks_or_one_way_symlink_to_same_file, are_hardlinks_to_same_file}; @@ -251,6 +251,17 @@ fn parse_paths(files: &[OsString], b: &Behavior) -> Vec { } fn handle_two_paths(source: &Path, target: &Path, b: &Behavior) -> UResult<()> { + if b.backup == BackupMode::SimpleBackup && source_is_target_backup(source, target, &b.suffix) { + return Err(io::Error::new( + io::ErrorKind::NotFound, + format!( + "backing up {} might destroy source; {} not moved", + target.quote(), + source.quote() + ), + ) + .into()); + } if source.symlink_metadata().is_err() { return Err(MvError::NoSuchFile(source.quote().to_string()).into()); } diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 0c292c50d85..ceaa4ba2271 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -510,6 +510,22 @@ fn test_mv_same_hardlink_backup_simple() { .succeeds(); } +#[test] +#[cfg(all(unix, not(target_os = "android")))] +fn test_mv_same_hardlink_backup_simple_destroy() { + let (at, mut ucmd) = at_and_ucmd!(); + let file_a = "test_mv_same_file_a~"; + let file_b = "test_mv_same_file_a"; + at.touch(file_a); + at.touch(file_b); + + ucmd.arg(file_a) + .arg(file_b) + .arg("--b=simple") + .fails() + .stderr_contains("backing up 'test_mv_same_file_a' might destroy source"); +} + #[test] fn test_mv_same_file_not_dot_dir() { let (at, mut ucmd) = at_and_ucmd!();