Repository: mina-sshd Updated Branches: refs/heads/master 642a8f3b2 -> ec901de77
[SSHD-790] Added capability to encode outgoing SFTP client names with non default charset Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/ec901de7 Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/ec901de7 Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/ec901de7 Branch: refs/heads/master Commit: ec901de77fb2da687c4882f057ec330352c2e5ff Parents: 642a8f3 Author: Lyor Goldstein <[email protected]> Authored: Wed Jan 3 20:32:24 2018 +0200 Committer: Lyor Goldstein <[email protected]> Committed: Wed Jan 3 20:32:54 2018 +0200 ---------------------------------------------------------------------- README.md | 13 ++- .../client/subsystem/sftp/SftpFileSystem.java | 1 - .../subsystem/sftp/impl/AbstractSftpClient.java | 74 ++++++++++------ .../sshd/common/subsystem/sftp/SftpHelper.java | 92 ++++++++++++-------- 4 files changed, 113 insertions(+), 67 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/ec901de7/README.md ---------------------------------------------------------------------- diff --git a/README.md b/README.md index 4e99d3e..5cc741f 100644 --- a/README.md +++ b/README.md @@ -732,10 +732,10 @@ throughout the SFTP server subsystem code. The accessor is registered/overwritte ``` -### SFTP received names encoding +### SFTP sent/received names encoding -By default, the SFTP client uses UTF-8 to decode any referenced file/folder name. However, some servers do not properly encode such names, and -thus the "decoded" names by the client become corrupted, or even worse - cause an exception upon decoding attempt. The `SftpClient` exposes +By default, the SFTP client uses UTF-8 to encode/decode any referenced file/folder name. However, some servers do not properly encode such names, +and thus the "visible" names by the client become corrupted, or even worse - cause an exception upon decoding attempt. The `SftpClient` exposes a `get/setNameDecodingCharset` method which enables the user to modify the charset - even while the SFTP session is in progress - e.g.: ```java @@ -799,6 +799,13 @@ public class MyCustomSftpClient extends DefaultSftpClient { Charset cs = detectCharset(bytes); return new String(bytes, cs); } + + @Override + protected <B extends Buffer> B putReferencedName(int cmd, B buf, String name) { + Charset cs = detectCharset(name); + buf.putString(name, cs); + return buf; + } } public class MyCustomSftpClientFactory extends DefaultSftpClientFactory { http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/ec901de7/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/SftpFileSystem.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/SftpFileSystem.java b/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/SftpFileSystem.java index 1ca0283..bc790b3 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/SftpFileSystem.java +++ b/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/SftpFileSystem.java @@ -210,7 +210,6 @@ public class SftpFileSystem extends BaseFileSystem<SftpPath> implements ClientSe } private final class Wrapper extends AbstractSftpClient { - private final SftpClient delegate; private final AtomicInteger count = new AtomicInteger(1); private final int readSize; http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/ec901de7/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/impl/AbstractSftpClient.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/impl/AbstractSftpClient.java b/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/impl/AbstractSftpClient.java index a1d2014..45173e2 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/impl/AbstractSftpClient.java +++ b/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/impl/AbstractSftpClient.java @@ -119,6 +119,19 @@ public abstract class AbstractSftpClient extends AbstractSubsystemClient impleme } /** + * @param <B> Type of {@link Buffer} being updated + * @param cmd The command for which this name is being added + * @param buf The buffer instance to update + * @param name The name to place in the buffer + * @return The updated buffer + */ + protected <B extends Buffer> B putReferencedName(int cmd, B buf, String name) { + Charset cs = getNameDecodingCharset(); + buf.putString(name, cs); + return buf; + } + + /** * Sends the specified command, waits for the response and then invokes {@link #checkResponseStatus(int, Buffer)} * @param cmd The command to send * @param request The request {@link Buffer} @@ -444,7 +457,7 @@ public abstract class AbstractSftpClient extends AbstractSubsystemClient impleme return attrs; } - protected void writeAttributes(Buffer buffer, Attributes attributes) throws IOException { + protected <B extends Buffer> B writeAttributes(int cmd, B buffer, Attributes attributes) throws IOException { int version = getVersion(); int flagsMask = 0; Collection<Attribute> flags = Objects.requireNonNull(attributes, "No attributes").getFlags(); @@ -489,8 +502,8 @@ public abstract class AbstractSftpClient extends AbstractSubsystemClient impleme } if ((flagsMask & SftpConstants.SSH_FILEXFER_ATTR_ACMODTIME) != 0) { - SftpHelper.writeTime(buffer, version, flagsMask, attributes.getAccessTime()); - SftpHelper.writeTime(buffer, version, flagsMask, attributes.getModifyTime()); + buffer = SftpHelper.writeTime(buffer, version, flagsMask, attributes.getAccessTime()); + buffer = SftpHelper.writeTime(buffer, version, flagsMask, attributes.getModifyTime()); } } else if (version >= SftpConstants.SFTP_V4) { for (Attribute a : flags) { @@ -538,26 +551,28 @@ public abstract class AbstractSftpClient extends AbstractSubsystemClient impleme buffer.putInt(attributes.getPermissions()); } if ((flagsMask & SftpConstants.SSH_FILEXFER_ATTR_ACCESSTIME) != 0) { - SftpHelper.writeTime(buffer, version, flagsMask, attributes.getAccessTime()); + buffer = SftpHelper.writeTime(buffer, version, flagsMask, attributes.getAccessTime()); } if ((flagsMask & SftpConstants.SSH_FILEXFER_ATTR_CREATETIME) != 0) { - SftpHelper.writeTime(buffer, version, flagsMask, attributes.getCreateTime()); + buffer = SftpHelper.writeTime(buffer, version, flagsMask, attributes.getCreateTime()); } if ((flagsMask & SftpConstants.SSH_FILEXFER_ATTR_MODIFYTIME) != 0) { - SftpHelper.writeTime(buffer, version, flagsMask, attributes.getModifyTime()); + buffer = SftpHelper.writeTime(buffer, version, flagsMask, attributes.getModifyTime()); } if ((flagsMask & SftpConstants.SSH_FILEXFER_ATTR_ACL) != 0) { - SftpHelper.writeACLs(buffer, version, attributes.getAcl()); + buffer = SftpHelper.writeACLs(buffer, version, attributes.getAcl()); } - // TODO: for v6+ add CTIME (see https://tools.ietf.org/html/draft-ietf-secsh-filexfer-13#page-21) + // TODO: for v5 ? 6? add CTIME (see https://tools.ietf.org/html/draft-ietf-secsh-filexfer-13#page-16 - v6) } else { throw new UnsupportedOperationException("writeAttributes(" + attributes + ") unsupported version: " + version); } if ((flagsMask & SftpConstants.SSH_FILEXFER_ATTR_EXTENDED) != 0) { - SftpHelper.writeExtensions(buffer, attributes.getExtensions()); + buffer = SftpHelper.writeExtensions(buffer, attributes.getExtensions()); } + + return buffer; } @Override @@ -574,7 +589,8 @@ public abstract class AbstractSftpClient extends AbstractSubsystemClient impleme } Buffer buffer = new ByteArrayBuffer(path.length() + Long.SIZE /* some extra fields */, false); - buffer.putString(path); + buffer = putReferencedName(SftpConstants.SSH_FXP_OPEN, buffer, path); + int version = getVersion(); int mode = 0; if (version < SftpConstants.SFTP_V5) { @@ -627,7 +643,7 @@ public abstract class AbstractSftpClient extends AbstractSubsystemClient impleme } } buffer.putInt(mode); - writeAttributes(buffer, fileOpenAttributes); + buffer = writeAttributes(SftpConstants.SSH_FXP_OPEN, buffer, fileOpenAttributes); CloseableHandle handle = new DefaultCloseableHandle(this, path, checkHandle(SftpConstants.SSH_FXP_OPEN, buffer)); if (log.isTraceEnabled()) { @@ -663,7 +679,7 @@ public abstract class AbstractSftpClient extends AbstractSubsystemClient impleme } Buffer buffer = new ByteArrayBuffer(path.length() + Long.SIZE /* some extra fields */, false); - buffer.putString(path); + buffer = putReferencedName(SftpConstants.SSH_FXP_REMOVE, buffer, path); checkCommandStatus(SftpConstants.SSH_FXP_REMOVE, buffer); } @@ -678,8 +694,8 @@ public abstract class AbstractSftpClient extends AbstractSubsystemClient impleme } Buffer buffer = new ByteArrayBuffer(oldPath.length() + newPath.length() + Long.SIZE /* some extra fields */, false); - buffer.putString(oldPath); - buffer.putString(newPath); + buffer = putReferencedName(SftpConstants.SSH_FXP_RENAME, buffer, oldPath); + buffer = putReferencedName(SftpConstants.SSH_FXP_RENAME, buffer, newPath); int numOptions = GenericUtils.size(options); int version = getVersion(); @@ -827,7 +843,7 @@ public abstract class AbstractSftpClient extends AbstractSubsystemClient impleme } Buffer buffer = new ByteArrayBuffer(path.length() + Long.SIZE /* some extra fields */, false); - buffer.putString(path); + buffer = putReferencedName(SftpConstants.SSH_FXP_MKDIR, buffer, path); buffer.putInt(0); int version = getVersion(); @@ -849,7 +865,7 @@ public abstract class AbstractSftpClient extends AbstractSubsystemClient impleme } Buffer buffer = new ByteArrayBuffer(path.length() + Long.SIZE /* some extra fields */, false); - buffer.putString(path); + buffer = putReferencedName(SftpConstants.SSH_FXP_RMDIR, buffer, path); checkCommandStatus(SftpConstants.SSH_FXP_RMDIR, buffer); } @@ -860,7 +876,7 @@ public abstract class AbstractSftpClient extends AbstractSubsystemClient impleme } Buffer buffer = new ByteArrayBuffer(path.length() + Long.SIZE /* some extra fields */, false); - buffer.putString(path); + buffer = putReferencedName(SftpConstants.SSH_FXP_OPENDIR, buffer, path); CloseableHandle handle = new DefaultCloseableHandle(this, path, checkHandle(SftpConstants.SSH_FXP_OPENDIR, buffer)); if (log.isTraceEnabled()) { @@ -968,7 +984,7 @@ public abstract class AbstractSftpClient extends AbstractSubsystemClient impleme } Buffer buffer = new ByteArrayBuffer(path.length() + Long.SIZE, false); - buffer.putString(path); + buffer = putReferencedName(SftpConstants.SSH_FXP_REALPATH, buffer, path); return checkOneName(SftpConstants.SSH_FXP_REALPATH, buffer); } @@ -979,7 +995,7 @@ public abstract class AbstractSftpClient extends AbstractSubsystemClient impleme } Buffer buffer = new ByteArrayBuffer(path.length() + Long.SIZE, false); - buffer.putString(path); + buffer = putReferencedName(SftpConstants.SSH_FXP_STAT, buffer, path); int version = getVersion(); if (version >= SftpConstants.SFTP_V4) { @@ -996,7 +1012,7 @@ public abstract class AbstractSftpClient extends AbstractSubsystemClient impleme } Buffer buffer = new ByteArrayBuffer(path.length() + Long.SIZE, false); - buffer.putString(path); + buffer = putReferencedName(SftpConstants.SSH_FXP_LSTAT, buffer, path); int version = getVersion(); if (version >= SftpConstants.SFTP_V4) { @@ -1035,8 +1051,8 @@ public abstract class AbstractSftpClient extends AbstractSubsystemClient impleme } Buffer buffer = new ByteArrayBuffer(); - buffer.putString(path); - writeAttributes(buffer, attributes); + buffer = putReferencedName(SftpConstants.SSH_FXP_SETSTAT, buffer, path); + buffer = writeAttributes(SftpConstants.SSH_FXP_SETSTAT, buffer, attributes); checkCommandStatus(SftpConstants.SSH_FXP_SETSTAT, buffer); } @@ -1052,7 +1068,7 @@ public abstract class AbstractSftpClient extends AbstractSubsystemClient impleme byte[] id = Objects.requireNonNull(handle, "No handle").getIdentifier(); Buffer buffer = new ByteArrayBuffer(id.length + (2 * Long.SIZE) /* some extras */, false); buffer.putBytes(id); - writeAttributes(buffer, attributes); + buffer = writeAttributes(SftpConstants.SSH_FXP_FSETSTAT, buffer, attributes); checkCommandStatus(SftpConstants.SSH_FXP_FSETSTAT, buffer); } @@ -1063,7 +1079,7 @@ public abstract class AbstractSftpClient extends AbstractSubsystemClient impleme } Buffer buffer = new ByteArrayBuffer(path.length() + Long.SIZE /* some extra fields */, false); - buffer.putString(path); + buffer = putReferencedName(SftpConstants.SSH_FXP_READLINK, buffer, path); return checkOneName(SftpConstants.SSH_FXP_READLINK, buffer); } @@ -1083,13 +1099,15 @@ public abstract class AbstractSftpClient extends AbstractSubsystemClient impleme if (!symbolic) { throw new UnsupportedOperationException("Hard links are not supported in sftp v" + version); } - buffer.putString(targetPath); - buffer.putString(linkPath); + buffer = putReferencedName(SftpConstants.SSH_FXP_SYMLINK, buffer, targetPath); + buffer = putReferencedName(SftpConstants.SSH_FXP_SYMLINK, buffer, linkPath); + checkCommandStatus(SftpConstants.SSH_FXP_SYMLINK, buffer); } else { - buffer.putString(targetPath); - buffer.putString(linkPath); + buffer = putReferencedName(SftpConstants.SSH_FXP_SYMLINK, buffer, targetPath); + buffer = putReferencedName(SftpConstants.SSH_FXP_SYMLINK, buffer, linkPath); buffer.putBoolean(symbolic); + checkCommandStatus(SftpConstants.SSH_FXP_LINK, buffer); } } http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/ec901de7/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/SftpHelper.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/SftpHelper.java b/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/SftpHelper.java index 7b06e11..5f83818 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/SftpHelper.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/SftpHelper.java @@ -195,17 +195,19 @@ public final class SftpHelper { /** * Writes a file / folder's attributes to a buffer * - * @param buffer The target {@link Buffer} + * @param <B> Type of {@link Buffer} being updated + * @param buffer The target buffer instance * @param version The output encoding version * @param attributes The {@link Map} of attributes + * @return The updated buffer * @see #writeAttrsV3(Buffer, int, Map) * @see #writeAttrsV4(Buffer, int, Map) */ - public static void writeAttrs(Buffer buffer, int version, Map<String, ?> attributes) { + public static <B extends Buffer> B writeAttrs(B buffer, int version, Map<String, ?> attributes) { if (version == SftpConstants.SFTP_V3) { - writeAttrsV3(buffer, version, attributes); + return writeAttrsV3(buffer, version, attributes); } else if (version >= SftpConstants.SFTP_V4) { - writeAttrsV4(buffer, version, attributes); + return writeAttrsV4(buffer, version, attributes); } else { throw new IllegalStateException("Unsupported SFTP version: " + version); } @@ -214,11 +216,13 @@ public final class SftpHelper { /** * Writes the retrieved file / directory attributes in V3 format * - * @param buffer The target {@link Buffer} + * @param <B> Type of {@link Buffer} being updated + * @param buffer The target buffer instance * @param version The actual version - must be {@link SftpConstants#SFTP_V3} * @param attributes The {@link Map} of attributes + * @return The updated buffer */ - public static void writeAttrsV3(Buffer buffer, int version, Map<String, ?> attributes) { + public static <B extends Buffer> B writeAttrsV3(B buffer, int version, Map<String, ?> attributes) { ValidateUtils.checkTrue(version == SftpConstants.SFTP_V3, "Illegal version: %d", version); boolean isReg = getBool((Boolean) attributes.get("isRegularFile")); @@ -247,22 +251,26 @@ public final class SftpHelper { buffer.putInt(attributesToPermissions(isReg, isDir, isLnk, perms)); } if ((flags & SftpConstants.SSH_FILEXFER_ATTR_ACMODTIME) != 0) { - writeTime(buffer, version, flags, lastAccessTime); - writeTime(buffer, version, flags, lastModifiedTime); + buffer = writeTime(buffer, version, flags, lastAccessTime); + buffer = writeTime(buffer, version, flags, lastModifiedTime); } if ((flags & SftpConstants.SSH_FILEXFER_ATTR_EXTENDED) != 0) { - writeExtensions(buffer, extensions); + buffer = writeExtensions(buffer, extensions); } + + return buffer; } /** * Writes the retrieved file / directory attributes in V4+ format * - * @param buffer The target {@link Buffer} + * @param <B> Type of {@link Buffer} being updated + * @param buffer The target buffer instance * @param version The actual version - must be at least {@link SftpConstants#SFTP_V4} * @param attributes The {@link Map} of attributes + * @return The updated buffer */ - public static void writeAttrsV4(Buffer buffer, int version, Map<String, ?> attributes) { + public static <B extends Buffer> B writeAttrsV4(B buffer, int version, Map<String, ?> attributes) { ValidateUtils.checkTrue(version >= SftpConstants.SFTP_V4, "Illegal version: %d", version); boolean isReg = getBool((Boolean) attributes.get("isRegularFile")); @@ -302,23 +310,25 @@ public final class SftpHelper { } if ((flags & SftpConstants.SSH_FILEXFER_ATTR_ACCESSTIME) != 0) { - writeTime(buffer, version, flags, lastAccessTime); + buffer = writeTime(buffer, version, flags, lastAccessTime); } if ((flags & SftpConstants.SSH_FILEXFER_ATTR_CREATETIME) != 0) { - writeTime(buffer, version, flags, lastAccessTime); + buffer = writeTime(buffer, version, flags, lastAccessTime); } if ((flags & SftpConstants.SSH_FILEXFER_ATTR_MODIFYTIME) != 0) { - writeTime(buffer, version, flags, lastModifiedTime); + buffer = writeTime(buffer, version, flags, lastModifiedTime); } if ((flags & SftpConstants.SSH_FILEXFER_ATTR_ACL) != 0) { - writeACLs(buffer, version, acl); + buffer = writeACLs(buffer, version, acl); } // TODO: ctime // TODO: bits if ((flags & SftpConstants.SSH_FILEXFER_ATTR_EXTENDED) != 0) { - writeExtensions(buffer, extensions); + buffer = writeExtensions(buffer, extensions); } + + return buffer; } /** @@ -644,21 +654,25 @@ public final class SftpHelper { return extended; } - public static void writeExtensions(Buffer buffer, Map<?, ?> extensions) { + public static <B extends Buffer> B writeExtensions(B buffer, Map<?, ?> extensions) { int numExtensions = GenericUtils.size(extensions); buffer.putInt(numExtensions); - if (numExtensions > 0) { - extensions.forEach((key, value) -> { - Objects.requireNonNull(key, "No extension type"); - Objects.requireNonNull(value, "No extension value"); - buffer.putString(key.toString()); - if (value instanceof byte[]) { - buffer.putBytes((byte[]) value); - } else { - buffer.putString(value.toString()); - } - }); + if (numExtensions <= 0) { + return buffer; } + + extensions.forEach((key, value) -> { + Objects.requireNonNull(key, "No extension type"); + Objects.requireNonNull(value, "No extension value"); + buffer.putString(key.toString()); + if (value instanceof byte[]) { + buffer.putBytes((byte[]) value); + } else { + buffer.putString(value.toString()); + } + }); + + return buffer; } public static NavigableMap<String, String> toStringExtensions(Map<String, ?> extensions) { @@ -834,14 +848,15 @@ public final class SftpHelper { return mask; } - public static void writeACLs(Buffer buffer, int version, Collection<? extends AclEntry> acl) { + public static <B extends Buffer> B writeACLs(B buffer, int version, Collection<? extends AclEntry> acl) { int lenPos = buffer.wpos(); buffer.putInt(0); // length placeholder - encodeACLs(buffer, version, acl); + buffer = encodeACLs(buffer, version, acl); BufferUtils.updateLengthPlaceholder(buffer, lenPos); + return buffer; } - public static void encodeACLs(Buffer buffer, int version, Collection<? extends AclEntry> acl) { + public static <B extends Buffer> B encodeACLs(B buffer, int version, Collection<? extends AclEntry> acl) { Objects.requireNonNull(acl, "No ACL"); if (version >= SftpConstants.SFTP_V6) { buffer.putInt(0); // TODO handle ACL flags @@ -851,12 +866,14 @@ public final class SftpHelper { buffer.putInt(numEntries); if (numEntries > 0) { for (AclEntry e : acl) { - writeAclEntry(buffer, e); + buffer = writeAclEntry(buffer, e); } } + + return buffer; } - public static void writeAclEntry(Buffer buffer, AclEntry acl) { + public static <B extends Buffer> B writeAclEntry(B buffer, AclEntry acl) { Objects.requireNonNull(acl, "No ACL"); AclEntryType type = acl.type(); @@ -868,6 +885,7 @@ public final class SftpHelper { Principal user = acl.principal(); buffer.putString(user.getName()); + return buffer; } /** @@ -982,12 +1000,14 @@ public final class SftpHelper { /** * Encodes a {@link FileTime} value into a buffer * - * @param buffer The target {@link Buffer} + * @param <B> Type of {@link Buffer} being updated + * @param buffer The target buffer instance * @param version The encoding version * @param flags The encoding flags * @param time The value to encode + * @return The updated buffer */ - public static void writeTime(Buffer buffer, int version, int flags, FileTime time) { + public static <B extends Buffer> B writeTime(B buffer, int version, int flags, FileTime time) { // for v3 see https://tools.ietf.org/html/draft-ietf-secsh-filexfer-02#page-8 // for v6 see https://tools.ietf.org/html/draft-ietf-secsh-filexfer-13#page-16 if (version >= SftpConstants.SFTP_V4) { @@ -1000,6 +1020,8 @@ public final class SftpHelper { } else { buffer.putInt(time.to(TimeUnit.SECONDS)); } + + return buffer; } /**
