rm: --dir (-d): fix bugs in handling of empty, inaccessible directories

* src/remove.c (prompt, rm_fts): In the dir-handling code of both of
these functions, relax a "get_dir_status (...) == DS_EMPTY" condition
to instead test only "get_dir_status (...) != 0", enabling flow control
to reach the prompt function also for unreadable directories. However,
that function itself also needed special handling for this case:
(prompt): Handle empty, inaccessible directories properly,
deleting them with -d (--dir), and prompting about whether to delete
with -i (--interactive).
* tests/rm/empty-inacc.sh: Add tests for the new code.
Reported by наб <nabijaczleweli@nabijaczleweli.xyz> in
bugs.debian.org/1015273
* NEWS (Bug fixes): Mention this.

diff --git a/NEWS b/NEWS
index d66ea0f6fb31a9ee4b846138775cf4028a5d80a7..a3250161f76800099a676b32f5e89bf3c626b008 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -36,6 +36,10 @@ GNU coreutils NEWS                                    -*- outline -*-
   'mv --backup=simple f d/' no longer mistakenly backs up d/f to f~.
   [bug introduced in coreutils-9.1]
 
+  rm -d (--dir) now properly handles unreadable empty directories.
+  E.g., before, this would fail to remove d: mkdir -m0 d; src/rm -d d
+  [bug introduced in v8.19 with the addition of this option]
+
   runcon --compute no longer looks up the specified command in the $PATH
   so that there is no mismatch between the inspected and executed file.
   [bug introduced when runcon was introduced in coreutils-6.9.90]

diff --git a/src/remove.c b/src/remove.c
index a1603722e9e643f03d5745ce30d2d9924acfd12b..e1da19e38b86e5b516fc62fcb6d6705fc3b2d293 100644 (file)
--- a/src/remove.c
+++ b/src/remove.c
@@ -212,7 +212,7 @@ prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
 
   int wp_errno = 0;
   if (!x->ignore_missing_files
-      && ((x->interactive == RMI_ALWAYS) || x->stdin_tty)
+      && (x->interactive == RMI_ALWAYS || x->stdin_tty)
       && dirent_type != DT_LNK)
     {
       write_protected = write_protected_non_symlink (fd_cwd, filename, sbuf);
@@ -254,7 +254,7 @@ prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
                 prompting the user  */
             if ( ! (x->recursive
                     || (x->remove_empty_directories
-                        && get_dir_status (fts, ent, dir_status) == DS_EMPTY)))
+                        && get_dir_status (fts, ent, dir_status) != 0)))
               {
                 write_protected = -1;
                 wp_errno = *dir_status <= 0 ? EISDIR : *dir_status;
@@ -281,8 +281,23 @@ prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
                  program_name, quoted_name);
       else if (0 < *dir_status)
         {
-          error (0, *dir_status, _("cannot remove %s"), quoted_name);
-          return RM_ERROR;
+          if ( ! (x->remove_empty_directories && *dir_status == EACCES))
+            {
+              error (0, *dir_status, _("cannot remove %s"), quoted_name);
+              return RM_ERROR;
+            }
+
+          /* The following code can lead to a successful deletion only with
+             the --dir (-d) option (remove_empty_directories) and an empty
+             inaccessible directory. In the first prompt call for a directory,
+             we'd normally ask whether to descend into it, but in this case
+             (it's inaccessible), that is not possible, so don't prompt.  */
+          if (mode == PA_DESCEND_INTO_DIR)
+            return RM_OK;
+
+          fprintf (stderr,
+               _("%s: attempt removal of inaccessible directory %s? "),
+                   program_name, quoted_name);
         }
       else
         {
@@ -434,7 +449,7 @@ rm_fts (FTS *fts, FTSENT *ent, struct rm_options const *x)
     case FTS_D:                        /* preorder directory */
       if (! x->recursive
           && !(x->remove_empty_directories
-               && get_dir_status (fts, ent, &dir_status) == DS_EMPTY))
+               && get_dir_status (fts, ent, &dir_status) != 0))
         {
           /* This is the first (pre-order) encounter with a directory
              that we cannot delete.

diff --git a/tests/rm/empty-inacc.sh b/tests/rm/empty-inacc.sh
index 5d61815095f839520523d4ca914bb8c64a183289..807ff4d1d58bcfdc0b6f45033eb7e8fc96ced7cf 100755 (executable)
--- a/tests/rm/empty-inacc.sh
+++ b/tests/rm/empty-inacc.sh
@@ -24,8 +24,7 @@ mkdir -m0 inacc || framework_failure_
 
 # Also exercise the different code path that's taken for a directory
 # that is empty (hence removable) and unreadable.
-mkdir -m a-r -p a/unreadable
-
+mkdir -m a-r -p a/unreadable || framework_failure_
 
 # This would fail for e.g., coreutils-5.93.
 rm -rf inacc || fail=1
@@ -35,4 +34,28 @@ test -d inacc && fail=1
 rm -rf a || fail=1
 test -d a && fail=1
 
+# Ensure that using rm -d removes an unreadable empty directory.
+mkdir -m a-r unreadable2 || framework_failure_
+mkdir -m0 inacc2 || framework_failure_
+
+# These would fail for coreutils-9.1 and prior.
+rm -d unreadable2 || fail=1
+test -d unreadable2 && fail=1
+rm -d inacc2 || fail=1
+test -d inacc2 && fail=1
+
+# Test the interactive code paths that are new with 9.2:
+mkdir -m0 inacc2 || framework_failure_
+
+echo n | rm ---presume-input-tty -di inacc2 > out 2>&1 || fail=1
+# decline: ensure it was not deleted, and the prompt was as expected.
+printf "rm: attempt removal of inaccessible directory 'inacc2'? " > exp
+test -d inacc2 || fail=1
+compare exp out || fail=1
+
+echo y | rm ---presume-input-tty -di inacc2 > out 2>&1 || fail=1
+# accept: ensure it **was** deleted, and the prompt was as expected.
+test -d inacc2 && fail=1
+compare exp out || fail=1
+
 Exit $fail