Created attachment 7417 [details]
This bug originally comes from https://bugzilla.redhat.com/show_bug.cgi?id=743598
Description of problem:
rsync daemon chooses wrong destination place for source paths containing space
and the module name.
Version-Release number of selected component (if applicable):
Latest upstream version is also having this bug.
Steps to Reproduce:
1. Create directory /tmp/foo.
2. Create a minimal rsyncd.conf, e.g.
use chroot = false
uid = devtest
path = /tmp/foo
read only = false
3. Start rsyncd with "sudo rsync --daemon --no-detach --config=rsyncd.conf".
4. Module name (in this case "foo") should appear after the space in
- mkdir -p '/tmp/foo/c foo'
- rsync '/etc/issue' 'rsync://blade36/foo/c foo/issue'
5. Notice that the file will end up in '/tmp/foo/c' instead of '/tmp/foo/c
File is getting created as '/tmp/foo/c' instead of '/tmp/foo/c foo/issue'.
File should be created at location '/tmp/foo/c foo/issue' instead of
is there any problem with reproduction or inclusion into upstream?
It looks like the proposed fix may break this usage (from the
'ADVANCED USAGE' section of the man page):
Older versions of rsync required using quoted spaces in the SRC, like
rsync -av host:'dir1/file1 dir2/file2' /dest
rsync host::'modname/dir1/file1 modname/dir2/file2' /dest
This word-splitting still works (by default) in the latest rsync, but
is not as easy to use as the first method.
Have you tested this?
Thank you Chris. I have not been aware about this rsync feature. I am getting the same result when I try to transmit mentioned file with --protect-args. Shouldn't this be working with --protected-args?
(In reply to comment #3)
> Thank you Chris. I have not been aware about this rsync feature. I am getting
> the same result when I try to transmit mentioned file with --protect-args.
> Shouldn't this be working with --protected-args?
Yes, I think --protect-args should work, but it doesn't, i.e. I believe you've found a real bug. Unfortunately your proposed fix would break the documented historical usage and thus, if applied, could have the effect of breaking someone's setup that relies on that historical usage.
I'm not sure what the correct solution should be: perhaps something like adding a new flag to enable the desired behavior, e.g. --protect-module-whitespace.
Created attachment 8876 [details]
New version of patch preserving historical usage
Attached patch enables transmission of files into directories containing the module name with --protect-args and simultaneously preserves historical rsync usage.
Apologies, my previous comment was a bit misleading. I meant that I would also have expected --protect-args to fix the problem you've identified, however, if the intention is to not affect people relying on the historical usage, you can't now change the behavior of --protect-args per your 2nd patch, which is why I suggested perhaps a new flag.
Maybe a good place to start nailing this down would be to create a new test in testsuite/ which explores the various possibilities, including tests that demonstrate the historical usage (with and without --protect-args), and tests that demonstrate the current bug (does it occur in both directions, i.e. when trying to pick up a file with embedded whitespace?). Then any proposed patch would need to fix the bug without breaking the historical usage tests.
I should point out that I have no official capacity within the rsync project, I'm merely an interested observer!
Speaking of "official capacity", Wayne, I see you have assigned this issue to yourself, do you have an opinion of how this should be resolved?
It is the intention of --protect-args (-s) to avoid such space splitting. The manpage in the cited section mentions that the space-splitting methods are "old", and later on talks about how -s can be used to avoid them. Those that are using newer versions of rsync should be using the non-deprecated, multi-arg calling syntax, OR the legacy parsing that is provided by the (usually) default --no-protect-args behavior. So, I consider this a bug in --protect-args.
I'm looking at a patch similar to the last-proposed one.
I'm committing a fix for this now.