bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [patch 1/4] do not force suid color


From: Jim Meyering
Subject: Re: [patch 1/4] do not force suid color
Date: Sat, 21 Jun 2008 19:37:52 +0200

Jan Engelhardt <address@hidden> wrote:
> By default, ls highlights setuid/setgid/etc. files with a color, but
> there is no way to restore the old (coreutils 5.x?) behavior, i.e.
> that the setuid file gets the same color as it would when not having
> suid.
>
> Assume an /etc/DIR_COLORS of:
>       NORMAL 0
>       FILE 0
>       EXEC 1;31 # bright red
>
> Coreutils 5.x:
>   -rw---S---  1 jengelh users    0 Oct 13 16:45 colorless
>   -rwx--S---  1 jengelh users    0 Oct 13 16:45 brightred
>
> Coreutils 6.x:
>   -rw---S---  1 jengelh users    0 Oct 13 16:45 hardcoded-sgid-color
>   -rwx--S---  1 jengelh users    0 Oct 13 16:45 hardcoded-sgid-color
>
> Adding any of the following to /etc/DIR_COLORS:
>
>   # Parse error
>   SGID
>
>   # -rw---S---  1 jengelh users    0 Oct 13 16:45 colorless
>   # -rwx--S---  1 jengelh users    0 Oct 13 16:45 colorless
>   SGID 0
>
> In short, there is no way to get at the 5.x behavior with config
> files only. This patch fixes the source code by removing any
> hardcoded defaults and changing the logic.
>
> Those who want that 6.x behavior shall use a DIR_COLORS file, such as
> the shipped one.
>
> Those who prefer the 5.x behavior can change their DIR_COLORS file to
> not include any SUID/SGID/STICKY/OWR/OWT lines. (Which is sufficient
> for me.)

Thanks for explaining the problem and taking the time to prepare those
patches.  However, upstream ls has never relied on /etc/DIR_COLORS,
and I'm confident that many people would complain if --color stopped
colorizing the output of ls.

On the bright side, the behavior you want should be possible
and we can provide the capability via a much less invasive change.
With the barely tested patch below, I can run ls (remember, with no
/etc/DIR_COLORS file and no need to eval dircolors output) like this:

    $ LS_COLORS=sg= ./ls -log --color
    total 0
    -rwxr-sr-x 1 0 Jun 21 16:17 exec
    -rw-r--r-- 1 0 Jun 21 16:17 plain

and "exec" is highlighted in green, as it would be
if it did not have the set-GID bit set.

To make it red instead of green, you might use this:

    $ LS_COLORS='ex=1;31:sg=' ./ls -log --color

Of course, using LS_COLORS directly is not a user-friendly interface.
And dircolors cannot yet emit such settings:

    $ echo SETGID|dircolors -
    dircolors: -:1: invalid line;  missing second token

If you (or anyone else) are interested in fixing that (maybe as easy
as removing the test and ensuring that downstream code can handle NULL
pointers or empty strings) and adding tests and documentation, that
would accelerate the process.  Otherwise, I'll do it eventually.


>From 9dde28f1ee2ccd8bbfff1f2ec893e446abc02175 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sat, 21 Jun 2008 19:05:02 +0200
Subject: [PATCH] ls: ignore a normally-colorizable attribute if its string has 
length 0

* ls.c (print_color_indicator): Skip each test for an attribute
like C_SETUID, C_SETGID, C_STICKY_OTHER_WRITABLE, etc. if the
corresponding string has 0 length.
Prompted by a report from Jan Engelhardt.
---
 src/ls.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/ls.c b/src/ls.c
index 4e044a9..0e1528b 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -3922,20 +3922,25 @@ print_color_indicator (const char *name, mode_t mode, 
int linkok,
       if (S_ISREG (mode))
        {
          type = C_FILE;
-         if ((mode & S_ISUID) != 0)
+         if ((mode & S_ISUID) != 0
+             && color_indicator[C_SETUID].len)
            type = C_SETUID;
-         else if ((mode & S_ISGID) != 0)
+         else if ((mode & S_ISGID) != 0
+                  && color_indicator[C_SETGID].len)
            type = C_SETGID;
          else if ((mode & S_IXUGO) != 0)
            type = C_EXEC;
        }
       else if (S_ISDIR (mode))
        {
-         if ((mode & S_ISVTX) && (mode & S_IWOTH))
+         if ((mode & S_ISVTX) && (mode & S_IWOTH)
+             && color_indicator[C_STICKY_OTHER_WRITABLE].len)
            type = C_STICKY_OTHER_WRITABLE;
-         else if ((mode & S_IWOTH) != 0)
+         else if ((mode & S_IWOTH) != 0
+                  && color_indicator[C_OTHER_WRITABLE].len)
            type = C_OTHER_WRITABLE;
-         else if ((mode & S_ISVTX) != 0)
+         else if ((mode & S_ISVTX) != 0
+                  && color_indicator[C_STICKY].len)
            type = C_STICKY;
          else
            type = C_DIR;
--
1.5.6.rc3.23.gc3bdd




reply via email to

[Prev in Thread] Current Thread [Next in Thread]