[Bash-completion-devel] [bash-completion-Bugs][312143] protect against user's personal binary preferences

bash-completion-bugs at alioth.debian.org bash-completion-bugs at alioth.debian.org
Tue Dec 15 21:59:42 UTC 2009


Bugs item #312143, was changed at 2009-12-08 02:29 by Ville Skyttä 
You can respond by visiting: 
https://alioth.debian.org/tracker/?func=detail&atid=413095&aid=312143&group_id=100114

>Status: Closed
Priority: 3
Submitted By: Kevin Hunter (hunteke-guest)
Assigned to: Ville Skyttä  (scop-guest)
Summary: protect against user's personal binary preferences 
Distribution: --Distribution-Agnostic--
Originally reported in: None
Milestone: None
>Status: Fix Committed
Original bug number: 


Initial Comment:
Since the bash_completion script is sourced /into/ a user's shell session, their personal preferences, e.g. aliases, would otherwise get used.  (e.g. alias grep='grep --color=always')  This is not what bash_completion needs.  In the example, this would return something like

$ sudo apt-get purge ggz[tab]
$ apt-get purge ggz^[\[m^[\[K^[\[m^[\[Kcore-bin

Instead, what we want is:

$ sudo apt-get purge ggz[tab]
$ sudo apt-get purge ggzcore-bin

Please see attached bashrc snippet and suggested patch to bash_completion.

Note: I did not change any of the comments.  Just the places where grep is actually used and called as 'grep'.

----------------------------------------------------------------------

>Comment By: Ville Skyttä  (scop-guest)
Date: 2009-12-15 23:59

Message:
Good point regarding echo/printf, moved that one to #312163.

Current git has all grep invocations replaced with "command grep", no other commands were treated that way at least yet due to lack of feedback for my cd, echo, head, and tail aliasing question below.

----------------------------------------------------------------------

Comment By: Freddy Vulto (fvu-guest)
Date: 2009-12-09 23:36

Message:
We'd better never use echo, but `printf' (or redirection) instead because if an argument to echo happens to be -e or -n, it is interpreted wrongly as an option:

   $ a=-n
   $ echo "$a"  # Error: Nothing is output
   $ printf "%s" "$a"  # Ok: -n gets output
   -n$

----------------------------------------------------------------------

Comment By: Ville Skyttä  (scop-guest)
Date: 2009-12-09 22:04

Message:
Just one more thing regarding other commands besides grep: echo should probably be prefixed by "builtin" rather than "command".  And to what would one generally want to alias cd, echo, head, and tail to?

By the way there's a bunch of echos for which we could nowadays use here strings (<<<foo), I'll take a look at those as well.

----------------------------------------------------------------------

Comment By: Ville Skyttä  (scop-guest)
Date: 2009-12-09 21:46

Message:
I wasn't even aware of the quoting and backslash tricks, and I thus I agree that they are obscure and that we shouldn't use them ;)

It seems there is a rough consensus that "command foo" is the way to go.  Thanks to Raph for the list; I'll go ahead and implement that.

(I wish there was a "local" variant of shopt, that way we could just do "shopt -u expand_aliases" at start of each completion function and be done with the alias problems...)

> In regards to using GREP_OPTIONS, that's a no-go.  As
> with my bashrc, I specifically set GREP_OPTIONS and
> would be annoyed if it was hijacked.

Those settings wouldn't be hijacked, GREP_OPTIONS would just be emptied for bash completion's grep invocations, like "GREP_OPTIONS= command grep ..." (all part of same command), nothing else.

----------------------------------------------------------------------

Comment By: Sung Pae (guns-guest)
Date: 2009-12-09 06:59

Message:
Regarding `command foo' being kind of ugly:

The alias expansion routine ignores words that have been quoted. This means that the functionality of the `command' builtin can be mimicked by simply quoting a command.

Incidentally, words are considered to be quoted when preceded by backslashes. This effectively means that one can shorten `command grep' to `\grep'.

    $ alias ls='ls -Ahl'
    $ ls
    total 0
    -rw-r--r--  1 guns  wheel     0B Dec  8 22:44 bar
    -rw-r--r--  1 guns  wheel     0B Dec  8 22:44 foo
    $ 'ls'
    bar foo
    $ \ls
    bar foo

This works in bash 2.0+ (AFAIK - it probably worked since the first version). You can see this for yourself in the source: see alias_expand() and skipws() in alias.c.

I'm not actually recommending that the project use this. It's a little obscure and not well documented. But it is considerably more compact than using the command builtin.


----------------------------------------------------------------------

Comment By: Kevin Hunter (hunteke-guest)
Date: 2009-12-09 06:02

Message:
Hoh!  Command is ... not the best keyword for which to search in the bash man page.  :-)  But, finally found it.  The only reason I didn't create that patch with command is because I didn't know about it.  I think it's a far cleaner solution.

Besides, I knoze you're smarter than me.  That's why I said "suggested" patch.  ;-)

Alright, I still don't have the development version, nor would I have time to test it out for at least another month or so, but here's an updated patch that responds to these criticisms:

1. not corrected - against old version of bash_completion

2. corrected - it would "pollute" users' environment with the GREP variable.

3. corrected (moot) - --color=auto is a GNU grep thing, it cannot be blindly used in bash completion.  As GREP is removed, issue solved.

For the record, I did *not* say =auto, I said =never.  I explicitly don't use auto, because I often want color /in the pipeline/, as for 'less -R'.  =auto tries to be smart by removing color if it's not directly to the terminal, which is less than ideal for my use cases.

4. corrected - grep is not always installed in /bin.

In regards to using GREP_OPTIONS, that's a no-go.  As with my bashrc, I specifically set GREP_OPTIONS and would be annoyed if it was hijacked.

(Raphaël, I have similar aliases well.  Would get into them, but ... not the time or place.  :-) )

----------------------------------------------------------------------

Comment By: Raphaël Droz (gibboris-guest)
Date: 2009-12-09 02:18

Message:
I vote for using command (or 'quoting' the command, but 'command' prefix is nice for grep'ing bash-comp source).

I think this should also be made for other potentially aliased commands :
'cp', 'rm', 'mv', 'less' aren't used.
'cd' is : pkgtools, dpkg
'echo' is
'head', 'tail' are : genisoimage, cowsay, apache2ctl.
'ls' is always prefixed by 'command'
(personally I also have an alias for 'df'' and 'wget')

----------------------------------------------------------------------

Comment By: Ville Skyttä  (scop-guest)
Date: 2009-12-09 00:56

Message:
The patch has a number of problems, it cannot be applied, at least as is.

First, it is apparently against an old version of bash completion, current development version looks substantially different.  Second, it would "pollute" users' environment with the GREP variable.  Third, --color=auto is a GNU grep thing, it cannot be blindly used in bash completion.  Fourth, grep is not always installed in /bin.

Wouldn't simply replacing all occurrences of "grep" with "command grep" (it's already that way in a couple of places) be a good enough solution?  Ditto egrep and fgrep.  This wouldn't protect against nasty things in GREP_OPTIONS but I'm not sure there's much we can sanely do about those anyway.  Hmm... or we could always do "GREP_OPTIONS= command grep ..." but I'm not sure if that ugliness is worth it.  Or we could do that and also replace uses of grep with some other less problematic tool but...

----------------------------------------------------------------------

Comment By: Kevin Hunter (hunteke-guest)
Date: 2009-12-08 02:38

Message:
For passersby reading this, I'm aware that I may have an unexpected setup for my grep.  I explain why in the comments of the attached bashrc snippet, but suffice it to say, this what I have:

GREP_OPTIONS='--color=none'  # default to sane options
alias grep='grep --color=always'   # if I'm typing it, I most-likely want colors
alias less='less -R'   # enable pass-through (RAW) colors from grep

For reference, here's the discussion that convinced me put it the way I have it.

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=475772#17


----------------------------------------------------------------------

You can respond by visiting: 
https://alioth.debian.org/tracker/?func=detail&atid=413095&aid=312143&group_id=100114



More information about the Bash-completion-devel mailing list