Fix SFTPClient.mkdirs to not inadvertently prefix with '/' (#415)

hierynomus · Aug 2, 2018 · deff097 · deff097
1 parent 7556a7f
commit deff097
Show file tree

Hide file tree

Showing 6 changed files with 84 additions and 14 deletions.
diff --git a/src/main/java/net/schmizz/sshj/sftp/PathComponents.java b/src/main/java/net/schmizz/sshj/sftp/PathComponents.java
@@ -18,8 +18,14 @@
 public class PathComponents {
 
     static String adjustForParent(String parent, String path, String pathSep) {
-        return (path.startsWith(pathSep)) ? path // Absolute path, nothing to adjust
-                : (parent + (parent.endsWith(pathSep) ? "" : pathSep) + path); // Relative path
+        if (path.startsWith(pathSep)) {
+            return path; // Absolute path, nothing to adjust
+        } else if (parent.endsWith(pathSep)) {
+            return parent + path; // Relative path, parent endsWith '/'
+        } else if (parent.isEmpty()) {
+            return path;
+        }
+        return parent + pathSep + path; // Relative path
     }
 
     static String trimTrailingSeparator(String somePath, String pathSep) {
@@ -33,7 +39,8 @@ static String trimTrailingSeparator(String somePath, String pathSep) {
     public PathComponents(String parent, String name, String pathSep) {
         this.parent = parent;
         this.name = name;
-        this.path = trimTrailingSeparator(adjustForParent(parent, name, pathSep), pathSep);
+        String adjusted = adjustForParent(parent, name, pathSep);
+        this.path = !pathSep.equals(adjusted) ? trimTrailingSeparator(adjusted, pathSep) : adjusted;
     }
 
     public String getParent() {

diff --git a/src/main/java/net/schmizz/sshj/sftp/PathHelper.java b/src/main/java/net/schmizz/sshj/sftp/PathHelper.java
@@ -70,22 +70,30 @@ public PathComponents getComponents(String parent, String name) {
      */
     public PathComponents getComponents(final String path)
             throws IOException {
-        if (path.equals(pathSep))
-            return getComponents("", "");
+        if (path.equals(pathSep)) {
+            return getComponents("", "/");
+        }
 
-        if (path.isEmpty() || ".".equals(path) || ("." + pathSep).equals(path))
+        if (path.isEmpty() || ".".equals(path) || ("." + pathSep).equals(path)) {
             return getComponents(getDotDir());
+        }
 
         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());
+        String parent;
+        String name;
+        if (lastSep == -1) {
+            parent = "";
+            name = withoutTrailSep;
+        } else {
+            parent = lastSep == 0 ? "/" : withoutTrailSep.substring(0, lastSep);
+            name = withoutTrailSep.substring(lastSep + pathSep.length());
+        }
 
         if (".".equals(name) || "..".equals(name)) {
             return getComponents(canonicalizer.canonicalize(path));
         } else {
             return getComponents(parent, name);
         }
     }
-
 }
diff --git a/src/test/groovy/com/hierynomus/sshj/sftp/SFTPClientSpec.groovy b/src/test/groovy/com/hierynomus/sshj/sftp/SFTPClientSpec.groovy
@@ -18,6 +18,7 @@ package com.hierynomus.sshj.sftp
 import com.hierynomus.sshj.test.SshFixture
 import com.hierynomus.sshj.test.util.FileUtil
 import net.schmizz.sshj.SSHClient
+import net.schmizz.sshj.sftp.FileMode
 import net.schmizz.sshj.sftp.SFTPClient
 import org.junit.Rule
 import org.junit.rules.TemporaryFolder
@@ -167,6 +168,40 @@ class SFTPClientSpec extends Specification {
         !new File(dest, "totototo.txt").exists()
     }
 
+    def "should mkdirs with existing parent path"() {
+        given:
+        SSHClient sshClient = fixture.setupConnectedDefaultClient()
+        sshClient.authPassword("test", "test")
+        SFTPClient ftp = sshClient.newSFTPClient()
+        ftp.mkdir("dir1")
+
+        when:
+        ftp.mkdirs("dir1/dir2/dir3/dir4")
+
+        then:
+        ftp.statExistence("dir1/dir2/dir3/dir4") != null
+
+        cleanup:
+        ["dir1/dir2/dir3/dir4", "dir1/dir2/dir3", "dir1/dir2", "dir1"].each {
+            ftp.rmdir(it)
+        }
+        ftp.close()
+        sshClient.disconnect()
+    }
+
+    def "should stat root"() {
+        given:
+        SSHClient sshClient = fixture.setupConnectedDefaultClient()
+        sshClient.authPassword("test", "test")
+        SFTPClient ftp = sshClient.newSFTPClient()
+
+        when:
+        def attrs = ftp.statExistence("/")
+
+        then:
+        attrs.type == FileMode.Type.DIRECTORY
+    }
+
     private void doUpload(File src, File dest) throws IOException {
         SSHClient sshClient = fixture.setupConnectedDefaultClient()
         sshClient.authPassword("test", "test")

diff --git a/src/test/java/com/hierynomus/sshj/connection/channel/direct/CommandTest.java b/src/test/java/com/hierynomus/sshj/connection/channel/direct/CommandTest.java
@@ -18,6 +18,7 @@
 import com.hierynomus.sshj.test.SshFixture;
 import net.schmizz.sshj.SSHClient;
 import net.schmizz.sshj.connection.channel.direct.Session;
+import net.schmizz.sshj.sftp.SFTPClient;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -46,6 +47,10 @@ public void shouldExecuteBackgroundCommand() throws IOException {
         exec.join();
         assertThat("File should exist", file.exists());
         assertThat("File should be directory", file.isDirectory());
-
+        SFTPClient sftpClient = sshClient.newSFTPClient();
+        if (sftpClient.statExistence("&") != null) {
+            sftpClient.rmdir("&");
+            // TODO fail here when this is fixed
+        }
     }
 }
diff --git a/src/test/java/net/schmizz/sshj/sftp/PathHelperTest.java b/src/test/java/net/schmizz/sshj/sftp/PathHelperTest.java
@@ -50,7 +50,7 @@ public void empty()
     public void root()
             throws IOException {
         final PathComponents comp = helper.getComponents("/");
-        assertEquals("", comp.getName());
+        assertEquals("/", comp.getName());
         assertEquals("", comp.getParent());
     }
 
@@ -67,17 +67,26 @@ public void dotDot()
             throws IOException {
         final PathComponents comp = helper.getComponents("..");
         assertEquals("home", comp.getName());
-        assertEquals("", comp.getParent());
+        assertEquals("/", comp.getParent());
     }
 
     @Test
     public void fileInHomeDir()
             throws IOException {
         final PathComponents comp = helper.getComponents("somefile");
         assertEquals("somefile", comp.getName());
+        assertEquals("somefile", comp.getPath());
         assertEquals("", comp.getParent());
     }
 
+    @Test
+    public void pathInHomeDir() throws IOException {
+        final PathComponents comp = helper.getComponents("dir1/dir2");
+        assertEquals("dir2", comp.getName());
+        assertEquals("dir1/dir2", comp.getPath());
+        assertEquals("dir1", comp.getParent());
+    }
+
     @Test
     public void fileSomeLevelsDeep()
             throws IOException {

diff --git a/src/test/java/net/schmizz/sshj/sftp/SFTPClientTest.java b/src/test/java/net/schmizz/sshj/sftp/SFTPClientTest.java
@@ -30,15 +30,21 @@ public class SFTPClientTest {
     @Before
     public void setPathHelper() throws Exception {
         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("."))
-                    return "/workingdirectory";
+                if ("".equals(path) || ".".equals(path) || "./".equals(path))
+                    return "/home/me";
+                if ("..".equals(path) || "../".equals(path))
+                    return "/home";
                 return path;
             }
         }, DEFAULT_PATH_SEPARATOR);
         when(sftpEngine.getPathHelper()).thenReturn(helper);
+        when(sftpEngine.stat("/")).thenReturn(new FileAttributes.Builder().withType(FileMode.Type.DIRECTORY).build());
         when(sftpEngine.getLoggerFactory()).thenReturn(LoggerFactory.DEFAULT);
     }