Subject: Re: bug#11922: df doesn't handle \n in mount entries appropriately

From: Pádraig Brady <[email-masked]>
Date: Mon, 16 Jul 2012 02:46:19 +0100

On 07/13/2012 04:05 PM, Pádraig Brady wrote:
> On 07/13/2012 04:03 PM, Jim Meyering wrote:
>> Pádraig Brady wrote:
>>> On 07/12/2012 05:11 PM, Pádraig Brady wrote:
> 
>>> * src/df.c (hide_problematic_chars): Replace '\n' with '\?'.
>>
>> Thanks.  That looks fine, but shouldn't the '\?' above be just '?'
> 
> oops thanks.

I adjusted a bit and now replace all control chars (< 0x20) with '?'.
They're easy to handle in all encodings and can only be problematic.
I added a test too.

I've pushed the attached, so I'm marking this done.

cheers,
Pádraig.
>From 8d91451e237c50284b7de99228ea814c55c5deb1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Fri, 13 Jul 2012 02:27:26 +0100
Subject: [PATCH] df: don't output control characters in a mount point name

It's awkward to read and problematic for scripts when
control characters like '\n' are output.

Note other fields are already handled with mbsalign,
which converts non printable chars to the replacement char.
A caveat to note with that, is the replacement char takes
a place in the field and so possibly truncates the field
if it was the widest field in the records.

Note a more general replacement function, that
handles all printable, or white space characters,
would require more sophisticated support for various
encodings, and the complexity vs benefit was not
deemed beneficial enough at present.
Perhaps in future a more general replacement function
could be shared between the various utilities.

Note <space> is unaffected in any field,
which could impact scripts processing the output.
However any of the number fields at least could have
spaces considering `LANG=fr_FR df -B\'1`, so it's
probably best to leave spaces, which also allows
scripts to handle mount points with spaces without change.

* src/df.c (hide_problematic_chars): Replace control chars with '?'.
* tests/df/problematic-chars: Add a new root only test.
* tests/Makefile.am: Reference the new test.
* NEWS: Mention the fix.
---
 NEWS                       |    5 +++++
 src/df.c                   |   24 +++++++++++++++++++++++-
 tests/Makefile.am          |    1 +
 tests/df/problematic-chars |   44 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 73 insertions(+), 1 deletions(-)
 create mode 100755 tests/df/problematic-chars

diff --git a/NEWS b/NEWS
index b8d3cbb..113b30a 100644
--- a/NEWS
+++ b/NEWS
@@ -13,6 +13,11 @@ GNU coreutils NEWS                                    -*- outline -*-
   date: invalid date '\260'
   [This bug was present in "the beginning".]
 
+  df no longer outputs control characters present in the mount point name.
+  Such characters are replaced with '?', so for example, scripts consuming
+  lines output by df, can work reliably.
+  [This bug was present in "the beginning".]
+
   head --lines=-N (-n-N) now resets the read pointer of a seekable input file.
   This means that "head -n-3" no longer consumes all of its input, and lines
   not output by head may be processed by other programs.  For example, this
diff --git a/src/df.c b/src/df.c
index 7e30d57..5dc3d2d 100644
--- a/src/df.c
+++ b/src/df.c
@@ -192,6 +192,23 @@ static struct option const long_options[] =
   {NULL, 0, NULL, 0}
 };
 
+/* Replace problematic chars with '?'.
+   Since only control characters are currently considered,
+   this should work in all encodings.  */
+
+static char*
+hide_problematic_chars (char *cell)
+{
+  char *p = cell;
+  while (*p)
+    {
+      if (iscntrl (to_uchar (*p)))
+        *p = '?';
+      p++;
+    }
+  return cell;
+}
+
 /* Dynamically allocate a row of pointers in TABLE, which
    can then be accessed with standard 2D array notation.  */
 
@@ -315,6 +332,8 @@ get_header (void)
       if (!cell)
         xalloc_die ();
 
+      hide_problematic_chars (cell);
+
       table[nrows-1][field] = cell;
 
       widths[field] = MAX (widths[field], mbswidth (cell, 0));
@@ -661,7 +680,10 @@ get_dev (char const *disk, char const *mount_point,
         }
 
       if (cell)
-        widths[field] = MAX (widths[field], mbswidth (cell, 0));
+        {
+          hide_problematic_chars (cell);
+          widths[field] = MAX (widths[field], mbswidth (cell, 0));
+        }
       table[nrows-1][field] = cell;
     }
 }
diff --git a/tests/Makefile.am b/tests/Makefile.am
index dfac9a0..944c7d2 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -31,6 +31,7 @@ root_tests =					\
   cp/capability					\
   cp/sparse-fiemap				\
   dd/skip-seek-past-dev				\
+  df/problematic-chars				\
   install/install-C-root			\
   ls/capability					\
   ls/nameless-uid				\
diff --git a/tests/df/problematic-chars b/tests/df/problematic-chars
new file mode 100755
index 0000000..801c4d9
--- /dev/null
+++ b/tests/df/problematic-chars
@@ -0,0 +1,44 @@
+#!/bin/sh
+# Ensure that df outputs one line per entry
+
+# Copyright (C) 2012 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+print_ver_ df
+require_root_
+
+mnt='mount
+point'
+
+cwd=$(pwd)
+cleanup_() { cd /; umount "$cwd/$mnt"; }
+
+skip=0
+# Create a file system, then mount it.
+dd if=/dev/zero of=blob bs=8192 count=200 > /dev/null 2>&1 \
+                                             || skip=1
+mkdir "$mnt"                                 || skip=1
+mkfs -t ext2 -F blob \
+  || skip_ "failed to create ext2 file system"
+
+mount -oloop blob "$mnt"                     || skip=1
+
+test $skip = 1 \
+  && skip_ "insufficient mount/ext2 support"
+
+test $(df "$mnt" | wc -l) = 2 || fail=1
+
+Exit $fail
-- 
1.7.6.4