rm: fix the usage of '/..' '/.' with -rf options

fix the test tests/rm/r-4

---------

Co-authored-by: Julian Beltz <[email protected]>
Co-authored-by: Sylvestre Ledru <[email protected]>

Author: 3 people

Cargo.lock

@@ -3,18 +3,23 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
-// spell-checker:ignore (path) eacces inacc
+// spell-checker:ignore (path) eacces inacc rm-r4
 
 use clap::{builder::ValueParser, crate_version, parser::ValueSource, Arg, ArgAction, Command};
 use std::collections::VecDeque;
 use std::ffi::{OsStr, OsString};
 use std::fs::{self, File, Metadata};
 use std::io::ErrorKind;
 use std::ops::BitOr;
+#[cfg(not(windows))]
+use std::os::unix::ffi::OsStrExt;
+use std::path::MAIN_SEPARATOR;
 use std::path::{Path, PathBuf};
 use uucore::display::Quotable;
 use uucore::error::{UResult, USimpleError, UUsageError};
-use uucore::{format_usage, help_about, help_section, help_usage, prompt_yes, show_error};
+use uucore::{
+    format_usage, help_about, help_section, help_usage, os_str_as_bytes, prompt_yes, show_error,
+};
 use walkdir::{DirEntry, WalkDir};
 
 #[derive(Eq, PartialEq, Clone, Copy)]
