>From fb35d840935e7927f6d9f018d36fe544f6e376df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= Date: Thu, 3 May 2018 21:19:15 -0700 Subject: [PATCH] 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 --- NEWS | 4 ++++ doc/coreutils.texi | 3 ++- src/cp.c | 26 +++++++++++++++++--------- tests/cp/dir-rm-dest.sh | 7 ++++++- tests/cp/thru-dangling.sh | 9 +++++---- 5 files changed, 34 insertions(+), 15 deletions(-) diff --git a/NEWS b/NEWS index de02814..3f34a11 100644 --- 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 cdde136..c8d9bd9 100644 --- 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 04ceb868..04cbd4b 100644 --- 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 1285b15..7de719d 100755 --- 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 e16a473..8fd452e 100755 --- 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 -- 2.9.3