From c49d8e6113a0182b5259450e0b76c1f0fbc64629 Mon Sep 17 00:00:00 2001 From: Phil Gebhardt Date: Sat, 18 Jun 2022 03:53:18 -0700 Subject: [PATCH 1/3] cp: make `--b=simple` protective of source When `--backup` is supplied, `cp` will take a backup of *destination* before *source* is copied. When `--backup=simple` is supplied, it is possible for the backup path for *destination* to equal the path for *source*, destroying source before the copy is made. This change prevents this by returning an error instead. This fixes https://github.com/uutils/coreutils/issues/3629 --- src/uu/cp/src/cp.rs | 11 ++++++++++- tests/by-util/test_cp.rs | 21 +++++++++++++++++++++ tests/fixtures/cp/protected.txt | 1 + tests/fixtures/cp/protected.txt.bak | 1 + 4 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/cp/protected.txt create mode 100644 tests/fixtures/cp/protected.txt.bak diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 669a38601b2..d9fae2d189a 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1256,7 +1256,16 @@ fn handle_existing_dest(source: &Path, dest: &Path, options: &Options) -> CopyRe let backup_path = backup_control::get_backup_path(options.backup, dest, &options.backup_suffix); if let Some(backup_path) = backup_path { - backup_dest(dest, &backup_path)?; + if paths_refer_to_same_file(source, &backup_path)? { + return Err(format!( + "backing up {} might destroy source; {} not copied", + dest.quote(), + source.quote() + ) + .into()); + } else { + backup_dest(dest, &backup_path)?; + } } match options.overwrite { diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 06134d2dd80..26a0e1d969c 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -38,6 +38,8 @@ static TEST_COPY_FROM_FOLDER: &str = "hello_dir_with_file/"; static TEST_COPY_FROM_FOLDER_FILE: &str = "hello_dir_with_file/hello_world.txt"; static TEST_COPY_TO_FOLDER_NEW: &str = "hello_dir_new"; static TEST_COPY_TO_FOLDER_NEW_FILE: &str = "hello_dir_new/hello_world.txt"; +static TEST_PROTECT_BACKUP_SRC: &str = "protected.txt.bak"; +static TEST_PROTECT_BACKUP_DEST: &str = "protected.txt"; #[cfg(any(target_os = "linux", target_os = "android", target_os = "freebsd"))] static TEST_MOUNT_COPY_FROM_FOLDER: &str = "dir_with_mount"; #[cfg(any(target_os = "linux", target_os = "android", target_os = "freebsd"))] @@ -558,6 +560,25 @@ fn test_cp_backup_simple() { ); } +#[test] +fn test_cp_backup_simple_protect_source() { + let (at, mut ucmd) = at_and_ucmd!(); + ucmd.arg("--backup=simple") + .arg("--suffix") + .arg(".bak") + .arg(TEST_PROTECT_BACKUP_SRC) + .arg(TEST_PROTECT_BACKUP_DEST) + .fails() + .stderr_only(format!( + "cp: backing up '{}' might destroy source; '{}' not copied", + TEST_PROTECT_BACKUP_DEST, + TEST_PROTECT_BACKUP_SRC, + )); + + assert_eq!(at.read(TEST_PROTECT_BACKUP_SRC), "original text\n"); + assert_eq!(at.read(TEST_PROTECT_BACKUP_DEST), "new text\n"); +} + #[test] fn test_cp_backup_never() { let (at, mut ucmd) = at_and_ucmd!(); diff --git a/tests/fixtures/cp/protected.txt b/tests/fixtures/cp/protected.txt new file mode 100644 index 00000000000..eee417f1b97 --- /dev/null +++ b/tests/fixtures/cp/protected.txt @@ -0,0 +1 @@ +new text diff --git a/tests/fixtures/cp/protected.txt.bak b/tests/fixtures/cp/protected.txt.bak new file mode 100644 index 00000000000..d65e2c6ab49 --- /dev/null +++ b/tests/fixtures/cp/protected.txt.bak @@ -0,0 +1 @@ +original text From 30f03b3ded427e1bba1afe3d4219149efd2871d0 Mon Sep 17 00:00:00 2001 From: Phil Gebhardt Date: Sat, 18 Jun 2022 09:51:39 -0700 Subject: [PATCH 2/3] use existing fixture file instead of new ones --- tests/by-util/test_cp.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 26a0e1d969c..c246d0ef93a 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -38,8 +38,6 @@ static TEST_COPY_FROM_FOLDER: &str = "hello_dir_with_file/"; static TEST_COPY_FROM_FOLDER_FILE: &str = "hello_dir_with_file/hello_world.txt"; static TEST_COPY_TO_FOLDER_NEW: &str = "hello_dir_new"; static TEST_COPY_TO_FOLDER_NEW_FILE: &str = "hello_dir_new/hello_world.txt"; -static TEST_PROTECT_BACKUP_SRC: &str = "protected.txt.bak"; -static TEST_PROTECT_BACKUP_DEST: &str = "protected.txt"; #[cfg(any(target_os = "linux", target_os = "android", target_os = "freebsd"))] static TEST_MOUNT_COPY_FROM_FOLDER: &str = "dir_with_mount"; #[cfg(any(target_os = "linux", target_os = "android", target_os = "freebsd"))] @@ -563,20 +561,19 @@ fn test_cp_backup_simple() { #[test] fn test_cp_backup_simple_protect_source() { let (at, mut ucmd) = at_and_ucmd!(); + let source = format!("{}~", TEST_HELLO_WORLD_SOURCE); + at.touch(&source); ucmd.arg("--backup=simple") - .arg("--suffix") - .arg(".bak") - .arg(TEST_PROTECT_BACKUP_SRC) - .arg(TEST_PROTECT_BACKUP_DEST) + .arg(&source) + .arg(TEST_HELLO_WORLD_SOURCE) .fails() .stderr_only(format!( "cp: backing up '{}' might destroy source; '{}' not copied", - TEST_PROTECT_BACKUP_DEST, - TEST_PROTECT_BACKUP_SRC, + TEST_HELLO_WORLD_SOURCE, source, )); - assert_eq!(at.read(TEST_PROTECT_BACKUP_SRC), "original text\n"); - assert_eq!(at.read(TEST_PROTECT_BACKUP_DEST), "new text\n"); + assert_eq!(at.read(TEST_HELLO_WORLD_SOURCE), "Hello, World!\n"); + assert_eq!(at.read(&source), ""); } #[test] From 098658a3217e633926e4319d25ddbcafd650295e Mon Sep 17 00:00:00 2001 From: Phil Gebhardt Date: Sat, 18 Jun 2022 09:56:27 -0700 Subject: [PATCH 3/3] rm unnecessary fixture files --- tests/fixtures/cp/protected.txt | 1 - tests/fixtures/cp/protected.txt.bak | 1 - 2 files changed, 2 deletions(-) delete mode 100644 tests/fixtures/cp/protected.txt delete mode 100644 tests/fixtures/cp/protected.txt.bak diff --git a/tests/fixtures/cp/protected.txt b/tests/fixtures/cp/protected.txt deleted file mode 100644 index eee417f1b97..00000000000 --- a/tests/fixtures/cp/protected.txt +++ /dev/null @@ -1 +0,0 @@ -new text diff --git a/tests/fixtures/cp/protected.txt.bak b/tests/fixtures/cp/protected.txt.bak deleted file mode 100644 index d65e2c6ab49..00000000000 --- a/tests/fixtures/cp/protected.txt.bak +++ /dev/null @@ -1 +0,0 @@ -original text