[waiting for #1371] fix(java,rsync,scp): handle quoted space in filepaths properly

bash_completion

@@ -188,11 +188,11 @@ _comp_dequote__initialize()
 _comp_dequote__initialize
 
 # This function expands a word using `eval` in a safe way.  This function can
-# be typically used to get the expanded value of `${word[i]}` as
-# `_comp_dequote "${word[i]}"`.  When the word contains unquoted shell special
-# characters, command substitutions, and other unsafe strings, the function
-# call fails before applying `eval`.  Otherwise, `eval` is applied to the
-# string to generate the result.
+# be typically used to get the expanded value of `${word[i]}` as `_comp_dequote
+# "${word[i]}"`.  When the word contains unquoted shell special characters,
+# command substitutions, and other unsafe strings, the function call fails
+# before applying `eval` and REPLY is set to be the literal string.  Otherwise,
+# `eval` is applied to the string to generate the result.
 #
 # @param    $1  String to be expanded.  A safe word consists of the following
 #               sequence of substrings:
@@ -207,7 +207,12 @@ _comp_dequote__initialize
 #                 quotations, parameter expansions are allowed.
 #
 # @var[out] REPLY  Array that contains the expanded results.  Multiple words or
-#                  no words may be generated through pathname expansions.
+#                  no words may be generated through pathname expansions.  If
+#                  $1 is not a safe word, REPLY contains the literal value of
+#                  $1.
+#
+# @return 0 if $1 is a safe word and the expansion result contains one word at
+#     least, or 1 otherwise.
 #
 # Note: This function allows parameter expansions as safe strings, which might
 # cause unexpected results:
@@ -235,8 +240,37 @@ _comp_dequote__initialize
 _comp_dequote()
 {
     REPLY=() # fallback value for unsafe word and failglob
-    [[ $1 =~ $_comp_dequote__regex_safe_word ]] || return 1
-    eval "REPLY=($1)" 2>/dev/null # may produce failglob
+    if [[ ${1-} =~ $_comp_dequote__regex_safe_word ]]; then
+        eval "REPLY=($1)" 2>/dev/null # may produce failglob
+        ((${#REPLY[@]} > 0))
+        return "$?"
+    else
+        # shellcheck disable=SC2178
+        REPLY=${1-}
+        return 1
+    fi
+}
+
+# Try to reconstruct an incomplete word and apply _comp_dequote.
+# @param    $1  String to be expanded.  The same as _comp_dequote, but
+#               incomplete backslash, single quotation, and double quotation
+#               are allowed.
+# @var[out] REPLY  Result.  The same as _comp_dequote.
+# @since 2.17
+_comp_dequote_incomplete()
+{
+    local _word=${1-}
+    if ! [[ $_word =~ $_comp_dequote__regex_safe_word ]]; then
+        # shellcheck disable=SC1003
+        if [[ ${_word%'\'} =~ $_comp_dequote__regex_safe_word ]]; then
+            _word=${_word%'\'}
+        elif [[ $_word\' =~ $_comp_dequote__regex_safe_word ]]; then
+            _word=$_word\'
+        elif [[ $_word\" =~ $_comp_dequote__regex_safe_word ]]; then
+            _word=$_word\"
+        fi
+    fi
+    _comp_dequote "$_word"
 }
 
 # Unset the given variables across a scope boundary. Useful for unshadowing
@@ -1559,7 +1593,7 @@ _comp_compgen_help__get_help_lines()
         --) shift 1 ;&
         *)
             local REPLY
-            _comp_dequote "${comp_args[0]-}" || REPLY=${comp_args[0]-}
+            _comp_dequote "${comp_args[0]-}"
             help_cmd=("${REPLY:-false}" "$@")
             ;;
     esac
@@ -2855,8 +2889,8 @@ _comp_command_offset()
     if ((COMP_CWORD == 0)); then
         _comp_compgen_commands
     else
-        _comp_dequote "${COMP_WORDS[0]}" || REPLY=${COMP_WORDS[0]}
-        local cmd=$REPLY compcmd=$REPLY
+        _comp_dequote "${COMP_WORDS[0]}"
+        local cmd=${REPLY-} compcmd=${REPLY-}
         local cspec=$(complete -p -- "$cmd" 2>/dev/null)
 
         # If we have no completion for $cmd yet, see if we have for basename

completions/java

@@ -113,12 +113,16 @@ _comp_cmd_java__packages()
     _comp_cmd_java__find_sourcepath || return 0
     local -a sourcepaths=("${REPLY[@]}")
 
+    local REPLY
+    _comp_dequote_incomplete "$cur"
+    local cur_val=${REPLY-}
+
     # convert package syntax to path syntax
-    local cur=${cur//.//}
+    local cur_val=${cur_val//.//}
     # parse each sourcepath element for packages
     for i in "${sourcepaths[@]}"; do
         if [[ -d $i ]]; then
-            _comp_expand_glob files '"$i/$cur"*' || continue
+            _comp_expand_glob files '"$i/$cur_val"*' || continue
             _comp_split -la COMPREPLY "$(
                 command ls -F -d "${files[@]}" 2>/dev/null |
                     command sed -e 's|^'"$i"'/||'

completions/make

@@ -121,7 +121,7 @@ _comp_cmd_make()
                 # Expand tilde expansion
                 local REPLY
                 _comp_dequote "${words[i + 1]-}" &&
-                    [[ -d ${REPLY-} ]] &&
+                    [[ -d $REPLY ]] &&
                     makef_dir=(-C "$REPLY")
                 break
             fi
@@ -134,7 +134,7 @@ _comp_cmd_make()
                 # Expand tilde expansion
                 local REPLY
                 _comp_dequote "${words[i + 1]-}" &&
-                    [[ -f ${REPLY-} ]] &&
+                    [[ -f $REPLY ]] &&
                     makef=(-f "$REPLY")
                 break
             fi

completions/mutt

@@ -32,7 +32,7 @@ _comp_cmd_mutt__get_muttrc()
         shift
     done
 
-    if [[ ! $REPLY ]]; then
+    if [[ ! ${REPLY-} ]]; then
         if [[ -f ~/.${muttcmd}rc ]]; then
             REPLY=\~/.${muttcmd}rc
         elif [[ -f ~/.${muttcmd}/${muttcmd}rc ]]; then
@@ -52,7 +52,7 @@ _comp_cmd_mutt__get_conffiles()
     local file
     for file; do
         _comp_dequote "$file"
-        _comp_cmd_mutt__get_conffiles__visit "$REPLY"
+        _comp_cmd_mutt__get_conffiles__visit "${REPLY-}"
     done
     ((${#conffiles[@]})) || return 1
     REPLY=("${conffiles[@]}")

completions/pkgutil

@@ -33,8 +33,7 @@ _comp_cmd_pkgutil()
             catalog_files=("$REPLY")
         elif [[ ${words[i]} == --config ]]; then
             local REPLY
-            _comp_dequote "${words[i + 1]}"
-            [[ ${REPLY-} ]] && configuration_files=("$REPLY")
+            _comp_dequote "${words[i + 1]}" && configuration_files=("$REPLY")
         elif [[ ${words[i]} == -@([iurdacUS]|-install|-upgrade|-remove|-download|-available|-compare|-catalog|-stream) ]]; then
             command="${words[i]}"
         fi

completions/ssh

@@ -459,9 +459,63 @@ _comp_cmd_sftp()
     shopt -u hostcomplete && complete -F _comp_cmd_sftp sftp
 
 # things we want to backslash escape in scp paths
-# shellcheck disable=SC2089
 _comp_cmd_scp__path_esc='[][(){}<>"'"'"',:;^&!$=?`\\|[:space:]]'
 
+# Escape shell special characters in filenames by backslash.  This also
+# suffixes a space or a slash based on the file type.
+#
+# Note: With a non-empty prefix ($1 of _comp_xfunc_scp_compgen_local_files),
+# Bash will not recognize any filenames, so we need to perform the proper
+# quoting manually.  We also need to manually suffix a space or a slash based
+# on the file type because "-o nospace" is specified.  One might think of using
+# "compopt +o nospace" instead, but it would suffix a space to directory names
+# unexpectedly.
+#
+# FIXME: With the current strategy of using "ls -FL", we cannot distinguish the
+# filenames that end with one of the type-classifier characters.  For example,
+# a regular file "pipe|" and a named pipe "pipe" would both produce the
+# identical result "pipe|" with "ls -1FL".  As a consequence, those characters
+# at the end of the filename are removed unexpectedly.  To solve this problem,
+# we need to give up relying on "ls -1FL".
+#
+# Options:
+#     -d  Only directory names are selected.
+# @param $2 escape_replacement - If a non-empty value is specified, special
+#     characters are replaced with the specified value (instead of the default
+#     '\\&').
+# @stdin List of filenames in the "ls -1F" format, where filenames are
+#     separated by newlines, and characters /=@|* are suffixed based on the
+#     types of the files.
+_comp_cmd_scp__escape_path()
+{
+    local OPTIND=1 OPTARG="" OPTERR=0 opt dirs_only=""
+    while getopts ':d' _flag "$@"; do
+        case $_flag in
+            d) dirs_only=set ;;
+            *)
+                echo "bash_completion: $FUNCNAME: usage error: $*" >&2
+                return 1
+                ;;
+        esac
+    done
+    shift "$((OPTIND - 1))"
+    local escape_replacement=${1:-'\\&'}
+
+    if [[ $dirs_only ]]; then
+        # escape problematic characters; remove non-dirs
+        command sed \
+            -e '/[^/]$/d' \
+            -e 's/'"$_comp_cmd_scp__path_esc"'/'"$escape_replacement"'/g'
+    else
+        # escape problematic characters; remove executables, aliases, pipes
+        # and sockets; add space at end of file names
+        command sed \
+            -e 's/[*@|=]$//g' \
+            -e 's/'"$_comp_cmd_scp__path_esc"'/'"$escape_replacement"'/g' \
+            -e 's/[^/]$/& /g'
+    fi
+}
+
 # Complete remote files with ssh. Returns paths escaped with three backslashes
 # (unless -l option is provided).
 # Options:
@@ -487,14 +541,30 @@ _comp_xfunc_scp_compgen_remote_files()
     done
 
     # remove backslash escape from the first colon
-    local cur=${cur/\\:/:}
-
-    local _userhost=${cur%%?(\\):*}
-    local _path=${cur#*:}
+    local REPLY=$cur
+    if [[ ! $_less_escaping ]]; then
+        # unescape (3 backslashes to 1 for chars we escaped)
+        #
+        # Note: We want to do the following, but POSIX BRE does not support \|:
+        #
+        #     REPLY=$(command sed -e 's/\\\\\\\('"$_comp_cmd_scp__path_esc"'\|$\)/\\\1/g' <<<"$REPLY")
+        #
+        # Note: We need to store \\\\\\ in a variable to work around "shopt -s
+        # compat31".
+        local _tail=$REPLY _regex_triple_backslashes='\\\\\\('$_comp_cmd_scp__path_esc'|$)(.*)$'
+        REPLY=
+        while [[ $_tail && $_tail =~ $_regex_triple_backslashes ]]; do
+            # shellcheck disable=SC1003
+            REPLY=${_tail::${#_tail}-${#BASH_REMATCH}}'\'${BASH_REMATCH[1]}
+            _tail=${BASH_REMATCH[2]}
+        done
+        REPLY+=$_tail
+    fi
+    _comp_dequote_incomplete "$REPLY"
+    local cur_val=${REPLY-}
 
-    # unescape (3 backslashes to 1 for chars we escaped)
-    # shellcheck disable=SC2090
-    _path=$(command sed -e 's/\\\\\\\('"$_comp_cmd_scp__path_esc"'\)/\\\1/g' <<<"$_path")
+    local _userhost=${cur_val%%:*}
+    local _path=${cur_val#*:}
 
     # default to home dir of specified user on remote host
     if [[ ! $_path ]]; then
@@ -507,21 +577,10 @@ _comp_xfunc_scp_compgen_remote_files()
     fi
 
     local _files
-    if [[ $_dirs_only ]]; then
-        # escape problematic characters; remove non-dirs
-        # shellcheck disable=SC2090
-        _files=$(ssh -o 'Batchmode yes' "$_userhost" \
-            command ls -aF1dL "$_path*" 2>/dev/null |
-            command sed -e 's/'"$_comp_cmd_scp__path_esc"'/'"$_escape_replacement"'/g' -e '/[^\/]$/d')
-    else
-        # escape problematic characters; remove executables, aliases, pipes
-        # and sockets; add space at end of file names
-        # shellcheck disable=SC2090
-        _files=$(ssh -o 'Batchmode yes' "$_userhost" \
-            command ls -aF1dL "$_path*" 2>/dev/null |
-            command sed -e 's/'"$_comp_cmd_scp__path_esc"'/'"$_escape_replacement"'/g' -e 's/[*@|=]$//g' \
-                -e 's/[^\/]$/& /g')
-    fi
+    _files=$(ssh -o 'Batchmode yes' "$_userhost" \
+        command ls -aF1dL "$_path*" 2>/dev/null |
+        _comp_cmd_scp__escape_path ${_dirs_only:+'-d'} -- \
+            "$_escape_replacement")
     _comp_compgen -R split -l -- "$_files"
 }
 
@@ -538,27 +597,22 @@ _scp_remote_files()
 # @since 2.12
 _comp_xfunc_scp_compgen_local_files()
 {
-    local _dirsonly=""
+    local _dirs_only=""
     if [[ ${1-} == -d ]]; then
-        _dirsonly=set
+        _dirs_only=set
         shift
     fi
 
+    local REPLY
+    _comp_dequote_incomplete "$cur"
+    local cur_val=${REPLY-}
+
     local files
-    _comp_expand_glob files '"$cur"*' || return 0
-    if [[ $_dirsonly ]]; then
-        _comp_compgen -U files split -l -- "$(
-            command ls -aF1dL "${files[@]}" 2>/dev/null |
-                command sed -e "s/$_comp_cmd_scp__path_esc/\\\\&/g" \
-                    -e '/[^\/]$/d' -e "s/^/${1-}/"
-        )"
-    else
-        _comp_compgen -U files split -l -- "$(
-            command ls -aF1dL "${files[@]}" 2>/dev/null |
-                command sed -e "s/$_comp_cmd_scp__path_esc/\\\\&/g" \
-                    -e 's/[*@|=]$//g' -e 's/[^\/]$/& /g' -e "s/^/${1-}/"
-        )"
-    fi
+    _comp_expand_glob files '"$cur_val"*' || return 0
+    _comp_compgen -RU files split -l ${1:+-P "$1"} -- "$(
+        command ls -aF1dL "${files[@]}" 2>/dev/null |
+            _comp_cmd_scp__escape_path ${_dirs_only:+'-d'}
+    )"
 }
 
 # @deprecated 2.12

test/t/test_cancel.py

@@ -16,7 +16,7 @@ def added_job(self, request, bash):
             )
         except AssertionError:
             pytest.skip("Could not add test print job")
-            return
+
         if len(got) > 3:
             request.addfinalizer(
                 lambda: assert_bash_exec(bash, "cancel %s" % got[3])

test/t/test_ls.py

@@ -33,7 +33,7 @@ def test_3(self, bash):
         part_full = find_unique_completion_pair(res)
         if not part_full:
             pytest.skip("No suitable test user found")
-            return
+
         part, full = part_full
         completion = assert_complete(bash, "ls ~%s" % part)
         assert completion == full[len(part) :]

test/t/test_man.py

@@ -23,7 +23,7 @@ def colonpath(self, request, bash):
             pass
         else:
             pytest.skip("Cygwin doesn't like paths with colons")
-            return
+
         tmpdir, _, _ = prepare_fixture_dir(
             request,
             files=["man/man3/Bash::Completion.3pm.gz"],

test/t/test_rsync.py

@@ -73,3 +73,15 @@ def test_remote_path_with_spaces(self, bash):
             completion == r"\ in\ filename.txt"
             or completion == r"\\\ in\\\ filename.txt"
         )
+
+    @pytest.mark.complete(r"rsync -na spaced\ ", cwd="scp")
+    def test_local_path_with_spaces(self, completion):
+        """This function tests xfunc _comp_xfunc_scp_compgen_local_files, which
+        is defined in completions/ssh, through the rsync interface.  We reuse
+        the fixture directory for the test of the scp completion.
+
+        The expected result depends on the rsync version, so we check the
+        result if it matches either one of two possible expected results.
+
+        """
+        assert completion == r"\ conf" or completion == r"\\\ conf"