Skip to content
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
19 changes: 18 additions & 1 deletion src/uu/rm/src/rm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,24 @@ fn remove_dir_recursive(
// a directory and we don't want to recurse. In particular, this
// avoids an infinite recursion in the case of a link to the current
// directory, like `ln -s . link`.
if !path.is_dir() || path.is_symlink() {
//
// On Windows, a symbolic link to a directory (a directory reparse
// point) cannot be removed with the file-deletion API used by
// `remove_file` -> it fails with `ERROR_ACCESS_DENIED`. Route such
// links through `remove_dir` (the directory-removal API), which
// removes the reparse point itself instead of following the link.
// This mirrors how the top-level `remove` already handles directory
Comment on lines +669 to +675

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please make the comment shorter

// symlinks. See microsoft/coreutils#84.
if path.is_symlink() {
#[cfg(windows)]
if let Ok(metadata) = fs::symlink_metadata(path) {
if is_symlink_dir(&metadata) {
return remove_dir(path, options, progress_bar);
}
}
return remove_file(path, options, progress_bar);
}
if !path.is_dir() {
return remove_file(path, options, progress_bar);
}

Expand Down
27 changes: 27 additions & 0 deletions tests/by-util/test_rm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,33 @@
scene.ucmd().arg("-r").arg(link).succeeds();
}

#[test]
fn test_recursive_removes_directory_symlink_in_tree() {
// `rm -r` over a tree that contains a symbolic link to a directory must
// remove the link itself, not follow it, and leave the target intact.
// On Windows this previously failed with "Permission denied" because
// directory symlinks were routed through the file-deletion API
// (DeleteFileW) instead of the directory-removal API (RemoveDirectoryW).
let (at, mut ucmd) = at_and_ucmd!();

let target = "test_rm_dir_symlink_in_tree_target";
let tree = "test_rm_dir_symlink_in_tree";
let link = "test_rm_dir_symlink_in_tree/link";

at.mkdir(target);
at.touch(format!("{target}/keepme"));
at.mkdir(tree);
at.symlink_dir(target, link);

ucmd.arg("-r").arg(tree).succeeds().no_stderr();

assert!(!at.dir_exists(tree));
// The link is gone (it lived inside `tree`), but the directory it pointed
// at must be untouched.
assert!(at.dir_exists(target));
assert!(at.file_exists(&format!("{target}/keepme")));

Check failure on line 488 in tests/by-util/test_rm.rs

View workflow job for this annotation

GitHub Actions / Style and Lint (unix)

ERROR: `cargo clippy`: the borrowed expression implements the required traits (file:'tests/by-util/test_rm.rs', line:488)
}

#[test]
fn test_invalid_symlink() {
let (at, mut ucmd) = at_and_ucmd!();
Expand Down
Loading