[Bash-completion-devel] [bash-completion-Bugs][311396] DBTS 511788: completion of usernames broken

bash-completion-bugs at alioth.debian.org bash-completion-bugs at alioth.debian.org
Mon Jan 25 20:22:33 UTC 2010


Bugs item #311396, was changed at 2009-01-30 09:52 by Freddy Vulto
You can respond by visiting: 
https://alioth.debian.org/tracker/?func=detail&atid=413095&aid=311396&group_id=100114

Status: Open
Priority: 3
Submitted By: David Paleino (hanska-guest)
>Assigned to: Freddy Vulto (fvu-guest)
Summary: DBTS 511788: completion of usernames broken 
Distribution: None
Originally reported in: Debian BTS
Milestone: None
Status: None
Original bug number: 511788


Initial Comment:
Hi.

Completion of usernames is broken and "\: " or \ is inserted
e.g.
chown cuser\:  (space at the end)
chown calestyo\\\:calestyo
etc. etc.

Completion of users/groups with a "." (which is widely used at big  
sites like firstname.secondname) and perhaps with other strange  
characters is even more broken ^^

Thanks,
Chris.


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

Comment By: Leonard Crestez (cdleonard-guest)
Date: 2010-01-15 10:20

Message:
Here's a revised version of the patch. The patch included some unrelated fixes
that are no longer relevant. Also, it did not apply cleanly to HEAD. 
Since I couldn't keep all the escaping and unescaping straight I added tests
instead. This makes this patch larger even though there are fewer changes in
bash_completion.

I could not get the existing assert_* functions to get exactly what I wanted
so I made my own pretty little helpers. Those helpers could probably be made
shorter by using more stuff from library.exp but they're very short anyway.
This my first time writing TCL; suggestions welcome.

I ran the tests on multiple versions of bash; the patch works the same on all
versions.

diff --git a/bash_completion b/bash_completion
index 48eb2fd..8b12127 100644
--- a/bash_completion
+++ b/bash_completion
@@ -775,20 +775,38 @@ _installed_modules()
         awk '{if (NR != 1) print $1}' )" -- "$1" ) )
 }
 
-# This function completes on user:group format
+# This function completes on user or user:group format; as for chown and cpio.
 #
+# The : must be added manually; it will only complete usernames initially.
+# The legacy user.group format is not supported.
+#
+# It assumes compopt -o filenames; but doesn't touch it.
 _usergroup()
 {
     local IFS=$'\n'
-    cur=${cur//\\\\ / }
-    if [[ $cur = *@(\\:|.)* ]]; then
-        user=${cur%%*([^:.])}
-        COMPREPLY=( $(compgen -P ${user/\\\\} -g -- ${cur##*[.:]}) )
+    if [[ $cur = *\\\\* || $cur = *:*:* ]]; then
+        # Give up early on if something seems horribly wrong.
+        return
+    elif [[ $cur = *\\:* ]]; then
+        # Completing group after 'user\:gr<TAB>'.
+        # Reply with a list of groups prefixed with 'user:', readline will
+        # escape to the colon.
+        local prefix
+        prefix=${cur%%*([^:])}
+        prefix=${prefix//\\}
+        COMPREPLY=( $( compgen -P "$prefix" -g -- "${cur#*[:]}" ) )
     elif [[ $cur = *:* ]]; then
-        COMPREPLY=( $( compgen -g -- ${cur##*[.:]} ) )
+        # Completing group after 'user:gr<TAB>'.
+        # Reply with a list of unprefixed groups since readline with split on :
+        # and only replace the 'gr' part
+        COMPREPLY=( $( compgen -g -- "${cur#*:}" ) )
     else
-        type compopt &>/dev/null && compopt -o nospace
-        COMPREPLY=( $( compgen -S : -u -- "$cur" ) )
+        # Completing a partial 'usernam<TAB>'.
+        #
+        # Don't suffix with a : because readline will escape it and add a
+gg        # slash. It's better to complete into 'chown username ' than 'chown
+        # username\:'.
+        COMPREPLY=( $( compgen -u -- "$cur" ) )
     fi
 }
 
@@ -912,8 +930,10 @@ complete -F _service service
 _chown()
 {
     local cur prev split=false
-    cur=`_get_cword`
-    prev=${COMP_WORDS[COMP_CWORD-1]}
+
+    # Get cur and prev words; but don't treat user:group as separate words.
+    cur=`_get_cword :`
+    prev=`_get_pword :`
 
     _split_longopt && split=true
 
@@ -930,22 +950,22 @@ _chown()
 
     $split && return 0
 
-    # options completion
     if [[ "$cur" == -* ]]; then
+        # Complete -options
         COMPREPLY=( $( compgen -W '-c -h -f -R -v --changes --dereference \
             --no-dereference --from --silent --quiet --reference --recursive \
             --verbose --help --version' -- "$cur" ) )
     else
-        _count_args
+        local args
 
-        case $args in
-            1)
-                _usergroup
-                ;;
-            *)
-                _filedir
-                ;;
-        esac
+        # The first argument is an usergroup; the rest are filedir.
+        _count_args :
+
+        if [[ $args == 1 ]]; then
+            _usergroup
+        else
+            _filedir
+        fi
     fi
 } # _chown()
 complete -F _chown -o filenames chown
diff --git a/contrib/cpio b/contrib/cpio
index e8e4a5a..dddfc19 100644
--- a/contrib/cpio
+++ b/contrib/cpio
@@ -11,8 +11,8 @@ _cpio()
     local cur prev split=false
 
     COMPREPLY=()
-    cur=`_get_cword`
-    prev=${COMP_WORDS[COMP_CWORD-1]}
+    cur=`_get_cword :` 
+    prev=`_get_pword :` 
 
     _split_longopt && split=true
 
@@ -91,7 +91,7 @@ _cpio()
         esac
     fi
 }
-complete -F _cpio cpio
+complete -F _cpio -o filenames cpio
 }
 
 # Local variables:
diff --git a/test/completion/chown.exp b/test/completion/chown.exp
new file mode 100644
index 0000000..05ad230
--- /dev/null
+++ b/test/completion/chown.exp
@@ -0,0 +1,3 @@
+if {[assert_bash_type chown]} {
+    source "lib/completions/chown.exp"
+}; # if
diff --git a/test/lib/completions/chown.exp b/test/lib/completions/chown.exp
new file mode 100644
index 0000000..6568aca
--- /dev/null
+++ b/test/lib/completions/chown.exp
@@ -0,0 +1,70 @@
+proc setup {} {
+    save_env
+}; # setup()
+
+proc teardown {} {
+    assert_env_unmodified
+}; # teardown()
+
+setup
+
+
+set test "Check anything happens"
+
+assert_complete_any "chown " $test
+
+sync_after_int
+
+
+# All the tests use the root:root user and group. It's assumed to exist.
+set fulluser "root"
+set fullgroup "root"
+
+# Partial username is assumed to be unambiguous.
+set partuser "roo"
+set partgroup "roo"
+
+if {[exec getent passwd | grep "^$partuser" | wc -l] > 1 ||
+    [exec getent passwd | grep "^$fulluser:" | wc -l] != 1 ||
+    [exec getent group | grep "^$partgroup" | wc -l] > 1 ||
+    [exec getent group | grep "^$fullgroup:" | wc -l] != 1} {
+    # This breaks test counts; but that's probably not important.
+    untested "Not running complex chown tests."
+} else {
+    assert_complete_into "chown $partuser" "chown $fulluser "
+    sync_after_int
+
+    assert_complete_into "chown $fulluser:$partgroup" "chown $fulluser:$fullgroup "
+    sync_after_int
+
+    # One slash should work correctly (doubled here for tcl).
+    assert_complete_into "chown $fulluser\\:$partgroup" "chown $fulluser\\:$fullgroup "
+    sync_after_int
+
+    foreach prefix {
+        "funky\\ user:" "funky\\ user\\:" "funky.user:" "funky\\.user:" "fu\\ nky.user\\:"
+        "f\\ o\\ o\\.\\bar:" "foo\\_b\\ a\\.r\\ :"
+    } {
+        set test "Check preserve special chars in $prefix$partgroup<TAB>"
+        assert_complete_into "chown $prefix$partgroup" "chown $prefix$fullgroup " $test
+        sync_after_int
+    }
+
+    # Check that we give up in degenerate cases instead of spewing various junk.
+
+    assert_no_complete "chown $fulluser\\\\:$partgroup"
+    sync_after_int
+
+    assert_no_complete "chown $fulluser\\\\\\:$partgroup"
+    sync_after_int
+
+    assert_no_complete "chown $fulluser\\\\\\\\:$partgroup"
+    sync_after_int
+
+    # Colons in user/groupnames are not usually allowed.
+    assert_no_complete "chown foo:bar:$partgroup"
+    sync_after_int
+}
+
+
+teardown
diff --git a/test/lib/library.exp b/test/lib/library.exp
index fe8d7e2..1ce8aab 100644
--- a/test/lib/library.exp
+++ b/test/lib/library.exp
@@ -215,13 +215,18 @@ proc _remove_cword_from_cmd {cmd {cword ""}} {
 }; # _remove_cword_from_cmd()
 
 
+# Escape regexp special characters
+proc _escape_regexp_chars {var} {
+    upvar $var str
+    regsub -all {([\^$+*?.|(){}[\]\\])} $str {\\\1} str
+}
+
 # Make sure any completions are returned
 proc assert_complete_any {cmd {test ""} {prompt /@}} {
     if {$test == ""} {set test "$cmd should show completions"}
     send "$cmd\t"
     expect -ex "$cmd"
-        # Escape special regexp characters
-    regsub -all {([\^$+*?.|(){}[\]\\])} $cmd {\\\1} cmd
+    _escape_regexp_chars cmd
     expect {
         -timeout 1
         # Match completions, multiple words
@@ -417,6 +422,45 @@ proc assert_exec {cmd {stdout ''} {test ''}} {
 }; # assert_exec()
 
 
+# Assert that $from completes exactly into $into.
+# This requires that the completion is unambiguous.
+#
+# Example: assert_complete_into "modprobe usb_storag" "modprobe usb_storage"
+#
+# Params:
+# @from The string to tab from
+# @into The expected result.
+# @test Optional parameter with test name.
+proc assert_complete_into {{from} {into} {test ""}} {
+    if {[string length $test] == 0} {
+        set test "Check '$from' completes into '$into'"
+    }
+
+    _escape_regexp_chars into
+    # We can't anchor on $, simulate typing a magical string instead.
+    set endguard "Magic End Guard"
+    send "$from\t$endguard"
+
+    expect {
+        -re "^$into$endguard" { pass "$test" }
+        default { fail "$test" }
+        timeout { fail "$test" }
+    }
+}
+
+# Check that no completion is attempted on a certain command.
+#
+# Params:
+# @cmd The command to attempt to complete.
+# @test Optional parameter with test name.
+proc assert_no_complete {{cmd} {test ""}} {
+    if {[string length $test] == 0} {
+        set test "Check no completion on '$cmd'"
+    }
+    assert_complete_into $cmd $cmd $test
+}
+
+
 # Sort list.
 # `exec sort' is used instead of `lsort' to achieve exactly the
 #  same sort order as in bash.
@@ -506,8 +550,7 @@ proc match_items {items test {prompt /@} {size 20}} {
         set expected ""
         for {set j 0} {$j < $size && $i + $j < [llength $items]} {incr j} {
             set item "[lindex $items [expr {$i + $j}]]"
-                # Escape special regexp characters
-            regsub -all {([\^$+*?.|(){}[\]\\])} $item {\\\1} item
+            _escape_regexp_chars item
             append expected $item
             if {[llength $items] > 1} {append expected {\s+}};
         }; # for

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

Comment By: Leonard Crestez (cdleonard-guest)
Date: 2009-11-15 13:58

Message:
Hello; this has been annoying me and I decided to take a look. This bug affects chown and cpio.

1) COMP_WORDBREAKS and chown

In bash4 completion words are also split on colons and this messes up the completion. The easiest solution would be to override COMP_WORDBREAKS for a single completion; but as far as I know that's not possible in bash.

_get_cword ":" can emulate old behaviour but a similar variant is not available for _count_args. I worked around that calling _get_cword ":" in a loop. Presumably this could done by adding a parameter to _count_args but I wanted to avoid that.

I hit an unrelated bug in _get_cword4; the original implementation did not properly deal with multiple separators and escapes. It now behaves the same as _get_cword3.

On usernames with dots in them: It seems that _usergroup tries hard to treat a dot just like a colon for legacy reasons. Here's a quote from the info page on chown (man page is shorter):

    Some older scripts may still use `.' in place of the `:' separator.
    POSIX 1003.1-2001 (*note Standards conformance::) does not require
    support for that, but for backward compatibility GNU `chown' supports
    `.' so long as no ambiguity results.  New scripts should avoid the use
    of `.' because it is not portable, and because it has undesirable
    results if the entire OWNER`.'GROUP happens to identify a user whose
    name contains `.'.

The entire _usergroup function was broken for bash 3 and above. I made it work; but it's still hacky. It treats 'user\:group' and 'user:group' separately because the former is one word to readline while the latter is interpreted as three worlds. This means that in the first case you have to prefix your replies with user: and rely on -o filenames to escape the colon.

I added -o filenames to cpio completion. This change makes it handle --file arguments with spaces correctly and hopefully doesn't break anything.

It would make a log of sense for a function that sets COMPREPLY to call compopt as well; but that's a bash4 feature. As far as I can tell calling _filedir without -o filenames is *always* wrong.

I tested this patch manually with bash4 in debian and the older 3.* upstream releases. I did not look at the automated testing support.

Here's the patch (inline; it seems I can't attach to an existing item in alioth):

diff --git a/bash_completion b/bash_completion
index 8c054a1..b7fd215 100644
--- a/bash_completion
+++ b/bash_completion
@@ -373,8 +373,12 @@ __get_cword4()
     # calculate current word, negatively offset by n_idx
     cur="${tmp:$(__word_start "$tmp")}"
     while [[ $n_idx -gt 0 ]]; do
-        local tmp="${tmp%[$WORDBREAKS]$cur}"    # truncate passed string
-        local cur="${tmp:$(__word_start "$tmp")}" # then recalculate
+        #tmp="${tmp%[$WORDBREAKS]$cur}"    # truncate passed string
+        # Truncate passed string. First remove {#cur} trailing chars then trailing $WORDBREAKS
+        tmp="${tmp:0:${#tmp} - ${#cur}}"
+        tmp="${tmp%%+([$WORDBREAKS])}"
+        # Then recalculate
+        local cur="${tmp:$(__word_start "$tmp")}"
         ((--n_idx))
     done
     printf "%s" "$cur"
@@ -656,19 +660,31 @@ _installed_modules()
         awk '{if (NR != 1) print $1}' )" -- "$1" ) )
 }
 
-# This function completes on user:group format
+# This function completes on user or user:group format; as for chown and cpio.
+# The : must be added manually; it will only complete usernames initially.
+# The legacy user.group format is not supported.
 #
 _usergroup()
 {
     local IFS=$'\n'
-    cur=${cur//\\\\ / }
-    if [[ $cur = *@(\\:|.)* ]]; then
-        user=${cur%%*([^:.])}
-        COMPREPLY=( $(compgen -P ${user/\\\\} -g -- ${cur##*[.:]}) )
+    cur=${cur//\\ / }
+    if [[ $cur = *\\:* ]]; then
+        # Completing group after 'user\:gr<TAB>'.
+        # Reply with a list of groups prefixed with 'user:'.
+        # Readline will replace the entire 'user\:gr' chunk and -o filenames
+        # will add a slash to our replies.
+        user=${cur%%*([^:])}
+        COMPREPLY=( $(compgen -P ${user/\\} -g -- ${cur##*[:]}) )
     elif [[ $cur = *:* ]]; then
-        COMPREPLY=( $( compgen -g -- ${cur##*[.:]} ) )
+        # Completing group after 'user:gr<TAB>'.
+        # Reply with a list of unprefixed groups since readline with
+        # split on : and replace the 'gr' part
+        COMPREPLY=( $( compgen -g -- ${cur##*:} ) )
     else
-        COMPREPLY=( $( compgen -S : -u -- "$cur" ) )
+        # Completing a partial 'usernam<TAB>'.
+        # Don't suffix with a : because readline will escape it and add a slash.
+        # It's better to complete into 'chown username ' than 'chown username\: '.
+        COMPREPLY=( $( compgen -u -- "$cur" ) )
     fi
 }
 
@@ -790,8 +806,10 @@ complete -F _service service
 _chown()
 {
     local cur prev split=false
-    cur=`_get_cword`
-    prev=${COMP_WORDS[COMP_CWORD-1]}
+
+    # Don't split user:group into separate words.
+    cur=`_get_cword ":"`
+    prev=`_get_pword ":"`
 
     _split_longopt && split=true
 
@@ -814,16 +832,30 @@ _chown()
             --no-dereference --from --silent --quiet --reference --recursive \
             --verbose --help --version' -- "$cur" ) )
     else
-        _count_args
+        # The first non-option argument is an _usergroup; the rest are _filedir
+        # In bash4 COMP_WORDS is also broken on colons by default so we loop
+        # backwards calling _get_cword.
+        #
+        # Does not deal with chown --from a b<TAB>; completes b as filename
+        # Should attempt to swallow --from and --reference.
+        local i=1 after_usergroup=false
+        while true ; do
+            prev=`_get_cword ":" $i`
+            if [[ $prev = chown ]]; then
+                # Stop looking when we find the actual chown command.
+                break;
+            elif [[ $prev != -* ]]; then
+                after_usergroup=true
+                break;
+            fi
+            ((++i))
+        done
 
-        case $args in
-            1)
-                _usergroup
-                ;;
-            *)
-                _filedir
-                ;;
-        esac
+        if [[ $after_usergroup = true ]]; then
+            _filedir
+        else
+            _usergroup
+        fi
     fi
 } # _chown()
 complete -F _chown -o filenames chown
diff --git a/contrib/cpio b/contrib/cpio
index f28b1b2..24f82c6 100644
--- a/contrib/cpio
+++ b/contrib/cpio
@@ -11,8 +11,8 @@ _cpio()
     local cur prev split=false
 
     COMPREPLY=()
-    cur=`_get_cword`
-    prev=${COMP_WORDS[COMP_CWORD-1]}
+    cur=`_get_cword :`
+    prev=`_get_pword :`
 
     _split_longopt && split=true
 
@@ -89,7 +89,7 @@ _cpio()
         esac
     fi
 }
-complete -F _cpio cpio
+complete -F _cpio -o filenames cpio
 }
 
 # Local variables:


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

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



More information about the Bash-completion-devel mailing list