Skip to content

Escaping of remote filenames containing spaces is broken for rsync #1255

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
omgold opened this issue Oct 25, 2024 · 7 comments · May be fixed by #1357
Open

Escaping of remote filenames containing spaces is broken for rsync #1255

omgold opened this issue Oct 25, 2024 · 7 comments · May be fixed by #1357

Comments

@omgold
Copy link

omgold commented Oct 25, 2024

Describe the bug

If I want to copy a file from remote to local containing spaces, the space will be escaped with triple backslashes, but it should be only one.

To reproduce

If there is a named test file.txt when I try to complete

rsync localhost:test

I get the completion

rsync localhost:test\\\ file.txt

which doesn't work.

Expected behavior

It should be

rsync localhost:test\ file.txt

Versions (please complete the following information)

  • [ x] Operating system name/distribution and version: Arch
  • [ x] bash version, echo "$BASH_VERSION": 5.2.37(1)-release
  • [ x] bash-completion version, (IFS=.; echo "${BASH_COMPLETION_VERSINFO[*]}"): 2.14.0
@yedayak
Copy link
Collaborator

yedayak commented Oct 25, 2024

Hi, what version of rsync are you using?
This should be fixed on the main branch, see this pr.
Can you try using the main branch and see if it works for you?

@omgold
Copy link
Author

omgold commented Oct 25, 2024

Okay tried it. It seems to be fix only partially, though.

It still misbehaves when there are 2 files present, test file.txt and test\ file.txt.

If I then try to complete

rsync localhost:test

at the first tab press I get

rsync localhost:test\

but on subsequent presses, I get no choices to pick from at all.

I'm using rsync 3.3.0

@yedayak
Copy link
Collaborator

yedayak commented Nov 2, 2024

Can you add set -x lines to completions/rsync?
like this:

106                 set -x #<------ Add this
107                 _comp_cmd_rsync__vercomp "$rsync_version" "3.2.4"
108                 if (($? == 2)); then
109                     _comp_compgen -x scp remote_files
110                 else
111                     _comp_compgen -x scp remote_files -l
112                 fi
113                 set +x <-----  # and this

And attach the output when completing with the issue?

@omgold
Copy link
Author

omgold commented Nov 4, 2024

@yedayak here you go.

rsync-complete-debug-output.txt

@scop
Copy link
Owner

scop commented Dec 10, 2024

I wonder if #1232 is the same issue as this one.

@akinomyoga
Copy link
Collaborator

I wonder if #1232 is the same issue as this one.

I'd say it is closely related, but it's a separate issue.

The original problem reported in #1255 (comment) was already fixed as I don't see the reported behavior in the latest version of bash_completion. I think #910 fixed it, as @yedayak has pointed out in #1255 (comment).

The second problem mentioned in #1255 (comment) is even another independent problem. The completion generates the following two candidates.

test\ file.txt
test\\\ file.txt

Then, Bash inserts the common prefix test\. The problem is that the inserted single \ (missing the next character) is syntactically invalid. We need to remove this backslash internally before using it as the completion prefix.

This seems to be fixed by extending _comp_quote after merging #1357:

diff --git a/bash_completion b/bash_completion
index 89a9584be..81b024dd2 100644
--- a/bash_completion
+++ b/bash_completion
@@ -235,8 +235,11 @@ _comp_dequote__initialize
 _comp_dequote()
 {
     REPLY=() # fallback value for unsafe word and failglob
-    [[ $1 =~ $_comp_dequote__regex_safe_word ]] || return 1
+    if [[ $1 =~ $_comp_dequote__regex_safe_word ]]; then
       eval "REPLY=($1)" 2>/dev/null # may produce failglob
+    elif [[ $1 =~ ${_comp_dequote__regex_safe_word%$}\\$ ]]; then
+      eval "REPLY=(${1%\\})" 2>/dev/null
+    fi
 }

 # Unset the given variables across a scope boundary. Useful for unshadowing

Although, I'm not sure if the above change wouldn't cause an issue in other places that use _comp_dequote.

akinomyoga added a commit to akinomyoga/bash-completion that referenced this issue Apr 5, 2025
@akinomyoga
Copy link
Collaborator

I decided to create a new function, _comp_dequote_incomplete, and include it in PR #1357. Commit 38855ad is the fix.

akinomyoga added a commit to akinomyoga/bash-completion that referenced this issue Apr 5, 2025
akinomyoga added a commit to akinomyoga/bash-completion that referenced this issue Apr 6, 2025
akinomyoga added a commit to akinomyoga/bash-completion that referenced this issue Apr 6, 2025
akinomyoga added a commit to akinomyoga/bash-completion that referenced this issue Apr 6, 2025
scop#1255 (comment)

Co-authored-by: Yedaya Katsman <yedaya.ka@gmail.com>
akinomyoga added a commit to akinomyoga/bash-completion that referenced this issue Apr 6, 2025
scop#1255 (comment)

Co-authored-by: Yedaya Katsman <yedaya.ka@gmail.com>
akinomyoga added a commit to akinomyoga/bash-completion that referenced this issue Apr 6, 2025
scop#1255 (comment)

Co-authored-by: Yedaya Katsman <yedaya.ka@gmail.com>
akinomyoga added a commit to akinomyoga/bash-completion that referenced this issue Apr 6, 2025
scop#1255 (comment)

Co-authored-by: Yedaya Katsman <yedaya.ka@gmail.com>
akinomyoga added a commit to akinomyoga/bash-completion that referenced this issue Apr 7, 2025
scop#1255 (comment)

Co-authored-by: Yedaya Katsman <yedaya.ka@gmail.com>
akinomyoga added a commit to akinomyoga/bash-completion that referenced this issue Apr 7, 2025
scop#1255 (comment)

Co-authored-by: Yedaya Katsman <yedaya.ka@gmail.com>
akinomyoga added a commit to akinomyoga/bash-completion that referenced this issue Apr 7, 2025
scop#1255 (comment)

Co-authored-by: Yedaya Katsman <yedaya.ka@gmail.com>
akinomyoga added a commit to akinomyoga/bash-completion that referenced this issue Apr 9, 2025
scop#1255 (comment)

Co-authored-by: Yedaya Katsman <yedaya.ka@gmail.com>
akinomyoga added a commit to akinomyoga/bash-completion that referenced this issue Apr 9, 2025
scop#1255 (comment)

Co-authored-by: Yedaya Katsman <yedaya.ka@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants