Skip to content

Commit e755c86

Browse files
sylvestresylvestre
committed
rm: on linux use the safe traversal in all cases
1 parent 8021acb commit e755c86

File tree

1 file changed

+99
-97
lines changed

1 file changed

+99
-97
lines changed

‎src/uu/rm/src/rm.rs‎

Lines changed: 99 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,7 @@ fn is_readable_metadata(metadata: &Metadata) -> bool {
395395

396396
/// Whether the given file or directory is readable.
397397
#[cfg(unix)]
398+
#[cfg(not(target_os = "linux"))]
398399
fn is_readable(path: &Path) -> bool {
399400
match fs::metadata(path) {
400401
Err(_) => false,
@@ -436,11 +437,29 @@ fn safe_remove_dir_recursive(path: &Path, options: &Options) -> bool {
436437
let dir_fd = match DirFd::open(path) {
437438
Ok(fd) => fd,
438439
Err(e) => {
439-
show_error!(
440-
"{}",
441-
e.map_err_context(|| translate!("rm-error-cannot-remove", "file" => path.quote()))
442-
);
443-
return true;
440+
// If we can't open the directory for safe traversal, try removing it as empty directory
441+
// This handles the case where it's an empty directory with no read permissions
442+
match fs::remove_dir(path) {
443+
Ok(_) => {
444+
if options.verbose {
445+
println!(
446+
"{}",
447+
translate!("rm-verbose-removed-directory", "file" => normalize(path).quote())
448+
);
449+
}
450+
return false;
451+
}
452+
Err(_) => {
453+
// If we can't remove it as empty dir either, report the original open error
454+
show_error!(
455+
"{}",
456+
e.map_err_context(
457+
|| translate!("rm-error-cannot-remove", "file" => path.quote())
458+
)
459+
);
460+
return true;
461+
}
462+
}
444463
}
445464
};
446465

@@ -457,28 +476,35 @@ fn safe_remove_dir_recursive(path: &Path, options: &Options) -> bool {
457476

458477
// Use regular fs::remove_dir for the root since we can't unlinkat ourselves
459478
match fs::remove_dir(path) {
460-
Ok(_) => false,
461-
Err(e) => {
479+
Ok(_) => {
480+
if options.verbose {
481+
println!(
482+
"{}",
483+
translate!("rm-verbose-removed-directory", "file" => normalize(path).quote())
484+
);
485+
}
486+
false
487+
}
488+
Err(e) if !error => {
462489
let e = e.map_err_context(
463490
|| translate!("rm-error-cannot-remove", "file" => path.quote()),
464491
);
465492
show_error!("{e}");
466493
true
467494
}
495+
Err(_) => {
496+
// If there has already been at least one error when
497+
// trying to remove the children, then there is no need to
498+
// show another error message as we return from each level
499+
// of the recursion.
500+
error
501+
}
468502
}
469503
}
470504
}
471505

472506
#[cfg(target_os = "linux")]
473507
fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Options) -> bool {
474-
// Check if we should descend into this directory
475-
if options.interactive == InteractiveMode::Always
476-
&& !is_dir_empty(path)
477-
&& !prompt_descend(path)
478-
{
479-
return false;
480-
}
481-
482508
// Read directory entries using safe traversal
483509
let entries = match dir_fd.read_dir() {
484510
Ok(entries) => entries,
@@ -518,35 +544,9 @@ fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Options
518544
let is_dir = (entry_stat.st_mode & libc::S_IFMT) == libc::S_IFDIR;
519545

520546
if is_dir {
521-
// Recursively remove directory
522-
let subdir_fd = match dir_fd.open_subdir(&entry_name) {
523-
Ok(fd) => fd,
524-
Err(e) => {
525-
let e = e.map_err_context(
526-
|| translate!("rm-error-cannot-remove", "file" => entry_path.quote()),
527-
);
528-
show_error!("{e}");
529-
error = true;
530-
continue;
531-
}
532-
};
533-
534-
let child_error = safe_remove_dir_recursive_impl(&entry_path, &subdir_fd, options);
547+
// Recursively remove subdirectory - handle in the style of the non-Linux version
548+
let child_error = remove_dir_recursive(&entry_path, options);
535549
error = error || child_error;
536-
537-
// Try to remove the directory (even if there were some child errors)
538-
// Ask user permission if needed
539-
if options.interactive == InteractiveMode::Always && !prompt_dir(&entry_path, options) {
540-
continue;
541-
}
542-
543-
if let Err(e) = dir_fd.unlink_at(&entry_name, true) {
544-
let e = e.map_err_context(
545-
|| translate!("rm-error-cannot-remove", "file" => entry_path.quote()),
546-
);
547-
show_error!("{e}");
548-
error = true;
549-
}
550550
} else {
551551
// Remove file - check if user wants to remove it first
552552
if prompt_file(&entry_path, options) {
@@ -556,6 +556,11 @@ fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Options
556556
);
557557
show_error!("{e}");
558558
error = true;
559+
} else if options.verbose {
560+
println!(
561+
"{}",
562+
translate!("rm-verbose-removed", "file" => normalize(&entry_path).quote())
563+
);
559564
}
560565
}
561566
}
@@ -590,17 +595,13 @@ fn remove_dir_recursive(path: &Path, options: &Options) -> bool {
590595
return false;
591596
}
592597

593-
// Use secure traversal on Linux for long paths
598+
// Use secure traversal on Linux for all recursive directory removals
594599
#[cfg(target_os = "linux")]
595600
{
596-
if let Some(s) = path.to_str() {
597-
if s.len() > 1000 {
598-
return safe_remove_dir_recursive(path, options);
599-
}
600-
}
601+
safe_remove_dir_recursive(path, options)
601602
}
602603

603-
// Fallback for non-Linux or shorter paths
604+
// Fallback for non-Linux or use fs::remove_dir_all for very long paths
604605
#[cfg(not(target_os = "linux"))]
605606
{
606607
if let Some(s) = path.to_str() {
@@ -617,62 +618,63 @@ fn remove_dir_recursive(path: &Path, options: &Options) -> bool {
617618
}
618619
}
619620
}
620-
}
621621

622-
// Recursive case: this is a directory.
623-
let mut error = false;
624-
match fs::read_dir(path) {
625-
Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => {
626-
// This is not considered an error.
627-
}
628-
Err(_) => error = true,
629-
Ok(iter) => {
630-
for entry in iter {
631-
match entry {
632-
Err(_) => error = true,
633-
Ok(entry) => {
634-
let child_error = remove_dir_recursive(&entry.path(), options);
635-
error = error || child_error;
622+
// Recursive case: this is a directory.
623+
let mut error = false;
624+
match fs::read_dir(path) {
625+
Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => {
626+
// This is not considered an error.
627+
}
628+
Err(_) => error = true,
629+
Ok(iter) => {
630+
for entry in iter {
631+
match entry {
632+
Err(_) => error = true,
633+
Ok(entry) => {
634+
let child_error = remove_dir_recursive(&entry.path(), options);
635+
error = error || child_error;
636+
}
636637
}
637638
}
638639
}
639640
}
640-
}
641641

642-
// Ask the user whether to remove the current directory.
643-
if options.interactive == InteractiveMode::Always && !prompt_dir(path, options) {
644-
return false;
645-
}
646-
647-
// Try removing the directory itself.
648-
match fs::remove_dir(path) {
649-
Err(_) if !error && !is_readable(path) => {
650-
// For compatibility with GNU test case
651-
// `tests/rm/unread2.sh`, show "Permission denied" in this
652-
// case instead of "Directory not empty".
653-
show_error!("cannot remove {}: Permission denied", path.quote());
654-
error = true;
655-
}
656-
Err(e) if !error => {
657-
let e =
658-
e.map_err_context(|| translate!("rm-error-cannot-remove", "file" => path.quote()));
659-
show_error!("{e}");
660-
error = true;
642+
// Ask the user whether to remove the current directory.
643+
if options.interactive == InteractiveMode::Always && !prompt_dir(path, options) {
644+
return false;
661645
}
662-
Err(_) => {
663-
// If there has already been at least one error when
664-
// trying to remove the children, then there is no need to
665-
// show another error message as we return from each level
666-
// of the recursion.
646+
647+
// Try removing the directory itself.
648+
match fs::remove_dir(path) {
649+
Err(_) if !error && !is_readable(path) => {
650+
// For compatibility with GNU test case
651+
// `tests/rm/unread2.sh`, show "Permission denied" in this
652+
// case instead of "Directory not empty".
653+
show_error!("cannot remove {}: Permission denied", path.quote());
654+
error = true;
655+
}
656+
Err(e) if !error => {
657+
let e = e.map_err_context(
658+
|| translate!("rm-error-cannot-remove", "file" => path.quote()),
659+
);
660+
show_error!("{e}");
661+
error = true;
662+
}
663+
Err(_) => {
664+
// If there has already been at least one error when
665+
// trying to remove the children, then there is no need to
666+
// show another error message as we return from each level
667+
// of the recursion.
668+
}
669+
Ok(_) if options.verbose => println!(
670+
"{}",
671+
translate!("rm-verbose-removed-directory", "file" => normalize(path).quote())
672+
),
673+
Ok(_) => {}
667674
}
668-
Ok(_) if options.verbose => println!(
669-
"{}",
670-
translate!("rm-verbose-removed-directory", "file" => normalize(path).quote())
671-
),
672-
Ok(_) => {}
673-
}
674675

675-
error
676+
error
677+
}
676678
}
677679

678680
fn handle_dir(path: &Path, options: &Options) -> bool {

0 commit comments

Comments
 (0)