You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2015/12/01 11:43:48 UTC
mina-sshd git commit: Using more robust code for SFTP extensions
handling
Repository: mina-sshd
Updated Branches:
refs/heads/master c11b57ad6 -> 3802e5212
Using more robust code for SFTP extensions handling
Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/3802e521
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/3802e521
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/3802e521
Branch: refs/heads/master
Commit: 3802e521256fc8c75bdb11f0d0b6a4088151bc68
Parents: c11b57a
Author: Lyor Goldstein <lg...@vmware.com>
Authored: Tue Dec 1 12:43:33 2015 +0200
Committer: Lyor Goldstein <lg...@vmware.com>
Committed: Tue Dec 1 12:43:33 2015 +0200
----------------------------------------------------------------------
.../subsystem/sftp/DefaultSftpClient.java | 4 +-
.../sftp/extensions/NewlineParser.java | 65 +++++++++++++++++---
.../sftp/extensions/VersionsParser.java | 30 ++++++---
.../openssh/AbstractOpenSSHExtensionParser.java | 4 +-
.../server/subsystem/sftp/SftpSubsystem.java | 54 ++++++++++++++--
.../sshd/client/subsystem/sftp/SftpTest.java | 26 +++++++-
6 files changed, 155 insertions(+), 28 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/3802e521/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/DefaultSftpClient.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/DefaultSftpClient.java b/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/DefaultSftpClient.java
index 858b866..fe19f73 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/DefaultSftpClient.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/DefaultSftpClient.java
@@ -334,11 +334,11 @@ public class DefaultSftpClient extends AbstractSftpClient {
Collection<String> extensions = ParserUtils.supportedExtensions(parsed);
if ((GenericUtils.size(extensions) > 0) && extensions.contains(SftpConstants.EXT_VERSION_SELECT)) {
Versions vers = GenericUtils.isEmpty(parsed) ? null : (Versions) parsed.get(SftpConstants.EXT_VERSIONS);
- Collection<String> reported = (vers == null) ? null : vers.versions;
+ Collection<String> reported = (vers == null) ? null : vers.getVersions();
if (GenericUtils.size(reported) > 0) {
for (String v : reported) {
if (!available.add(Integer.valueOf(v))) {
- continue;
+ continue; // debug breakpoint
}
}
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/3802e521/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/extensions/NewlineParser.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/extensions/NewlineParser.java b/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/extensions/NewlineParser.java
index 1043fff..b9ac5bb 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/extensions/NewlineParser.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/extensions/NewlineParser.java
@@ -19,7 +19,9 @@
package org.apache.sshd.common.subsystem.sftp.extensions;
+import java.io.Serializable;
import java.nio.charset.StandardCharsets;
+import java.util.Objects;
import org.apache.sshd.common.subsystem.sftp.SftpConstants;
import org.apache.sshd.common.subsystem.sftp.extensions.NewlineParser.Newline;
@@ -36,17 +38,62 @@ public class NewlineParser extends AbstractParser<Newline> {
*
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
- public static class Newline {
- // CHECKSTYLE:OFF
- public String newline;
- // CHECKSTYLE:ON
+ public static class Newline implements Cloneable, Serializable {
+ private static final long serialVersionUID = 2010656704254497899L;
+ private String newline;
+
+ public Newline() {
+ this(null);
+ }
+
+ public Newline(String newline) {
+ this.newline = newline;
+ }
+
+ public String getNewline() {
+ return newline;
+ }
+
+ public void setNewline(String newline) {
+ this.newline = newline;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(getNewline());
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (obj == null) {
+ return false;
+ }
+ if (obj == this) {
+ return true;
+ }
+ if (obj.getClass() != getClass()) {
+ return false;
+ }
+
+ return Objects.equals(((Newline) obj).getNewline(), getNewline());
+ }
+
+ @Override
+ public Newline clone() {
+ try {
+ return getClass().cast(super.clone());
+ } catch (CloneNotSupportedException e) {
+ throw new RuntimeException("Failed to clone " + toString() + ": " + e.getMessage(), e);
+ }
+ }
@Override
public String toString() {
- if (GenericUtils.isEmpty(newline)) {
- return newline;
+ String nl = getNewline();
+ if (GenericUtils.isEmpty(nl)) {
+ return nl;
} else {
- return BufferUtils.printHex(':', newline.getBytes(StandardCharsets.UTF_8));
+ return BufferUtils.printHex(':', nl.getBytes(StandardCharsets.UTF_8));
}
}
}
@@ -63,8 +110,6 @@ public class NewlineParser extends AbstractParser<Newline> {
}
public Newline parse(String value) {
- Newline nl = new Newline();
- nl.newline = value;
- return nl;
+ return new Newline(value);
}
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/3802e521/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/extensions/VersionsParser.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/extensions/VersionsParser.java b/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/extensions/VersionsParser.java
index df96595..7299a3c 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/extensions/VersionsParser.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/extensions/VersionsParser.java
@@ -21,8 +21,8 @@ package org.apache.sshd.common.subsystem.sftp.extensions;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
-import java.util.Collection;
import java.util.Collections;
+import java.util.List;
import org.apache.sshd.common.subsystem.sftp.SftpConstants;
import org.apache.sshd.common.subsystem.sftp.extensions.VersionsParser.Versions;
@@ -41,13 +41,27 @@ public class VersionsParser extends AbstractParser<Versions> {
public static class Versions {
public static final char SEP = ',';
- // CHECKSTYLE:OFF
- public Collection<String> versions;
- // CHECKSTYLE:ON
+ private List<String> versions;
+
+ public Versions() {
+ this(null);
+ }
+
+ public Versions(List<String> versions) {
+ this.versions = versions;
+ }
+
+ public List<String> getVersions() {
+ return versions;
+ }
+
+ public void setVersions(List<String> versions) {
+ this.versions = versions;
+ }
@Override
public String toString() {
- return GenericUtils.join(versions, ',');
+ return GenericUtils.join(getVersions(), ',');
}
}
@@ -64,10 +78,6 @@ public class VersionsParser extends AbstractParser<Versions> {
public Versions parse(String value) {
String[] comps = GenericUtils.split(value, Versions.SEP);
- Versions v = new Versions();
- v.versions = GenericUtils.isEmpty(comps)
- ? Collections.<String>emptyList()
- : Arrays.asList(comps);
- return v;
+ return new Versions(GenericUtils.isEmpty(comps) ? Collections.<String>emptyList() : Arrays.asList(comps));
}
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/3802e521/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/extensions/openssh/AbstractOpenSSHExtensionParser.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/extensions/openssh/AbstractOpenSSHExtensionParser.java b/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/extensions/openssh/AbstractOpenSSHExtensionParser.java
index a1c1764..8590e64 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/extensions/openssh/AbstractOpenSSHExtensionParser.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/extensions/openssh/AbstractOpenSSHExtensionParser.java
@@ -19,6 +19,7 @@
package org.apache.sshd.common.subsystem.sftp.extensions.openssh;
+import java.io.Serializable;
import java.nio.charset.StandardCharsets;
import java.util.Objects;
@@ -33,7 +34,8 @@ import org.apache.sshd.common.util.ValidateUtils;
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public abstract class AbstractOpenSSHExtensionParser extends AbstractParser<OpenSSHExtension> {
- public static class OpenSSHExtension implements NamedResource, Cloneable {
+ public static class OpenSSHExtension implements NamedResource, Cloneable, Serializable {
+ private static final long serialVersionUID = 5902797870154506909L;
private final String name;
private String version;
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/3802e521/sshd-core/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java b/sshd-core/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
index 8449f5c..e67804a 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
@@ -27,6 +27,7 @@ import java.io.OutputStream;
import java.io.StreamCorruptedException;
import java.nio.ByteBuffer;
import java.nio.channels.FileChannel;
+import java.nio.charset.StandardCharsets;
import java.nio.file.AccessDeniedException;
import java.nio.file.CopyOption;
import java.nio.file.FileAlreadyExistsException;
@@ -243,6 +244,12 @@ public class SftpSubsystem
SftpConstants.SSH_ACL_CAP_ALARM)));
/**
+ * Property that can be used to set the reported NL value.
+ * If not set, then {@link IoUtils#EOL} is used
+ */
+ public static final String NEWLINE_VALUE = "sftp-newline";
+
+ /**
* A {@link Map} of {@link FileInfoExtractor}s to be used to complete
* attributes that are deemed important enough to warrant an extra
* effort if not accessible via the file system attributes views
@@ -2210,7 +2217,7 @@ public class SftpSubsystem
protected void appendExtensions(Buffer buffer, String supportedVersions) {
appendVersionsExtension(buffer, supportedVersions);
- appendNewlineExtension(buffer, IoUtils.EOL);
+ appendNewlineExtension(buffer, resolveNewlineValue(getServerSession()));
appendVendorIdExtension(buffer, VersionProperties.getVersionProperties());
appendOpenSSHExtensions(buffer);
appendAclSupportedExtension(buffer);
@@ -2365,10 +2372,18 @@ public class SftpSubsystem
* or use the correct extension name
*
* @param buffer The {@link Buffer} to append to
- * @param value The recommended value
+ * @param value The recommended value - ignored if {@code null}/empty
* @see SftpConstants#EXT_VERSIONS
*/
protected void appendVersionsExtension(Buffer buffer, String value) {
+ if (GenericUtils.isEmpty(value)) {
+ return;
+ }
+
+ if (log.isDebugEnabled()) {
+ log.debug("appendVersionsExtension({}) value={}", getServerSession(), value);
+ }
+
buffer.putString(SftpConstants.EXT_VERSIONS);
buffer.putString(value);
}
@@ -2379,25 +2394,56 @@ public class SftpSubsystem
* or use the correct extension name
*
* @param buffer The {@link Buffer} to append to
- * @param value The recommended value
+ * @param value The recommended value - ignored if {@code null}/empty
* @see SftpConstants#EXT_NEWLINE
*/
protected void appendNewlineExtension(Buffer buffer, String value) {
+ if (GenericUtils.isEmpty(value)) {
+ return;
+ }
+
+ if (log.isDebugEnabled()) {
+ log.debug("appendNewlineExtension({}) value={}",
+ getServerSession(), BufferUtils.printHex(':', value.getBytes(StandardCharsets.UTF_8)));
+ }
+
buffer.putString(SftpConstants.EXT_NEWLINE);
buffer.putString(value);
}
+ protected String resolveNewlineValue(ServerSession session) {
+ String value = PropertyResolverUtils.getString(session, NEWLINE_VALUE);
+ if (value == null) {
+ return IoUtils.EOL;
+ } else {
+ return value; // empty means disabled
+ }
+ }
+
/**
* Appends the "vendor-id" extension to the buffer. <B>Note:</B>
* if overriding this method make sure you either do not append anything
* or use the correct extension name
*
* @param buffer The {@link Buffer} to append to
- * @param versionProperties The currently available version properties
+ * @param versionProperties The currently available version properties - ignored
+ * if {@code null}/empty. The code expects the following values:
+ * <UL>
+ * <LI>{@code groupId} - as the vendor name</LI>
+ * <LI>{@code artifactId} - as the product name</LI>
+ * <LI>{@code version} - as the product version</LI>
+ * </UL>
* @see SftpConstants#EXT_VENDOR_ID
* @see <A HREF="http://tools.ietf.org/wg/secsh/draft-ietf-secsh-filexfer/draft-ietf-secsh-filexfer-09.txt">DRAFT 09 - section 4.4</A>
*/
protected void appendVendorIdExtension(Buffer buffer, Map<String, ?> versionProperties) {
+ if (GenericUtils.isEmpty(versionProperties)) {
+ return;
+ }
+
+ if (log.isDebugEnabled()) {
+ log.debug("appendVendorIdExtension({}): {}", getServerSession(), versionProperties);
+ }
buffer.putString(SftpConstants.EXT_VENDOR_ID);
PropertyResolver resolver = PropertyResolverUtils.toPropertyResolver(Collections.unmodifiableMap(versionProperties));
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/3802e521/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java b/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java
index 9274c5b..a902e88 100644
--- a/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java
@@ -63,13 +63,16 @@ import org.apache.sshd.common.random.Random;
import org.apache.sshd.common.session.Session;
import org.apache.sshd.common.subsystem.sftp.SftpConstants;
import org.apache.sshd.common.subsystem.sftp.extensions.AclSupportedParser.AclCapabilities;
+import org.apache.sshd.common.subsystem.sftp.extensions.NewlineParser.Newline;
import org.apache.sshd.common.subsystem.sftp.extensions.ParserUtils;
import org.apache.sshd.common.subsystem.sftp.extensions.Supported2Parser.Supported2;
import org.apache.sshd.common.subsystem.sftp.extensions.SupportedParser.Supported;
+import org.apache.sshd.common.subsystem.sftp.extensions.VersionsParser.Versions;
import org.apache.sshd.common.subsystem.sftp.extensions.openssh.AbstractOpenSSHExtensionParser.OpenSSHExtension;
import org.apache.sshd.common.util.GenericUtils;
import org.apache.sshd.common.util.OsUtils;
import org.apache.sshd.common.util.ValidateUtils;
+import org.apache.sshd.common.util.buffer.BufferUtils;
import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
import org.apache.sshd.common.util.io.IoUtils;
import org.apache.sshd.server.Command;
@@ -950,6 +953,10 @@ public class SftpTest extends AbstractSftpClientTestSupport {
assertSupportedExtensions(extName, ((Supported2) extValue).extensionNames);
} else if (SftpConstants.EXT_ACL_SUPPORTED.equalsIgnoreCase(extName)) {
assertSupportedAclCapabilities((AclCapabilities) extValue);
+ } else if (SftpConstants.EXT_VERSIONS.equalsIgnoreCase(extName)) {
+ assertSupportedVersions((Versions) extValue);
+ } else if (SftpConstants.EXT_NEWLINE.equalsIgnoreCase(extName)) {
+ assertNewlineValue((Newline) extValue);
}
}
@@ -993,7 +1000,6 @@ public class SftpTest extends AbstractSftpClientTestSupport {
}
private static Map<String, OptionalFeature> EXPECTED_EXTENSIONS = SftpSubsystem.DEFAULT_SUPPORTED_CLIENT_EXTENSIONS;
-
private static void assertSupportedExtensions(String extName, Collection<String> extensionNames) {
assertEquals(extName + "[count]", EXPECTED_EXTENSIONS.size(), GenericUtils.size(extensionNames));
@@ -1008,6 +1014,24 @@ public class SftpTest extends AbstractSftpClientTestSupport {
}
}
+ private static void assertSupportedVersions(Versions vers) {
+ List<String> values = vers.getVersions();
+ assertEquals("Mismatched reported versions size: " + values,
+ 1 + SftpSubsystem.HIGHER_SFTP_IMPL - SftpSubsystem.LOWER_SFTP_IMPL,
+ GenericUtils.size(values));
+ for (int expected = SftpSubsystem.LOWER_SFTP_IMPL, index = 0; expected <= SftpSubsystem.HIGHER_SFTP_IMPL; expected++, index++) {
+ String e = Integer.toString(expected);
+ String a = values.get(index);
+ assertEquals("Missing value at index=" + index + ": " + values, e, a);
+ }
+ }
+
+ private static void assertNewlineValue(Newline nl) {
+ assertEquals("Mismatched NL value",
+ BufferUtils.printHex(':', IoUtils.EOL.getBytes(StandardCharsets.UTF_8)),
+ BufferUtils.printHex(':', nl.getNewline().getBytes(StandardCharsets.UTF_8)));
+ }
+
private static void assertSupportedAclCapabilities(AclCapabilities caps) {
Set<Integer> actual = AclCapabilities.deconstructAclCapabilities(caps.getCapabilities());
assertEquals("Mismatched ACL capabilities count", SftpSubsystem.DEFAULT_ACL_SUPPORTED_MASK.size(), actual.size());