On 01/05/18 11:14, Ernesto Alfonso wrote: > > cp -f fails when destination is a recursive symlink: > > $ ln -s self self > $ cat self > self: Too many levels of symbolic links > $ touch a > $ cp a self > cp: failed to access 'self': Too many levels of symbolic links > $ cp -f a self > cp: failed to access 'self': Too many levels of symbolic links > > >>From the man page: > > -f, --force > if an existing destination file cannot be opened, remove it > and try again (this option is ignored when > the -n option is also used)
Note cp will still write through symlinks with -f. For example with dangling symlinks one gets: cp: not writing through dangling symlink '...' I.E. -f currently only removes the symlink if the destination exists but can't be opened. This looks to be an explicit decision which I'd be reluctant to change. I've added a test and some docs to make this apparent. Now there is also the --remove-destination option which is a more explicit request to always delete the destination first. Would that suffice? Note that doesn't even work currently, so the attached enables that, so that command line args are treated similarly to traversed files. cheers, Pádraig
>From fb35d840935e7927f6d9f018d36fe544f6e376df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]> 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
