From d65df3c9bc6b3eb3b966bfa3dac5bfa280137457 Mon Sep 17 00:00:00 2001 From: Shikhar Bhushan Date: Sat, 12 May 2012 13:51:05 +0100 Subject: [PATCH] - Move trailing slash removal from SFTPEngine.mkdirs() to PathHelper.getComponents() - Try to make the PathHelper.getComponents() code clearer - Added some tests for PathHelper.getComponents() --- .../net/schmizz/sshj/sftp/PathHelper.java | 52 +++++++------ .../net/schmizz/sshj/sftp/SFTPClient.java | 1 - .../net/schmizz/sshj/sftp/PathHelperTest.java | 74 +++++++++++++++++++ 3 files changed, 104 insertions(+), 23 deletions(-) create mode 100644 src/test/java/net/schmizz/sshj/sftp/PathHelperTest.java diff --git a/src/main/java/net/schmizz/sshj/sftp/PathHelper.java b/src/main/java/net/schmizz/sshj/sftp/PathHelper.java index c977f952..598ab581 100644 --- a/src/main/java/net/schmizz/sshj/sftp/PathHelper.java +++ b/src/main/java/net/schmizz/sshj/sftp/PathHelper.java @@ -19,6 +19,13 @@ import java.io.IOException; public class PathHelper { + public interface Canonicalizer { + + String canonicalize(String path) + throws IOException; + + } + public static final String DEFAULT_PATH_SEPARATOR = "/"; private final Canonicalizer canonicalizer; @@ -26,9 +33,9 @@ public class PathHelper { private String dotDir; - public interface Canonicalizer { - String canonicalize(String path) - throws IOException; + private synchronized String getDotDir() // cached + throws IOException { + return (dotDir != null) ? dotDir : (dotDir = canonicalizer.canonicalize(".")); } public PathHelper(Canonicalizer canonicalizer, String pathSep) { @@ -52,32 +59,33 @@ public class PathHelper { return new PathComponents(parent, name, pathSep); } - public PathComponents getComponents(String path) + /** + * Divide the path into {@code PathComponents(parent, name)} while making sure {@code name != "." && name != ".."} + * + * @param path to convert + * + * @return PathComponents + * + * @throws IOException + */ + public PathComponents getComponents(final String path) throws IOException { - if (path.isEmpty() || path.equals(".")) + if (path.equals(pathSep)) + return getComponents("", ""); + + if (path.isEmpty() || path.equals(".") || path.equals("." + pathSep)) return getComponents(getDotDir()); - final int lastSlash = path.lastIndexOf(pathSep); + final String withoutTrailSep = trimTrailingSeparator(path); + final int lastSep = withoutTrailSep.lastIndexOf(pathSep); + final String parent = (lastSep == -1) ? "" : withoutTrailSep.substring(0, lastSep); + final String name = (lastSep == -1) ? withoutTrailSep : withoutTrailSep.substring(lastSep + pathSep.length()); - if (lastSlash == -1) // Relative path - if (path.equals("..")) - return getComponents(canonicalizer.canonicalize(path)); - else - return getComponents(getDotDir(), path); - - final String name = path.substring(lastSlash + pathSep.length()); - - if (name.equals(".") || name.equals("..")) + if (name.equals(".") || name.equals("..")) { return getComponents(canonicalizer.canonicalize(path)); - else { - final String parent = path.substring(0, lastSlash); + } else { return getComponents(parent, name); } } - private synchronized String getDotDir() - throws IOException { - return (dotDir != null) ? dotDir : (dotDir = canonicalizer.canonicalize(".")); - } - } \ No newline at end of file diff --git a/src/main/java/net/schmizz/sshj/sftp/SFTPClient.java b/src/main/java/net/schmizz/sshj/sftp/SFTPClient.java index 64dfd9cb..f0aad64d 100644 --- a/src/main/java/net/schmizz/sshj/sftp/SFTPClient.java +++ b/src/main/java/net/schmizz/sshj/sftp/SFTPClient.java @@ -90,7 +90,6 @@ public class SFTPClient public void mkdirs(String path) throws IOException { final Deque dirsToMake = new LinkedList(); - path = PathComponents.trimTrailingSeparator(path, engine.getPathHelper().getPathSeparator()); for (PathComponents current = engine.getPathHelper().getComponents(path); ; current = engine.getPathHelper().getComponents(current.getParent())) { final FileAttributes attrs = statExistence(current.getPath()); diff --git a/src/test/java/net/schmizz/sshj/sftp/PathHelperTest.java b/src/test/java/net/schmizz/sshj/sftp/PathHelperTest.java new file mode 100644 index 00000000..a6256e97 --- /dev/null +++ b/src/test/java/net/schmizz/sshj/sftp/PathHelperTest.java @@ -0,0 +1,74 @@ +package net.schmizz.sshj.sftp; + +import org.junit.Test; + +import java.io.IOException; + +import static junit.framework.Assert.assertEquals; + +public class PathHelperTest { + + private final PathHelper helper = new PathHelper(new PathHelper.Canonicalizer() { + /** + * Very basic, it does not try to canonicalize relative bits in the middle of a path. + */ + @Override + public String canonicalize(String path) + throws IOException { + if (path.equals("") || path.equals(".") || path.equals("./")) + return "/home/me"; + if (path.equals("..") || path.equals("../")) + return "/home"; + return path; + } + }, "/"); + + @Test + public void empty() + throws IOException { + final PathComponents comp = helper.getComponents(""); + assertEquals("me", comp.getName()); + assertEquals("/home", comp.getParent()); + } + + @Test + public void root() + throws IOException { + final PathComponents comp = helper.getComponents("/"); + assertEquals("", comp.getName()); + assertEquals("", comp.getParent()); + } + + @Test + public void dot() + throws IOException { + final PathComponents comp = helper.getComponents("."); + assertEquals("me", comp.getName()); + assertEquals("/home", comp.getParent()); + } + + @Test + public void dotDot() + throws IOException { + final PathComponents comp = helper.getComponents(".."); + assertEquals("home", comp.getName()); + assertEquals("", comp.getParent()); + } + + @Test + public void fileInHomeDir() + throws IOException { + final PathComponents comp = helper.getComponents("somefile"); + assertEquals("somefile", comp.getName()); + assertEquals("", comp.getParent()); + } + + @Test + public void fileSomeLevelsDeep() + throws IOException { + final PathComponents comp = helper.getComponents("/home/me/../somedir/somefile"); + assertEquals("somefile", comp.getName()); + assertEquals("/home/me/../somedir", comp.getParent()); + } + +}