Skip to content

Commit 4fe6b36

Browse files
sylvestresylvestre
committed
rm: on linux use the safe traversal in all cases
1 parent e105eb5 commit 4fe6b36

File tree

1 file changed

+139
-82
lines changed

1 file changed

+139
-82
lines changed

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

Lines changed: 139 additions & 82 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,16 +544,31 @@ 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
547+
// Recursively remove directory by opening it first
522548
let subdir_fd = match dir_fd.open_subdir(&entry_name) {
523549
Ok(fd) => fd,
524550
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;
551+
// If we can't open the subdirectory, try removing it directly as empty directory
552+
match fs::remove_dir(&entry_path) {
553+
Ok(_) => {
554+
if options.verbose {
555+
println!(
556+
"{}",
557+
translate!("rm-verbose-removed-directory", "file" => normalize(&entry_path).quote())
558+
);
559+
}
560+
continue; // Successfully removed, move to next entry
561+
}
562+
Err(_) => {
563+
// If we can't remove it as empty dir either, report the original open error
564+
let e = e.map_err_context(
565+
|| translate!("rm-error-cannot-remove", "file" => entry_path.quote()),
566+
);
567+
show_error!("{e}");
568+
error = true;
569+
continue;
570+
}
571+
}
531572
}
532573
};
533574

@@ -540,12 +581,26 @@ fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Options
540581
continue;
541582
}
542583

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;
584+
match dir_fd.unlink_at(&entry_name, true) {
585+
Err(e) if !child_error => {
586+
let e = e.map_err_context(
587+
|| translate!("rm-error-cannot-remove", "file" => entry_path.quote()),
588+
);
589+
show_error!("{e}");
590+
error = true;
591+
}
592+
Err(_) => {
593+
// If there were child errors, suppress directory removal errors
594+
error = true;
595+
}
596+
Ok(_) => {
597+
if options.verbose {
598+
println!(
599+
"{}",
600+
translate!("rm-verbose-removed-directory", "file" => normalize(&entry_path).quote())
601+
);
602+
}
603+
}
549604
}
550605
} else {
551606
// Remove file - check if user wants to remove it first
@@ -556,6 +611,11 @@ fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Options
556611
);
557612
show_error!("{e}");
558613
error = true;
614+
} else if options.verbose {
615+
println!(
616+
"{}",
617+
translate!("rm-verbose-removed", "file" => normalize(&entry_path).quote())
618+
);
559619
}
560620
}
561621
}
@@ -590,17 +650,13 @@ fn remove_dir_recursive(path: &Path, options: &Options) -> bool {
590650
return false;
591651
}
592652

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

603-
// Fallback for non-Linux or shorter paths
659+
// Fallback for non-Linux or use fs::remove_dir_all for very long paths
604660
#[cfg(not(target_os = "linux"))]
605661
{
606662
if let Some(s) = path.to_str() {
@@ -617,62 +673,63 @@ fn remove_dir_recursive(path: &Path, options: &Options) -> bool {
617673
}
618674
}
619675
}
620-
}
621676

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;
677+
// Recursive case: this is a directory.
678+
let mut error = false;
679+
match fs::read_dir(path) {
680+
Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => {
681+
// This is not considered an error.
682+
}
683+
Err(_) => error = true,
684+
Ok(iter) => {
685+
for entry in iter {
686+
match entry {
687+
Err(_) => error = true,
688+
Ok(entry) => {
689+
let child_error = remove_dir_recursive(&entry.path(), options);
690+
error = error || child_error;
691+
}
636692
}
637693
}
638694
}
639695
}
640-
}
641696

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;
697+
// Ask the user whether to remove the current directory.
698+
if options.interactive == InteractiveMode::Always && !prompt_dir(path, options) {
699+
return false;
661700
}
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.
701+
702+
// Try removing the directory itself.
703+
match fs::remove_dir(path) {
704+
Err(_) if !error && !is_readable(path) => {
705+
// For compatibility with GNU test case
706+
// `tests/rm/unread2.sh`, show "Permission denied" in this
707+
// case instead of "Directory not empty".
708+
show_error!("cannot remove {}: Permission denied", path.quote());
709+
error = true;
710+
}
711+
Err(e) if !error => {
712+
let e = e.map_err_context(
713+
|| translate!("rm-error-cannot-remove", "file" => path.quote()),
714+
);
715+
show_error!("{e}");
716+
error = true;
717+
}
718+
Err(_) => {
719+
// If there has already been at least one error when
720+
// trying to remove the children, then there is no need to
721+
// show another error message as we return from each level
722+
// of the recursion.
723+
}
724+
Ok(_) if options.verbose => println!(
725+
"{}",
726+
translate!("rm-verbose-removed-directory", "file" => normalize(path).quote())
727+
),
728+
Ok(_) => {}
667729
}
668-
Ok(_) if options.verbose => println!(
669-
"{}",
670-
translate!("rm-verbose-removed-directory", "file" => normalize(path).quote())
671-
),
672-
Ok(_) => {}
673-
}
674730

675-
error
731+
error
732+
}
676733
}
677734

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

0 commit comments

Comments
 (0)