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 6e68b7621edcb620be010277d234962a6abfbc3f Author: Lyor Goldstein <lgoldst...@apache.org> AuthorDate: Thu Sep 17 11:02:32 2020 +0300 [SSHD-1077] Added command line option to request specific SFTP version in SftpCommandMain --- CHANGES.md | 1 + .../apache/sshd/cli/client/SftpCommandMain.java | 113 +++++++++++++++------ .../java/org/apache/sshd/common/NamedResource.java | 6 ++ .../sshd/sftp/client/SftpVersionSelector.java | 89 +++++++++++++--- .../sshd/sftp/client/SftpVersionResolverTest.java | 100 ++++++++++++++++++ 5 files changed, 264 insertions(+), 45 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index a86f891..938bfd5 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -27,6 +27,7 @@ or `-key-file` command line option. * [SSHD-1060](https://issues.apache.org/jira/browse/SSHD-1060) Do not store logger level in fields. * [SSHD-1064](https://issues.apache.org/jira/browse/SSHD-1064) Fixed `ClientSession#executeRemoteCommand` handling of STDERR in case of exception to behave according to its documentation * [SSHD-1076](https://issues.apache.org/jira/browse/SSHD-1076) Break down `ClientUserAuthService#auth` method into several to allow for flexible override +* [SSHD-1077](https://issues.apache.org/jira/browse/SSHD-1077) Added command line option to request specific SFTP version in `SftpCommandMain` ## Behavioral changes and enhancements diff --git a/sshd-cli/src/main/java/org/apache/sshd/cli/client/SftpCommandMain.java b/sshd-cli/src/main/java/org/apache/sshd/cli/client/SftpCommandMain.java index 399208f..42a4d1f 100644 --- a/sshd-cli/src/main/java/org/apache/sshd/cli/client/SftpCommandMain.java +++ b/sshd-cli/src/main/java/org/apache/sshd/cli/client/SftpCommandMain.java @@ -65,10 +65,13 @@ import org.apache.sshd.common.util.buffer.BufferUtils; import org.apache.sshd.common.util.io.IoUtils; import org.apache.sshd.common.util.io.NoCloseInputStream; import org.apache.sshd.common.util.threads.ThreadUtils; +import org.apache.sshd.server.config.SshServerConfigFileReader; import org.apache.sshd.sftp.client.SftpClient; import org.apache.sshd.sftp.client.SftpClient.Attributes; import org.apache.sshd.sftp.client.SftpClient.DirEntry; import org.apache.sshd.sftp.client.SftpClientFactory; +import org.apache.sshd.sftp.client.SftpVersionSelector; +import org.apache.sshd.sftp.client.SftpVersionSelector.NamedVersionSelector; import org.apache.sshd.sftp.client.extensions.openssh.OpenSSHStatExtensionInfo; import org.apache.sshd.sftp.client.extensions.openssh.OpenSSHStatPathExtension; import org.apache.sshd.sftp.client.fs.SftpFileSystemProvider; @@ -268,34 +271,21 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { return sb; } - public static SftpClientFactory resolveSftpClientFactory(String... args) { - int numArgs = GenericUtils.length(args); - ClassLoader cl = ThreadUtils.resolveDefaultClassLoader(SftpClientFactory.class); - for (int index = 0; index < numArgs; index++) { - String argVal = args[index]; - if ("-o".equals(argVal)) { - index++; - - String opt = args[index]; - int idx = opt.indexOf('='); - ValidateUtils.checkTrue(idx > 0, "bad syntax for option: %s %s", argVal, opt); - - String optName = opt.substring(0, idx); - String optValue = opt.substring(idx + 1); - if (!Objects.equals(SftpClientFactory.class.getSimpleName(), optName)) { - continue; - } + /* -------------------------------------------------------------------- */ - try { - Class<?> clazz = cl.loadClass(optValue); - return SftpClientFactory.class.cast(clazz.newInstance()); - } catch (Throwable t) { - System.err.append("Failed (").append(t.getClass().getSimpleName()).append(')') - .append(" to instantiate ").append(optValue) - .append(": ").println(t.getMessage()); - System.err.flush(); - throw GenericUtils.toRuntimeException(t, true); - } + public static SftpClientFactory resolveSftpClientFactory(ClientSession session) { + ClassLoader cl = ThreadUtils.resolveDefaultClassLoader(SftpClientFactory.class); + String factoryName = session.getString(SftpClientFactory.class.getSimpleName()); + if (GenericUtils.isNotEmpty(factoryName)) { + try { + Class<?> clazz = cl.loadClass(factoryName); + return SftpClientFactory.class.cast(clazz.newInstance()); + } catch (Throwable t) { + System.err.append("Failed (").append(t.getClass().getSimpleName()).append(')') + .append(" to instantiate ").append(factoryName) + .append(": ").println(t.getMessage()); + System.err.flush(); + throw GenericUtils.toRuntimeException(t, true); } } @@ -313,6 +303,15 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { return SftpClientFactory.instance(); } + /* -------------------------------------------------------------------- */ + + public static NamedVersionSelector resolveVersionSelector(ClientSession session) { + String selector = session.getString(SshServerConfigFileReader.SFTP_FORCED_VERSION_PROP.getName()); + return SftpVersionSelector.resolveVersionSelector(selector); + } + + /* -------------------------------------------------------------------- */ + public static void main(String[] args) throws Exception { PrintStream stdout = System.out; PrintStream stderr = System.err; @@ -330,15 +329,23 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { if (session == null) { System.err.println("usage: sftp [-v[v][v]] [-E logoutput] [-i identity] [-io nio2|mina|netty]" + " [-J proxyJump] [-l login] [" + SFTP_PORT_OPTION + " port] [-o option=value]" - + " [-w password] [-c cipherlist] [-m maclist] [-C] hostname/user@host"); + + " [-w password] [-c cipherlist] [-m maclist] [-C] hostname/user@host"); System.exit(-1); return; } try { - // TODO allow command-line specification of SftpClientFactory - SftpClientFactory clientFactory = resolveSftpClientFactory(args); - try (SftpClient sftpClient = clientFactory.createSftpClient(session); + SftpClientFactory clientFactory = resolveSftpClientFactory(session); + if ((level != null) && (level.intValue() <= Level.INFO.intValue())) { + stdout.append("Using factory=").println(clientFactory.getClass().getSimpleName()); + } + + SftpVersionSelector versionSelector = resolveVersionSelector(session); + if ((level != null) && (level.intValue() <= Level.INFO.intValue())) { + stdout.append("Using version selector=").println(versionSelector); + } + + try (SftpClient sftpClient = clientFactory.createSftpClient(session, versionSelector); SftpCommandMain sftp = new SftpCommandMain(sftpClient)) { // TODO allow injection of extra CommandExecutor(s) via command line and/or service loading sftp.doInteractive(stdin, stdout, stderr); @@ -353,6 +360,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + ////////////////////////////////////////////////////////////////////////// + private static class ExitCommandExecutor implements SftpCommandExecutor { ExitCommandExecutor() { super(); @@ -373,6 +382,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class PwdCommandExecutor implements SftpCommandExecutor { protected PwdCommandExecutor() { super(); @@ -394,6 +405,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class SessionCommandExecutor implements SftpCommandExecutor { SessionCommandExecutor() { super(); @@ -424,6 +437,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class KexCommandExecutor implements SftpCommandExecutor { KexCommandExecutor() { super(); @@ -456,6 +471,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class ClientCommandExecutor implements SftpCommandExecutor { ClientCommandExecutor() { super(); @@ -494,6 +511,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class InfoCommandExecutor implements SftpCommandExecutor { InfoCommandExecutor() { super(); @@ -534,6 +553,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class VersionCommandExecutor implements SftpCommandExecutor { VersionCommandExecutor() { super(); @@ -555,6 +576,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class CdCommandExecutor extends PwdCommandExecutor { CdCommandExecutor() { super(); @@ -578,6 +601,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class LcdCommandExecutor extends PwdCommandExecutor { LcdCommandExecutor() { super(); @@ -605,6 +630,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class MkdirCommandExecutor implements SftpCommandExecutor { MkdirCommandExecutor() { super(); @@ -628,6 +655,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class LsCommandExecutor implements SftpCommandExecutor { LsCommandExecutor() { super(); @@ -670,6 +699,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class RmCommandExecutor implements SftpCommandExecutor { RmCommandExecutor() { super(); @@ -757,6 +788,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class RmdirCommandExecutor implements SftpCommandExecutor { RmdirCommandExecutor() { super(); @@ -780,6 +813,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class RenameCommandExecutor implements SftpCommandExecutor { RenameCommandExecutor() { super(); @@ -805,6 +840,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class StatVfsCommandExecutor implements SftpCommandExecutor { StatVfsCommandExecutor() { super(); @@ -846,6 +883,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class LStatCommandExecutor implements SftpCommandExecutor { LStatCommandExecutor() { super(); @@ -871,6 +910,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class ReadLinkCommandExecutor implements SftpCommandExecutor { ReadLinkCommandExecutor() { super(); @@ -895,6 +936,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class HelpCommandExecutor implements SftpCommandExecutor { HelpCommandExecutor() { super(); @@ -918,6 +961,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private abstract class TransferCommandExecutor implements SftpCommandExecutor { protected TransferCommandExecutor() { super(); @@ -1101,6 +1146,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class GetCommandExecutor extends TransferCommandExecutor { GetCommandExecutor() { super(); @@ -1120,6 +1167,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class PutCommandExecutor extends TransferCommandExecutor { PutCommandExecutor() { super(); @@ -1139,6 +1188,8 @@ public class SftpCommandMain extends SshClientCliSupport implements Channel { } } + /* -------------------------------------------------------------------- */ + private class ProgressCommandExecutor implements SftpCommandExecutor { ProgressCommandExecutor() { super(); diff --git a/sshd-common/src/main/java/org/apache/sshd/common/NamedResource.java b/sshd-common/src/main/java/org/apache/sshd/common/NamedResource.java index 6421539..256ecfd 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/NamedResource.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/NamedResource.java @@ -122,4 +122,10 @@ public interface NamedResource { } }; } + + static int safeCompareByName(NamedResource r1, NamedResource r2, boolean caseSensitive) { + String n1 = (r1 == null) ? null : r1.getName(); + String n2 = (r2 == null) ? null : r2.getName(); + return GenericUtils.safeCompare(n1, n2, caseSensitive); + } } diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/SftpVersionSelector.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/SftpVersionSelector.java index 0099960..3dac99e 100644 --- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/SftpVersionSelector.java +++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/SftpVersionSelector.java @@ -21,9 +21,12 @@ package org.apache.sshd.sftp.client; import java.util.Collection; import java.util.List; +import java.util.Objects; +import java.util.stream.Stream; import java.util.stream.StreamSupport; import org.apache.sshd.client.session.ClientSession; +import org.apache.sshd.common.NamedResource; import org.apache.sshd.common.util.GenericUtils; import org.apache.sshd.common.util.NumberUtils; import org.apache.sshd.common.util.ValidateUtils; @@ -36,12 +39,12 @@ public interface SftpVersionSelector { /** * An {@link SftpVersionSelector} that returns the current version */ - SftpVersionSelector CURRENT = new NamedVersionSelector("CURRENT", (session, initial, current, available) -> current); + NamedVersionSelector CURRENT = new NamedVersionSelector("CURRENT", (session, initial, current, available) -> current); /** * An {@link SftpVersionSelector} that returns the maximum available version */ - SftpVersionSelector MAXIMUM = new NamedVersionSelector( + NamedVersionSelector MAXIMUM = new NamedVersionSelector( "MAXIMUM", (session, initial, current, available) -> GenericUtils.stream(available).mapToInt(Integer::intValue).max() .orElse(current)); @@ -49,7 +52,7 @@ public interface SftpVersionSelector { /** * An {@link SftpVersionSelector} that returns the minimum available version */ - SftpVersionSelector MINIMUM = new NamedVersionSelector( + NamedVersionSelector MINIMUM = new NamedVersionSelector( "MINIMUM", (session, initial, current, available) -> GenericUtils.stream(available).mapToInt(Integer::intValue).min() .orElse(current)); @@ -70,9 +73,9 @@ public interface SftpVersionSelector { * eventually thrown by the client during re-negotiation phase. * * @param version The requested version - * @return The {@link SftpVersionSelector} + * @return The {@link NamedVersionSelector} wrapping the requested version */ - static SftpVersionSelector fixedVersionSelector(int version) { + static NamedVersionSelector fixedVersionSelector(int version) { return new NamedVersionSelector(Integer.toString(version), (session, initial, current, available) -> version); } @@ -83,10 +86,10 @@ public interface SftpVersionSelector { * * @param preferred The preferred versions in decreasing order of preference (i.e., most preferred is 1st) - may * not be {@code null}/empty - * @return A {@link SftpVersionSelector} that attempts to select the most preferred version that is also + * @return A {@link NamedVersionSelector} that attempts to select the most preferred version that is also * listed as available. */ - static SftpVersionSelector preferredVersionSelector(int... preferred) { + static NamedVersionSelector preferredVersionSelector(int... preferred) { return preferredVersionSelector(NumberUtils.asList(preferred)); } @@ -96,10 +99,10 @@ public interface SftpVersionSelector { * is invoked * * @param preferred The preferred versions in decreasing order of preference (i.e., most preferred is 1st) - * @return A {@link SftpVersionSelector} that attempts to select the most preferred version that is also + * @return A {@link NamedVersionSelector} that attempts to select the most preferred version that is also * listed as available. */ - static SftpVersionSelector preferredVersionSelector(Iterable<? extends Number> preferred) { + static NamedVersionSelector preferredVersionSelector(Iterable<? extends Number> preferred) { ValidateUtils.checkNotNullAndNotEmpty((Collection<?>) preferred, "Empty preferred versions"); return new NamedVersionSelector( GenericUtils.join(preferred, ','), @@ -111,13 +114,46 @@ public interface SftpVersionSelector { "Preferred versions (" + preferred + ") not available: " + available))); } - class NamedVersionSelector implements SftpVersionSelector { + /** + * Parses the input string to see if it matches one of the "known" selectors names (case insensitive). If + * not, then checks if it is a single number and uses it as a {@link #fixedVersionSelector(int) fixed} version. + * Otherwise, assumes a comma separated list of versions in preferred order. + * + * @param selector The selector value - if {@code null}/empty then returns {@link #CURRENT} + * @return Parsed {@link NamedVersionSelector} + */ + static NamedVersionSelector resolveVersionSelector(String selector) { + if (GenericUtils.isEmpty(selector)) { + return SftpVersionSelector.CURRENT; + } else if (selector.equalsIgnoreCase(SftpVersionSelector.CURRENT.getName())) { + return SftpVersionSelector.CURRENT; + } else if (selector.equalsIgnoreCase(SftpVersionSelector.MINIMUM.getName())) { + return SftpVersionSelector.MINIMUM; + } else if (selector.equalsIgnoreCase(SftpVersionSelector.MAXIMUM.getName())) { + return SftpVersionSelector.MAXIMUM; + } else if (NumberUtils.isIntegerNumber(selector)) { + return SftpVersionSelector.fixedVersionSelector(Integer.parseInt(selector)); + } else { + String[] preferred = GenericUtils.split(selector, ','); + int[] prefs = Stream.of(preferred).mapToInt(Integer::parseInt).toArray(); + return SftpVersionSelector.preferredVersionSelector(prefs); + } + } + + /** + * Wraps a {@link SftpVersionSelector} and assigns it a name. <B>Note:</B> {@link NamedVersionSelector} are + * considered equal if they are assigned the same name - case <U>insensitive</U> + * + * @author <a href="mailto:d...@mina.apache.org">Apache MINA SSHD Project</a> + */ + class NamedVersionSelector implements SftpVersionSelector, NamedResource { + protected final SftpVersionSelector selector; + private final String name; - private final SftpVersionSelector selector; public NamedVersionSelector(String name, SftpVersionSelector selector) { - this.name = name; - this.selector = selector; + this.name = ValidateUtils.checkNotNullAndNotEmpty(name, "No name provided"); + this.selector = Objects.requireNonNull(selector, "No delegate selector provided"); } @Override @@ -126,8 +162,33 @@ public interface SftpVersionSelector { } @Override - public String toString() { + public String getName() { return name; } + + @Override + public int hashCode() { + return GenericUtils.hashCode(getName(), Boolean.TRUE); + } + + @Override + public boolean equals(Object obj) { + if (obj == null) { + return false; + } + if (obj == this) { + return true; + } + if (getClass() != obj.getClass()) { + return false; + } + + return NamedResource.safeCompareByName(this, (NamedVersionSelector) obj, false) == 0; + } + + @Override + public String toString() { + return getName(); + } } } diff --git a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpVersionResolverTest.java b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpVersionResolverTest.java new file mode 100644 index 0000000..527fc8e --- /dev/null +++ b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpVersionResolverTest.java @@ -0,0 +1,100 @@ +/* + * 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.sftp.client; + +import java.util.Collection; +import java.util.LinkedList; + +import org.apache.sshd.common.NamedResource; +import org.apache.sshd.sftp.client.SftpVersionSelector.NamedVersionSelector; +import org.apache.sshd.util.test.JUnit4ClassRunnerWithParametersFactory; +import org.apache.sshd.util.test.JUnitTestSupport; +import org.apache.sshd.util.test.NoIoTestCase; +import org.junit.Assume; +import org.junit.FixMethodOrder; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.runner.RunWith; +import org.junit.runners.MethodSorters; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; +import org.junit.runners.Parameterized.UseParametersRunnerFactory; + +/** + * @author <a href="mailto:d...@mina.apache.org">Apache MINA SSHD Project</a> + */ +@RunWith(Parameterized.class) // see https://github.com/junit-team/junit/wiki/Parameterized-tests +@UseParametersRunnerFactory(JUnit4ClassRunnerWithParametersFactory.class) +@FixMethodOrder(MethodSorters.NAME_ASCENDING) +@Category({ NoIoTestCase.class }) +public class SftpVersionResolverTest extends JUnitTestSupport { + private final NamedVersionSelector expected; + private final NamedVersionSelector actual; + + public SftpVersionResolverTest(String selector, NamedVersionSelector expected) { + this.expected = expected; + this.actual = SftpVersionSelector.resolveVersionSelector(selector); + } + + @Parameters(name = "selector={0}") + public static Collection<Object[]> parameters() { + return new LinkedList<Object[]>() { + // Not serializing it + private static final long serialVersionUID = 1L; + + { + addTestCase(null, SftpVersionSelector.CURRENT); + addTestCase("", SftpVersionSelector.CURRENT); + addTestCase(SftpVersionSelector.CURRENT); + addTestCase(SftpVersionSelector.MINIMUM); + addTestCase(SftpVersionSelector.MAXIMUM); + addTestCase(SftpVersionSelector.fixedVersionSelector(3)); + addTestCase(SftpVersionSelector.preferredVersionSelector(3, 4, 5)); + } + + private void addTestCase(NamedVersionSelector expected) { + addTestCase(expected.getName(), expected); + } + + private void addTestCase(String selector, NamedVersionSelector expected) { + add(new Object[] { selector, expected }); + } + }; + } + + @Test + public void testResolvedResult() { + assertEquals(expected, actual); + } + + @Test + public void testPreDefinedSelectorResolution() { + Assume.assumeTrue("Pre-defined selector ?", + (NamedResource.safeCompareByName(SftpVersionSelector.CURRENT, expected, false) == 0) + || (NamedResource.safeCompareByName(SftpVersionSelector.MINIMUM, expected, false) == 0) + || (NamedResource.safeCompareByName(SftpVersionSelector.MAXIMUM, expected, false) == 0)); + assertSame(expected, actual); + } + + @Override + public String toString() { + return getClass().getSimpleName() + "[expected=" + expected + ", actual=" + actual + "]"; + } +}