From 07ac056e81a07df130e44365dd28698c37ef7fd8 Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sat, 5 Apr 2025 13:08:25 +0900 Subject: [PATCH 1/9] [waiting for PR1371] * fix(scp): fix a bug that `-F[TAB]` did not complete at all * fix(rsync,ssh,sshfs): do not generate regular files *'\' as dirs * fix(rsync,scp): remove file-type marks from "ls -F" properly * refactor(ssh): remove outdated disable=SC2089,SC2090 * refactor(scp): use "_comp_compgen_split -P" to add prefix * fix(_comp_command_offset): work around nounset * refactor(scp): rename local var "_dirs{ => _}only" for consistency * test(cancel,ls,man): remove redundant "return" --- bash_completion | 2 +- completions/ssh | 28 ++++++++-------- test/fixtures/sshfs/local_path-dir/dummy.txt | 0 "test/fixtures/sshfs/local_path-file\\" | 0 test/t/test_cancel.py | 2 +- test/t/test_ls.py | 2 +- test/t/test_man.py | 2 +- test/t/test_scp.py | 34 +++++++++++++++++++- test/t/test_sshfs.py | 4 +++ 9 files changed, 54 insertions(+), 20 deletions(-) create mode 100644 test/fixtures/sshfs/local_path-dir/dummy.txt create mode 100644 "test/fixtures/sshfs/local_path-file\\" diff --git a/bash_completion b/bash_completion index 89a9584be12..cf16b672924 100644 --- a/bash_completion +++ b/bash_completion @@ -2856,7 +2856,7 @@ _comp_command_offset() _comp_compgen_commands else _comp_dequote "${COMP_WORDS[0]}" || REPLY=${COMP_WORDS[0]} - local cmd=$REPLY compcmd=$REPLY + 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 diff --git a/completions/ssh b/completions/ssh index b8e84b38f88..55753333684 100644 --- a/completions/ssh +++ b/completions/ssh @@ -459,7 +459,6 @@ _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:]]' # Complete remote files with ssh. Returns paths escaped with three backslashes @@ -493,7 +492,6 @@ _comp_xfunc_scp_compgen_remote_files() local _path=${cur#*:} # 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") # default to home dir of specified user on remote host @@ -509,18 +507,17 @@ _comp_xfunc_scp_compgen_remote_files() 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') + 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') + command sed -e 's/[*@|=]$//g' \ + -e 's/'"$_comp_cmd_scp__path_esc"'/'"$_escape_replacement"'/g' \ + -e 's/[^/]$/& /g') fi _comp_compgen -R split -l -- "$_files" } @@ -538,25 +535,26 @@ _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 files _comp_expand_glob files '"$cur"*' || return 0 - if [[ $_dirsonly ]]; then - _comp_compgen -U files split -l -- "$( + if [[ $_dirs_only ]]; then + _comp_compgen -RU files split -l ${1:+-P "$1"} -- "$( command ls -aF1dL "${files[@]}" 2>/dev/null | command sed -e "s/$_comp_cmd_scp__path_esc/\\\\&/g" \ - -e '/[^\/]$/d' -e "s/^/${1-}/" + -e '/[^/]$/d' )" else - _comp_compgen -U files split -l -- "$( + _comp_compgen -RU files split -l ${1:+-P "$1"} -- "$( 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-}/" + command sed -e 's/[*@|=]$//g' \ + -e "s/$_comp_cmd_scp__path_esc/\\\\&/g" \ + -e 's/[^/]$/& /g' )" fi } diff --git a/test/fixtures/sshfs/local_path-dir/dummy.txt b/test/fixtures/sshfs/local_path-dir/dummy.txt new file mode 100644 index 00000000000..e69de29bb2d diff --git "a/test/fixtures/sshfs/local_path-file\\" "b/test/fixtures/sshfs/local_path-file\\" new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/t/test_cancel.py b/test/t/test_cancel.py index 4aeafd2c5fa..9384908c630 100644 --- a/test/t/test_cancel.py +++ b/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]) diff --git a/test/t/test_ls.py b/test/t/test_ls.py index f91ee6b2e2e..56f0ee71e90 100644 --- a/test/t/test_ls.py +++ b/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) :] diff --git a/test/t/test_man.py b/test/t/test_man.py index 081b8fcc1e7..bec41de9613 100644 --- a/test/t/test_man.py +++ b/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"], diff --git a/test/t/test_scp.py b/test/t/test_scp.py index b91e5476827..e630bbdc9f9 100644 --- a/test/t/test_scp.py +++ b/test/t/test_scp.py @@ -2,7 +2,12 @@ import pytest -from conftest import assert_bash_exec, assert_complete, bash_env_saved +from conftest import ( + assert_bash_exec, + assert_complete, + bash_env_saved, + prepare_fixture_dir, +) LIVE_HOST = "bash_completion" @@ -55,6 +60,14 @@ def test_capital_f_without_space(self, completion): "option requires an argument -- F" in x for x in completion ) + @pytest.mark.complete("scp -Fconf", cwd="scp") + def test_capital_f_without_space_2(self, completion): + assert completion == "ig" + + @pytest.mark.complete("scp -Fbi", cwd="scp") + def test_capital_f_without_space_3(self, completion): + assert completion == "n/" + @pytest.fixture(scope="class") def live_pwd(self, bash): try: @@ -141,3 +154,22 @@ def test_xfunc_remote_files(self, bash): "shared/default/foo ", "shared/default/foo.d/", ] + + @pytest.fixture + def tmpdir_mkfifo(self, request, bash): + tmpdir, _, _ = prepare_fixture_dir(request, files=[], dirs=[]) + + try: + assert_bash_exec(bash, "mkfifo '%s/local_path_1-pipe'" % tmpdir) + except Exception: + pytest.skip( + "The present system does not allow creating a named pipe." + ) + + return tmpdir + + def test_local_path_mark_1(self, bash, tmpdir_mkfifo): + completion = assert_complete( + bash, "scp local_path_1-", cwd=tmpdir_mkfifo + ) + assert completion == "pipe" diff --git a/test/t/test_sshfs.py b/test/t/test_sshfs.py index cb4189bf5e1..71977adf367 100644 --- a/test/t/test_sshfs.py +++ b/test/t/test_sshfs.py @@ -6,3 +6,7 @@ class TestSshfs: @pytest.mark.complete("sshfs ./") def test_1(self, completion): assert completion + + @pytest.mark.complete("sshfs local_path", cwd="sshfs") + def test_local_path_suffix_1(self, completion): + assert completion == "-dir/" From 6bc12ac1c98a5ad01d978c78a36c2eb3bd50adca Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sat, 5 Apr 2025 15:27:26 +0900 Subject: [PATCH 2/9] refactor(scp): factorize common codes --- completions/ssh | 71 ++++++++++++++++++++++++++++++------------------- 1 file changed, 43 insertions(+), 28 deletions(-) diff --git a/completions/ssh b/completions/ssh index 55753333684..8c9f1508a16 100644 --- a/completions/ssh +++ b/completions/ssh @@ -461,6 +461,42 @@ _comp_cmd_sftp() # things we want to backslash escape in scp paths _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. +# +# 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 dirs_only=$1 escape_replacement=${2:-'\\&'} + 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: @@ -505,20 +541,9 @@ _comp_xfunc_scp_compgen_remote_files() fi local _files - if [[ $_dirs_only ]]; then - # escape problematic characters; remove non-dirs - _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 - _files=$(ssh -o 'Batchmode yes' "$_userhost" \ - command ls -aF1dL "$_path*" 2>/dev/null | - command sed -e 's/[*@|=]$//g' \ - -e 's/'"$_comp_cmd_scp__path_esc"'/'"$_escape_replacement"'/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" "$_escape_replacement") _comp_compgen -R split -l -- "$_files" } @@ -543,20 +568,10 @@ _comp_xfunc_scp_compgen_local_files() local files _comp_expand_glob files '"$cur"*' || return 0 - if [[ $_dirs_only ]]; then - _comp_compgen -RU files split -l ${1:+-P "$1"} -- "$( - command ls -aF1dL "${files[@]}" 2>/dev/null | - command sed -e "s/$_comp_cmd_scp__path_esc/\\\\&/g" \ - -e '/[^/]$/d' - )" - else - _comp_compgen -RU files split -l ${1:+-P "$1"} -- "$( - command ls -aF1dL "${files[@]}" 2>/dev/null | - command sed -e 's/[*@|=]$//g' \ - -e "s/$_comp_cmd_scp__path_esc/\\\\&/g" \ - -e 's/[^/]$/& /g' - )" - fi + _comp_compgen -RU files split -l ${1:+-P "$1"} -- "$( + command ls -aF1dL "${files[@]}" 2>/dev/null | + _comp_cmd_scp__escape_path "$_dirs_only" + )" } # @deprecated 2.12 From cf59417d847edc8fc8db0bf4d14cc4f2afe28757 Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sat, 5 Apr 2025 13:35:25 +0900 Subject: [PATCH 3/9] fix(java,rsync,scp): handle quoted space in filepaths properly Filenames containing a special character are not properly completed [1]. These completions generate filenames by pathname expanaion using $cur. However, $cur contains the word on the command line including quotaing, such as cur='file\ with\ space.txt' and cur='"a b c.txt"'. This patch obtains the value of "cur" using _comp_dequote. [1] https://github.com/scop/bash-completion/issues/1232 This patch also fixes a similar case in completions/java. Co-authored-by: Yedaya Katsman --- completions/java | 8 ++++++-- completions/ssh | 21 ++++++++++++++------- test/t/test_rsync.py | 12 ++++++++++++ test/t/test_scp.py | 8 ++++++++ 4 files changed, 40 insertions(+), 9 deletions(-) diff --git a/completions/java b/completions/java index 24c6a8a1987..0827f7fd8c7 100644 --- a/completions/java +++ b/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 "$cur" || REPLY=$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"'/||' diff --git a/completions/ssh b/completions/ssh index 8c9f1508a16..779ffad7df1 100644 --- a/completions/ssh +++ b/completions/ssh @@ -522,13 +522,16 @@ _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) + REPLY=$(command sed -e 's/\\\\\\\('"$_comp_cmd_scp__path_esc"'\)/\\\1/g' <<<"$REPLY") + fi + _comp_dequote "$REPLY" + local cur_val=${REPLY-} - # unescape (3 backslashes to 1 for chars we escaped) - _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 @@ -566,8 +569,12 @@ _comp_xfunc_scp_compgen_local_files() shift fi + local REPLY + _comp_dequote "$cur" || REPLY=$cur + local cur_val=${REPLY-} + local files - _comp_expand_glob files '"$cur"*' || return 0 + _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" diff --git a/test/t/test_rsync.py b/test/t/test_rsync.py index 56fa95aa51a..e440c6ee81c 100644 --- a/test/t/test_rsync.py +++ b/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" diff --git a/test/t/test_scp.py b/test/t/test_scp.py index e630bbdc9f9..933fbb410cd 100644 --- a/test/t/test_scp.py +++ b/test/t/test_scp.py @@ -173,3 +173,11 @@ def test_local_path_mark_1(self, bash, tmpdir_mkfifo): bash, "scp local_path_1-", cwd=tmpdir_mkfifo ) assert completion == "pipe" + + @pytest.mark.complete("scp spa", cwd="scp") + def test_local_path_with_spaces_1(self, completion): + assert completion == r"ced\ \ conf" + + @pytest.mark.complete(r"scp spaced\ ", cwd="scp") + def test_local_path_with_spaces_2(self, completion): + assert completion == r"\ conf" From 6ddd0187a6edcbedd5cf7c1ba3efd3aedfc40b7d Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sat, 5 Apr 2025 16:01:02 +0900 Subject: [PATCH 4/9] test(scp): leave comments for a failing test case --- completions/ssh | 7 +++++++ test/t/test_scp.py | 15 ++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/completions/ssh b/completions/ssh index 779ffad7df1..17e7957f7cd 100644 --- a/completions/ssh +++ b/completions/ssh @@ -471,6 +471,13 @@ _comp_cmd_scp__path_esc='[][(){}<>"'"'"',:;^&!$=?`\\|[:space:]]' # "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 diff --git a/test/t/test_scp.py b/test/t/test_scp.py index 933fbb410cd..43497b014db 100644 --- a/test/t/test_scp.py +++ b/test/t/test_scp.py @@ -157,7 +157,13 @@ def test_xfunc_remote_files(self, bash): @pytest.fixture def tmpdir_mkfifo(self, request, bash): - tmpdir, _, _ = prepare_fixture_dir(request, files=[], dirs=[]) + # We prepare two files: 1) a named pipe and 2) a regular file ending + # with the same name but an extra special character "|". + tmpdir, _, _ = prepare_fixture_dir( + request, + files=["local_path_2-pipe|"], + dirs=[], + ) try: assert_bash_exec(bash, "mkfifo '%s/local_path_1-pipe'" % tmpdir) @@ -174,6 +180,13 @@ def test_local_path_mark_1(self, bash, tmpdir_mkfifo): ) assert completion == "pipe" + # FIXME: This test currently fails. + # def test_local_path_mark_2(self, bash, tmpdir_mkfifo): + # completion = assert_complete( + # bash, "scp local_path_2-", cwd=tmpdir_mkfifo + # ) + # assert completion == "pipe\\|" + @pytest.mark.complete("scp spa", cwd="scp") def test_local_path_with_spaces_1(self, completion): assert completion == r"ced\ \ conf" From 11e318e4f0096863daa06ae706514b989a7e5f3b Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sat, 5 Apr 2025 21:53:07 +0900 Subject: [PATCH 5/9] fix(_comp_dequote): set the literal value to REPLY as a fallback It is more useful to set the literal value of $1 to REPLY even when $1 is an unsafe word. When the caller wants to use the result only when REPLY is a safe string and suceceds without failglob, the caller can reference the exit status $?. When the caller wants to use any string, it is more useful if REPLY contains a fallback value. This patch changes the behavior so that REPLY is set to be the literal value of $1 even when an unsafe value is specified to $1. We also change the exit status to be 0 only when at least one word is generated by a safe string. --- bash_completion | 32 ++++++++++++++++++++++---------- completions/java | 2 +- completions/make | 4 ++-- completions/mutt | 4 ++-- completions/pkgutil | 3 +-- completions/ssh | 2 +- test/t/unit/test_unit_dequote.py | 12 ++++++------ 7 files changed, 35 insertions(+), 24 deletions(-) diff --git a/bash_completion b/bash_completion index cf16b672924..31233c6db96 100644 --- a/bash_completion +++ b/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,15 @@ _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 } # Unset the given variables across a scope boundary. Useful for unshadowing @@ -1559,7 +1571,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,7 +2867,7 @@ _comp_command_offset() if ((COMP_CWORD == 0)); then _comp_compgen_commands else - _comp_dequote "${COMP_WORDS[0]}" || REPLY=${COMP_WORDS[0]} + _comp_dequote "${COMP_WORDS[0]}" local cmd=${REPLY-} compcmd=${REPLY-} local cspec=$(complete -p -- "$cmd" 2>/dev/null) diff --git a/completions/java b/completions/java index 0827f7fd8c7..4ea6b817635 100644 --- a/completions/java +++ b/completions/java @@ -114,7 +114,7 @@ _comp_cmd_java__packages() local -a sourcepaths=("${REPLY[@]}") local REPLY - _comp_dequote "$cur" || REPLY=$cur + _comp_dequote "$cur" local cur_val=${REPLY-} # convert package syntax to path syntax diff --git a/completions/make b/completions/make index 94e2b73b9de..a0e491cd78c 100644 --- a/completions/make +++ b/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 diff --git a/completions/mutt b/completions/mutt index 5ebf65a463f..a741c8cdf9b 100644 --- a/completions/mutt +++ b/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[@]}") diff --git a/completions/pkgutil b/completions/pkgutil index 0b2e4ec5d4b..d80f3679e91 100644 --- a/completions/pkgutil +++ b/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 diff --git a/completions/ssh b/completions/ssh index 17e7957f7cd..4623f9cae8b 100644 --- a/completions/ssh +++ b/completions/ssh @@ -577,7 +577,7 @@ _comp_xfunc_scp_compgen_local_files() fi local REPLY - _comp_dequote "$cur" || REPLY=$cur + _comp_dequote "$cur" local cur_val=${REPLY-} local files diff --git a/test/t/unit/test_unit_dequote.py b/test/t/unit/test_unit_dequote.py index 117a487758d..5082f60af06 100644 --- a/test/t/unit/test_unit_dequote.py +++ b/test/t/unit/test_unit_dequote.py @@ -25,7 +25,7 @@ def test_2_str(self, bash, functions): assert output.strip() == "" def test_3_null(self, bash, functions): - output = assert_bash_exec(bash, "__tester ''", want_output=True) + output = assert_bash_exec(bash, "! __tester ''", want_output=True) assert output.strip() == "" def test_4_empty(self, bash, functions): @@ -108,25 +108,25 @@ def test_unsafe_1(self, bash, functions): output = assert_bash_exec( bash, "! __tester '$(echo hello >&2)'", want_output=True ) - assert output.strip() == "" + assert output.strip() == "<$(echo hello >&2)>" def test_unsafe_2(self, bash, functions): output = assert_bash_exec( bash, "! __tester '|echo hello >&2'", want_output=True ) - assert output.strip() == "" + assert output.strip() == "<|echo hello >&2>" def test_unsafe_3(self, bash, functions): output = assert_bash_exec( bash, "! __tester '>| important_file.txt'", want_output=True ) - assert output.strip() == "" + assert output.strip() == "<>| important_file.txt>" def test_unsafe_4(self, bash, functions): output = assert_bash_exec( bash, "! __tester '`echo hello >&2`'", want_output=True ) - assert output.strip() == "" + assert output.strip() == "<`echo hello >&2`>" def test_glob_default(self, bash, functions): with bash_env_saved(bash) as bash_env: @@ -160,6 +160,6 @@ def test_glob_nullglob(self, bash, functions): bash_env.shopt("failglob", False) bash_env.shopt("nullglob", True) output = assert_bash_exec( - bash, "__tester 'non-existent-*.txt'", want_output=True + bash, "! __tester 'non-existent-*.txt'", want_output=True ) assert output.strip() == "" From 4eddf94890604d13af0c090922a007eeb3b379d1 Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sat, 5 Apr 2025 22:14:35 +0900 Subject: [PATCH 6/9] fix(java,rsync,ssh): complete syntactically incomplete cur https://github.com/scop/bash-completion/issues/1255#issuecomment-2437499379 Co-authored-by: Yedaya Katsman --- bash_completion | 22 +++++ completions/java | 2 +- completions/ssh | 4 +- .../fixtures/scp/local_path/backslash-a b.txt | 0 .../scp/local_path/backslash-a\\ b.txt" | 0 test/t/test_scp.py | 35 +++++++- test/t/unit/Makefile.am | 1 + test/t/unit/test_unit_dequote_incomplete.py | 83 +++++++++++++++++++ 8 files changed, 142 insertions(+), 5 deletions(-) create mode 100644 test/fixtures/scp/local_path/backslash-a b.txt create mode 100644 "test/fixtures/scp/local_path/backslash-a\\ b.txt" create mode 100644 test/t/unit/test_unit_dequote_incomplete.py diff --git a/bash_completion b/bash_completion index 31233c6db96..52129777d83 100644 --- a/bash_completion +++ b/bash_completion @@ -251,6 +251,28 @@ _comp_dequote() 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 # global scoped variables. Note that simply calling unset on a local variable # will not unshadow the global variable. Rather, the result will be a local diff --git a/completions/java b/completions/java index 4ea6b817635..1a3630ded49 100644 --- a/completions/java +++ b/completions/java @@ -114,7 +114,7 @@ _comp_cmd_java__packages() local -a sourcepaths=("${REPLY[@]}") local REPLY - _comp_dequote "$cur" + _comp_dequote_incomplete "$cur" local cur_val=${REPLY-} # convert package syntax to path syntax diff --git a/completions/ssh b/completions/ssh index 4623f9cae8b..66d0b07f7cc 100644 --- a/completions/ssh +++ b/completions/ssh @@ -534,7 +534,7 @@ _comp_xfunc_scp_compgen_remote_files() # unescape (3 backslashes to 1 for chars we escaped) REPLY=$(command sed -e 's/\\\\\\\('"$_comp_cmd_scp__path_esc"'\)/\\\1/g' <<<"$REPLY") fi - _comp_dequote "$REPLY" + _comp_dequote_incomplete "$REPLY" local cur_val=${REPLY-} local _userhost=${cur_val%%:*} @@ -577,7 +577,7 @@ _comp_xfunc_scp_compgen_local_files() fi local REPLY - _comp_dequote "$cur" + _comp_dequote_incomplete "$cur" local cur_val=${REPLY-} local files diff --git a/test/fixtures/scp/local_path/backslash-a b.txt b/test/fixtures/scp/local_path/backslash-a b.txt new file mode 100644 index 00000000000..e69de29bb2d diff --git "a/test/fixtures/scp/local_path/backslash-a\\ b.txt" "b/test/fixtures/scp/local_path/backslash-a\\ b.txt" new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/t/test_scp.py b/test/t/test_scp.py index 43497b014db..ab0ac1e97c9 100644 --- a/test/t/test_scp.py +++ b/test/t/test_scp.py @@ -28,7 +28,13 @@ def test_basic(self, hosts, completion): ) ), # Local filenames - ["bin/", "config", "known_hosts", r"spaced\ \ conf"], + [ + "bin/", + "config", + "known_hosts", + "local_path/", + r"spaced\ \ conf", + ], ) ) assert completion == expected @@ -48,7 +54,13 @@ def test_basic_spaced_conf(self, hosts, completion): ) ), # Local filenames - ["bin/", "config", "known_hosts", r"spaced\ \ conf"], + [ + "bin/", + "config", + "known_hosts", + "local_path/", + r"spaced\ \ conf", + ], ) ) assert completion == expected @@ -115,6 +127,19 @@ def test_remote_path_with_spaces(self, bash): assert_bash_exec(bash, "unset -f ssh") assert completion == r"\\\ in\\\ filename.txt" + def test_remote_path_with_backslash(self, bash): + assert_bash_exec( + bash, r"ssh() { printf '%s\n' 'abc def.txt' 'abc\ def.txt'; }" + ) + completion = assert_complete(bash, "scp remote_host:abc\\") + assert_bash_exec(bash, "unset -f ssh") + + # Note: The number of backslash escaping differs depending on the scp + # version. + assert completion == sorted( + [r"abc\ def.txt", r"abc\\\ def.txt"] + ) or completion == sorted([r"abc\\\ def.txt", r"abc\\\\\\\ def.txt"]) + def test_xfunc_remote_files(self, bash): with bash_env_saved(bash) as bash_env: bash_env.save_variable("COMPREPLY") @@ -194,3 +219,9 @@ def test_local_path_with_spaces_1(self, completion): @pytest.mark.complete(r"scp spaced\ ", cwd="scp") def test_local_path_with_spaces_2(self, completion): assert completion == r"\ conf" + + @pytest.mark.complete("scp backslash-a\\", cwd="scp/local_path") + def test_local_path_backslash(self, completion): + assert completion == sorted( + [r"backslash-a\ b.txt", r"backslash-a\\\ b.txt"] + ) diff --git a/test/t/unit/Makefile.am b/test/t/unit/Makefile.am index fc4cdec3a68..cb5967deab4 100644 --- a/test/t/unit/Makefile.am +++ b/test/t/unit/Makefile.am @@ -19,6 +19,7 @@ EXTRA_DIST = \ test_unit_delimited.py \ test_unit_deprecate_func.py \ test_unit_dequote.py \ + test_unit_dequote_incomplete.py \ test_unit_expand.py \ test_unit_expand_glob.py \ test_unit_expand_tilde.py \ diff --git a/test/t/unit/test_unit_dequote_incomplete.py b/test/t/unit/test_unit_dequote_incomplete.py new file mode 100644 index 00000000000..e82e60b38a5 --- /dev/null +++ b/test/t/unit/test_unit_dequote_incomplete.py @@ -0,0 +1,83 @@ +import pytest + +from conftest import assert_bash_exec + + +@pytest.mark.bashcomp( + cmd=None, + cwd="_filedir", + ignore_env=r"^\+declare -f __tester$", +) +class TestDequoteIncomplete: + @pytest.fixture + def functions(self, bash): + assert_bash_exec( + bash, + '__tester() { local REPLY=dummy v=var;_comp_dequote_incomplete "$1";local ext=$?;((${#REPLY[@]}))&&printf \'<%s>\' "${REPLY[@]}";echo;return $ext;}', + ) + + def test_basic_1(self, bash, functions): + output = assert_bash_exec(bash, "__tester a", want_output=True) + assert output.strip() == "" + + def test_basic_2(self, bash, functions): + output = assert_bash_exec(bash, "__tester abc", want_output=True) + assert output.strip() == "" + + def test_basic_3_null(self, bash, functions): + output = assert_bash_exec(bash, "! __tester ''", want_output=True) + assert output.strip() == "" + + def test_basic_4_empty(self, bash, functions): + output = assert_bash_exec(bash, "__tester \"''\"", want_output=True) + assert output.strip() == "<>" + + def test_basic_5_brace(self, bash, functions): + output = assert_bash_exec(bash, "__tester 'a{1..3}'", want_output=True) + assert output.strip() == "" + + def test_basic_6_glob(self, bash, functions): + output = assert_bash_exec(bash, "__tester 'a?b'", want_output=True) + assert output.strip() == "" + + def test_quote_1(self, bash, functions): + output = assert_bash_exec( + bash, "__tester '\"a\"'\\'b\\'\\$\\'c\\'", want_output=True + ) + assert output.strip() == "" + + def test_quote_2(self, bash, functions): + output = assert_bash_exec( + bash, "__tester '\\\"\\'\\''\\$\\`'", want_output=True + ) + assert output.strip() == "<\"'$`>" + + def test_quote_3(self, bash, functions): + output = assert_bash_exec( + bash, "__tester \\$\\'a\\\\tb\\'", want_output=True + ) + assert output.strip() == "" + + def test_quote_4(self, bash, functions): + output = assert_bash_exec( + bash, '__tester \'"abc\\"def"\'', want_output=True + ) + assert output.strip() == '' + + def test_quote_5(self, bash, functions): + output = assert_bash_exec( + bash, "__tester \\'abc\\'\\\\\\'\\'def\\'", want_output=True + ) + assert output.strip() == "" + + def test_incomplete_1(self, bash, functions): + output = assert_bash_exec(bash, "__tester 'a\\'", want_output=True) + assert output.strip() == "" + + def test_incomplete_2(self, bash, functions): + output = assert_bash_exec(bash, '__tester "\'a b "', want_output=True) + assert output.strip() == "" + + def test_incomplete_3(self, bash, functions): + output = assert_bash_exec(bash, "__tester '\"a b '", want_output=True) + assert output.strip() == "" From c4488833cea98295f8a94c38daa3e39c527ca27f Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Mon, 7 Apr 2025 06:33:41 +0900 Subject: [PATCH 7/9] refactor(ssh): accept dironly flag as an option --- completions/ssh | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/completions/ssh b/completions/ssh index 66d0b07f7cc..44c2c7054ed 100644 --- a/completions/ssh +++ b/completions/ssh @@ -488,7 +488,19 @@ _comp_cmd_scp__path_esc='[][(){}<>"'"'"',:;^&!$=?`\\|[:space:]]' # types of the files. _comp_cmd_scp__escape_path() { - local dirs_only=$1 escape_replacement=${2:-'\\&'} + 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 \ @@ -553,7 +565,8 @@ _comp_xfunc_scp_compgen_remote_files() local _files _files=$(ssh -o 'Batchmode yes' "$_userhost" \ command ls -aF1dL "$_path*" 2>/dev/null | - _comp_cmd_scp__escape_path "$_dirs_only" "$_escape_replacement") + _comp_cmd_scp__escape_path ${_dirs_only:+'-d'} -- \ + "$_escape_replacement") _comp_compgen -R split -l -- "$_files" } @@ -584,7 +597,7 @@ _comp_xfunc_scp_compgen_local_files() _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" + _comp_cmd_scp__escape_path ${_dirs_only:+'-d'} )" } From 95c107777d5aaf32ac8518b663cc1daa88948e11 Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Mon, 7 Apr 2025 07:27:30 +0900 Subject: [PATCH 8/9] test(dequote{,_incomplete}): use raw stringliteral r"" --- test/t/unit/test_unit_dequote.py | 4 ++-- test/t/unit/test_unit_dequote_incomplete.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/t/unit/test_unit_dequote.py b/test/t/unit/test_unit_dequote.py index 5082f60af06..392b54266f7 100644 --- a/test/t/unit/test_unit_dequote.py +++ b/test/t/unit/test_unit_dequote.py @@ -54,7 +54,7 @@ def test_7_quote_2(self, bash, functions): def test_7_quote_3(self, bash, functions): output = assert_bash_exec( - bash, "__tester \\$\\'a\\\\tb\\'", want_output=True + bash, r"__tester \$\'a\\tb\'", want_output=True ) assert output.strip() == "" @@ -66,7 +66,7 @@ def test_7_quote_4(self, bash, functions): def test_7_quote_5(self, bash, functions): output = assert_bash_exec( - bash, "__tester \\'abc\\'\\\\\\'\\'def\\'", want_output=True + bash, r"__tester \'abc\'\\\'\'def\'", want_output=True ) assert output.strip() == "" diff --git a/test/t/unit/test_unit_dequote_incomplete.py b/test/t/unit/test_unit_dequote_incomplete.py index e82e60b38a5..33e6db8f608 100644 --- a/test/t/unit/test_unit_dequote_incomplete.py +++ b/test/t/unit/test_unit_dequote_incomplete.py @@ -54,7 +54,7 @@ def test_quote_2(self, bash, functions): def test_quote_3(self, bash, functions): output = assert_bash_exec( - bash, "__tester \\$\\'a\\\\tb\\'", want_output=True + bash, r"__tester \$\'a\\tb\'", want_output=True ) assert output.strip() == "" @@ -66,12 +66,12 @@ def test_quote_4(self, bash, functions): def test_quote_5(self, bash, functions): output = assert_bash_exec( - bash, "__tester \\'abc\\'\\\\\\'\\'def\\'", want_output=True + bash, r"__tester \'abc\'\\\'\'def\'", want_output=True ) assert output.strip() == "" def test_incomplete_1(self, bash, functions): - output = assert_bash_exec(bash, "__tester 'a\\'", want_output=True) + output = assert_bash_exec(bash, r"__tester 'a\'", want_output=True) assert output.strip() == "" def test_incomplete_2(self, bash, functions): From c8de1594ef0c0edd0b78043c3d4a890fb97fc77e Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Mon, 7 Apr 2025 08:07:58 +0900 Subject: [PATCH 9/9] fix(scp): work around incomplete triple backslashes for remote paths --- completions/ssh | 16 +++++++++++++++- test/t/test_scp.py | 9 +++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/completions/ssh b/completions/ssh index 44c2c7054ed..4beab5771fc 100644 --- a/completions/ssh +++ b/completions/ssh @@ -544,7 +544,21 @@ _comp_xfunc_scp_compgen_remote_files() local REPLY=$cur if [[ ! $_less_escaping ]]; then # unescape (3 backslashes to 1 for chars we escaped) - REPLY=$(command sed -e 's/\\\\\\\('"$_comp_cmd_scp__path_esc"'\)/\\\1/g' <<<"$REPLY") + # + # 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-} diff --git a/test/t/test_scp.py b/test/t/test_scp.py index ab0ac1e97c9..03a43e29f9d 100644 --- a/test/t/test_scp.py +++ b/test/t/test_scp.py @@ -140,6 +140,15 @@ def test_remote_path_with_backslash(self, bash): [r"abc\ def.txt", r"abc\\\ def.txt"] ) or completion == sorted([r"abc\\\ def.txt", r"abc\\\\\\\ def.txt"]) + def test_remote_path_with_backslash_2(self, bash): + assert_bash_exec( + bash, r"ssh() { [[ $1 == abc ]]; printf '%s\n' 'abc OK'; }" + ) + completion = assert_complete(bash, "scp remote_host:abc\\\\\\") + assert_bash_exec(bash, "unset -f ssh") + + assert completion == "OK" + def test_xfunc_remote_files(self, bash): with bash_env_saved(bash) as bash_env: bash_env.save_variable("COMPREPLY")