Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rm: r4_gnu_test #6621

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions .cargo/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@ linker = "x86_64-unknown-redox-gcc"

[target.'cfg(clippy)']
rustflags = [
"-Wclippy::use_self",
"-Wclippy::needless_pass_by_value",
"-Wclippy::semicolon_if_nothing_returned",
"-Wclippy::single_char_pattern",
"-Wclippy::explicit_iter_loop",
"-Wclippy::if_not_else",
"-Wclippy::use_self",
"-Wclippy::needless_pass_by_value",
"-Wclippy::semicolon_if_nothing_returned",
"-Wclippy::single_char_pattern",
"-Wclippy::explicit_iter_loop",
"-Wclippy::if_not_else",
]

2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

88 changes: 85 additions & 3 deletions src/uu/rm/src/rm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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() {
Expand All @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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]);
}
AnirbanHalder654322 marked this conversation as resolved.
Show resolved Hide resolved
false
}

// For windows we can use windows metadata trait and file attributes to see if a directory is readonly
#[cfg(windows)]
Expand Down Expand Up @@ -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())
}
Expand All @@ -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 {

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
use super::*;

This way, you don't have to use crate::foo_bar; everything in each test again and again.

#[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));
}
}
62 changes: 62 additions & 0 deletions tests/by-util/test_rm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,68 @@ fn test_remove_inaccessible_dir() {
assert!(!at.dir_exists(dir_1));
}

#[test]
#[cfg(not(windows))]
AnirbanHalder654322 marked this conversation as resolved.
Show resolved Hide resolved
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() {
Expand Down