diff --git a/Cargo.lock b/Cargo.lock index 0586e8f62..03eaa7abb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2818,6 +2818,7 @@ name = "uu_rm" version = "0.0.16" dependencies = [ "clap 4.0.18", + "libc", "remove_dir_all 0.7.0", "uucore", "walkdir", diff --git a/src/uu/rm/Cargo.toml b/src/uu/rm/Cargo.toml index 9d372a5f5..3cd49b11b 100644 --- a/src/uu/rm/Cargo.toml +++ b/src/uu/rm/Cargo.toml @@ -20,6 +20,9 @@ walkdir = "2.2" remove_dir_all = "0.7.0" uucore = { version=">=0.0.16", package="uucore", path="../../uucore", features=["fs"] } +[target.'cfg(unix)'.dependencies] +libc = "0.2.137" + [target.'cfg(windows)'.dependencies] windows-sys = { version = "0.42.0", default-features = false, features = ["Win32_Storage_FileSystem"] } diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index dcd457aa6..f65ebb2fa 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -10,13 +10,11 @@ #[macro_use] extern crate uucore; -use clap::{crate_version, Arg, ArgAction, Command}; +use clap::{crate_version, parser::ValueSource, Arg, ArgAction, Command}; use remove_dir_all::remove_dir_all; use std::collections::VecDeque; -use std::fs; -use std::fs::File; -use std::io::ErrorKind; -use std::io::{stderr, stdin, BufRead, Write}; +use std::fs::{self, File, Metadata}; +use std::io::{stderr, stdin, BufRead, ErrorKind, Write}; use std::ops::BitOr; use std::path::{Path, PathBuf}; use uucore::display::Quotable; @@ -87,17 +85,30 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .map(|v| v.map(ToString::to_string).collect()) .unwrap_or_default(); - let force = matches.get_flag(OPT_FORCE); + let force_flag = matches.get_flag(OPT_FORCE); - if files.is_empty() && !force { + // If -f(--force) is before any -i (or variants) we want prompts else no prompts + let force_prompt_never: bool = force_flag && { + let force_index = matches.index_of(OPT_FORCE).unwrap_or(0); + ![OPT_PROMPT, OPT_PROMPT_MORE, OPT_INTERACTIVE] + .iter() + .any(|flag| { + matches.value_source(flag) == Some(ValueSource::CommandLine) + && matches.index_of(flag).unwrap_or(0) > force_index + }) + }; + + if files.is_empty() && !force_flag { // Still check by hand and not use clap // Because "rm -f" is a thing return Err(UUsageError::new(1, "missing operand")); } else { let options = Options { - force, + force: force_flag, interactive: { - if matches.get_flag(OPT_PROMPT) { + if force_prompt_never { + InteractiveMode::Never + } else if matches.get_flag(OPT_PROMPT) { InteractiveMode::Always } else if matches.get_flag(OPT_PROMPT_MORE) { InteractiveMode::Once @@ -159,12 +170,17 @@ pub fn uu_app() -> Command { Arg::new(OPT_PROMPT) .short('i') .help("prompt before every removal") + .overrides_with_all(&[OPT_PROMPT_MORE, OPT_INTERACTIVE]) + .action(ArgAction::SetTrue), + ) + .arg( + Arg::new(OPT_PROMPT_MORE) + .short('I') + .help("prompt once before removing more than three files, or when removing recursively. \ + Less intrusive than -i, while still giving some protection against most mistakes") + .overrides_with_all(&[OPT_PROMPT, OPT_INTERACTIVE]) .action(ArgAction::SetTrue), ) - .arg(Arg::new(OPT_PROMPT_MORE).short('I').help( - "prompt once before removing more than three files, or when removing recursively. \ - Less intrusive than -i, while still giving some protection against most mistakes", - ).action(ArgAction::SetTrue)) .arg( Arg::new(OPT_INTERACTIVE) .long(OPT_INTERACTIVE) @@ -172,7 +188,8 @@ pub fn uu_app() -> Command { "prompt according to WHEN: never, once (-I), or always (-i). Without WHEN, \ prompts always", ) - .value_name("WHEN"), + .value_name("WHEN") + .overrides_with_all(&[OPT_PROMPT, OPT_PROMPT_MORE]), ) .arg( Arg::new(OPT_ONE_FILE_SYSTEM) @@ -361,12 +378,7 @@ fn handle_dir(path: &Path, options: &Options) -> bool { } fn remove_dir(path: &Path, options: &Options) -> bool { - let response = if options.interactive == InteractiveMode::Always { - prompt_file(path, true) - } else { - true - }; - if response { + if prompt_file(path, options, true) { if let Ok(mut read_dir) = fs::read_dir(path) { if options.dir || options.recursive { if read_dir.next().is_none() { @@ -411,12 +423,7 @@ fn remove_dir(path: &Path, options: &Options) -> bool { } fn remove_file(path: &Path, options: &Options) -> bool { - let response = if options.interactive == InteractiveMode::Always { - prompt_file(path, false) - } else { - true - }; - if response && prompt_write_protected(path, false, options) { + if prompt_file(path, options, false) { match fs::remove_file(path) { Ok(_) => { if options.verbose { @@ -438,46 +445,138 @@ 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) { - Ok(_) => true, - Err(err) => { - if err.kind() == ErrorKind::PermissionDenied { - if is_dir { - prompt(&(format!("rm: remove write-protected directory {}? ", path.quote()))) - } else { - if fs::metadata(path).unwrap().len() == 0 { - return prompt( - &(format!( - "rm: remove write-protected regular empty file {}? ", - path.quote() - )), - ); + // If interactive is Always we want to check if the file is symlink to prompt the right message + if options.interactive == InteractiveMode::Always { + if let Ok(metadata) = fs::symlink_metadata(path) { + if metadata.is_symlink() { + return prompt(&(format!("remove symbolic link {}? ", path.quote()))); + } + } + } + if is_dir { + // We can't use metadata.permissions.readonly for directories because it only works on files + // So we have to handle wether a directory is writable on not manually + 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 { - prompt(&(format!("rm: descend into directory {}? ", path.quote()))) +// 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 +// Most cases are covered by keep eye out for edge cases +#[cfg(unix)] +fn handle_writable_directory(path: &Path, options: &Options, metadata: &Metadata) -> bool { + use std::os::unix::fs::PermissionsExt; + let mode = metadata.permissions().mode(); + // Check if directory has user write permissions + // Why is S_IWUSR showing up as a u16 on macos? + let user_writable = (mode & (libc::S_IWUSR as u32)) != 0; + 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 { - if is_dir { - prompt(&(format!("rm: remove directory {}? ", path.quote()))) +// For windows we can use windows metadata trait and file attributes to see if a directory is readonly +#[cfg(windows)] +fn handle_writable_directory(path: &Path, options: &Options, metadata: &Metadata) -> bool { + use std::os::windows::prelude::MetadataExt; + use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_READONLY; + let not_user_writable = (metadata.file_attributes() & FILE_ATTRIBUTE_READONLY) != 0; + if not_user_writable { + 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 } } +// I have this here for completeness but it will always return "remove directory {}" because metadata.permissions().readonly() only works for file not directories +#[cfg(not(windows))] +#[cfg(not(unix))] +fn handle_writable_directory(path: &Path, options: &Options, metadata: &Metadata) -> bool { + if options.interactive == InteractiveMode::Always { + prompt(&(format!("remove directory {}? ", path.quote()))) + } else { + 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 @@ -487,7 +586,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(); @@ -501,15 +600,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; - -#[cfg(windows)] -fn is_symlink_dir(metadata: &fs::Metadata) -> bool { +fn is_symlink_dir(metadata: &Metadata) -> bool { + use std::os::windows::prelude::MetadataExt; use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_DIRECTORY; metadata.file_type().is_symlink() diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs index b600c2090..98273fedd 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -400,6 +400,151 @@ 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] +fn test_rm_force_prompts_order() { + 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 yes = format!("y{}", END_OF_LINE); + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let empty_file = "empty"; + + at.touch(empty_file); + + // This should cause rm to prompt to remove regular empty file + let mut child: Child = scene.ucmd().arg("-fi").arg(empty_file).run_no_wait(); + + let mut child_stdin = child.stdin.take().unwrap(); + child_stdin.write_all(yes.as_bytes()).unwrap(); + child_stdin.flush().unwrap(); + + let output = child.wait_with_output().unwrap(); + let string_output = + String::from_utf8(output.stderr).expect("Couldn't convert output.stderr to string"); + assert!(string_output.trim() == "rm: remove regular empty file 'empty'?"); + assert!(!at.file_exists(empty_file)); + + at.touch(empty_file); + + // This should not cause rm to prompt to remove regular empty file + scene + .ucmd() + .arg("-if") + .arg(empty_file) + .succeeds() + .no_stderr(); + assert!(!at.file_exists(empty_file)); +} + #[test] #[ignore = "issue #3722"] fn test_rm_directory_rights_rm1() {