Bug 784467 - inconsistent recursive directory copy
Summary: inconsistent recursive directory copy
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: rsh
Version: 16
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Adam Tkac
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-01-25 02:45 UTC by JW
Modified: 2013-04-30 23:50 UTC (History)
2 users (show)

Fixed In Version: rsh-0.17-68.fc16
Clone Of:
Environment:
Last Closed: 2012-02-17 00:53:01 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description JW 2012-01-25 02:45:17 UTC
Description of problem:
Recursively copying a directory is inconsistent.  It changes according to whether the source directory has a trailing / or not.  The copy should be consistent regardless of the trailing /.


Version-Release number of selected component (if applicable):
/rsh-0.17-65

How reproducible:
Always

Steps to Reproduce:
1. on local: mkdir src; touch src/file; chmod o-rx src
2. on remote: mkdir dest; chmod a+rwx dest
3. on local: rcp -pr src remote:dest
4. on remote ls -al dest
5. on remote: rm -fr dest; mkdir dest; chmod a+rwx dest\
6. on local: rcp -pr src/ remote:dest
  
Actual results:
4. 
drwxrwxrwx 2 root root 4096 2012-01-25 13:28 ./
drwxr-xr-x 3 root root 4096 2012-01-25 13:28 ../
-rw-r--r-- 1 root root    0 2012-01-25 13:28 src
5.
drwxr--x--- 2 root root 4096 2012-01-25 13:28 ./
drwxr-xr-x 3 root root 4096 2012-01-25 13:28 ../
-rw-r--r-- 1 root root    0 2012-01-25 13:28 file

Expected results:
4. ok, exactly as expected
5. same as 4

Additional info:
Note very carefully that steps 3 and 6 only differ by the trailing / on src.

This is certainly a bug and not reasonable behavior.  The src is a directory regardless of whether a / is appended. The trailing / should never make a difference.

In the code there is (in source() and in rsource()):

    last = strrchr(name, '/');
    if (last == 0)
        last = name;
    else
        last++;

With a trailing / 'last' points to a null charactor which results in the sending of "D0750 0 " with an empty name component! Normally would be "D0750 0 src".  That certainly has never been an acceptable implementation of the rcp protocol. On the remote the faulty line gets conveniently converted into "dest/" which means that src never gets created on the remote.

Clearly what is really needed is:
    last = strrchr_which_isnt_the_last_character(name, '/');
but that doesn't seem reasonable.

Either the command-line args such as src/ should have their trailing / removed in main() else the following code could be added after each such strrchr():
    if (last && !last[1])
    {
        *last = '\0';
        last = strrchr(name, '/');
    }

Comment 1 JW 2012-01-25 02:59:18 UTC
Also worth mentioning are the disastrous consequences to be had if root recursively copies a directory like this: "rcp -pr src/ remote:/tmp".  That will have the effect of changing the modes on /tmp to the same as src and many processes will find themselves unable to write into /tmp.

Note also, that a user always has the option of using "rcp -pr src/* remote:dest" if they really do want to copy the contents of src only and not the directory itself.

As it stands the rcp implementation is plainly defective.

AND PLEASE DONT BOTHER TELLING ME I SHOULD NOT BE USING rcp.  Not that you need to know, but in a heavily congested server environment on a closed network there is a lot to be said for using rcp instead of scp. And have you ever tried reading scp traffic over a LAN when you really need to be able to?

Comment 2 Adam Tkac 2012-01-31 15:08:50 UTC
(In reply to comment #1)
> AND PLEASE DONT BOTHER TELLING ME I SHOULD NOT BE USING rcp.  Not that you need
> to know, but in a heavily congested server environment on a closed network
> there is a lot to be said for using rcp instead of scp. And have you ever tried
> reading scp traffic over a LAN when you really need to be able to?

If you shouldn't use rcp, it wouldn't be in distribution. You are right it's better than scp in certain networks.

If we are going to handle all corner cases then

    if (last && !last[1])
    {
        *last = '\0';
        last = strrchr(name, '/');
    }

after strrchr() is not sufficient because one can supply 'dirname///' as a directory name. I'm investigating more general fix.

Comment 3 Adam Tkac 2012-01-31 16:37:18 UTC
I've just pushed following patch which should handle all corner cases:

http://pkgs.fedoraproject.org/gitweb/?p=rsh.git;a=blob_plain;f=netkit-rsh-0.17-rh784467.patch;hb=c0f15b59f4dd0967d6099eb3ba8de841f01840ac

Comment 4 Fedora Update System 2012-01-31 16:42:03 UTC
rsh-0.17-68.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/rsh-0.17-68.fc16

Comment 5 Fedora Update System 2012-01-31 22:05:24 UTC
Package rsh-0.17-68.fc16:
* should fix your issue,
* was pushed to the Fedora 16 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing rsh-0.17-68.fc16'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-1094/rsh-0.17-68.fc16
then log in and leave karma (feedback).

Comment 6 Fedora Update System 2012-02-17 00:53:01 UTC
rsh-0.17-68.fc16 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.