Bug 8838 - rsync daemon chooses wrong destination place if space and the module name is part of it
Summary: rsync daemon chooses wrong destination place if space and the module name is ...
Status: RESOLVED FIXED
Alias: None
Product: rsync
Classification: Unclassified
Component: core (show other bugs)
Version: 3.1.0
Hardware: All Linux
: P5 normal (vote)
Target Milestone: ---
Assignee: Wayne Davison
QA Contact: Rsync QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-02 14:46 UTC by Michal Luscon (mail bounces back)
Modified: 2013-06-02 22:53 UTC (History)
0 users

See Also:


Attachments
proposed patch (1.26 KB, patch)
2012-04-02 14:46 UTC, Michal Luscon (mail bounces back)
no flags Details
New version of patch preserving historical usage (840 bytes, patch)
2013-05-10 09:50 UTC, Michal Luscon (mail bounces back)
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michal Luscon (mail bounces back) 2012-04-02 14:46:41 UTC
Created attachment 7417 [details]
proposed patch

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.

How reproducible:
Every time.


Steps to Reproduce:

1. Create directory /tmp/foo.
2. Create a minimal rsyncd.conf, e.g.
"""
use chroot = false
uid = devtest
[foo]
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
destination path. 
 - 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
foo/issue'


Actual results:
File is getting created as '/tmp/foo/c' instead of '/tmp/foo/c foo/issue'.


Expected results:
File should be created at location '/tmp/foo/c foo/issue' instead of
'/tmp/foo/c'.
Comment 1 Michal Luscon (mail bounces back) 2013-04-12 15:40:18 UTC
Hi,
is there any problem with reproduction or inclusion into upstream?
Comment 2 Chris Dunlop 2013-04-15 02:08:20 UTC
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
  these examples:

     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?

Cheers,

Chris
Comment 3 Michal Luscon (mail bounces back) 2013-05-03 14:01:08 UTC
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?
Comment 4 Chris Dunlop 2013-05-06 03:54:58 UTC
(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.
Comment 5 Michal Luscon (mail bounces back) 2013-05-10 09:50:58 UTC
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.
Comment 6 Chris Dunlop 2013-05-13 07:00:07 UTC
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?
Comment 7 Wayne Davison 2013-05-19 19:56:15 UTC
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.
Comment 8 Wayne Davison 2013-06-02 22:53:37 UTC
I'm committing a fix for this now.