rm: rm3 now passes

src/uu/rm/src/rm.rs

@@ -13,10 +13,8 @@ extern crate uucore;
 use clap::{crate_version, Arg, Command};
 use remove_dir_all::remove_dir_all;
 use std::collections::VecDeque;
-use std::fs;
+use std::fs::{self, File, Metadata};
-use std::fs::File;
+use std::io::{stderr, stdin, BufRead, ErrorKind, Write};
-use std::io::ErrorKind;
-use std::io::{stderr, stdin, BufRead, Write};
 use std::ops::BitOr;
 use std::path::{Path, PathBuf};
 use uucore::display::Quotable;
@@ -365,12 +363,7 @@ fn handle_dir(path: &Path, options: &Options) -> bool {
 }
 
 fn remove_dir(path: &Path, options: &Options) -> bool {
-    let response = if options.interactive == InteractiveMode::Always {
+    if prompt_file(path, options, true) {
-        prompt_file(path, true)
-    } else {
-        true
-    };
-    if response {
         if let Ok(mut read_dir) = fs::read_dir(path) {
             if options.dir || options.recursive {
                 if read_dir.next().is_none() {
@@ -415,12 +408,7 @@ fn remove_dir(path: &Path, options: &Options) -> bool {
 }
 
 fn remove_file(path: &Path, options: &Options) -> bool {
-    let response = if options.interactive == InteractiveMode::Always {
+    if prompt_file(path, options, false) {
-        prompt_file(path, false)
-    } else {
-        true
-    };
-    if response && prompt_write_protected(path, false, options) {
         match fs::remove_file(path) {
             Ok(_) => {
                 if options.verbose {
@@ -442,46 +430,122 @@ fn remove_file(path: &Path, options: &Options) -> bool {
     false
 }
 
-fn prompt_write_protected(path: &Path, is_dir: bool, options: &Options) -> bool {
+fn prompt_file(path: &Path, options: &Options, is_dir: bool) -> bool {
+    // If interactive is Never we never want to send prompts
     if options.interactive == InteractiveMode::Never {
         return true;
     }
-    match File::open(path) {
+    // If interactive is Always we want to check if the file is symlink to prompt the right message
-        Ok(_) => true,
+    if options.interactive == InteractiveMode::Always {
-        Err(err) => {
+        if let Ok(metadata) = fs::symlink_metadata(path) {
-            if err.kind() == ErrorKind::PermissionDenied {
+            if metadata.is_symlink() {
-                if is_dir {
+                return prompt(&(format!("remove symbolic link {}? ", path.quote())));
-                    prompt(&(format!("rm: remove write-protected directory {}? ", path.quote())))
+            }
-                } else {
+        }
-                    if fs::metadata(path).unwrap().len() == 0 {
+    }
-                        return prompt(
+    if is_dir {
-                            &(format!(
+        // We can't use metadata.permissions.readonly for directories because it only works on files
-                                "rm: remove write-protected regular empty file {}? ",
+        // So we have to handle wether a directory is writable on not manually
-                                path.quote()
+        if let Ok(metadata) = fs::metadata(path) {
-                            )),
+            handle_writable_directory(path, options, &metadata)
-                        );
+        } else {
+            true
+        }
+    } else {
+        // File::open(path) doesn't open the file in write mode so we need to use file options to open it in also write mode to check if it can written too
+        match File::options().read(true).write(true).open(path) {
+            Ok(file) => {
+                if let Ok(metadata) = file.metadata() {
+                    if metadata.permissions().readonly() {
+                        if metadata.len() == 0 {
+                            prompt(
+                                &(format!(
+                                    "remove write-protected regular empty file {}? ",
+                                    path.quote()
+                                )),
+                            )
+                        } else {
+                            prompt(
+                                &(format!(
+                                    "remove write-protected regular file {}? ",
+                                    path.quote()
+                                )),
+                            )
+                        }
+                    } else if options.interactive == InteractiveMode::Always {
+                        if metadata.len() == 0 {
+                            prompt(&(format!("remove regular empty file {}? ", path.quote())))
+                        } else {
+                            prompt(&(format!("remove file {}? ", path.quote())))
+                        }
+                    } else {
+                        true
                     }
-                    prompt(&(format!("rm: remove write-protected regular file {}? ", path.quote())))
+                } else {
+                    true
+                }
+            }
+            Err(err) => {
+                if err.kind() == ErrorKind::PermissionDenied {
+                    if let Ok(metadata) = fs::metadata(path) {
+                        if metadata.len() == 0 {
+                            prompt(
+                                &(format!(
+                                    "remove write-protected regular empty file {}? ",
+                                    path.quote()
+                                )),
+                            )
+                        } else {
+                            prompt(
+                                &(format!(
+                                    "remove write-protected regular file {}? ",
+                                    path.quote()
+                                )),
+                            )
+                        }
+                    } else {
+                        prompt(&(format!("remove write-protected regular file {}? ", path.quote())))
+                    }
+                } else {
+                    true
                 }
-            } else {
-                true
             }
         }
     }
 }
 
-fn prompt_descend(path: &Path) -> bool {
+// For directories finding if they are writable or not is a hassle. In Unix we can use the built-in rust crate to to check mode bits. But other os don't have something similar afaik
-    prompt(&(format!("rm: descend into directory {}? ", path.quote())))
+#[cfg(unix)]
+fn handle_writable_directory(path: &Path, options: &Options, metadata: &Metadata) -> bool {
+    use std::os::unix::fs::PermissionsExt;
+    let mode = metadata.permissions().mode();
+    let user_write_permission = (mode & 0b1_1100_0000) >> 6;
+    let user_writable = !matches!(user_write_permission, 0o0 | 0o1 | 0o4 | 0o5);
+    if !user_writable {
+        prompt(&(format!("remove write-protected directory {}? ", path.quote())))
+    } else if options.interactive == InteractiveMode::Always {
+        prompt(&(format!("remove directory {}? ", path.quote())))
+    } else {
+        true
+    }
 }
 
-fn prompt_file(path: &Path, is_dir: bool) -> bool {
+// I have this here for completeness but it will always return "remove directory {}" because metadata.permissions().readonly() only works for file not directories
-    if is_dir {
+#[cfg(not(unix))]
-        prompt(&(format!("rm: remove directory {}? ", path.quote())))
+fn handle_writable_directory(path: &Path, options: &Options, metadata: &Metadata) -> bool {
+    if metadata.permissions().readonly() {
+        prompt(&(format!("remove write-protected directory {}? ", path.quote())))
+    } else if options.interactive == InteractiveMode::Always {
+        prompt(&(format!("remove directory {}? ", path.quote())))
     } else {
-        prompt(&(format!("rm: remove file {}? ", path.quote())))
+        true
     }
 }
 
+fn prompt_descend(path: &Path) -> bool {
+    prompt(&(format!("descend into directory {}? ", path.quote())))
+}
+
 fn normalize(path: &Path) -> PathBuf {
     // copied from https://github.com/rust-lang/cargo/blob/2e4cfc2b7d43328b207879228a2ca7d427d188bb/src/cargo/util/paths.rs#L65-L90
     // both projects are MIT https://github.com/rust-lang/cargo/blob/master/LICENSE-MIT
@@ -491,7 +555,7 @@ fn normalize(path: &Path) -> PathBuf {
 }
 
 fn prompt(msg: &str) -> bool {
-    let _ = stderr().write_all(msg.as_bytes());
+    let _ = stderr().write_all(format!("{}: {}", uucore::util_name(), msg).as_bytes());
     let _ = stderr().flush();
 
     let mut buf = Vec::new();
@@ -505,15 +569,13 @@ fn prompt(msg: &str) -> bool {
 }
 
 #[cfg(not(windows))]
-fn is_symlink_dir(_metadata: &fs::Metadata) -> bool {
+fn is_symlink_dir(_metadata: &Metadata) -> bool {
     false
 }
 
 #[cfg(windows)]
-use std::os::windows::prelude::MetadataExt;
+fn is_symlink_dir(metadata: &Metadata) -> bool {
-
+    use std::os::windows::prelude::MetadataExt;
-#[cfg(windows)]
-fn is_symlink_dir(metadata: &fs::Metadata) -> bool {
     use winapi::um::winnt::FILE_ATTRIBUTE_DIRECTORY;
 
     metadata.file_type().is_symlink()

tests/by-util/test_rm.rs

@@ -400,6 +400,109 @@ fn test_rm_descend_directory() {
     assert!(!at.file_exists(file_2));
 }
 
+#[cfg(feature = "chmod")]
+#[test]
+fn test_rm_prompts() {
+    use std::io::Write;
+    use std::process::Child;
+
+    // Needed for talking with stdin on platforms where CRLF or LF matters
+    const END_OF_LINE: &str = if cfg!(windows) { "\r\n" } else { "\n" };
+
+    let mut answers = vec![
+        "rm: descend into directory 'a'?",
+        "rm: remove write-protected regular empty file 'a/empty-no-write'?",
+        "rm: remove symbolic link 'a/slink'?",
+        "rm: remove symbolic link 'a/slink-dot'?",
+        "rm: remove write-protected regular file 'a/f-no-write'?",
+        "rm: remove regular empty file 'a/empty'?",
+        "rm: remove directory 'a/b'?",
+        "rm: remove write-protected directory 'a/b-no-write'?",
+        "rm: remove directory 'a'?",
+    ];
+
+    answers.sort();
+
+    let yes = format!("y{}", END_OF_LINE);
+
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    at.mkdir("a/");
+
+    let file_1 = "a/empty";
+    let file_2 = "a/empty-no-write";
+    let file_3 = "a/f-no-write";
+
+    at.touch(file_1);
+    at.touch(file_2);
+    at.make_file(file_3)
+        .write_all(b"not-empty")
+        .expect("Couldn't write to a/f-no-write");
+
+    at.symlink_dir("a/empty-f", "a/slink");
+    at.symlink_dir(".", "a/slink-dot");
+
+    let dir_1 = "a/b/";
+    let dir_2 = "a/b-no-write/";
+
+    at.mkdir(dir_1);
+    at.mkdir(dir_2);
+
+    scene
+        .ccmd("chmod")
+        .arg("u-w")
+        .arg(file_3)
+        .arg(dir_2)
+        .arg(file_2)
+        .succeeds();
+
+    let mut child: Child = scene.ucmd().arg("-ri").arg("a").run_no_wait();
+
+    let mut child_stdin = child.stdin.take().unwrap();
+    child_stdin.write_all(yes.as_bytes()).unwrap();
+    child_stdin.flush().unwrap();
+    child_stdin.write_all(yes.as_bytes()).unwrap();
+    child_stdin.flush().unwrap();
+    child_stdin.write_all(yes.as_bytes()).unwrap();
+    child_stdin.flush().unwrap();
+    child_stdin.write_all(yes.as_bytes()).unwrap();
+    child_stdin.flush().unwrap();
+    child_stdin.write_all(yes.as_bytes()).unwrap();
+    child_stdin.flush().unwrap();
+    child_stdin.write_all(yes.as_bytes()).unwrap();
+    child_stdin.flush().unwrap();
+    child_stdin.write_all(yes.as_bytes()).unwrap();
+    child_stdin.flush().unwrap();
+    child_stdin.write_all(yes.as_bytes()).unwrap();
+    child_stdin.flush().unwrap();
+    child_stdin.write_all(yes.as_bytes()).unwrap();
+    child_stdin.flush().unwrap();
+
+    let output = child.wait_with_output().unwrap();
+
+    let mut trimmed_output = Vec::new();
+    for string in String::from_utf8(output.stderr)
+        .expect("Couldn't convert output.stderr to string")
+        .split("rm: ")
+    {
+        if !string.is_empty() {
+            let trimmed_string = format!("rm: {}", string).trim().to_string();
+            trimmed_output.push(trimmed_string);
+        }
+    }
+
+    trimmed_output.sort();
+
+    assert!(trimmed_output.len() == answers.len());
+
+    for (i, checking_string) in trimmed_output.iter().enumerate() {
+        assert!(checking_string == answers[i]);
+    }
+
+    assert!(!at.dir_exists("a"));
+}
+
 #[test]
 #[ignore = "issue #3722"]
 fn test_rm_directory_rights_rm1() {