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 2019/02/10 05:57:01 UTC

[mina-sshd] branch master updated (f95df91 -> 4cee648)

This is an automated email from the ASF dual-hosted git repository.

lgoldstein pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git.


    from f95df91  [maven-release-plugin] prepare for next development iteration
     new b4fc8db  [SSHD-890] SFTP susbsystem implementation does not clear response buffer when sending unsupported extension status message
     new 4cee648  Some minor code-style modifications to SFTP test code

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../sftp/AbstractSftpSubsystemHelper.java          |   2 +-
 .../sshd/server/subsystem/sftp/SftpSubsystem.java  |   2 +-
 .../sshd/client/subsystem/sftp/SftpTest.java       | 116 +++++++++++++--------
 .../client/subsystem/sftp/SftpVersionsTest.java    |  24 +++--
 .../sftp/extensions/UnsupportedExtensionTest.java  |  71 +++++++++++++
 .../helpers/AbstractCheckFileExtensionTest.java    |   4 +-
 .../helpers/AbstractMD5HashExtensionTest.java      |   4 +-
 .../helpers/CopyDataExtensionImplTest.java         |   4 +-
 .../helpers/CopyFileExtensionImplTest.java         |   4 +-
 .../openssh/helpers/OpenSSHExtensionsTest.java     |  12 ++-
 10 files changed, 183 insertions(+), 60 deletions(-)
 create mode 100644 sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/UnsupportedExtensionTest.java


[mina-sshd] 02/02: Some minor code-style modifications to SFTP test code

Posted by lg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git

commit 4cee648938f794d6485d5bc520f2f2f27892d2aa
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Sat Feb 9 10:48:33 2019 +0200

    Some minor code-style modifications to SFTP test code
---
 .../sshd/client/subsystem/sftp/SftpTest.java       | 116 +++++++++++++--------
 .../client/subsystem/sftp/SftpVersionsTest.java    |  24 +++--
 .../helpers/AbstractCheckFileExtensionTest.java    |   4 +-
 .../helpers/AbstractMD5HashExtensionTest.java      |   4 +-
 .../helpers/CopyDataExtensionImplTest.java         |   4 +-
 .../helpers/CopyFileExtensionImplTest.java         |   4 +-
 .../openssh/helpers/OpenSSHExtensionsTest.java     |  12 ++-
 7 files changed, 110 insertions(+), 58 deletions(-)

