ScpClient does not properly quote paths #256
Comments
|
Do you happen to have a device that I could test against? |
|
Device is prepared. Is there any private way to tell you credentials and connection info? |
|
You should be able to contact me through my profile. |
|
What (remote) path did you specify when you got the error you mentioned above? |
|
|
Was able to reproduce the issue. |
|
I've fixed this issue (and a few related ones) locally. |
drieseng
changed the title
|
The problem here was that we were using a over-simplified quoting mechanism (meaning surround any path with double quotes). Apparently MikroTik does not handle (double) quoted paths. I've now updated our implementation to apply the following scheme:
This should work with all shells. |
|
Enclosing characters in single quotes should preserve the literal value of each character within the single-quotes. However, I've just learned that MikroTik instead adds the single quotes to the actual file or directory name :( This is not the first quirk I've seen in the MikroTik SCP implementation. Meanwhile, I'll consider revisit our quoting scheme... |
|
I've contacted Mikrotik Support to discuss these issues. |
|
Please note, that is not newest version of MikroTik. It may be solved in 6.x version (I sent you access). |
|
MikroTik support asked me to run tests on version 6.40.1, however I'm convinced that this will not change much. Problem is that shell-based SCP implementations require certain characters to be quoted or escaped, and non-shell based implementation - such as, but not limited to, MikroTik - do not support/expect quoting. As I see it, we have two options:
|
|
Property name look good. My program is able to detect on which MikroTik version is running, so I can change behavior for some versions. I have updated device to 6.40.1. So you can test against that version. |
|
I'm currently also busy with uploading a file to a switch and there it's also not possible to use upload because the path+filename is automatically quoted by the scpClient. Do you already know a date when this bugfix may be released? |
|
I've given this a little more thought, and came up with the following extensible implementation: /// <summary>
/// Represents a transformation that can be applied to a remote path.
/// </summary>
public interface IRemotePathTransformation
{
/// <summary>
/// Transforms the specified remote path.
/// </summary>
/// <param name="path">The path to transform.</param>
/// <returns>
/// The transformed path.
/// </returns>
string Transform(string path);
}
/// <summary>
/// Encloses a path in quotes, and escapes any embedded quote character with a backslash.
/// </summary>
internal class QuotePathTransformation : IRemotePathTransformation
{
/// <summary>
/// Encloses the specified path in quotes, and escapes any embedded quote character with a backslash.
/// </summary>
/// <param name="path">The path to transform.</param>
/// <returns>
/// The transformed path.
/// </returns>
/// <exception cref="ArgumentNullException"><paramref name="path"/> is <c>null</c>.</exception>
public string Transform(string path)
{
var transformed = new StringBuilder(path.Length);
transformed.Append("\"");
foreach (var c in path)
{
if (c == '"')
transformed.Append('\\');
transformed.Append(c);
}
transformed.Append("\"");
return transformed.ToString();
}
}
/// <summary>
/// Escapes any character that can have a special meaning for a shell with a backslash.
/// </summary>
internal class EscapePathTransformation : IRemotePathTransformation
{
/// <summary>
/// Escapes any character in the specified path that can have a special meaning for a shell with a
/// backslash.
/// </summary>
/// <param name="path">The path to transform.</param>
/// <returns>
/// The transformed path.
/// </returns>
/// <exception cref="ArgumentNullException"><paramref name="path"/> is <c>null</c>.</exception>
public string Transform(string path)
{
StringBuilder transformed = new StringBuilder(path.Length);
foreach (var c in path)
{
switch (c)
{
case '|':
case '&':
case ';':
case '<':
case '>':
case '(':
case ')':
case '$':
case '`':
case '\\':
case '"':
case ' ':
case '\t':
case '\n':
case '*':
case '?':
case '[':
case '#':
case '~':
case '=':
case '%':
case '\'':
case '!':
transformed.Append('\\');
break;
}
transformed.Append(c);
}
return transformed.ToString();
}
}
/// <summary>
/// Performs no transformation.
/// </summary>
internal class NonePathTransformation : IRemotePathTransformation
{
/// <summary>
/// Returns the specified path without applying a transformation.
/// </summary>
/// <param name="path">The path to transform.</param>
/// <returns>
/// The specified path as is.
/// </returns>
public string Transform(string path)
{
return path;
}
}
/// <summary>
/// Allow access to built-in remote path transformations.
/// </summary>
public static class RemotePathTransformation
{
private static IRemotePathTransformation _quote = new QuotePathTransformation();
private static IRemotePathTransformation _escape = new EscapePathTransformation();
private static IRemotePathTransformation _none = new NonePathTransformation();
/// <summary>
/// Encloses a path in quotes, and escapes any embedded quote character with a backslash.
/// </summary>
public static IRemotePathTransformation Quote
{
get { return _quote; }
}
/// <summary>
/// Escapes any character in a path that can have a special meaning for a shell with a backslash.
/// </summary>
/// <remarks>
/// This is the recommended transformation for shell-based servers.
/// </remarks>
public static IRemotePathTransformation Escape
{
get { return _escape; }
}
/// <summary>
/// Performs no transformation.
/// </summary>
/// <remarks>
/// This transformation should be used for servers that do not support escape sequences in paths
/// or paths enclosed in quotes, or would preserve the escape characters or quotes in the path that
/// is handed off to the IO layer. This is common for servers that are not shell-based.
/// </remarks>
public static IRemotePathTransformation None
{
get { return _none; }
}
}
public class ScpClient
{
/// <summary>
/// Gets or sets the transformation to apply to remote paths.
/// </summary>
/// <value>
/// The transformation to apply to remote paths. The default is <see cref="RemotePathTransformation.Escape"/>.
/// </value>
/// <remarks>
/// This transformation is applied to the remote file or directory path that is passed to the
/// <c>scp</c> command.
/// </remarks>
public IRemotePathTransformation RemotePathTransformation { get; set; }
...
}Usage: using (ScpClient client = new ScpClient("HOST", 22, "USER", "PWD"))
{
client.RemotePathTransformation = RemotePathTransformation.None;
client.Upload(...);
} |
|
@tmds Can you have a look at this issue, and my proposal? |
|
@drieseng I find the public void DownloadMikroTik(this ScpClient client, string filename, Stream destination) => client.Download(MikroTikTransform(filename), destination, escapeFilename: false);A property that controls the escaping for the existing overload is interesting when no transform is needed. So the API could be something like: // existing:
ScpClient.Download(string filename, Stream destination)
ScpClient.Upload(Stream source, string path)
// new:
ScpClient.Download(string filename, Stream destination, bool escapeFilename)
ScpClient.Upload(Stream source, string path, bool escapeFilename)
ScpClient.EscapeFilename = true // controls behavior of existing methodsWhatever the API, you can CC me on the PR. |
|
@tmds Disabling the quoting is not only required for MikroTik routers, but also for certain non-shell based SSH/SCP software (eg. SolarWinds). Cisco routers also do not support enclosing the path in quote (see issue #108), but I have yet to verify whether they support and/or require other escaping mechanisms. Up until - and including - 2016.1.0-beta2, we just enclosed path in quotes ("<path>"). This is a simplistic approach, as it would - for example - not support embedded quotes in the path and does not preserve the literal value of some other special characters. In the develop branch, I've implemented a better quoting mechanism (see With the information that I have right now, we need to at least allow devs to disable quoting of remote paths. In my proposal I included three options: escape all known special characters, simple quoting and no quoting/escaping at all. The simple quoting option should be replaced with the quoting mechanism that is implemented in the develop branch. Whether we need another option depends on the outcome of issue #108. How we this expose option to developers is still up for grabs, but:
|
… to control if and how a remote path should be transformed before passed on to the scp command. This allows for a custom transformation (escaping/quoting) - that implements IRemotePathTransformation - to be plugged into ScpClient. Out of the box, we only provide two implementations that are exposed through the RemotePathTransformation (enum-like) class: Quote and None. Fixes issue #256.
|
I've merged the SCP remote path transformation PR (#290) into the develop branch. |
|
I've just released version 2016.1.0-beta3 of SSH.NET, which includes a fix for this issue. |
scpClient.Download does not work on MikroTik 5.26
I am getting exception Renci.SshNet.Common.ScpException: scp error: /"file.backup": no such file or directory!
When tested scp from Fedora box, it worked without problems.
Please provide info what informations you will need for fixing (debugging).
The text was updated successfully, but these errors were encountered: