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 <[email protected]> Authored: Thu Jul 2 11:32:58 2015 +0300 Committer: Lyor Goldstein <[email protected]> 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); + } }
