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/07/02 10:33:07 UTC
mina-sshd git commit: [SSHD-415] Use the full URI in order to
identify SFTP file systems
Repository: mina-sshd
Updated Branches:
refs/heads/master 95e307d9c -> 178c73e41
[SSHD-415] Use the full URI in order to identify SFTP file systems
Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/178c73e4
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/178c73e4
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/178c73e4
Branch: refs/heads/master
Commit: 178c73e418097ef441de3492267d1257a9df4f2f
Parents: 95e307d
Author: Lyor Goldstein <lg...@vmware.com>
Authored: Thu Jul 2 11:32:58 2015 +0300
Committer: Lyor Goldstein <lg...@vmware.com>
Committed: Thu Jul 2 11:32:58 2015 +0300
----------------------------------------------------------------------
.../subsystem/sftp/SftpFileSystemProvider.java | 64 +++++++++++------
.../server/subsystem/sftp/SftpSubsystem.java | 19 ++++--
.../subsystem/sftp/SftpFileSystemTest.java | 72 ++++++++++++++++++--
3 files changed, 124 insertions(+), 31 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/178c73e4/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/SftpFileSystemProvider.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/SftpFileSystemProvider.java b/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/SftpFileSystemProvider.java
index 8cf981c..df0c1e8 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/SftpFileSystemProvider.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/SftpFileSystemProvider.java
@@ -138,8 +138,8 @@ public class SftpFileSystemProvider extends FileSystemProvider {
return SftpConstants.SFTP_SUBSYSTEM_NAME;
}
- @Override
- public FileSystem newFileSystem(URI uri, Map<String, ?> env) throws IOException {
+ @Override // NOTE: co-variant return
+ public SftpFileSystem newFileSystem(URI uri, Map<String, ?> env) throws IOException {
String host = ValidateUtils.checkNotNullAndNotEmpty(uri.getHost(), "Host not provided", GenericUtils.EMPTY_OBJECT_ARRAY);
int port = uri.getPort();
if (port <= 0) {
@@ -150,7 +150,7 @@ public class SftpFileSystemProvider extends FileSystemProvider {
String[] ui = GenericUtils.split(userInfo, ':');
ValidateUtils.checkTrue(GenericUtils.length(ui) == 2, "Invalid user info: %s", userInfo);
String username = ui[0], password = ui[1];
- String id = getFileSystemIdentifier(host, port, userInfo);
+ String id = getFileSystemIdentifier(host, port, username);
SftpFileSystem fileSystem;
synchronized (fileSystems) {
@@ -158,6 +158,7 @@ public class SftpFileSystemProvider extends FileSystemProvider {
throw new FileSystemAlreadyExistsException(id);
}
+ // TODO try and find a way to avoid doing this while locking the file systems cache
ClientSession session=null;
try {
session = client.connect(username, host, port)
@@ -199,18 +200,7 @@ public class SftpFileSystemProvider extends FileSystemProvider {
}
public SftpFileSystem newFileSystem(ClientSession session) throws IOException {
- IoSession ioSession = session.getIoSession();
- SocketAddress addr = ioSession.getRemoteAddress();
- String username = session.getUsername();
- String userauth = username + ":" + session.toString();
- String id;
- if (addr instanceof InetSocketAddress) {
- InetSocketAddress inetAddr = (InetSocketAddress) addr;
- id = getFileSystemIdentifier(inetAddr.getHostString(), inetAddr.getPort(), userauth);
- } else {
- id = getFileSystemIdentifier(addr.toString(), SshConfigFileReader.DEFAULT_PORT, userauth);
- }
-
+ String id = getFileSystemIdentifier(session);
SftpFileSystem fileSystem;
synchronized (fileSystems) {
if ((fileSystem=fileSystems.get(id)) != null) {
@@ -255,7 +245,7 @@ public class SftpFileSystemProvider extends FileSystemProvider {
* @param id File system identifier - ignored if {@code null}/empty
* @return The cached {@link SftpFileSystem} - {@code null} if no match
*/
- protected SftpFileSystem getFileSystem(String id) {
+ public SftpFileSystem getFileSystem(String id) {
if (GenericUtils.isEmpty(id)) {
return null;
}
@@ -980,11 +970,47 @@ public class SftpFileSystemProvider extends FileSystemProvider {
return p;
}
+ /**
+ * Uses the host, port and username to create a unique identifier
+ * @param uri The {@link URI} - <B>Note:</B> not checked to make sure
+ * that the scheme is {@code sftp://}
+ * @return The unique identifier
+ * @see #getFileSystemIdentifier(String, int, String)
+ */
public static final String getFileSystemIdentifier(URI uri) {
- return getFileSystemIdentifier(uri.getHost(), uri.getPort(), uri.getUserInfo());
+ String userInfo = ValidateUtils.checkNotNullAndNotEmpty(uri.getUserInfo(), "UserInfo not provided", GenericUtils.EMPTY_OBJECT_ARRAY);
+ String[] ui = GenericUtils.split(userInfo, ':');
+ ValidateUtils.checkTrue(GenericUtils.length(ui) == 2, "Invalid user info: %s", userInfo);
+ return getFileSystemIdentifier(uri.getHost(), uri.getPort(), ui[0]);
}
- public static final String getFileSystemIdentifier(String host, int port, String userAuth) {
- return userAuth;
+ /**
+ * Uses the remote host address, port and current username to create a unique identifier
+ * @param session The {@link ClientSession}
+ * @return The unique identifier
+ * @see #getFileSystemIdentifier(String, int, String)
+ */
+ public static final String getFileSystemIdentifier(ClientSession session) {
+ IoSession ioSession = session.getIoSession();
+ SocketAddress addr = ioSession.getRemoteAddress();
+ String username = session.getUsername();
+ if (addr instanceof InetSocketAddress) {
+ InetSocketAddress inetAddr = (InetSocketAddress) addr;
+ return getFileSystemIdentifier(inetAddr.getHostString(), inetAddr.getPort(), username);
+ } else {
+ return getFileSystemIdentifier(addr.toString(), SshConfigFileReader.DEFAULT_PORT, username);
+ }
+ }
+
+ public static final String getFileSystemIdentifier(String host, int port, String username) {
+ return new StringBuilder(GenericUtils.length(host) + 1 + /* port */ + 5 + 1 + GenericUtils.length(username))
+ .append(GenericUtils.trimToEmpty(host))
+ .append(':').append((port <= 0) ? SshConfigFileReader.DEFAULT_PORT : port)
+ .append(':').append(GenericUtils.trimToEmpty(username))
+ .toString();
+ }
+
+ public static final URI createFileSystemURI(String host, int port, String username, String password) {
+ return URI.create(SftpConstants.SFTP_SUBSYSTEM_NAME + "://" + username + ":" + password + "@" + host + ":" + port + "/");
}
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/178c73e4/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 70d729f..9788ca2 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
@@ -113,7 +113,13 @@ public class SftpSubsystem extends AbstractLoggingBean implements Command, Runna
public static final int LOWER_SFTP_IMPL = SFTP_V3; // Working implementation from v3
public static final int HIGHER_SFTP_IMPL = SFTP_V6; // .. up to
public static final String ALL_SFTP_IMPL;
- public static final int MAX_PACKET_LENGTH = 1024 * 16;
+
+ /**
+ * Force the use of a max. packet length - especially for {@link #doReadDir(Buffer, int)}
+ * @see #DEFAULT_MAX_PACKET_LENGTH
+ */
+ public static final String MAX_PACKET_LENGTH_PROP = "sftp-max-packet-length";
+ public static final int DEFAULT_MAX_PACKET_LENGTH = 1024 * 16;
static {
StringBuilder sb = new StringBuilder(2 * (1 + (HIGHER_SFTP_IMPL - LOWER_SFTP_IMPL)));
@@ -939,12 +945,13 @@ public class SftpSubsystem extends AbstractLoggingBean implements Command, Runna
return;
}
- if (((DirectoryHandle) p).isDone()) {
+ DirectoryHandle dh = (DirectoryHandle) p;
+ if (dh.isDone()) {
sendStatus(id, SSH_FX_EOF, "", "");
return;
}
- Path file = p.getFile();
+ Path file = dh.getFile();
LinkOption[] options = IoUtils.getLinkOptions(false);
Boolean status = IoUtils.checkFileExists(file, options);
if (status == null) {
@@ -958,7 +965,6 @@ public class SftpSubsystem extends AbstractLoggingBean implements Command, Runna
} else if (!Files.isReadable(file)) {
sendStatus(id, SSH_FX_PERMISSION_DENIED, file.toString());
} else {
- DirectoryHandle dh = (DirectoryHandle) p;
if (dh.hasNext()) {
// There is at least one file in the directory.
// Send only a few files at a time to not create packets of a too
@@ -1374,8 +1380,9 @@ public class SftpSubsystem extends AbstractLoggingBean implements Command, Runna
buffer.putInt(id);
int wpos = buffer.wpos();
buffer.putInt(0);
- int nb = 0;
- while (files.hasNext() && (buffer.wpos() < MAX_PACKET_LENGTH)) {
+
+ int nb = 0, maxSize = FactoryManagerUtils.getIntProperty(session, MAX_PACKET_LENGTH_PROP, DEFAULT_MAX_PACKET_LENGTH);
+ while (files.hasNext() && (buffer.wpos() < maxSize)) {
Path f = files.next();
String shortName = getShortName(f);
buffer.putString(shortName, StandardCharsets.UTF_8);
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/178c73e4/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpFileSystemTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpFileSystemTest.java b/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpFileSystemTest.java
index ff909f8..aa7054f 100644
--- a/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpFileSystemTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpFileSystemTest.java
@@ -40,16 +40,21 @@ import java.nio.file.attribute.PosixFilePermissions;
import java.nio.file.attribute.UserPrincipalLookupService;
import java.nio.file.attribute.UserPrincipalNotFoundException;
import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
+import java.util.HashSet;
+import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
+import org.apache.sshd.client.SshClient;
import org.apache.sshd.common.NamedFactory;
import org.apache.sshd.common.file.FileSystemFactory;
import org.apache.sshd.common.file.root.RootedFileSystemProvider;
import org.apache.sshd.common.session.Session;
import org.apache.sshd.common.subsystem.sftp.SftpConstants;
+import org.apache.sshd.common.util.GenericUtils;
import org.apache.sshd.common.util.OsUtils;
import org.apache.sshd.common.util.io.IoUtils;
import org.apache.sshd.server.Command;
@@ -111,8 +116,7 @@ public class SftpFileSystemTest extends BaseTestSupport {
Path lclSftp = Utils.resolve(targetPath, SftpConstants.SFTP_SUBSYSTEM_NAME, getClass().getSimpleName(), getCurrentTestName());
Utils.deleteRecursive(lclSftp);
- try(FileSystem fs = FileSystems.newFileSystem(
- URI.create("sftp://" + getCurrentTestName() + ":" + getCurrentTestName() + "@localhost:" + port + "/"),
+ try(FileSystem fs = FileSystems.newFileSystem(createDefaultFileSystemURI(),
new TreeMap<String,Object>() {
private static final long serialVersionUID = 1L; // we're not serializing it
@@ -230,8 +234,7 @@ public class SftpFileSystemTest extends BaseTestSupport {
Path lclSftp = Utils.resolve(targetPath, SftpConstants.SFTP_SUBSYSTEM_NAME, getClass().getSimpleName(), getCurrentTestName());
Utils.deleteRecursive(lclSftp);
- try(FileSystem fs = FileSystems.newFileSystem(
- URI.create("sftp://" + getCurrentTestName() + ":" + getCurrentTestName() + "@localhost:" + port + "/"),
+ try(FileSystem fs = FileSystems.newFileSystem(createDefaultFileSystemURI(),
new TreeMap<String,Object>() {
private static final long serialVersionUID = 1L; // we're not serializing it
@@ -289,8 +292,7 @@ public class SftpFileSystemTest extends BaseTestSupport {
@Test
public void testFileStore() throws IOException {
- try(FileSystem fs = FileSystems.newFileSystem(
- URI.create("sftp://" + getCurrentTestName() + ":" + getCurrentTestName() + "@localhost:" + port + "/"), Collections.<String,Object>emptyMap())) {
+ try(FileSystem fs = FileSystems.newFileSystem(createDefaultFileSystemURI(), Collections.<String,Object>emptyMap())) {
Iterable<FileStore> iter = fs.getFileStores();
assertTrue("Not a list", iter instanceof List<?>);
@@ -310,4 +312,62 @@ public class SftpFileSystemTest extends BaseTestSupport {
}
}
}
+
+ @Test
+ public void testMultipleFileStoresOnSameProvider() throws IOException {
+ try(SshClient client = SshClient.setUpDefaultClient()) {
+ client.start();
+
+ SftpFileSystemProvider provider = new SftpFileSystemProvider(client);
+ Collection<SftpFileSystem> fsList = new LinkedList<>();
+ try {
+ Collection<String> idSet = new HashSet<>();
+ for (int index=0; index < 4; index++) {
+ String credentials = getCurrentTestName() + "-user-" + index;
+ SftpFileSystem expected = provider.newFileSystem(createFileSystemURI(credentials), Collections.<String,Object>emptyMap());
+ fsList.add(expected);
+
+ String id = expected.getId();
+ assertTrue("Non unique file system id: " + id, idSet.add(id));
+
+ SftpFileSystem actual = provider.getFileSystem(id);
+ assertSame("Mismatched cached instances for " + id, expected, actual);
+ System.out.println("Created file system id: " + id);
+ }
+
+ for (SftpFileSystem fs : fsList) {
+ String id = fs.getId();
+ fs.close();
+ assertNull("File system not removed from cache: " + id, provider.getFileSystem(id));
+ }
+ } finally {
+ IOException err = null;
+ for (FileSystem fs : fsList) {
+ try {
+ fs.close();
+ } catch(IOException e) {
+ err = GenericUtils.accumulateException(err, e);
+ }
+ }
+
+ client.stop();
+
+ if (err != null) {
+ throw err;
+ }
+ }
+ }
+ }
+
+ private URI createDefaultFileSystemURI() {
+ return createFileSystemURI(getCurrentTestName());
+ }
+
+ private URI createFileSystemURI(String username) {
+ return createFileSystemURI(username, port);
+ }
+
+ private static URI createFileSystemURI(String username, int port) {
+ return SftpFileSystemProvider.createFileSystemURI("localhost", port, username, username);
+ }
}