Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Aug 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/main/java/net/schmizz/sshj/sftp/PathComponents.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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() {
Expand Down
20 changes: 14 additions & 6 deletions src/main/java/net/schmizz/sshj/sftp/PathHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

}
35 changes: 35 additions & 0 deletions src/test/groovy/com/hierynomus/sshj/sftp/SFTPClientSpec.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
}
}
}
13 changes: 11 additions & 2 deletions src/test/java/net/schmizz/sshj/sftp/PathHelperTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand All @@ -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 {
Expand Down
10 changes: 8 additions & 2 deletions src/test/java/net/schmizz/sshj/sftp/SFTPClientTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down