From 769eb29cd3503d7734776572f7875a864c847c92 Mon Sep 17 00:00:00 2001
From: David Matos <dmatos2012@users.noreply.github.com>
Date: Tue, 24 Oct 2023 10:54:23 +0200
Subject: [PATCH] mv: moving directory itself should fail (#5429)

* mv: moving directory itself should fail

* mv: Check trailing slash also fails on target containing itself

* mv: add "spell-checker:ignore mydir" to test

---------

Co-authored-by: Daniel Hofstetter <daniel.hofstetter@42dh.com>
---
 src/uu/mv/src/error.rs   |  5 +++++
 src/uu/mv/src/mv.rs      | 22 ++++++++++++++++++++++
 tests/by-util/test_mv.rs | 25 +++++++++++++++++++++++++
 3 files changed, 52 insertions(+)

diff --git a/src/uu/mv/src/error.rs b/src/uu/mv/src/error.rs
index a6605e232..e891fc2ec 100644
--- a/src/uu/mv/src/error.rs
+++ b/src/uu/mv/src/error.rs
@@ -12,6 +12,7 @@ pub enum MvError {
     NoSuchFile(String),
     SameFile(String, String),
     SelfSubdirectory(String),
+    SelfTargetSubdirectory(String, String),
     DirectoryToNonDirectory(String),
     NonDirectoryToDirectory(String, String),
     NotADirectory(String),
@@ -29,6 +30,10 @@ impl Display for MvError {
                 f,
                 "cannot move '{s}' to a subdirectory of itself, '{s}/{s}'"
             ),
+            Self::SelfTargetSubdirectory(s, t) => write!(
+                f,
+                "cannot move '{s}' to a subdirectory of itself, '{t}/{s}'"
+            ),
             Self::DirectoryToNonDirectory(t) => {
                 write!(f, "cannot overwrite directory {t} with non-directory")
             }
diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs
index 7236907da..47e0b864d 100644
--- a/src/uu/mv/src/mv.rs
+++ b/src/uu/mv/src/mv.rs
@@ -326,6 +326,28 @@ fn handle_two_paths(source: &Path, target: &Path, opts: &Options) -> UResult<()>
                 Err(MvError::DirectoryToNonDirectory(target.quote().to_string()).into())
             }
         } else {
+            // Check that source & target  do not contain same subdir/dir when both exist
+            // mkdir dir1/dir2; mv dir1 dir1/dir2
+            let target_contains_itself = target
+                .as_os_str()
+                .to_str()
+                .ok_or("not a valid unicode string")
+                .and_then(|t| {
+                    source
+                        .as_os_str()
+                        .to_str()
+                        .ok_or("not a valid unicode string")
+                        .map(|s| t.contains(s))
+                })
+                .unwrap();
+
+            if target_contains_itself {
+                return Err(MvError::SelfTargetSubdirectory(
+                    source.display().to_string(),
+                    target.display().to_string(),
+                )
+                .into());
+            }
             move_files_into_dir(&[source.to_path_buf()], target, opts)
         }
     } else if target.exists() && source.is_dir() {
diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs
index f7f9622f5..e88667320 100644
--- a/tests/by-util/test_mv.rs
+++ b/tests/by-util/test_mv.rs
@@ -2,6 +2,8 @@
 //
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
+//
+// spell-checker:ignore mydir
 use crate::common::util::TestScenario;
 use filetime::FileTime;
 use std::thread::sleep;
@@ -1389,6 +1391,29 @@ fn test_mv_into_self_data() {
     assert!(at.file_exists(file2));
     assert!(!at.file_exists(file1));
 }
+
+#[test]
+fn test_mv_directory_into_subdirectory_of_itself_fails() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    let dir1 = "mydir";
+    let dir2 = "mydir/mydir_2";
+    at.mkdir(dir1);
+    at.mkdir(dir2);
+    scene.ucmd().arg(dir1).arg(dir2).fails().stderr_contains(
+        "mv: cannot move 'mydir' to a subdirectory of itself, 'mydir/mydir_2/mydir'",
+    );
+
+    // check that it also errors out with /
+    scene
+        .ucmd()
+        .arg(format!("{}/", dir1))
+        .arg(dir2)
+        .fails()
+        .stderr_contains(
+            "mv: cannot move 'mydir/' to a subdirectory of itself, 'mydir/mydir_2/mydir/'",
+        );
+}
 // Todo:
 
 // $ at.touch a b