jbardin / scp.py

scp module for paramiko
Other
529 stars 139 forks source link

Hanging copying files to Windows: path sanitization issue #147

Open madscientist opened 3 years ago

madscientist commented 3 years ago

I was having a problem with scp hanging copying files to Windows (using the Windows-provided Windows 10 OpenSSH server on the remote). After a number of hours of debugging I have discovered that the problem is quoting of the path.

If I write to a Windows-form path like C:\foo\bar\myfile then the scp put() function hangs either forever, or until the timeout if you specify one. If I write to a path using UNIX slashes like C:/foo/bar/myfile then it works fine.

It appears that the default sanitize function is causing this, by wrapping the filename in single quotes if it contains characters which are special to POSIX systems, such as backslash. If I replace the sanitize with a method that simply returns a double-quoted string always then it works fine:

def winsan(pth):
    return '"'+pth+'"'

with scp.SCPClient(..., sanitize=winsan) as con:
    con.put(src, 'C:\\foo\\bar')

However I realize that using double-quotes on POSIX systems is much more complicated than using single quotes, and the results likely won't be portable anyway in some situations (if the path contains double-quotes for example).

I'm not exactly sure if there's any actual fix that can be made in scp itself here, but it seems like this might at least be worth a mention in the README etc. to hopefully save other people the above-mentioned hours.

remram44 commented 3 years ago

Hi @madscientist, there are a few issues similar to this like #138. Can you check if #139 helps?

madscientist commented 3 years ago

Thanks for that pointer. Some comments in the bug and PR imply that you must use POSIX-style paths (forward slashes) with Windows OpenSSH server. In fact, the Windows OpenSSH server doesn't require this: it works fine to use backslashes in paths even if you don't add any quotes. The only time you need quotes on Windows is if your path contains whitespace (as far as I know--I'm no expert on Windows but I have had to do a lot of portability stuff).

Regarding the PR #139 if you really want your sanitizer to be maximally portable, it needs to be more complex. You can find the rules for escaping inside double-quoted strings in the POSIX spec here: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_02_03

Given that definition, I think the escaping in that PR is too aggressive.

I put up an alternate #148 that tries to be more specific. Let me know what you think.