Ensure that same name subdirs aren't omitted when doing SFTP (#253)

* Updated version string in config

* Fixed #252 with backwards compatible behaviour

* Fixed logic and expanded test
This commit is contained in:
Jeroen van Erp
2016-07-05 12:21:51 +02:00
committed by GitHub
parent 3229584a95
commit 1ab72b7eaf
5 changed files with 317 additions and 96 deletions

View File

@@ -1,10 +1,11 @@
plugins {
id "java"
id "groovy"
id "maven"
id "idea"
id "signing"
id "osgi"
id "org.ajoberstar.release-opinion" version "1.4.0-rc.1"
id "org.ajoberstar.release-opinion" version "1.4.2"
id "com.github.hierynomus.license" version "0.12.1"
}
@@ -72,6 +73,7 @@ dependencies {
compile "net.vrallev.ecc:ecc-25519-java:1.0.1"
testCompile "junit:junit:4.11"
testCompile 'org.spockframework:spock-core:1.0-groovy-2.4'
testCompile "org.mockito:mockito-core:1.9.5"
testCompile "org.apache.sshd:sshd-core:1.1.0"
testRuntime "ch.qos.logback:logback-classic:1.1.2"
@@ -204,4 +206,5 @@ uploadArchives {
}
}
tasks.compileGroovy.onlyIf { false }
tasks.release.dependsOn 'build', 'uploadArchives'

View File

