From c7be2064a8c9dbe70608d4535626ff0bec9a2928 Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sat, 5 Apr 2025 13:35:25 +0900 Subject: [PATCH 1/2] 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 189995176b0..14ac725f639 100644 --- a/completions/ssh +++ b/completions/ssh @@ -530,13 +530,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 @@ -575,8 +578,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:+'-d'} 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 d28a0eb6ada..e7e9e18e291 100644 --- a/test/t/test_scp.py +++ b/test/t/test_scp.py @@ -205,3 +205,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 badc247263341e460e56dcbb29e8df2750363952 Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sat, 5 Apr 2025 16:01:02 +0900 Subject: [PATCH 2/2] 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 14ac725f639..abe41c010a5 100644 --- a/completions/ssh +++ b/completions/ssh @@ -467,6 +467,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 $1 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 e7e9e18e291..1555fc7fcb5 100644 --- a/test/t/test_scp.py +++ b/test/t/test_scp.py @@ -183,7 +183,13 @@ def test_remote_path_ending_with_backslash(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=[], + ) # If the system allows creating a named pipe, we create it in a # temporary directory and returns the path. We cannot check the @@ -206,6 +212,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"