fix(rsync,ssh): do not overescape spaces in remote filenames (#910)

* fix(rsync,ssh): do not overescape spaces in remote filenames

Fixes #848.

If a remote machine contains a file named `a b`, completing rsync
command results in `rsync remote:a\\\ b`, which results in rsync
failing to find the file.

This commit removes the extra slashes, and now completion results in
`rsync remote:a\ b`.

scp somehow accepts both variants, so this change won't break it.

* fix(rsync): overescape remote paths if rsync version is < 3.2.4
* test(rsync,ssh): test remote filenames with spaces using mock commands

completions/rsync

@@ -1,5 +1,25 @@
 # bash completion for rsync                                -*- shell-script -*-
 
+_comp_cmd_rsync__vercomp()
+{
+    if [[ $1 == "$2" ]]; then
+        return 0
+    fi
+    local i ver1 ver2
+    _comp_split -F . ver1 "$1"
+    _comp_split -F . ver2 "$2"
+    local n=$((${#ver1[@]} >= ${#ver2[@]} ? ${#ver1[@]} : ${#ver2[@]}))
+    for ((i = 0; i < n; i++)); do
+        if ((10#${ver1[i]:-0} > 10#${ver2[i]:-0})); then
+            return 1
+        fi
+        if ((10#${ver1[i]:-0} < 10#${ver2[i]:-0})); then
+            return 2
+        fi
+    done
+    return 0
+}
+
 _comp_cmd_rsync()
 {
     local cur prev words cword was_split comp_args
@@ -81,7 +101,15 @@ _comp_cmd_rsync()
                     break
                 fi
             done
-            [[ $shell == ssh ]] && _comp_compgen -x scp remote_files
+            if [[ $shell == ssh ]]; then
+                local rsync_version=$("$1" --version 2>/dev/null | sed -n '1s/.*rsync *version \([0-9.]*\).*/\1/p')
+                _comp_cmd_rsync__vercomp "$rsync_version" "3.2.4"
+                if (($? == 2)); then
+                    _comp_compgen -x scp remote_files
+                else
+                    _comp_compgen -x scp remote_files -l
+                fi
+            fi
             ;;
         *)
             _comp_compgen_known_hosts -c -a -- "$cur"

completions/ssh

@@ -462,12 +462,30 @@ _comp_cmd_sftp()
 # shellcheck disable=SC2089
 _comp_cmd_scp__path_esc='[][(){}<>"'"'"',:;^&!$=?`\\|[:space:]]'
 
-# Complete remote files with ssh.  If the first arg is -d, complete on dirs
-# only.  Returns paths escaped with three backslashes.
+# Complete remote files with ssh. Returns paths escaped with three backslashes
+# (unless -l option is provided).
+# Options:
+#     -d  Complete on dirs only.
+#     -l  Return paths escaped with one backslash instead of three.
 # @since 2.12
 # shellcheck disable=SC2120
 _comp_xfunc_scp_compgen_remote_files()
 {
+    local _dirs_only=""
+    local _less_escaping=""
+
+    local _flag OPTIND=1 OPTARG="" OPTERR=0
+    while getopts "dl" _flag "$@"; do
+        case $_flag in
+            d) _dirs_only=set ;;
+            l) _less_escaping=set ;;
+            *)
+                echo "bash_completion: $FUNCNAME: usage error: $*" >&2
+                return 1
+                ;;
+        esac
+    done
+
     # remove backslash escape from the first colon
     local cur=${cur/\\:/:}
 
@@ -483,20 +501,25 @@ _comp_xfunc_scp_compgen_remote_files()
         _path=$(ssh -o 'Batchmode yes' "$_userhost" pwd 2>/dev/null)
     fi
 
+    local _escape_replacement='\\\\\\&'
+    if [[ $_less_escaping ]]; then
+        _escape_replacement='\\&'
+    fi
+
     local _files
-    if [[ ${1-} == -d ]]; then
+    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"'/\\\\\\&/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"'/\\\\\\&/g' -e 's/[*@|=]$//g' \
+            command sed -e 's/'"$_comp_cmd_scp__path_esc"'/'"$_escape_replacement"'/g' -e 's/[*@|=]$//g' \
                 -e 's/[^\/]$/& /g')
     fi
     _comp_compgen -R split -l -- "$_files"

test/t/test_rsync.py

@@ -1,5 +1,7 @@
 import pytest
 
+from conftest import assert_bash_exec, assert_complete
+
 
 @pytest.mark.bashcomp(ignore_env=r"^[+-]_comp_cmd_scp__path_esc=")
 class TestRsync:
@@ -18,3 +20,56 @@ def test_3(self, completion):
     @pytest.mark.complete("rsync --", require_cmd=True)
     def test_4(self, completion):
         assert "--help" in completion
+
+    @pytest.mark.parametrize(
+        "ver1,ver2,result",
+        [
+            ("1", "1", "="),
+            ("1", "2", "<"),
+            ("2", "1", ">"),
+            ("1.1", "1.2", "<"),
+            ("1.2", "1.1", ">"),
+            ("1.1", "1.1.1", "<"),
+            ("1.1.1", "1.1", ">"),
+            ("1.1.1", "1.1.1", "="),
+            ("2.1", "2.2", "<"),
+            ("3.0.4.10", "3.0.4.2", ">"),
+            ("4.08", "4.08.01", "<"),
+            ("3.2.1.9.8144", "3.2", ">"),
+            ("3.2", "3.2.1.9.8144", "<"),
+            ("1.2", "2.1", "<"),
+            ("2.1", "1.2", ">"),
+            ("5.6.7", "5.6.7", "="),
+            ("1.01.1", "1.1.1", "="),
+            ("1.1.1", "1.01.1", "="),
+            ("1", "1.0", "="),
+            ("1.0", "1", "="),
+            ("1.0.2.0", "1.0.2", "="),
+            ("1..0", "1.0", "="),
+            ("1.0", "1..0", "="),
+        ],
+    )
+    def test_vercomp(self, bash, ver1, ver2, result):
+        output = assert_bash_exec(
+            bash,
+            f"_comp_cmd_rsync__vercomp {ver1} {ver2}; echo $?",
+            want_output=True,
+        ).strip()
+
+        if result == "=":
+            assert output == "0"
+        elif result == ">":
+            assert output == "1"
+        elif result == "<":
+            assert output == "2"
+        else:
+            raise Exception(f"Unsupported comparison result: {result}")
+
+    def test_remote_path_with_spaces(self, bash):
+        assert_bash_exec(bash, "ssh() { echo 'spaces in filename.txt'; }")
+        completion = assert_complete(bash, "rsync remote_host:spaces")
+        assert_bash_exec(bash, "unset -f ssh")
+        assert (
+            completion == r"\ in\ filename.txt"
+            or completion == r"\\\ in\\\ filename.txt"
+        )

test/t/test_scp.py

@@ -2,7 +2,7 @@
 
 import pytest
 
-from conftest import assert_bash_exec
+from conftest import assert_bash_exec, assert_complete
 
 LIVE_HOST = "bash_completion"
 
@@ -95,3 +95,9 @@ def test_remote_path_with_nullglob(self, completion):
     )
     def test_remote_path_with_failglob(self, completion):
         assert not completion
+
+    def test_remote_path_with_spaces(self, bash):
+        assert_bash_exec(bash, "ssh() { echo 'spaces in filename.txt'; }")
+        completion = assert_complete(bash, "scp remote_host:spaces")
+        assert_bash_exec(bash, "unset -f ssh")
+        assert completion == r"\\\ in\\\ filename.txt"