[prev in list] [next in list] [prev in thread] [next in thread]
List: mina-commits
Subject: [mina-sshd] branch master updated: [SSHD-909] SFTP versions extension handler ignores non-numerical
From: lgoldstein () apache ! org
Date: 2019-03-21 16:28:49
Message-ID: 155318572992.2940.4319201190692957349 () gitbox ! apache ! org
[Download RAW message or body]
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
The following commit(s) were added to refs/heads/master by this push:
new 2fb7ebb [SSHD-909] SFTP versions extension handler ignores non-numerical \
versions when resolving the available ones 2fb7ebb is described below
commit 2fb7ebb487154283dd6dc5db88676c179ebb01e0
Author: Lyor Goldstein <lgoldstein@apache.org>
AuthorDate: Thu Mar 21 09:03:11 2019 +0200
[SSHD-909] SFTP versions extension handler ignores non-numerical versions when \
resolving the available ones
---
CHANGES.md | 1 +
.../subsystem/sftp/impl/DefaultSftpClient.java | 46 +++++++++++--------
.../subsystem/sftp/extensions/VersionsParser.java | 31 +++++++++++++
.../sftp/extensions/VersionParserTest.java | 53 ++++++++++++++++++++++
4 files changed, 112 insertions(+), 19 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 899a31d..1a97a69 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -121,3 +121,4 @@ in order to avoid client-side session timeout due to no traffic \
from server.
* [SSHD-907](https://issues.apache.org/jira/browse/SSHD-907) - `StpEventListener` \
invokes (new) `exiting` method to inform about SFTP subsystem exiting and therefore \
closing all currently tracked file/directory handles.
+* [SSHD-909](https://issues.apache.org/jira/browse/SSHD-909) - SFTP versions \
extension handler ignores non-numerical versions when resolving the \
available ones.
diff --git a/sshd-sftp/src/main/java/org/apache/sshd/client/subsystem/sftp/impl/DefaultSftpClient.java \
b/sshd-sftp/src/main/java/org/apache/sshd/client/subsystem/sftp/impl/DefaultSftpClient.java
index c765f8d..d985994 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/client/subsystem/sftp/impl/DefaultSftpClient.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/client/subsystem/sftp/impl/DefaultSftpClient.java
@@ -27,15 +27,14 @@ import java.io.OutputStream;
import java.io.StreamCorruptedException;
import java.net.SocketTimeoutException;
import java.nio.charset.Charset;
-import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
+import java.util.List;
import java.util.Map;
import java.util.NavigableMap;
import java.util.Objects;
-import java.util.Set;
import java.util.TreeMap;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -431,41 +430,50 @@ public class DefaultSftpClient extends AbstractSftpClient {
* @throws IOException If failed to negotiate
*/
public int negotiateVersion(SftpVersionSelector selector) throws IOException {
+ boolean debugEnabled = log.isDebugEnabled();
+ ClientChannel clientChannel = getClientChannel();
int current = getVersion();
if (selector == null) {
+ if (debugEnabled) {
+ log.debug("negotiateVersion({}) no selector to override current={}", \
clientChannel, current); + }
return current;
}
- Set<Integer> available = \
GenericUtils.asSortedSet(Collections.singleton(current)); Map<String, ?> parsed = \
getParsedServerExtensions();
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.getVersions();
- if (GenericUtils.size(reported) > 0) {
- for (String v : reported) {
- if (!available.add(Integer.valueOf(v))) {
- continue; // debug breakpoint
- }
- }
- }
+ List<Integer> availableVersions = Collections.emptyList();
+ if ((GenericUtils.size(extensions) > 0)
+ && extensions.contains(SftpConstants.EXT_VERSION_SELECT)) {
+ Versions vers = GenericUtils.isEmpty(parsed)
+ ? null
+ : (Versions) parsed.get(SftpConstants.EXT_VERSIONS);
+ availableVersions = (vers == null)
+ ? Collections.singletonList(current)
+ : vers.resolveAvailableVersions(current);
+ } else {
+ availableVersions = Collections.singletonList(current);
}
- int selected = selector.selectVersion(getClientSession(), current, new \
ArrayList<>(available));
- if (log.isDebugEnabled()) {
- log.debug("negotiateVersion({}) current={} {} -> {}", \
getClientChannel(), current, available, selected); + ClientSession session = \
getClientSession(); + int selected = selector.selectVersion(session, current, \
availableVersions); + if (debugEnabled) {
+ log.debug("negotiateVersion({}) current={} {} -> {}",
+ clientChannel, current, availableVersions, selected);
}
if (selected == current) {
return current;
}
- if (!available.contains(selected)) {
- throw new StreamCorruptedException("Selected version (" + selected + ") \
not part of available: " + available); + if \
(!availableVersions.contains(selected)) { + throw new \
StreamCorruptedException( + "Selected version (" + selected + ") not \
part of available: " + availableVersions); }
String verVal = String.valueOf(selected);
- Buffer buffer = new ByteArrayBuffer(Integer.BYTES + \
SftpConstants.EXT_VERSION_SELECT.length() // extension name + Buffer \
buffer = new ByteArrayBuffer( + Integer.BYTES + \
SftpConstants.EXT_VERSION_SELECT.length() // extension name + Integer.BYTES + \
verVal.length() + Byte.SIZE, false); \
buffer.putString(SftpConstants.EXT_VERSION_SELECT); buffer.putString(verVal);
diff --git a/sshd-sftp/src/main/java/org/apache/sshd/common/subsystem/sftp/extensions/VersionsParser.java \
b/sshd-sftp/src/main/java/org/apache/sshd/common/subsystem/sftp/extensions/VersionsParser.java
index 51b31f6..0489334 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/common/subsystem/sftp/extensions/VersionsParser.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/common/subsystem/sftp/extensions/VersionsParser.java
@@ -20,13 +20,17 @@
package org.apache.sshd.common.subsystem.sftp.extensions;
import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
import java.util.List;
+import java.util.Set;
import org.apache.sshd.common.subsystem.sftp.SftpConstants;
import org.apache.sshd.common.subsystem.sftp.extensions.VersionsParser.Versions;
import org.apache.sshd.common.util.GenericUtils;
+import org.apache.sshd.common.util.NumberUtils;
/**
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
@@ -59,6 +63,33 @@ public class VersionsParser extends AbstractParser<Versions> {
this.versions = versions;
}
+ public List<Integer> resolveAvailableVersions(int current) {
+ List<Integer> currentlyAvailable = Collections.singletonList(current);
+ Collection<String> reported = getVersions();
+ if (GenericUtils.isEmpty(reported)) {
+ return currentlyAvailable;
+ }
+
+ Set<Integer> available = GenericUtils.asSortedSet(currentlyAvailable);
+ for (String v : reported) {
+ /*
+ * According to \
https://tools.ietf.org/html/draft-ietf-secsh-filexfer-11#section-5.5 + \
* versions may contain not only numbers + */
+ if (!NumberUtils.isIntegerNumber(v)) {
+ continue;
+ }
+
+ if (!available.add(Integer.valueOf(v))) {
+ continue; // debug breakpoint
+ }
+ }
+
+ return (available.size() == 1)
+ ? currentlyAvailable
+ : new ArrayList<>(available);
+ }
+
@Override
public String toString() {
return GenericUtils.join(getVersions(), ',');
diff --git a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/VersionParserTest.java \
b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/VersionParserTest.java
new file mode 100644
index 0000000..9f1c2fa
--- /dev/null
+++ b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/extensions/VersionParserTest.java
@@ -0,0 +1,53 @@
+/*
+ * 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.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import org.apache.sshd.common.subsystem.sftp.extensions.VersionsParser.Versions;
+import org.apache.sshd.util.test.JUnitTestSupport;
+import org.junit.FixMethodOrder;
+import org.junit.Test;
+import org.junit.runners.MethodSorters;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+@FixMethodOrder(MethodSorters.NAME_ASCENDING)
+public class VersionParserTest extends JUnitTestSupport {
+ public VersionParserTest() {
+ super();
+ }
+
+ @Test // see SSHD-909
+ public void testIgnoreNonNumbersWhenResolvingAvailableVersions() {
+ List<Integer> expected = Arrays.asList(3, 4, 5, 6);
+ List<String> values = expected.stream()
+ .map(Object::toString)
+ .collect(Collectors.toList());
+ values.addAll(Arrays.asList(
+ "draft-ietf-secsh-filexfer-11@vandyke.com", "partial-v6@vandyke.com"));
+ Versions v = new Versions(values);
+ List<Integer> actual = v.resolveAvailableVersions(3);
+ assertListEquals(getCurrentTestName(), expected, actual);
+ }
+}
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic