From deff0971703a5f240ccfc93db94cf5d4b0ad3e17 Mon Sep 17 00:00:00 2001 From: Jeroen van Erp Date: Thu, 2 Aug 2018 13:11:09 +0200 Subject: [PATCH] Fix SFTPClient.mkdirs to not inadvertently prefix with '/' (#415) --- .../net/schmizz/sshj/sftp/PathComponents.java | 13 +++++-- .../net/schmizz/sshj/sftp/PathHelper.java | 20 +++++++---- .../sshj/sftp/SFTPClientSpec.groovy | 35 +++++++++++++++++++ .../channel/direct/CommandTest.java | 7 +++- .../net/schmizz/sshj/sftp/PathHelperTest.java | 13 +++++-- .../net/schmizz/sshj/sftp/SFTPClientTest.java | 10 ++++-- 6 files changed, 84 insertions(+), 14 deletions(-) diff --git a/src/main/java/net/schmizz/sshj/sftp/PathComponents.java b/src/main/java/net/schmizz/sshj/sftp/PathComponents.java index 37ad0bedf..6774c602e 100644 --- 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 index b40fcd11b..e8a01fda1 100644 --- a/src/main/java/net/schmizz/sshj/sftp/PathHelper.java +++ b/src/main/java/net/schmizz/sshj/sftp/PathHelper.java @@ -70,16 +70,25 @@ 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)); @@ -87,5 +96,4 @@ public PathComponents getComponents(final String path) return getComponents(parent, name); } } - } \ No newline at end of file diff --git a/src/test/groovy/com/hierynomus/sshj/sftp/SFTPClientSpec.groovy b/src/test/groovy/com/hierynomus/sshj/sftp/SFTPClientSpec.groovy index 09764c1b4..1fd48615b 100644 --- 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 index bdd119475..3c62c9a1a 100644 --- 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 index 5c4f129fa..957ad099e 100644 --- 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,7 +67,7 @@ public void dotDot() throws IOException { final PathComponents comp = helper.getComponents(".."); assertEquals("home", comp.getName()); - assertEquals("", comp.getParent()); + assertEquals("/", comp.getParent()); } @Test @@ -75,9 +75,18 @@ 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 index bf2c25df4..61888ddf1 100644 --- 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); }