@@ -84,7 +84,7 @@ public class DefaultConfig
private final Logger log = LoggerFactory.getLogger(getClass());
private static final String VERSION = "SSHJ_0_14_0";
private static final String VERSION = "SSHJ_0_16_0";
public DefaultConfig() {
setVersion(VERSION);

View File

@@ -29,6 +29,7 @@ import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.util.EnumSet;
import java.util.List;
public class SFTPFileTransfer
extends AbstractFileTransfer
@@ -67,7 +68,7 @@ public class SFTPFileTransfer
@Override
public void upload(LocalSourceFile localFile, String remotePath)
throws IOException {
new Uploader().upload(getTransferListener(), localFile, remotePath);
new Uploader(localFile, remotePath).upload(getTransferListener());
}
@Override
@@ -173,6 +174,31 @@ public class SFTPFileTransfer
private class Uploader {
private final LocalSourceFile source;
private final String remote;
private Uploader(final LocalSourceFile source, final String remote) {
this.source = source;
this.remote = remote;
}
private void upload(final TransferListener listener) throws IOException {
if (source.isDirectory()) {
makeDirIfNotExists(remote); // Ensure that the directory exists
uploadDir(listener.directory(source.getName()), source, remote);
setAttributes(source, remote);
} else if (source.isFile() && isDirectory(remote)) {
String adjustedRemote = engine.getPathHelper().adjustForParent(this.remote, source.getName());
uploadFile(listener.file(source.getName(), source.getLength()), source, adjustedRemote);
setAttributes(source, adjustedRemote);
} else if (source.isFile()) {
uploadFile(listener.file(source.getName(), source.getLength()), source, remote);
setAttributes(source, remote);
} else {
throw new IOException(source + " is not a file or directory");
}
}
private void upload(final TransferListener listener,
final LocalSourceFile local,
final String remote)
@@ -182,20 +208,26 @@ public class SFTPFileTransfer
adjustedPath = uploadDir(listener.directory(local.getName()), local, remote);
} else if (local.isFile()) {
adjustedPath = uploadFile(listener.file(local.getName(), local.getLength()), local, remote);
} else
} else {
throw new IOException(local + " is not a file or directory");
if (getPreserveAttributes())
engine.setAttributes(adjustedPath, getAttributes(local));
}
setAttributes(local, adjustedPath);
}
private void setAttributes(LocalSourceFile local, String remotePath) throws IOException {
if (getPreserveAttributes()) {
engine.setAttributes(remotePath, getAttributes(local));
}
}
private String uploadDir(final TransferListener listener,
final LocalSourceFile local,
final String remote)
throws IOException {
final String adjusted = prepareDir(local, remote);
makeDirIfNotExists(remote);
for (LocalSourceFile f : local.getChildren(getUploadFilter()))
upload(listener, f, adjusted);
return adjusted;
upload(listener, f, engine.getPathHelper().adjustForParent(remote, f.getName()));
return remote;
}
private String uploadFile(final StreamCopier.Listener listener,
@@ -203,52 +235,50 @@ public class SFTPFileTransfer
final String remote)
throws IOException {
final String adjusted = prepareFile(local, remote);
final RemoteFile rf = engine.open(adjusted, EnumSet.of(OpenMode.WRITE,
OpenMode.CREAT,
OpenMode.TRUNC));
try {
final InputStream fis = local.getInputStream();
final RemoteFile.RemoteFileOutputStream rfos = rf.new RemoteFileOutputStream(0, 16);
try {
try (RemoteFile rf = engine.open(adjusted, EnumSet.of(OpenMode.WRITE, OpenMode.CREAT, OpenMode.TRUNC))) {
try (InputStream fis = local.getInputStream();
RemoteFile.RemoteFileOutputStream rfos = rf.new RemoteFileOutputStream(0, 16)) {
new StreamCopier(fis, rfos)
.bufSize(engine.getSubsystem().getRemoteMaxPacketSize() - rf.getOutgoingPacketOverhead())
.keepFlushing(false)
.listener(listener)
.copy();
} finally {
fis.close();
rfos.close();
}
} finally {
rf.close();
}
return adjusted;
}
private String prepareDir(final LocalSourceFile local, final String remote)
throws IOException {
final FileAttributes attrs;
private boolean makeDirIfNotExists(final String remote) throws IOException {
try {
attrs = engine.stat(remote);
FileAttributes attrs = engine.stat(remote);
if (attrs.getMode().getType() != FileMode.Type.DIRECTORY) {
throw new IOException(remote + " exists and should be a directory, but was a " + attrs.getMode().getType());
}
// Was not created, but existed.
return false;
} catch (SFTPException e) {
if (e.getStatusCode() == StatusCode.NO_SUCH_FILE) {
log.debug("probeDir: {} does not exist, creating", remote);
log.debug("makeDir: {} does not exist, creating", remote);
engine.makeDir(remote);
return remote;
} else
throw e;
}
if (attrs.getMode().getType() == FileMode.Type.DIRECTORY)
if (engine.getPathHelper().getComponents(remote).getName().equals(local.getName())) {
log.debug("probeDir: {} already exists", remote);
return remote;
return true;
} else {
log.debug("probeDir: {} already exists, path adjusted for {}", remote, local.getName());
return prepareDir(local, engine.getPathHelper().adjustForParent(remote, local.getName()));
throw e;
}
else
throw new IOException(attrs.getMode().getType() + " file already exists at " + remote);
}
}
private boolean isDirectory(final String remote) throws IOException {
try {
FileAttributes attrs = engine.stat(remote);
return attrs.getMode().getType() == FileMode.Type.DIRECTORY;
} catch (SFTPException e) {
if (e.getStatusCode() == StatusCode.NO_SUCH_FILE) {
log.debug("isDir: {} does not exist", remote);
return false;
} else {
throw e;
}
}
}
private String prepareFile(final LocalSourceFile local, final String remote)
@@ -264,8 +294,7 @@ public class SFTPFileTransfer
throw e;
}
if (attrs.getMode().getType() == FileMode.Type.DIRECTORY) {
log.debug("probeFile: {} was directory, path adjusted for {}", remote, local.getName());
return engine.getPathHelper().adjustForParent(remote, local.getName());
throw new IOException("Trying to upload file " + local.getName() + " to path " + remote + " but that is a directory");
} else {
log.debug("probeFile: {} is a {} file that will be replaced", remote, attrs.getMode().getType());
return remote;
@@ -281,5 +310,4 @@ public class SFTPFileTransfer
}
}
}

