jbardin / scp.py

scp module for paramiko
Other
529 stars 139 forks source link

scp.put fails when source directory has trailing slash #146

Closed process0 closed 2 years ago

process0 commented 3 years ago

When coping a directory recusively, if the source directory has a trailing slash the copy will fail. This does not happen when there is no trailing slash.

>>> scp.put('input/', '/tmp/', recursive=True)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "path/venv/lib/python3.7/site-packages/scp.py", line 164, in put
    self._send_recursive(files)
  File "path/venv/lib/python3.7/site-packages/scp.py", line 326, in _send_recursive
    self._chdir(last_dir, asbytes(root))
  File "path/venv/lib/python3.7/site-packages/scp.py", line 316, in _chdir
    self._send_pushd(to_dir)
  File "path/venv/lib/python3.7/site-packages/scp.py", line 340, in _send_pushd
    self._recv_confirm()
  File "path/venv/lib/python3.7/site-packages/scp.py", line 363, in _recv_confirm
    raise SCPException(asunicode(msg[1:]))
scp.SCPException: scp: error: unexpected filename: 
remram44 commented 3 years ago

Yep confirmed! Thanks for the report!

This will need to be fixed.

dmiro commented 3 years ago

I think that this bug is caused by change in openbsd

https://github.com/openbsd/src/commit/4667d15cf6214d4cb6b8c347f13864ca8c72002e#diff-e958952155bec91aa3a85f32af5428e5R1086

A little more information about the update:

https://superuser.com/questions/1403473/scp-error-unexpected-filename

etirta commented 3 years ago

Dear maintainer,

I'm affected by this bug. I've done some trouble shooting and believe it is a very simply fix to address the issue.

When the source path has trailing slash, say '/path/to/src/', this will cause the first attempt to recurse inside _send_pushd() will end up with basename as empty string (because the directory will have trailing slash.

On the older openssh this is not an issue:

$ { echo 'D0755 0  '; } | scp -rt . | hd
00000000  00 00                                             |..|
00000002

However, this is now an error in the newer version of openssh:

$ { echo 'D0755 0 .'; } | scp -rt . | hd
00000000  00 01 73 63 70 3a 20 65  72 72 6f 72 3a 20 75 6e  |..scp: error: un|
00000010  65 78 70 65 63 74 65 64  20 66 69 6c 65 6e 61 6d  |expected filenam|
00000020  65 3a 20 2e 0a                                    |e: ..|
00000025

If you don't mind my suggestion, it is easily fixed inside _send_recursive():

        for base in files:
            if not os.path.isdir(base):
                # filename mixed into the bunch
                self._send_files([base])
                continue
            last_dir = asbytes(base)
            for root, dirs, fls in os.walk(base):
                if not root.endswith('/'):
                    self._chdir(last_dir, asbytes(root))
                self._send_files([os.path.join(root, f) for f in fls])
                last_dir = asbytes(root)
            # back out of the directory
            while self._pushed > 0:
                self._send_popd()

I didn't propose to modify the base because I believe people would like to keep a nice feature that scp.put('/path/to/src', '/dst/', recursive=True) is different with scp.put('/path/to/src/', '/dst/', recursive=True). The earlier will yield /dst/src/src-content..., while the later will yield /dst/src-content..., just like before the openssh change.

I'm sorry I can't directly propose the change on the pull request, as I'm very newby with the git. This is my first github colaboration TBH.

I wish that now there is a viable and simple solution, you can fix this bug and release it quickly. I kind of need to have this fixed, otherwise, at this point I have to manually glob the /path/to/src/* and supply it to scp.put().

remram44 commented 3 years ago

Thanks for the help @etirta! I committed your change as pull request #161