Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quote paths with double quotes not single quotes #148

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

madscientist
Copy link

The previous scp single-quote algorithm for pathnames worked well for
POSIX servers but had problems on Windows servers, where single quotes
are not recognized (just treated like normal characters).

Use double quotes instead, and implement proper double-quoted escaping
for POSIX strings. This gives 100% correct paths on POSIX servers but
could still yield incorrect paths on Windows if the path contains
unusual characters like $ or `. If the user requires 100% Windows
compatibility they'll need to provide their own sanitizer function when
communicating with a Windows server.

Addresses issue #138 and issue #147

The previous scp single-quote algorithm for pathnames worked well for
POSIX servers but had problems on Windows servers, where single quotes
are not recognized (just treated like normal characters).

Use double quotes instead, and implement proper double-quoted escaping
for POSIX strings.  This gives 100% correct paths on POSIX servers but
could still yield incorrect paths on Windows if the path contains
unusual characters like $ or `.  If the user requires 100% Windows
compatibility they'll need to provide their own sanitizer function when
communicating with a Windows server.

Addresses issue jbardin#138 and issue jbardin#147
@remram44
Copy link
Collaborator

remram44 commented Dec 4, 2020

I'm surprised not to see backslashes in the list, is that correct?

@madscientist
Copy link
Author

Within a double-quoted string backslashes do not need to be escaped unless they are escaping a special character. See the POSIX spec link I put into the code review. So, for example, echo "foo\bar" will print out the string foo\bar: no need to escape the backslash.

However, you're right that my solution is missing something because I need to handle backslashes that DO come before special characters. In other words, say I have the literal filename foo\$bar (that is, the backslash is part of the filename). My previous solution would turn this into "foo\\$bar" which is clearly not right: it needs to be "foo\\\$bar" (escape the backslash, then escape the $).

I will post an update to handle this.

POSIX double-quoted strings don't require backslash characters to be
escaped _unless_ they are escaping a special character.  So, for a
filename foo\bar the quoting can be "foo\bar".  But for a filename
such as foo\$bar the quoting must be "foo\\\$bar".

Prefer to avoid escaping backslashes if not necessary to be more
compatible with Windows paths.
@madscientist
Copy link
Author

Just to point out, we should probably provide an update to the README that mentions this portability concern for pathnames when talking to Windows servers and suggests a sanitizer function.

We could even provide a default sanitizer function for Windows servers so users could just choose that, if any of us feels sufficiently knowledgeable as to write one :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants