Repository: mina-sshd Updated Branches: refs/heads/master 8b9024e4d -> 622889259
[SSHD-544] Take into account target file system separator when resolving "local" paths Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/62288925 Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/62288925 Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/62288925 Branch: refs/heads/master Commit: 62288925936ce7f0e6d1744e92af2d53a54db3f6 Parents: 8b9024e Author: Lyor Goldstein <[email protected]> Authored: Mon Jul 27 10:39:33 2015 +0300 Committer: Lyor Goldstein <[email protected]> Committed: Mon Jul 27 10:39:33 2015 +0300 ---------------------------------------------------------------------- .../sshd/common/file/util/BaseFileSystem.java | 4 +- .../apache/sshd/common/file/util/BasePath.java | 26 +++--- .../org/apache/sshd/common/scp/ScpHelper.java | 8 +- .../apache/sshd/common/util/SelectorUtils.java | 88 ++++++++++++++++++++ .../server/subsystem/sftp/SftpSubsystem.java | 7 +- .../subsystem/sftp/SftpFileSystemTest.java | 4 +- .../sshd/client/subsystem/sftp/SftpTest.java | 2 +- .../sshd/common/util/SelectorUtilsTest.java | 59 +++++++++---- 8 files changed, 157 insertions(+), 41 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/62288925/sshd-core/src/main/java/org/apache/sshd/common/file/util/BaseFileSystem.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/common/file/util/BaseFileSystem.java b/sshd-core/src/main/java/org/apache/sshd/common/file/util/BaseFileSystem.java index 386df6f..4c4a0df 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/file/util/BaseFileSystem.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/file/util/BaseFileSystem.java @@ -101,7 +101,7 @@ public abstract class BaseFileSystem<T extends Path> extends FileSystem { return create(root, names); } - private void appendDedupSep(StringBuilder sb, String s) { + protected void appendDedupSep(StringBuilder sb, CharSequence s) { for (int i = 0; i < s.length(); i++) { char ch = s.charAt(i); if ((ch != '/') || (sb.length() == 0) || (sb.charAt(sb.length() - 1) != '/')) { @@ -140,7 +140,7 @@ public abstract class BaseFileSystem<T extends Path> extends FileSystem { }; } - private String globToRegex(String pattern) { + protected String globToRegex(String pattern) { StringBuilder sb = new StringBuilder(pattern.length()); int inGroup = 0; int inClass = 0; http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/62288925/sshd-core/src/main/java/org/apache/sshd/common/file/util/BasePath.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/common/file/util/BasePath.java b/sshd-core/src/main/java/org/apache/sshd/common/file/util/BasePath.java index 3a795da..a7e6dad 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/file/util/BasePath.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/file/util/BasePath.java @@ -42,12 +42,12 @@ import org.apache.sshd.common.util.ValidateUtils; public abstract class BasePath<T extends BasePath<T, FS>, FS extends BaseFileSystem<T>> implements Path { - protected final FS fileSystem; protected final String root; protected final ImmutableList<String> names; + private final FS fileSystem; public BasePath(FS fileSystem, String root, ImmutableList<String> names) { - this.fileSystem = fileSystem; + this.fileSystem = ValidateUtils.checkNotNull(fileSystem, "No file system provided"); this.root = root; this.names = names; } @@ -126,7 +126,7 @@ public abstract class BasePath<T extends BasePath<T, FS>, FS extends BaseFileSys return create(null, names.subList(beginIndex, endIndex)); } - private static boolean startsWith(List<?> list, List<?> other) { + protected boolean startsWith(List<?> list, List<?> other) { return list.size() >= other.size() && list.subList(0, other.size()).equals(other); } @@ -144,7 +144,7 @@ public abstract class BasePath<T extends BasePath<T, FS>, FS extends BaseFileSys return startsWith(getFileSystem().getPath(other)); } - private static boolean endsWith(List<?> list, List<?> other) { + protected boolean endsWith(List<?> list, List<?> other) { return other.size() <= list.size() && list.subList(list.size() - other.size(), list.size()).equals(other); } @@ -163,7 +163,7 @@ public abstract class BasePath<T extends BasePath<T, FS>, FS extends BaseFileSys return endsWith(getFileSystem().getPath(other)); } - private boolean isNormal() { + protected boolean isNormal() { int count = getNameCount(); if ((count == 0) || ((count == 1) && !isAbsolute())) { return true; @@ -346,7 +346,7 @@ public abstract class BasePath<T extends BasePath<T, FS>, FS extends BaseFileSys return p1.names.size() - p2.names.size(); } - private int compare(String s1, String s2) { + protected int compare(String s1, String s2) { if (s1 == null) { return s2 == null ? 0 : -1; } else { @@ -355,7 +355,7 @@ public abstract class BasePath<T extends BasePath<T, FS>, FS extends BaseFileSys } @SuppressWarnings("unchecked") - private T checkPath(Path paramPath) { + protected T checkPath(Path paramPath) { ValidateUtils.checkNotNull(paramPath, "Missing path argument"); if (paramPath.getClass() != getClass()) { throw new ProviderMismatchException("Path is not of this class: " + paramPath + "[" + paramPath.getClass().getSimpleName() + "]"); @@ -371,11 +371,11 @@ public abstract class BasePath<T extends BasePath<T, FS>, FS extends BaseFileSys @Override public int hashCode() { - int hash = getFileSystem().hashCode(); + int hash = Objects.hashCode(getFileSystem()); // use hash codes from toString() form of names - hash = 31 * hash + (root == null ? 0 : root.hashCode()); + hash = 31 * hash + Objects.hashCode(root); for (String name : names) { - hash = 31 * hash + name.hashCode(); + hash = 31 * hash + Objects.hashCode(name); } return hash; } @@ -392,9 +392,11 @@ public abstract class BasePath<T extends BasePath<T, FS>, FS extends BaseFileSys if (root != null) { sb.append(root); } + + String separator = getFileSystem().getSeparator(); for (String name : names) { - if (sb.length() > 0 && sb.charAt(sb.length() - 1) != '/') { - sb.append(fileSystem.getSeparator()); + if ((sb.length() > 0) && (sb.charAt(sb.length() - 1) != '/')) { + sb.append(separator); } sb.append(name); } http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/62288925/sshd-core/src/main/java/org/apache/sshd/common/scp/ScpHelper.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/common/scp/ScpHelper.java b/sshd-core/src/main/java/org/apache/sshd/common/scp/ScpHelper.java index 0829d69..276f5ed 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/scp/ScpHelper.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/scp/ScpHelper.java @@ -482,16 +482,14 @@ public class ScpHelper extends AbstractLoggingBean { } /** - * @param commandPath The original command path using <U>local</U> separator + * @param commandPath The command path using the <U>local</U> file separator * @return The resolved absolute and normalized local path {@link Path} * @throws IOException If failed to resolve the path * @throws InvalidPathException If invalid local path value */ public Path resolveLocalPath(String commandPath) throws IOException, InvalidPathException { - // In case double slashes and other patterns are used - String path = SelectorUtils.applySlashifyRules(commandPath, File.separatorChar); - String localPath = SelectorUtils.translateToLocalPath(path); - Path lcl = fileSystem.getPath(localPath); + String path = SelectorUtils.translateToLocalFileSystemPath(commandPath, File.separatorChar, fileSystem); + Path lcl = fileSystem.getPath(path); Path abs = lcl.isAbsolute() ? lcl : lcl.toAbsolutePath(); Path p = abs.normalize(); if (log.isTraceEnabled()) { http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/62288925/sshd-core/src/main/java/org/apache/sshd/common/util/SelectorUtils.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/common/util/SelectorUtils.java b/sshd-core/src/main/java/org/apache/sshd/common/util/SelectorUtils.java index 00af8b2..e7319c8 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/util/SelectorUtils.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/util/SelectorUtils.java @@ -20,8 +20,10 @@ package org.apache.sshd.common.util; import java.io.File; import java.io.IOException; +import java.nio.file.FileSystem; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import java.util.StringTokenizer; /** @@ -593,6 +595,42 @@ public final class SelectorUtils { } /** + * Converts a path to one matching the target file system by applying the + * "slashification" rules, converting it to a local path and + * then translating its separator to the target file system one (if different + * than local one) + * @param path The input path + * @param pathSeparator The separator used to build the input path + * @param fs The target {@link FileSystem} - may not be {@code null} + * @return The transformed path + * @see #translateToLocalFileSystemPath(String, char, String) + */ + public static String translateToLocalFileSystemPath(String path, char pathSeparator, FileSystem fs) { + return translateToLocalFileSystemPath(path, pathSeparator, ValidateUtils.checkNotNull(fs, "No target file system").getSeparator()); + } + + /** + * Converts a path to one matching the target file system by applying the + * "slashification" rules, converting it to a local path and + * then translating its separator to the target file system one (if different + * than local one) + * @param path The input path + * @param pathSeparator The separator used to build the input path + * @param fsSeparator The target file system separator + * @return The transformed path + * @see #applySlashifyRules(String, char) + * @see #translateToLocalPath(String) + * @see #translateToFileSystemPath(String, String, String) + */ + public static String translateToLocalFileSystemPath(String path, char pathSeparator, String fsSeparator) { + // In case double slashes and other patterns are used + String slashified = applySlashifyRules(path, pathSeparator); + // In case we are running on Windows + String localPath = translateToLocalPath(slashified); + return translateToFileSystemPath(localPath, File.separator, fsSeparator); + } + + /** * Applies the "slashification" rules as specified in * <A HREF="http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap03.html#tag_03_266";>Single Unix Specification version 3, section 3.266</A> * and <A HREF="http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap04.html#tag_04_11";>section 4.11 - Pathname resolution</A> @@ -727,6 +765,56 @@ public final class SelectorUtils { } /** + * Converts a path containing a specific separator to one using the + * specified file-system one + * @param path The input path - ignored if {@code null}/empty + * @param pathSeparator The separator used to build the input path - may not + * be {@code null}/empty + * @param fs The target {@link FileSystem} - may not be {@code null} + * @return The path where the separator used to build it is replaced by + * the file-system one (if different) + * @see FileSystem#getSeparator() + * @see #translateToFileSystemPath(String, String, String) + */ + public static String translateToFileSystemPath(String path, String pathSeparator, FileSystem fs) { + return translateToFileSystemPath(path, pathSeparator, ValidateUtils.checkNotNull(fs, "No target file system").getSeparator()); + } + + /** + * Converts a path containing a specific separator to one using the + * specified file-system one + * @param path The input path - ignored if {@code null}/empty + * @param pathSeparator The separator used to build the input path - may not + * be {@code null}/empty + * @param fsSeparator The target file system separator - may not be {@code null}/empty + * @return The path where the separator used to build it is replaced by + * the file-system one (if different) + * @throws IllegalArgumentException if path or file-system separator are {@code null}/empty + * or if the separators are different and the path contains the target + * file-system separator as it would create an ambiguity + */ + public static String translateToFileSystemPath(String path, String pathSeparator, String fsSeparator) { + ValidateUtils.checkNotNullAndNotEmpty(pathSeparator, "Missing path separator"); + ValidateUtils.checkNotNullAndNotEmpty(fsSeparator, "Missing file-system separator"); + + if (GenericUtils.isEmpty(path) || Objects.equals(pathSeparator, fsSeparator)) { + return path; + } + + // make sure path does not contain the target separator + if (path.indexOf(fsSeparator) >= 0) { + ValidateUtils.throwIllegalArgumentException("File system replacement may yield ambiguous result for %s with separator=%s", path, fsSeparator); + } + + // check most likely case + if ((pathSeparator.length() == 1) && (fsSeparator.length() == 1)) { + return path.replace(pathSeparator.charAt(0), fsSeparator.charAt(0)); + } else { + return path.replace(pathSeparator, fsSeparator); + } + } + + /** * Returns dependency information on these two files. If src has been * modified later than target, it returns true. If target doesn't exist, * it likewise returns true. Otherwise, target is newer than src and http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/62288925/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 1160d57..54acc9a 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 @@ -2932,11 +2932,8 @@ public class SftpSubsystem extends AbstractLoggingBean implements Command, Runna * @throws InvalidPathException If bad local path specification */ protected Path resolveFile(String remotePath) throws IOException, InvalidPathException { - // In case double slashes and other patterns are used - String path = SelectorUtils.applySlashifyRules(remotePath, '/'); - // In case we are running on Windows - String localPath = SelectorUtils.translateToLocalPath(path); - Path p = defaultDir.resolve(localPath); + String path = SelectorUtils.translateToLocalFileSystemPath(remotePath, '/', defaultDir.getFileSystem()); + Path p = defaultDir.resolve(path); if (log.isTraceEnabled()) { log.trace("resolveFile({}) {}", remotePath, p); } http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/62288925/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 1623d98..7578df5 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 @@ -374,7 +374,7 @@ public class SftpFileSystemTest extends BaseTestSupport { Files.move(file2, file1); Map<String, Object> attrs = Files.readAttributes(file1, "*"); - System.out.append(file1.toString()).append(" attributes: ").println(attrs); + System.out.append('\t').append(file1.toString()).append(" attributes: ").println(attrs); // TODO: symbolic links only work for absolute files // Path link = fs.getPath("target/sftp/client/test2.txt"); @@ -396,7 +396,7 @@ public class SftpFileSystemTest extends BaseTestSupport { } attrs = Files.readAttributes(file1, "*", LinkOption.NOFOLLOW_LINKS); - System.out.append(file1.toString()).append(" no-follow attributes: ").println(attrs); + System.out.append('\t').append(file1.toString()).append(" no-follow attributes: ").println(attrs); assertEquals("Mismatched symlink data", expected, new String(Files.readAllBytes(file1))); http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/62288925/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java b/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java index e7af9ee..c5fb7c4 100644 --- a/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java @@ -537,7 +537,7 @@ public class SftpTest extends AbstractSftpClientTestSupport { path = path.replace('\\', '/'); Vector<?> res = c.ls(path); for (Object f : res) { - System.out.println(f.toString()); + System.out.append('\t').println(f.toString()); } } finally { c.disconnect(); http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/62288925/sshd-core/src/test/java/org/apache/sshd/common/util/SelectorUtilsTest.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/test/java/org/apache/sshd/common/util/SelectorUtilsTest.java b/sshd-core/src/test/java/org/apache/sshd/common/util/SelectorUtilsTest.java index 82d6367..4b557f5 100644 --- a/sshd-core/src/test/java/org/apache/sshd/common/util/SelectorUtilsTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/common/util/SelectorUtilsTest.java @@ -18,6 +18,7 @@ */ package org.apache.sshd.common.util; +import java.io.File; import java.util.Random; import org.apache.sshd.util.BaseTestSupport; @@ -35,12 +36,23 @@ public class SelectorUtilsTest extends BaseTestSupport { } @Test - public void testApplySlashifyRules() { + public void testApplyLinuxSeparatorSlashifyRules() { + testApplySlashifyRules('/'); + } + + @Test + public void testApplyWindowsSeparatorSlashifyRules() { + testApplySlashifyRules('\\'); + } + + private void testApplySlashifyRules(char slash) { for (String expected : new String[] { - null, "", getCurrentTestName(), getClass().getSimpleName() + "/" + getCurrentTestName(), - "/" + getClass().getSimpleName(), "/" + getClass().getSimpleName() + "/" + getCurrentTestName() + null, "", getCurrentTestName(), + getClass().getSimpleName() + String.valueOf(slash) + getCurrentTestName(), + String.valueOf(slash) + getClass().getSimpleName(), + String.valueOf(slash) + getClass().getSimpleName() + String.valueOf(slash) + getCurrentTestName() }) { - String actual = SelectorUtils.applySlashifyRules(expected, '/'); + String actual = SelectorUtils.applySlashifyRules(expected, slash); assertSame("Mismatched results for '" + expected + "'", expected, actual); } @@ -54,49 +66,68 @@ public class SelectorUtilsTest extends BaseTestSupport { boolean prepend = rnd.nextBoolean(); if (prepend) { - slashify(sb, rnd); + slashify(sb, rnd, slash); } sb.append(comps[0]); for (int j = 1; j < comps.length; j++) { - slashify(sb, rnd); + slashify(sb, rnd, slash); sb.append(comps[j]); } boolean append = rnd.nextBoolean(); if (append) { - slashify(sb, rnd); + slashify(sb, rnd, slash); } String path = sb.toString(); sb.setLength(0); if (prepend) { - sb.append('/'); + sb.append(slash); } sb.append(comps[0]); for (int j = 1; j < comps.length; j++) { - sb.append('/').append(comps[j]); + sb.append(slash).append(comps[j]); } if (append) { - sb.append('/').append('.'); + sb.append(slash).append('.'); } String expected = sb.toString(); - String actual = SelectorUtils.applySlashifyRules(path, '/'); + String actual = SelectorUtils.applySlashifyRules(path, slash); assertEquals("Mismatched results for path=" + path, expected, actual); } } - - private static int slashify(StringBuilder sb, Random rnd) { + private static int slashify(StringBuilder sb, Random rnd, char slash) { int slashes = 1 /* at least one slash */ + rnd.nextInt(Byte.SIZE); for (int k = 0; k < slashes; k++) { - sb.append('/'); + sb.append(slash); } return slashes; } + @Test + public void testTranslateToFileSystemPath() { + String path = getClass().getPackage().getName().replace('.', File.separatorChar) + + File.separator + getClass().getSimpleName() + + File.separator + getCurrentTestName() + ; + for (String expected : new String[] { null, "", path }) { + String actual = SelectorUtils.translateToFileSystemPath(expected, File.separator, File.separator); + assertSame("Mismatched instance for translated result", expected, actual); + } + + for (String fsSeparator : new String[] { String.valueOf('.'), "##" }) { + String expected = path.replace(File.separator, fsSeparator); + String actual = SelectorUtils.translateToFileSystemPath(path, File.separator, fsSeparator); + assertEquals("Mismatched translation result for separator='" + fsSeparator + "'", expected, actual); + + actual = SelectorUtils.translateToFileSystemPath(actual, fsSeparator, File.separator); + assertEquals("Mismatched translation revert for separator='" + fsSeparator + "'", path, actual); + } + } }
