ls: protect newlines for non terminal output

Add extra protection for newlines that might be output to log files etc.

* doc/coreutils.texi (ls invocation): Expand --literal description.
* src/ls.c (quote_name_buf): Replace '\n' with '?' if appropriate.
* tests/ls/dired.sh: Add a negative test case.
* tests/ls/zero-option.sh: Likewise.
* tests/ls/m-option.sh: Likewise. Also add a positive test case.
* NEWS: Mention the improvement.

NEWS

@@ -87,6 +87,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   'ls -m' now quotes files names containing commas when appropriate,
   so users can better distinguish separating commas.
 
+  'ls' now replaces newlines in file names if ambiguous with separators.
+  Previously newlines were protected only when outputting to a terminal.
+
   'sort' will now better use available memory and parallel operation
   when reading from unknown sized inputs like pipes.
 

doc/coreutils.texi

@@ -8466,8 +8466,10 @@ backslash sequences like those used in C.
 @opindex --quoting-style
 Do not quote file names.  However, with @command{ls} nongraphic
 characters are still printed as question marks if the output is a
-terminal and you do not specify the @option{--show-control-chars}
-option.
+terminal and you do not specify the @option{--show-control-chars} option.
+Also newline characters in file names are printed as question marks
+if ambiguous with separators, irrespective of whether output
+is a terminal or not.
 
 @optItem{ls,-q,}
 @optItemx{ls,--hide-control-chars,}

src/ls.c

@@ -741,6 +741,11 @@ static struct ignore_pattern *hide_patterns;
    quoting methods pass through control chars as-is.  */
 static bool qmark_funny_chars;
 
+/* True means extra replacement of newline with '?'
+   in cases where a newline in a file name would be
+   ambiguous with newline separators.  */
+static bool qmark_newline;
+
 /* Quoting options for file and dir name output.  */
 
 static struct quoting_options *filename_quoting_options;
@@ -2335,6 +2340,12 @@ decode_switches (int argc, char **argv)
                        ? ls_mode == LS_LS && stdout_isatty ()
                        : hide_control_chars_opt);
 
+  qmark_newline = hide_control_chars_opt != false && eolbyte == '\n'
+                  && ((line_length
+                       && (format == with_commas
+                           || format == many_per_line || format == horizontal))
+                      || format == one_per_line || format == long_format);
+
   int qs = quoting_style_opt;
   if (qs < 0)
     qs = getenv_quoting_style ();
@@ -4498,10 +4509,21 @@ quote_name_buf (char **inbuf, size_t bufsize, char *name,
   bool quoted;
 
   enum quoting_style qs = get_quoting_style (options);
-  bool needs_further_quoting = qmark_funny_chars
-                               && (qs == shell_quoting_style
-                                   || qs == shell_always_quoting_style
-                                   || qs == literal_quoting_style);
+  bool partial_quoting = qs == shell_quoting_style
+                         || qs == shell_always_quoting_style
+                         || qs == literal_quoting_style;
+  bool needs_further_quoting = qmark_funny_chars && partial_quoting;
+
+  /* Note we provide extra protection only for newline separators.
+     I.e., we don't bother protecting "double space" or "comma space"
+     separated entries, as for interactive output, shell escape quoting
+     is best to disambiguate, and for programmatic, --zero is best.
+     Extra processing to mark ', ' and '  ' would be overkill
+     as interactively there will still be ambiguities with multi-byte spaces,
+     and marked characters impact programmatic processing anyway.  */
+  bool extra_newline_protection = (!needs_further_quoting
+                                   && qmark_newline && partial_quoting
+                                   && !dired && strchr (name, '\n'));
 
   if (needs_general_quoting != 0)
     {
@@ -4514,7 +4536,7 @@ quote_name_buf (char **inbuf, size_t bufsize, char *name,
 
       quoted = (*name != *buf) || strlen (name) != len;
     }
-  else if (needs_further_quoting)
+  else if (needs_further_quoting || extra_newline_protection)
     {
       len = strlen (name);
       if (bufsize <= len)
@@ -4530,6 +4552,13 @@ quote_name_buf (char **inbuf, size_t bufsize, char *name,
       quoted = false;
     }
 
+  /* It's safe to replace newlines without multi-byte iteration, as newlines
+     do not appear as part of any multi-byte encoded character.  */
+  if (extra_newline_protection)
+    for (char *p = buf; p < buf + len; p++)
+      if (eolbyte && *p == '\n')
+        *p = '?';
+
   if (needs_further_quoting)
     {
       if (MB_CUR_MAX > 1)

tests/ls/dired.sh

@@ -76,5 +76,18 @@ while test "$#" -gt 0; do
   index=$(($index + 1))
 done
 
+# Ensure literal quoting preserves newlines for dired consumers.
+newline='n
+l'
+mkdir newline-dir || framework_failure_
+touch "newline-dir/$newline" || framework_failure_
+ls -l --dired --quoting-style=literal newline-dir > out || fail=1
+set -- $(sed -n 's|^//DIRED// ||p' out)
+test "$#" -eq 2 || framework_failure_
+dd bs=1 skip="$1" count="$(($2 - $1))" < out > actual 2>/dev/null \
+  || framework_failure_
+printf %s "$newline" > exp || framework_failure_
+compare exp actual || fail=1
+
 
 Exit $fail

tests/ls/m-option.sh

@@ -72,4 +72,17 @@ for qs in $(cut -d: -f1 exp); do
 done > out
 compare exp out || fail=1
 
+# Newlines are preserved with unlimited width, where -m does not wrap,
+newline='n
+l'
+touch "$newline" || framework_failure_
+printf '%s\n' "$newline" > exp || framework_failure_
+ls -m -w0 "$newline" > out || fail=1
+compare exp out || fail=1
+
+# Otherwise protect newlines that could be confused with separators.
+printf '%s\n' 'n?l' > exp || framework_failure_
+ls -m "$newline" > out || fail=1
+compare exp out || fail=1
+
 Exit $fail

tests/ls/zero-option.sh

@@ -29,12 +29,11 @@ disallowed_options='-l --dired'  # dired only enabled with -l
 returns_ 2 ls $disallowed_options --zero dir || fail=1
 
 disabled_options='--color=always -x -m -C -Q -q'
+newline='n
+l'
+touch dir/'com,ma' "dir/$newline" || framework_failure_
 LC_ALL=C ls $disabled_options --zero dir >out || fail=1
-tr '\n' '\0' <<EOF >exp
-a
-b
-cc
-EOF
+printf '%s\0' a b cc 'com,ma' "$newline" >exp || framework_failure_
 
 compare exp out || fail=1