bug#31335: unexpected cp -f behavior

[Pádraig Brady]

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?= 
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 

bug#31184: tail -f on Network FS not refreshing as soon as the file is changed.

[Pádraig Brady]

On 02/05/18 06:39, Jewsco Pius Jacquez wrote:
> I tried and the --follow=name doesn't refresh either. The stat output is 
> changing.
> 
> [jjacquez@cmilsbtest03 ~]$ tail --follow=name /media/samba/test.file
> append Wed May  2 08:35:54 CDT 2018
> append Wed May  2 08:35:54 CDT 2018
> append Wed May  2 08:35:54 CDT 2018
> append Wed May  2 08:35:54 CDT 2018
> append Wed May  2 08:35:54 CDT 2018
> append Wed May  2 08:35:54 CDT 2018
> append Wed May  2 08:35:54 CDT 2018
> append Wed May  2 08:35:54 CDT 2018
> append Wed May  2 08:35:54 CDT 2018
> append Wed May  2 08:35:55 CDT 2018
> ^C
> [jjacquez@cmilsbtest03 ~]$ tail --follow=name /media/samba/test.file
> append Wed May  2 08:36:01 CDT 2018
> append Wed May  2 08:36:01 CDT 2018
> append Wed May  2 08:36:01 CDT 2018
> append Wed May  2 08:36:02 CDT 2018
> append Wed May  2 08:36:02 CDT 2018
> append Wed May  2 08:36:02 CDT 2018
> append Wed May  2 08:36:02 CDT 2018
> append Wed May  2 08:36:02 CDT 2018
> append Wed May  2 08:36:02 CDT 2018
> append Wed May  2 08:36:02 CDT 2018
> ^C
> [jjacquez@cmilsbtest03 ~]$ while :; do stat -c '%s' /media/samba/test.file; 
> sleep 1; done
> 1649717
> 1650077
> 1650401
> 1650761
> 1651121
> 1651445
> 1651769
> 1652093
> 1652417
> 
> 
> About the reverting the patch, so I need to download the source and patch it?
> 
>   cd coreutils-8.29 &&
>   wget 'https://github.com/coreutils/coreutils/commit/7fc7206.patch' -O- |
>   patch -R -p1 &&
>   make

You'd done that previously, for for completeness
yes you need to download the source first like:

  wget https://ftpmirror.gnu.org/coreutils/coreutils-8.29.tar.xz
  tar -xf coreutils-8.29.tar.xz

thanks,
Pádraig

bug#31353: ls unexpectedly quoting filenames

[billy noah]

Ok I stand corrected about no one liking it.  But I just read upwards of
100 comments on that linked SO question and I don't recall a single person
speaking up in favor of the new behaviour.  While you may have a point
about security issues, the new default should probably be a user selectable
option, not the default.  Have you conducted any kind of poll to determine
what the vast majority of users prefer?  Can you please illustrate a
scenario where non-quoted filenames would be a security issue?

On Thu, May 3, 2018 at 2:01 AM, Paul Eggert  wrote:

> billy noah wrote:
>
> No one seems to
>> like this new behaviour
>>
> I like the new behavior. The old behavior had security problems, since
> file names with unusual characters could confuse users into thinking that
> the file system was X when it really was Y.  The new behavior avoids these
> problems, and so is safer and more predictable.  You can resurrect the old
> behavior, if you like, by using a shell script called 'ls' that invokes
> /usr/bin/ls with the settings you prefer.  However, the default settings
> should be safe, not risky.
>

bug#31353: ls unexpectedly quoting filenames

[Bernhard Voelker]

On 05/03/2018 08:01 AM, Paul Eggert wrote:
> I like the new behavior.

+1

bug#31353: ls unexpectedly quoting filenames

[Paul Eggert]

billy noah wrote:


No one seems to
like this new behaviour
I like the new behavior. The old behavior had security problems, since file 
names with unusual characters could confuse users into thinking that the file 
system was X when it really was Y.  The new behavior avoids these problems, and 
so is safer and more predictable.  You can resurrect the old behavior, if you 
like, by using a shell script called 'ls' that invokes /usr/bin/ls with the 
settings you prefer.  However, the default settings should be safe, not risky.