View File

@@ -0,0 +1,244 @@
/*
* Copyright (C)2009 - SSHJ Contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
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.SFTPClient
import org.junit.Rule
import org.junit.rules.TemporaryFolder
import spock.lang.Specification
import spock.lang.Unroll
import static org.codehaus.groovy.runtime.IOGroovyMethods.withCloseable
class SFTPClientTest extends Specification {
@Rule
public SshFixture fixture = new SshFixture()
@Rule
public TemporaryFolder temp = new TemporaryFolder()
@Unroll
def "should copy #sourceType->#targetType if #targetExists with #named name"() {
given:
File src = source
File dest = target
when:
doUpload(src, dest)
then:
exists.each { f ->
if (dest.isDirectory()) {
assert new File(dest, f).exists()
} else {
assert dest.exists()
}
}
// Dest is also counted by recursiveCount if it is a dir
exists.size() + (dest.isDirectory() ? 1 : 0) == recursiveCount(dest)
cleanup:
// Delete the temp directories
recursiveDelete(source.getParentFile())
recursiveDelete(target.getParentFile())
where:
source | target || exists
sourceTree() | existingTargetDir("dest") || ["toto.txt", "tata.txt", "tutu", "tutu/tutu.txt"]
sourceTree() | newTargetDir("dest") || ["toto.txt", "tata.txt", "tutu", "tutu/tutu.txt"]
sourceTree() | existingTargetDir("toto") || ["toto.txt", "tata.txt", "tutu", "tutu/tutu.txt"]
sourceTree() | newTargetDir("toto") || ["toto.txt", "tata.txt", "tutu", "tutu/tutu.txt"]
sourceFile() | existingTargetDir("dest") || ["toto.txt"]
sourceFile() | newTargetFile("toto.txt") || ["toto.txt"]
sourceFile() | newTargetFile("diff.txt") || ["diff.txt"]
sourceType = source.isDirectory() ? "dir" : "file"
targetType = target.isDirectory() ? "dir" : sourceType
targetExists = target.exists() ? "exists" : "not exists"
named = (target.name == source.name) ? "same" : "different"
}
def "should not throw exception on close before disconnect"() {
given:
File file = temp.newFile("source.txt")
FileUtil.writeToFile(file, "This is the source")
when:
doUpload(file, temp.newFile("dest.txt"))
then:
noExceptionThrown()
}
//
// def "should copy dir->dir if exists and same name"() {
// given:
// File srcDir = temp.newFolder("toto")
// File destDir = temp.newFolder("dest", "toto")
// FileUtil.writeToFile(new File(srcDir, "toto.txt"), "Toto file")
//
// when:
// doUpload(srcDir, destDir)
//
// then:
// destDir.exists()
// !new File(destDir, "toto").exists()
// new File(destDir, "toto.txt").exists()
// }
//
// def "should copy dir->dir if exists and different name"() {
// given:
// File srcDir = temp.newFolder("toto")
// File destDir = temp.newFolder("dest")
// FileUtil.writeToFile(new File(srcDir, "toto.txt"), "Toto file")
//
// when:
// doUpload(srcDir, destDir)
//
// then:
// destDir.exists()
// new File(destDir, "toto.txt").exists()
// }
//
// def "should copy dir->dir if not exists and same name"() {
// given:
// File dd = temp.newFolder("dest")
// File destDir = new File(dd, "toto")
//
// when:
// doUpload(srcDir, destDir)
//
// then:
// destDir.exists()
// new File(destDir, "toto.txt").exists()
// }
//
// def "should copy dir->dir if not exists and different name"() {
// given:
// File srcDir = temp.newFolder("toto")
// File destDir = new File(temp.getRoot(), "dest")
// FileUtil.writeToFile(new File(srcDir, "toto.txt"), "Toto file")
//
// when:
// doUpload(srcDir, destDir)
//
// then:
// destDir.exists()
// new File(destDir, "toto.txt").exists()
// }
def "should not merge same name subdirs (GH #252)"() {
given:
File toto = temp.newFolder("toto")
File tutu = mkdir(toto, "tutu")
File toto2 = mkdir(toto, "toto")
File dest = temp.newFolder("dest")
FileUtil.writeToFile(new File(toto, "toto.txt"), "Toto file")
FileUtil.writeToFile(new File(tutu, "tototutu.txt"), "Toto/Tutu file")
FileUtil.writeToFile(new File(toto2, "totototo.txt"), "Toto/Toto file")
when:
doUpload(toto, dest)
then:
new File(dest, "toto").exists()
new File(dest, "toto.txt").exists()
new File(dest, "tutu").exists()
new File(dest, "tutu/tototutu.txt").exists()
new File(dest, "toto").exists()
new File(dest, "toto/totototo.txt").exists()
!new File(dest, "totototo.txt").exists()
}
private void doUpload(File src, File dest) throws IOException {
SSHClient sshClient = fixture.setupConnectedDefaultClient()
sshClient.authPassword("test", "test")
try {
withCloseable(sshClient.newSFTPClient()) { SFTPClient sftpClient ->
sftpClient.put(src.getPath(), dest.getPath())
}
} finally {
sshClient.disconnect()
}
}
private File mkdir(File parent, String name) {
File file = new File(parent, name)
file.mkdirs()
return file
}
private def sourceTree() {
def tempDir = File.createTempDir()
File srcDir = mkdir(tempDir, "toto")
FileUtil.writeToFile(new File(srcDir, "toto.txt"), "Toto file")
FileUtil.writeToFile(new File(srcDir, "tata.txt"), "Tata file")
File tutuDir = mkdir(srcDir, "tutu")
FileUtil.writeToFile(new File(tutuDir, "tutu.txt"), "Tutu file")
return srcDir
}
private def sourceFile() {
def tempDir = File.createTempDir()
def totoFile = new File(tempDir, "toto.txt")
FileUtil.writeToFile(totoFile, "Bare toto file")
return totoFile
}
private def existingTargetDir(String name) {
def tempDir = File.createTempDir("sftp", "tmp")
tempDir.deleteOnExit()
return mkdir(tempDir, name)
}
private def newTargetFile(String name) {
def tempDir = File.createTempDir("sftp", "tmp")
tempDir.deleteOnExit()
return new File(tempDir, name)
}
private def newTargetDir(String name) {
def tempDir = File.createTempDir("sftp", "tmp")
tempDir.deleteOnExit()
return new File(tempDir, name)
}
private int recursiveCount(File file) {
if (file.isFile()) {
return 1
}
File[] files = file.listFiles();
if (files != null) {
return 1 + (files.collect({ f -> recursiveCount(f) }).sum() as int)
} else {
return 1
}
}
private void recursiveDelete(File file) {
File[] files = file.listFiles();
if (files != null) {
for (File each : files) {
recursiveDelete(each);
}
}
file.delete();
}
}

View File

@@ -1,54 +0,0 @@
/*
* Copyright (C)2009 - SSHJ Contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
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.SFTPClient;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import java.io.File;
import java.io.IOException;
public class SFTPClientTest {
@Rule
public SshFixture fixture = new SshFixture();
@Rule
public TemporaryFolder temp = new TemporaryFolder();
@Test
public void shouldNotThrowExceptionOnCloseBeforeDisconnect() throws IOException {
SSHClient sshClient = fixture.setupConnectedDefaultClient();
sshClient.authPassword("test", "test");
SFTPClient sftpClient = sshClient.newSFTPClient();
File file = temp.newFile("source.txt");
FileUtil.writeToFile(file, "This is the source");
try {
try {
sftpClient.put(file.getPath(), temp.newFile("dest.txt").getPath());
} finally {
sftpClient.close();
}
} finally {
sshClient.disconnect();
}
}
}