rsync - Jan 2008 - DO NOT REPLY [Bug 5201] New: Rsync lets user corrupt dest by applying non-inplace batch in inplace mode

https://bugzilla.samba.org/show_bug.cgi?id=5201

           Summary: Rsync lets user corrupt dest by applying non-inplace
                    batch in inplace mode
           Product: rsync
           Version: 3.0.0
          Platform: Other
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: P3
         Component: core
        AssignedTo: wayned@samba.org
        ReportedBy: matt@mattmccutchen.net
         QAContact: rsync-qa@samba.org


I originally reported this bug here:

http://lists.samba.org/archive/rsync/2007-December/019369.html

I noticed that rsync will let me apply a non-inplace batch file in inplace
mode.  This corrupts the destination file if the batch file copies any data
forward (from earlier offsets to later ones).  Of course, the post-transfer
checksum detects the corruption and gives the "ERROR: <file> failed
verification" message, but rsync doesn't give the user a clue why the
corruption occurred.

I think rsync should at least warn the user and possibly refuse outright to
apply a non-inplace batch in inplace mode.  However, existing batch
files don't indicate which mode they were written in, and it would be nice
to
let a user apply a non-inplace batch if it happened not to use
any forward copies.  Thus, the best thing to do might be to issue an error like
this the first time a forward copy is seen for each file
(followed, of course, by the post-transfer checksum failure):

rsync: error: batch file delta for <file> contains a forward copy, which
cannot
be performed in --inplace mode (probably the batch file was written without
--inplace); the update will probably be corrupted

Here is a script that demonstrates the current behavior:

#!/bin/bash
rm -rf src dest batch*
mkdir src dest
wget http://rsync.samba.org/ftp/rsync/rsync-3.0.0pre8.tar.gz -O dest/file
head -c 1000000 /dev/zero | cat - dest/file >src/file
rsync --no-whole-file --only-write-batch=batch src/file dest/
rsync --inplace --read-batch=batch dest/

Output:

ERROR: file failed verification -- update retained.
(No batched update for "file")

Two additional problems seem to have arisen since my original mail to the list:

1. The inaccurate message `(No batched update for "file")'
appears.
Furthermore, in the case of a solo file, this message shows its source name
while the previous line shows its destination name, which seems inconsistent.

2. Rsync isn't exiting with code 23, which might mislead a script to think
that
the copy was successful.


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=5201


wayned@samba.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED




------- Comment #1 from wayned@samba.org  2008-01-19 13:27 CST -------
I added option checking for --inpace and several others that need to be set
correctly for the proper reading of the batch file.


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=5201


matt@mattmccutchen.net changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |




------- Comment #2 from matt@mattmccutchen.net  2008-01-23 23:27 CST -------
With --read-batch, when rsync processes the flags, the protocol_version is
always 30 (it has not yet been set from the batch file).  As a result,
--inplace is incorrectly forced off for any pre-protocol-30 batch file,
regardless of whether the batch file was written with --inplace.

Once the above is fixed, I am still hoping that rsync 3 will fail more
gracefully when applying a *pre-protocol-30* non-inplace batch file in inplace
mode.


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=5201


wayned@samba.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |FIXED




------- Comment #3 from wayned@samba.org  2008-01-26 16:28 CST -------
I fixed the problem with the parsing of the batch flags before the
protocol_version of the batch file is known.

As for further checking, I don't think I'll add anything else.  The
batch.sh
file has the options they should use to parse the batch file.  If they mess
things up, it's good for us to try to help them out, but I don't want to
clutter the transfer code with sanity checks for startup options.


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=5201


matt@mattmccutchen.net changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |




------- Comment #4 from matt@mattmccutchen.net  2008-01-26 17:09 CST -------
(In reply to comment #3)
> I don't want to
> clutter the transfer code with sanity checks for startup options.
That's reasonable, but the lack of an exit code 23 when the post-transfer
checksum fails still needs to be fixed.  An rsync 2.6.9 receiver gets this
right, and it also prints `(No batched update for resend of
"file")' (note the
`resend of').  Perhaps the same code path does both the `resend of' and
the
exit code 23, and it just isn't being triggered properly in the rsync 3.0.0
receiver.


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=5201


wayned@samba.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |FIXED




------- Comment #5 from wayned@samba.org  2008-01-27 16:44 CST -------
OK, I missed the extra errors at the end of your report the first time around. 
They should now be fixed (as well as a couple other minor glitches in the same
area).  Thanks!


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=5201


matt@mattmccutchen.net changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |VERIFIED




------- Comment #6 from matt@mattmccutchen.net  2008-01-27 22:11 CST -------
Wow!  This is actually completely fixed!


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=5201





------- Comment #7 from matt@mattmccutchen.net  2008-01-27 22:14 CST -------
If it is safe to apply an inplace batch file in non-inplace mode, it would be
nice to allow this.


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.