@@ -290,6 +295,7 @@ pub fn remove(files: &[&OsStr], options: &Options) -> bool {
 
     for filename in files {
         let file = Path::new(filename);
+
         had_err = match file.symlink_metadata() {
             Ok(metadata) => {
                 if metadata.is_dir() {
@@ -300,6 +306,7 @@ pub fn remove(files: &[&OsStr], options: &Options) -> bool {
                     remove_file(file, options)
                 }
             }
+
             Err(_e) => {
                 // TODO: actually print out the specific error
                 // TODO: When the error is not about missing files
@@ -326,6 +333,15 @@ pub fn remove(files: &[&OsStr], options: &Options) -> bool {
 fn handle_dir(path: &Path, options: &Options) -> bool {
     let mut had_err = false;
 
+    let path = clean_trailing_slashes(path);
+    if path_is_current_or_parent_directory(path) {
+        show_error!(
+            "refusing to remove '.' or '..' directory: skipping '{}'",
+            path.display()
+        );
+        return true;
+    }
+
     let is_root = path.has_root() && path.parent().is_none();
     if options.recursive && (!is_root || !options.preserve_root) {
         if options.interactive != InteractiveMode::Always && !options.verbose {
@@ -396,7 +412,11 @@ fn handle_dir(path: &Path, options: &Options) -> bool {
     } else if options.dir && (!is_root || !options.preserve_root) {
         had_err = remove_dir(path, options).bitor(had_err);
     } else if options.recursive {
-        show_error!("could not remove directory {}", path.quote());
+        show_error!(
+            "it is dangerous to operate recursively on '{}'",
+            MAIN_SEPARATOR
+        );
+        show_error!("use --no-preserve-root to override this failsafe");
         had_err = true;
     } else {
         show_error!(
@@ -559,6 +579,20 @@ fn handle_writable_directory(path: &Path, options: &Options, metadata: &Metadata
         true
     }
 }
+/// Checks if the path is referring to current or parent directory , if it is referring to current or any parent directory in the file tree e.g  '/../..' , '../..'
+fn path_is_current_or_parent_directory(path: &Path) -> bool {
+    let path_str = os_str_as_bytes(path.as_os_str());
+    let dir_separator = MAIN_SEPARATOR as u8;
+    if let Ok(path_bytes) = path_str {
+        return path_bytes == ([b'.'])
+            || path_bytes == ([b'.', b'.'])
+            || path_bytes.ends_with(&[dir_separator, b'.'])
+            || path_bytes.ends_with(&[dir_separator, b'.', b'.'])
+            || path_bytes.ends_with(&[dir_separator, b'.', dir_separator])
+            || path_bytes.ends_with(&[dir_separator, b'.', b'.', dir_separator]);
+    }
+    false
+}
 
 // For windows we can use windows metadata trait and file attributes to see if a directory is readonly
 #[cfg(windows)]
@@ -586,6 +620,40 @@ fn handle_writable_directory(path: &Path, options: &Options, metadata: &Metadata
     }
 }
 
+/// Removes trailing slashes, for example 'd/../////' yield 'd/../' required to fix rm-r4 GNU test
+fn clean_trailing_slashes(path: &Path) -> &Path {
+    let path_str = os_str_as_bytes(path.as_os_str());
+    let dir_separator = MAIN_SEPARATOR as u8;
+
+    if let Ok(path_bytes) = path_str {
+        let mut idx = if path_bytes.len() > 1 {
+            path_bytes.len() - 1
+        } else {
+            return path;
+        };
+        // Checks if element at the end is a '/'
+        if path_bytes[idx] == dir_separator {
+            for i in (1..path_bytes.len()).rev() {
+                // Will break at the start of the continuous sequence of '/', eg: "abc//////" , will break at
+                // "abc/", this will clean ////// to the root '/', so we have to be careful to not
+                // delete the root.
+                if path_bytes[i - 1] != dir_separator {
+                    idx = i;
+                    break;
+                }
+            }
+            #[cfg(unix)]
+            return Path::new(OsStr::from_bytes(&path_bytes[0..=idx]));
+
+            #[cfg(not(unix))]
+            // Unwrapping is fine here as os_str_as_bytes() would return an error on non unix
+            // systems with non utf-8 characters and thus bypass the if let Ok branch
+            return Path::new(std::str::from_utf8(&path_bytes[0..=idx]).unwrap());
+        }
+    }
+    path
+}
+
 fn prompt_descend(path: &Path) -> bool {
     prompt_yes!("descend into directory {}?", path.quote())
 }
@@ -611,3 +679,17 @@ fn is_symlink_dir(metadata: &Metadata) -> bool {
     metadata.file_type().is_symlink()
         && ((metadata.file_attributes() & FILE_ATTRIBUTE_DIRECTORY) != 0)
 }
+
+mod tests {
+
+    #[test]
+    // Testing whether path the `/////` collapses to `/`
+    fn test_collapsible_slash_path() {
+        use std::path::Path;
+
+        use crate::clean_trailing_slashes;
+        let path = Path::new("/////");
+
+        assert_eq!(Path::new("/"), clean_trailing_slashes(path));
+    }
+}

src/uu/rm/src/rm.rs

@@ -677,6 +677,68 @@ fn test_remove_inaccessible_dir() {
     assert!(!at.dir_exists(dir_1));
 }
 
+#[test]
+#[cfg(not(windows))]
+fn test_rm_current_or_parent_dir_rm4() {
+    let ts = TestScenario::new(util_name!());
+    let at = &ts.fixtures;
+
+    at.mkdir("d");
+
+    let answers = [
+        "rm: refusing to remove '.' or '..' directory: skipping 'd/.'",
+        "rm: refusing to remove '.' or '..' directory: skipping 'd/./'",
+        "rm: refusing to remove '.' or '..' directory: skipping 'd/./'",
+        "rm: refusing to remove '.' or '..' directory: skipping 'd/..'",
+        "rm: refusing to remove '.' or '..' directory: skipping 'd/../'",
+    ];
+    let std_err_str = ts
+        .ucmd()
+        .arg("-rf")
+        .arg("d/.")
+        .arg("d/./")
+        .arg("d/.////")
+        .arg("d/..")
+        .arg("d/../")
+        .fails()
+        .stderr_move_str();
+
+    for (idx, line) in std_err_str.lines().enumerate() {
+        assert_eq!(line, answers[idx]);
+    }
+}
+
+#[test]
+#[cfg(windows)]
+fn test_rm_current_or_parent_dir_rm4_windows() {
+    let ts = TestScenario::new(util_name!());
+    let at = &ts.fixtures;
+
+    at.mkdir("d");
+
+    let answers = [
+        "rm: refusing to remove '.' or '..' directory: skipping 'd\\.'",
+        "rm: refusing to remove '.' or '..' directory: skipping 'd\\.\\'",
+        "rm: refusing to remove '.' or '..' directory: skipping 'd\\.\\'",
+        "rm: refusing to remove '.' or '..' directory: skipping 'd\\..'",
+        "rm: refusing to remove '.' or '..' directory: skipping 'd\\..\\'",
+    ];
+    let std_err_str = ts
+        .ucmd()
+        .arg("-rf")
+        .arg("d\\.")
+        .arg("d\\.\\")
+        .arg("d\\.\\\\\\\\")
+        .arg("d\\..")
+        .arg("d\\..\\")
+        .fails()
+        .stderr_move_str();
+
+    for (idx, line) in std_err_str.lines().enumerate() {
+        assert_eq!(line, answers[idx]);
+    }
+}
+
 #[test]
 #[cfg(not(windows))]
 fn test_fifo_removal() {