diff --git a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java
index 5a07733..d417225 100644
--- a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java
+++ b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java
@@ -119,7 +119,8 @@ import com.jcraft.jsch.JSch;
 @FixMethodOrder(MethodSorters.NAME_ASCENDING)
 @SuppressWarnings("checkstyle:MethodCount")
 public class SftpTest extends AbstractSftpClientTestSupport {
-    private static final Map<String, OptionalFeature> EXPECTED_EXTENSIONS = AbstractSftpSubsystemHelper.DEFAULT_SUPPORTED_CLIENT_EXTENSIONS;
+    private static final Map<String, OptionalFeature> EXPECTED_EXTENSIONS =
+        AbstractSftpSubsystemHelper.DEFAULT_SUPPORTED_CLIENT_EXTENSIONS;
 
     private com.jcraft.jsch.Session session;
 
@@ -157,7 +158,9 @@ public class SftpTest extends AbstractSftpClientTestSupport {
         rnd.fill(expectedRandom);
 
         byte[] expectedText = (getClass().getName() + "#" + getCurrentTestName()).getBytes(StandardCharsets.UTF_8);
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
@@ -203,7 +206,9 @@ public class SftpTest extends AbstractSftpClientTestSupport {
         rnd.fill(expected);
         Files.write(testFile, expected);
 
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
@@ -238,10 +243,12 @@ public class SftpTest extends AbstractSftpClientTestSupport {
     @Test   // see extra fix for SSHD-538
     public void testNavigateBeyondRootFolder() throws Exception {
         Path rootLocation = Paths.get(OsUtils.isUNIX() ? "/" : "C:\\");
-        final FileSystem fsRoot = rootLocation.getFileSystem();
+        FileSystem fsRoot = rootLocation.getFileSystem();
         sshd.setFileSystemFactory(session1 -> fsRoot);
 
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
@@ -269,7 +276,9 @@ public class SftpTest extends AbstractSftpClientTestSupport {
         assertHierarchyTargetFolderExists(lclSftp);
         sshd.setFileSystemFactory(new VirtualFileSystemFactory(lclSftp));
 
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
@@ -293,15 +302,17 @@ public class SftpTest extends AbstractSftpClientTestSupport {
                         ? SftpConstants.SSH_FX_INVALID_FILENAME
                         : SftpConstants.SSH_FX_NO_SUCH_FILE;
                 assertEquals("Mismatched status for " + escapePath,
-                             SftpConstants.getStatusName(expected),
-                             SftpConstants.getStatusName(e.getStatus()));
+                     SftpConstants.getStatusName(expected),
+                     SftpConstants.getStatusName(e.getStatus()));
             }
         }
     }
 
     @Test
     public void testNormalizeRemoteRootValues() throws Exception {
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
@@ -336,7 +347,9 @@ public class SftpTest extends AbstractSftpClientTestSupport {
 
         Factory<? extends Random> factory = client.getRandomFactory();
         Random rnd = factory.create();
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
@@ -389,7 +402,9 @@ public class SftpTest extends AbstractSftpClientTestSupport {
         File javaFile = testFile.toFile();
         assertHierarchyTargetFolderExists(javaFile.getParentFile());
 
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
@@ -483,7 +498,9 @@ public class SftpTest extends AbstractSftpClientTestSupport {
         Files.createDirectories(localFile.getParent());
         byte[] data = (getClass().getName() + "#" + getCurrentTestName() + "[" + localFile + "]").getBytes(StandardCharsets.UTF_8);
         Files.write(localFile, data, StandardOpenOption.CREATE);
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
@@ -543,9 +560,10 @@ public class SftpTest extends AbstractSftpClientTestSupport {
             AtomicReference<Path> dirHolder = new AtomicReference<>();
             factory.setFileSystemAccessor(new SftpFileSystemAccessor() {
                 @Override
-                public SeekableByteChannel openFile(ServerSession session, SftpEventListenerManager subsystem, Path file,
+                public SeekableByteChannel openFile(
+                        ServerSession session, SftpEventListenerManager subsystem, Path file,
                         String handle, Set<? extends OpenOption> options, FileAttribute<?>... attrs)
-                                throws IOException {
+                            throws IOException {
                     fileHolder.set(file);
                     return SftpFileSystemAccessor.super.openFile(session, subsystem, file, handle, options, attrs);
                 }
@@ -569,7 +587,9 @@ public class SftpTest extends AbstractSftpClientTestSupport {
             Files.createDirectories(localFile.getParent());
             byte[] expected = (getClass().getName() + "#" + getCurrentTestName() + "[" + localFile + "]").getBytes(StandardCharsets.UTF_8);
             Files.write(localFile, expected, StandardOpenOption.CREATE);
-            try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+            try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                    .verify(7L, TimeUnit.SECONDS)
+                    .getSession()) {
                 session.addPasswordIdentity(getCurrentTestName());
                 session.auth().verify(5L, TimeUnit.SECONDS);
 
@@ -623,18 +643,18 @@ public class SftpTest extends AbstractSftpClientTestSupport {
         assertObjectInstanceOf("Not an SFTP subsystem factory", SftpSubsystemFactory.class, f);
 
         SftpSubsystemFactory factory = (SftpSubsystemFactory) f;
-        final AtomicInteger versionHolder = new AtomicInteger(-1);
-        final AtomicInteger openCount = new AtomicInteger(0);
-        final AtomicInteger closeCount = new AtomicInteger(0);
-        final AtomicLong readSize = new AtomicLong(0L);
-        final AtomicLong writeSize = new AtomicLong(0L);
-        final AtomicInteger entriesCount = new AtomicInteger(0);
-        final AtomicInteger creatingCount = new AtomicInteger(0);
-        final AtomicInteger createdCount = new AtomicInteger(0);
-        final AtomicInteger removingCount = new AtomicInteger(0);
-        final AtomicInteger removedCount = new AtomicInteger(0);
-        final AtomicInteger modifyingCount = new AtomicInteger(0);
-        final AtomicInteger modifiedCount = new AtomicInteger(0);
+        AtomicInteger versionHolder = new AtomicInteger(-1);
+        AtomicInteger openCount = new AtomicInteger(0);
+        AtomicInteger closeCount = new AtomicInteger(0);
+        AtomicLong readSize = new AtomicLong(0L);
+        AtomicLong writeSize = new AtomicLong(0L);
+        AtomicInteger entriesCount = new AtomicInteger(0);
+        AtomicInteger creatingCount = new AtomicInteger(0);
+        AtomicInteger createdCount = new AtomicInteger(0);
+        AtomicInteger removingCount = new AtomicInteger(0);
+        AtomicInteger removedCount = new AtomicInteger(0);
+        AtomicInteger modifyingCount = new AtomicInteger(0);
+        AtomicInteger modifiedCount = new AtomicInteger(0);
         SftpEventListener listener = new AbstractSftpEventListenerAdapter() {
             @Override
             public void initialized(ServerSession session, int version) {
@@ -788,7 +808,9 @@ public class SftpTest extends AbstractSftpClientTestSupport {
         };
         factory.addSftpEventListener(listener);
 
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
@@ -821,7 +843,9 @@ public class SftpTest extends AbstractSftpClientTestSupport {
      */
     @Test
     public void testWriteChunking() throws Exception {
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
@@ -1036,7 +1060,9 @@ public class SftpTest extends AbstractSftpClientTestSupport {
 
         Path parentPath = targetPath.getParent();
         Path clientFolder = assertHierarchyTargetFolderExists(lclSftp.resolve("client"));
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
@@ -1076,7 +1102,9 @@ public class SftpTest extends AbstractSftpClientTestSupport {
 
     @Test
     public void testServerExtensionsDeclarations() throws Exception {
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
@@ -1159,8 +1187,8 @@ public class SftpTest extends AbstractSftpClientTestSupport {
     private static void assertSupportedVersions(Versions vers) {
         List<String> values = vers.getVersions();
         assertEquals("Mismatched reported versions size: " + values,
-                     1 + SftpSubsystemEnvironment.HIGHER_SFTP_IMPL - SftpSubsystemEnvironment.LOWER_SFTP_IMPL,
-                     GenericUtils.size(values));
+             1 + SftpSubsystemEnvironment.HIGHER_SFTP_IMPL - SftpSubsystemEnvironment.LOWER_SFTP_IMPL,
+             GenericUtils.size(values));
         for (int expected = SftpSubsystemEnvironment.LOWER_SFTP_IMPL, index = 0; expected <= SftpSubsystemEnvironment.HIGHER_SFTP_IMPL; expected++, index++) {
             String e = Integer.toString(expected);
             String a = values.get(index);
@@ -1170,20 +1198,20 @@ public class SftpTest extends AbstractSftpClientTestSupport {
 
     private static void assertNewlineValue(Newline nl) {
         assertEquals("Mismatched NL value",
-                     BufferUtils.toHex(':', IoUtils.EOL.getBytes(StandardCharsets.UTF_8)),
-                     BufferUtils.toHex(':', nl.getNewline().getBytes(StandardCharsets.UTF_8)));
+             BufferUtils.toHex(':', IoUtils.EOL.getBytes(StandardCharsets.UTF_8)),
+             BufferUtils.toHex(':', nl.getNewline().getBytes(StandardCharsets.UTF_8)));
     }
 
     private static void assertSupportedAclCapabilities(AclCapabilities caps) {
         Set<Integer> actual = AclCapabilities.deconstructAclCapabilities(caps.getCapabilities());
         assertEquals("Mismatched ACL capabilities count", AbstractSftpSubsystemHelper.DEFAULT_ACL_SUPPORTED_MASK.size(), actual.size());
         assertTrue("Missing capabilities - expected=" + AbstractSftpSubsystemHelper.DEFAULT_ACL_SUPPORTED_MASK + ", actual=" + actual,
-                   actual.containsAll(AbstractSftpSubsystemHelper.DEFAULT_ACL_SUPPORTED_MASK));
+           actual.containsAll(AbstractSftpSubsystemHelper.DEFAULT_ACL_SUPPORTED_MASK));
     }
 
     @Test
     public void testSftpVersionSelector() throws Exception {
-        final AtomicInteger selected = new AtomicInteger(-1);
+        AtomicInteger selected = new AtomicInteger(-1);
         SftpVersionSelector selector = (session, current, available) -> {
             int value = GenericUtils.stream(available)
                     .mapToInt(Integer::intValue)
@@ -1194,7 +1222,9 @@ public class SftpTest extends AbstractSftpClientTestSupport {
             return value;
         };
 
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
@@ -1211,7 +1241,9 @@ public class SftpTest extends AbstractSftpClientTestSupport {
         assertEquals("Mismatched subsystem factories count", 1, GenericUtils.size(factories));
 
         sshd.setSubsystemFactories(null);
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
@@ -1331,7 +1363,7 @@ public class SftpTest extends AbstractSftpClientTestSupport {
         assertObjectInstanceOf("Not an SFTP subsystem factory", SftpSubsystemFactory.class, f);
 
         SftpSubsystemFactory factory = (SftpSubsystemFactory) f;
-        final AtomicReference<LinkData> linkDataHolder = new AtomicReference<>();
+        AtomicReference<LinkData> linkDataHolder = new AtomicReference<>();
         SftpEventListener listener = new AbstractSftpEventListenerAdapter() {
             @Override
             public void linking(ServerSession session, Path src, Path target, boolean symLink) {
@@ -1417,7 +1449,9 @@ public class SftpTest extends AbstractSftpClientTestSupport {
         Files.deleteIfExists(lclFile);
         byte[] expected = (getClass().getName() + "#" + getCurrentTestName() + "(" + new Date() + ")").getBytes(StandardCharsets.UTF_8);
 
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
diff --git a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpVersionsTest.java b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpVersionsTest.java
index 8fa7eca..5ae7064 100644
--- a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpVersionsTest.java
+++ b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpVersionsTest.java
@@ -228,21 +228,22 @@ public class SftpVersionsTest extends AbstractSftpClientTestSupport {
         final List<AclEntry> aclExpected = new ArrayList<>(types.length);
         for (AclEntryType t : types) {
             aclExpected.add(AclEntry.newBuilder()
-                                .setType(t)
-                                .setFlags(EnumSet.allOf(AclEntryFlag.class))
-                                .setPermissions(EnumSet.allOf(AclEntryPermission.class))
-                                .setPrincipal(new DefaultGroupPrincipal(getCurrentTestName() + "@" + getClass().getPackage().getName()))
-                                .build());
+                .setType(t)
+                .setFlags(EnumSet.allOf(AclEntryFlag.class))
+                .setPermissions(EnumSet.allOf(AclEntryPermission.class))
+                .setPrincipal(new DefaultGroupPrincipal(getCurrentTestName() + "@" + getClass().getPackage().getName()))
+                .build());
         }
 
-        final AtomicInteger numInvocations = new AtomicInteger(0);
+        AtomicInteger numInvocations = new AtomicInteger(0);
         SftpSubsystemFactory factory = new SftpSubsystemFactory() {
             @Override
             public Command create() {
                 SftpSubsystem subsystem = new SftpSubsystem(getExecutorService(),
                         getUnsupportedAttributePolicy(), getFileSystemAccessor(), getErrorStatusDataHandler()) {
                     @Override
-                    protected NavigableMap<String, Object> resolveFileAttributes(Path file, int flags, LinkOption... options) throws IOException {
+                    protected NavigableMap<String, Object> resolveFileAttributes(Path file, int flags, LinkOption... options)
+                            throws IOException {
                         NavigableMap<String, Object> attrs = super.resolveFileAttributes(file, flags, options);
                         if (GenericUtils.isEmpty(attrs)) {
                             attrs = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
@@ -257,7 +258,8 @@ public class SftpVersionsTest extends AbstractSftpClientTestSupport {
                     }
 
                     @Override
-                    protected void setFileAccessControl(Path file, List<AclEntry> aclActual, LinkOption... options) throws IOException {
+                    protected void setFileAccessControl(Path file, List<AclEntry> aclActual, LinkOption... options)
+                            throws IOException {
                         if (aclActual != null) {
                             assertListEquals("Mismatched ACL set for file=" + file, aclExpected, aclActual);
                             numInvocations.incrementAndGet();
@@ -364,7 +366,8 @@ public class SftpVersionsTest extends AbstractSftpClientTestSupport {
                 SftpSubsystem subsystem = new SftpSubsystem(getExecutorService(),
                         getUnsupportedAttributePolicy(), getFileSystemAccessor(), getErrorStatusDataHandler()) {
                     @Override
-                    protected NavigableMap<String, Object> resolveFileAttributes(Path file, int flags, LinkOption... options) throws IOException {
+                    protected NavigableMap<String, Object> resolveFileAttributes(Path file, int flags, LinkOption... options)
+                            throws IOException {
                         NavigableMap<String, Object> attrs = super.resolveFileAttributes(file, flags, options);
                         if (GenericUtils.isEmpty(attrs)) {
                             attrs = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
@@ -379,7 +382,8 @@ public class SftpVersionsTest extends AbstractSftpClientTestSupport {
                     }
 
                     @Override
-                    protected void setFileExtensions(Path file, Map<String, byte[]> extensions, LinkOption... options) throws IOException {
+                    protected void setFileExtensions(Path file, Map<String, byte[]> extensions, LinkOption... options)
+                            throws IOException {
                         assertExtensionsMapEquals("setFileExtensions(" + file + ")", expExtensions, extensions);
                         numInvocations.incrementAndGet();
 
diff --git a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/helpers/AbstractCheckFileExtensionTest.java b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/helpers/AbstractCheckFileExtensionTest.java
index 5754a2a..e0beae0 100644
--- a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/helpers/AbstractCheckFileExtensionTest.java
+++ b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/helpers/AbstractCheckFileExtensionTest.java
@@ -177,7 +177,9 @@ public class AbstractCheckFileExtensionTest extends AbstractSftpClientTestSuppor
         Path parentPath = targetPath.getParent();
         String srcPath = CommonTestSupportUtils.resolveRelativeRemotePath(parentPath, srcFile);
         String srcFolder = CommonTestSupportUtils.resolveRelativeRemotePath(parentPath, srcFile.getParent());
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
diff --git a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/helpers/AbstractMD5HashExtensionTest.java b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/helpers/AbstractMD5HashExtensionTest.java
index 2393026..44799f6 100644
--- a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/helpers/AbstractMD5HashExtensionTest.java
+++ b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/helpers/AbstractMD5HashExtensionTest.java
@@ -134,7 +134,9 @@ public class AbstractMD5HashExtensionTest extends AbstractSftpClientTestSupport
         Path parentPath = targetPath.getParent();
         String srcPath = CommonTestSupportUtils.resolveRelativeRemotePath(parentPath, srcFile);
         String srcFolder = CommonTestSupportUtils.resolveRelativeRemotePath(parentPath, srcFile.getParent());
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
diff --git a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/helpers/CopyDataExtensionImplTest.java b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/helpers/CopyDataExtensionImplTest.java
index 2b928d4..8414ca4 100644
--- a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/helpers/CopyDataExtensionImplTest.java
+++ b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/helpers/CopyDataExtensionImplTest.java
@@ -161,7 +161,9 @@ public class CopyDataExtensionImplTest extends AbstractSftpClientTestSupport {
             }
         }
 
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
diff --git a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/helpers/CopyFileExtensionImplTest.java b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/helpers/CopyFileExtensionImplTest.java
index 74d3573..a976774 100644
--- a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/helpers/CopyFileExtensionImplTest.java
+++ b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/helpers/CopyFileExtensionImplTest.java
@@ -72,7 +72,9 @@ public class CopyFileExtensionImplTest extends AbstractSftpClientTestSupport {
         LinkOption[] options = IoUtils.getLinkOptions(true);
         assertFalse("Destination file unexpectedly exists", Files.exists(dstFile, options));
 
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
diff --git a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/openssh/helpers/OpenSSHExtensionsTest.java b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/openssh/helpers/OpenSSHExtensionsTest.java
index d778393..5e31a23 100644
--- a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/openssh/helpers/OpenSSHExtensionsTest.java
+++ b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/openssh/helpers/OpenSSHExtensionsTest.java
@@ -81,7 +81,9 @@ public class OpenSSHExtensionsTest extends AbstractSftpClientTestSupport {
 
         Path parentPath = targetPath.getParent();
         String srcPath = CommonTestSupportUtils.resolveRelativeRemotePath(parentPath, srcFile);
-        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
             session.addPasswordIdentity(getCurrentTestName());
             session.auth().verify(5L, TimeUnit.SECONDS);
 
@@ -167,7 +169,9 @@ public class OpenSSHExtensionsTest extends AbstractSftpClientTestSupport {
         try (SshClient client = setupTestClient()) {
             client.start();
 
-            try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+            try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                    .verify(7L, TimeUnit.SECONDS)
+                    .getSession()) {
                 session.addPasswordIdentity(getCurrentTestName());
                 session.auth().verify(5L, TimeUnit.SECONDS);
 
@@ -190,7 +194,9 @@ public class OpenSSHExtensionsTest extends AbstractSftpClientTestSupport {
         }
     }
 
-    private static void assertOpenSSHStatExtensionInfoEquals(String extension, OpenSSHStatExtensionInfo expected, OpenSSHStatExtensionInfo actual) throws Exception {
+    private static void assertOpenSSHStatExtensionInfoEquals(
+            String extension, OpenSSHStatExtensionInfo expected, OpenSSHStatExtensionInfo actual)
+                throws Exception {
         Field[] fields = expected.getClass().getFields();
         for (Field f : fields) {
             String name = f.getName();


[mina-sshd] 01/02: [SSHD-890] SFTP susbsystem implementation does not clear response buffer when sending unsupported extension status message

Posted by lg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git

commit b4fc8db6bc3fdc1907e619cd8b5043e9d2f87e3b
Author: Sakthipriyan Vairamani (thefourtheye) <th...@gmail.com>
AuthorDate: Sat Feb 9 10:32:29 2019 +0200

    [SSHD-890] SFTP susbsystem implementation does not clear response buffer when sending unsupported extension status message
---
 .../sftp/AbstractSftpSubsystemHelper.java          |  2 +-
 .../sshd/server/subsystem/sftp/SftpSubsystem.java  |  2 +-
 .../sftp/extensions/UnsupportedExtensionTest.java  | 71 ++++++++++++++++++++++
 3 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/AbstractSftpSubsystemHelper.java b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/AbstractSftpSubsystemHelper.java
index 2ef4b60..bd4ece6 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/AbstractSftpSubsystemHelper.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/AbstractSftpSubsystemHelper.java
@@ -1651,7 +1651,7 @@ public abstract class AbstractSftpSubsystemHelper
         if (log.isDebugEnabled()) {
             log.debug("executeExtendedCommand({}) received unsupported SSH_FXP_EXTENDED({})", getServerSession(), extension);
         }
-        sendStatus(buffer, id, SftpConstants.SSH_FX_OP_UNSUPPORTED,
+        sendStatus(prepareReply(buffer), id, SftpConstants.SSH_FX_OP_UNSUPPORTED,
             "Command SSH_FXP_EXTENDED(" + extension + ") is unsupported or not implemented");
     }
 
diff --git a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
index 66a0ced..e7f1538 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
@@ -947,7 +947,7 @@ public class SftpSubsystem
     @Override
     protected Buffer prepareReply(Buffer buffer) {
         buffer.clear();
-        buffer.putInt(0);
+        buffer.putInt(0);   // reserve space for actual packet length
         return buffer;
     }
 
diff --git a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/UnsupportedExtensionTest.java b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/UnsupportedExtensionTest.java
new file mode 100644
index 0000000..6dcdea0
--- /dev/null
+++ b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/UnsupportedExtensionTest.java
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.sshd.client.subsystem.sftp.extensions;
+
+import java.io.IOException;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.sshd.client.session.ClientSession;
+import org.apache.sshd.client.subsystem.sftp.AbstractSftpClientTestSupport;
+import org.apache.sshd.client.subsystem.sftp.RawSftpClient;
+import org.apache.sshd.client.subsystem.sftp.SftpClient;
+import org.apache.sshd.common.subsystem.sftp.SftpConstants;
+import org.apache.sshd.common.util.GenericUtils;
+import org.apache.sshd.common.util.buffer.Buffer;
+import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
+import org.junit.FixMethodOrder;
+import org.junit.Test;
+import org.junit.runners.MethodSorters;
+
+@FixMethodOrder(MethodSorters.NAME_ASCENDING)
+public class UnsupportedExtensionTest extends AbstractSftpClientTestSupport {
+    public UnsupportedExtensionTest() throws IOException {
+        super();
+    }
+
+    @Test   // see SSHD-890
+    public void testUnsupportedExtension() throws IOException {
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+                .verify(7L, TimeUnit.SECONDS)
+                .getSession()) {
+            session.addPasswordIdentity(getCurrentTestName());
+            session.auth().verify(5L, TimeUnit.SECONDS);
+
+            try (SftpClient sftpClient = createSftpClient(session)) {
+                String opcode = getCurrentTestName();
+                Buffer buffer = new ByteArrayBuffer(Integer.BYTES + GenericUtils.length(opcode) + Byte.SIZE, false);
+                buffer.putString(opcode);
+
+                assertObjectInstanceOf("Not a raw SFTP client", RawSftpClient.class, sftpClient);
+                RawSftpClient sftp = (RawSftpClient) sftpClient;
+                int cmd = sftp.send(SftpConstants.SSH_FXP_EXTENDED, buffer);
+                Buffer responseBuffer = sftp.receive(cmd);
+
+                responseBuffer.getInt();                    // Ignoring length
+                int type = responseBuffer.getUByte();
+                responseBuffer.getInt();                    // Ignoring message ID
+                int substatus = responseBuffer.getInt();
+
+                assertEquals("Type is not STATUS", SftpConstants.SSH_FXP_STATUS, type);
+                assertEquals("Sub-Type is not UNSUPPORTED", SftpConstants.SSH_FX_OP_UNSUPPORTED, substatus);
+            }
+        }
+    }
+}
\ No newline at end of file