cp: ensure --remove-destination doesn't traverse symlinks

* src/cp.c (target_directory_operand): Allow through inaccessible
arguments with -f or --remove.
* doc/coreutils.texi (cp invocation): Clarify that -f doesn't directly
impact the removal of non-traversable symlinks.
* tests/cp/dir-rm-dest.sh: Test the new behavior.
* tests/cp/thru-dangling.sh: Enforce -f behavior wrt symlinks.
* NEWS: Mention the bug fix.
Fixes https://bugs.gnu.org/31335

diff --git a/NEWS b/NEWS
index de028141b7aff65207715c2c4c95c78dd2e832a2..3f34a11094fb8820edae666b126900ec9512b8f3 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -24,6 +24,10 @@ GNU coreutils NEWS                                    -*- outline -*-
   Previously it would have set executable bits on created special files.
   [bug introduced with coreutils-8.20]
 
+  'cp --remove-destination file symlink' now removes the symlink
+  even if it can't be traversed.
+  [bug introduced with --remove-destination in fileutils-4.1.1]
+
   ls no longer truncates the abbreviated month names that have a
   display width between 6 and 12 inclusive.  Previously this would have
   output ambiguous months for Arabic or Catalan locales.

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index cdde136bfb8221ed2dca708673a4620a76c35bab..c8d9bd9798083be499b7a53d6e9feb9967542c3e 100644 (file)
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -8534,7 +8534,8 @@ Equivalent to @option{--no-dereference --preserve=links}.
 When copying without this option and an existing destination file cannot
 be opened for writing, the copy fails.  However, with @option{--force},
 when a destination file cannot be opened, @command{cp} then
-tries to recreate the file by first removing it.
+tries to recreate the file by first removing it.  Note @option{--force}
+alone will not remove symlinks that can't be traversed.
 When this option is combined with
 @option{--link} (@option{-l}) or @option{--symbolic-link}
 (@option{-s}), the destination link is replaced, and unless

diff --git a/src/cp.c b/src/cp.c
index 04ceb8687fd543c6dc1a9c0560be1583c9c709dc..04cbd4b3399f5c4553f5e20b2ddf7cd770e67193 100644 (file)
--- a/src/cp.c
+++ b/src/cp.c
@@ -559,23 +559,27 @@ make_dir_parents_private (char const *const_dir, size_t src_offset,
 
 /* FILE is the last operand of this command.
    Return true if FILE is a directory.
-   But report an error and exit if there is a problem accessing FILE,
-   or if FILE does not exist but would have to refer to an existing
-   directory if it referred to anything at all.
 
-   If the file exists, store the file's status into *ST.
+   Without -f, report an error and exit if FILE exists
+   but can't be accessed.
+
+   If the file exists and is accessible store the file's status into *ST.
    Otherwise, set *NEW_DST.  */
 
 static bool
-target_directory_operand (char const *file, struct stat *st, bool *new_dst)
+target_directory_operand (char const *file, struct stat *st,
+                          bool *new_dst, bool forcing)
 {
   int err = (stat (file, st) == 0 ? 0 : errno);
   bool is_a_dir = !err && S_ISDIR (st->st_mode);
   if (err)
     {
-      if (err != ENOENT)
+      if (err == ENOENT)
+        *new_dst = true;
+      else if (forcing)
+        st->st_mode = 0;  /* clear so we don't enter --backup case below.  */
+      else
         die (EXIT_FAILURE, err, _("failed to access %s"), quoteaf (file));
-      *new_dst = true;
     }
   return is_a_dir;
 }
@@ -590,6 +594,8 @@ do_copy (int n_files, char **file, const char *target_directory,
   struct stat sb;
   bool new_dst = false;
   bool ok = true;
+  bool forcing = x->unlink_dest_before_opening
+                 || x->unlink_dest_after_failed_open;
 
   if (n_files <= !target_directory)
     {
@@ -613,12 +619,14 @@ do_copy (int n_files, char **file, const char *target_directory,
           usage (EXIT_FAILURE);
         }
       /* Update NEW_DST and SB, which may be checked below.  */
-      ignore_value (target_directory_operand (file[n_files -1], &sb, &new_dst));
+      ignore_value (target_directory_operand (file[n_files -1], &sb, &new_dst,
+                                              forcing));
     }
   else if (!target_directory)
     {
       if (2 <= n_files
-          && target_directory_operand (file[n_files - 1], &sb, &new_dst))
+          && target_directory_operand (file[n_files - 1], &sb, &new_dst,
+                                       forcing))
         target_directory = file[--n_files];
       else if (2 < n_files)
         die (EXIT_FAILURE, 0, _("target %s is not a directory"),

diff --git a/tests/cp/dir-rm-dest.sh b/tests/cp/dir-rm-dest.sh
index 1285b15326ecf244f98d7831b2debd295847e645..7de719dc26e006cb21143a2b2b0bbda54993fde2 100755 (executable)
--- a/tests/cp/dir-rm-dest.sh
+++ b/tests/cp/dir-rm-dest.sh
@@ -1,5 +1,5 @@
 #!/bin/sh
-# verify that cp's --remove-destination option works with -R
+# verify cp's --remove-destination option
 
 # Copyright (C) 2000-2018 Free Software Foundation, Inc.
 
@@ -27,4 +27,9 @@ cp -R --remove-destination d e || fail=1
 # ...and again, with an existing destination.
 cp -R --remove-destination d e || fail=1
 
+# verify no ELOOP which was the case in <= 8.29
+ln -s loop loop || framework_failure_
+touch file || framework_failure_
+cp --remove-destination file loop || fail=1
+
 Exit $fail

diff --git a/tests/cp/thru-dangling.sh b/tests/cp/thru-dangling.sh
index e16a473c36a746cdc2de66297a4fc61d7d691a27..8fd452e8143f214b0453d76ea0e8aab3bada98ce 100755 (executable)
--- a/tests/cp/thru-dangling.sh
+++ b/tests/cp/thru-dangling.sh
@@ -27,10 +27,11 @@ echo "cp: not writing through dangling symlink 'dangle'" \
 
 
 # Starting with 6.9.90, this usage fails, by default:
-cp f dangle > err 2>&1 && fail=1
-
-compare exp-err err || fail=1
-test -f no-such && fail=1
+for opt in '' '-f'; do
+  cp $opt f dangle > err 2>&1 && fail=1
+  compare exp-err err || fail=1
+  test -f no-such && fail=1
+done
 
 # But you can set POSIXLY_CORRECT to get the historical behavior.
 env POSIXLY_CORRECT=1 cp f dangle > out 2>&1 || fail=1