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

SCPEngine should not quote remote paths #184

Closed
fbacher opened this issue Mar 13, 2015 · 7 comments
Closed

SCPEngine should not quote remote paths #184

fbacher opened this issue Mar 13, 2015 · 7 comments
Milestone

Comments

@fbacher
Copy link

fbacher commented Mar 13, 2015

SCPEngine.execSCPWith wraps the given path with single quotes and adds an escape character to any quotes in the path. This creates problems for some remote systems, such as vxWorks, which, at least on the version I'm communicating with, does not strip the quotes, resulting in invalid file name errors.

  • scp should leave it up to the application to properly quote paths
  • Applications can not work around the automatic quoting that occurs today
  • openssh scp source, does not appear to perform any quoting or globbing of paths.It passes them along as is to the remote shell to handle.
@hierynomus
Copy link
Owner

Would it work if we escaped the spaces using a \? I agree that the quoting that was previously put in is indeed dangerous and broken. Also see #152 for a PR which fixes it in an even worse way.

@bkarge
Copy link
Contributor

bkarge commented Mar 27, 2015

The escaping does not seem to work on Linux, either.
replaceAll("'", "'"'"'") did work for me (http://stackoverflow.com/questions/1250079/how-to-escape-single-quotes-within-single-quoted-strings)

@fbacher
Copy link
Author

fbacher commented Mar 27, 2015

I think that sshj should do what openssh does.

On Fri, Mar 27, 2015 at 11:33 AM, Björn Karge notifications@github.com
wrote:

The escaping does not seem to work on Linux, either.
replaceAll("'", "'"'"'") did work for me (
http://stackoverflow.com/questions/1250079/how-to-escape-single-quotes-within-single-quoted-strings
)


Reply to this email directly or view it on GitHub
#184 (comment).

@bkarge
Copy link
Contributor

bkarge commented Mar 28, 2015

I'm not sure, what scp "does". All I see is it "does not care" and subjects the interpretation of the file name to the escaping rules of the target shell (and probably other things).
However, sshj provides a Java API and the semantics clearly are "get me that file", and not run bash -c scp -qf . So I would rather propose an escaping that works in 99.9% of the cases, and to add a possibility to explicitly opt out of escaping if this goes wrong.

@hierynomus
Copy link
Owner

@fbacher I disagree. I think that SSHJ should be compatible with as many different SSH implementations as possible. This means that the scope is broader than OpenSSH. See for instance the issues against PSFTP, WinSSHD, etc... I indeed want to aim for a near 100% solution like @bkarge says. For this we need to find a correct quoting/escaping. That is not trivial, but the "just send it unquoted" option will definitely not work in a lot of cases.

@hierynomus hierynomus added this to the 1.0.0 milestone Jun 19, 2015
@jpstotz
Copy link
Contributor

jpstotz commented Sep 15, 2015

If the correct escaping and/or quoting is that difficult you should make to configurable.
The way it is in v0.13.0 is bad IMHO as it prevents performing SCP commands on remote files that contains a single quote in the filename - example:

/home/user/test'ed

Such a filename can not be used (e.g. download) with sshj at the moment, as the resulting command is:

scp -f -q -p -r '/home/user/test'ed'

@jpstotz
Copy link
Contributor

jpstotz commented Sep 15, 2015

Furthermore the implemented escaping for single quotes is defect:

SCPEngine line 112:
cmd.append("'").append(path.replaceAll("'", "\\'")).append("'");

The used method replaceAll requires an regex (and a single quote is not a valid regex).
Therefore path.replaceAll("'", "\\'") does not change or escape anything.
Change it to path.replace("'", "\\'")

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

No branches or pull requests

4 participants