- Move trailing slash removal from SFTPEngine.mkdirs() to PathHelper.…

…getComponents()

- Try to make the PathHelper.getComponents() code clearer
- Added some tests for PathHelper.getComponents()

src/main/java/net/schmizz/sshj/sftp/PathHelper.java

@@ -19,16 +19,23 @@
 
 public class PathHelper {
 
+    public interface Canonicalizer {
+
+        String canonicalize(String path)
+                throws IOException;
+
+    }
+
     public static final String DEFAULT_PATH_SEPARATOR = "/";
 
     private final Canonicalizer canonicalizer;
     private final String pathSep;
 
     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 PathComponents getComponents(String parent, String name) {
         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("."))
-            return getComponents(getDotDir());
+        if (path.equals(pathSep))
+            return getComponents("", "");
 
-        final int lastSlash = path.lastIndexOf(pathSep);
-
-        if (lastSlash == -1) // Relative path
-            if (path.equals(".."))
-                return getComponents(canonicalizer.canonicalize(path));
-            else
-                return getComponents(getDotDir(), path);
+        if (path.isEmpty() || path.equals(".") || path.equals("." + pathSep))
+            return getComponents(getDotDir());
 
-        final String name = path.substring(lastSlash + pathSep.length());
+        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 (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("."));
-    }
-
 }

src/main/java/net/schmizz/sshj/sftp/SFTPClient.java

@@ -90,7 +90,6 @@ public void mkdir(String dirname)
     public void mkdirs(String path)
             throws IOException {
         final Deque<String> dirsToMake = new LinkedList<String>();
-        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());

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());
+    }
+
+}