Updated README

* Updated version string in config

* Fixed #252 with backwards compatible behaviour

* Fixed logic and expanded test

README.adoc

@@ -1,10 +1,12 @@
 = sshj - SSHv2 library for Java
 Jeroen van Erp
 :sshj_groupid: com.hierynomus
-:sshj_version: 0.15.0
+:sshj_version: 0.16.0
 :source-highlighter: pygments
 
-image::https://travis-ci.org/hierynomus/sshj.svg?branch=master[]
+image:https://travis-ci.org/hierynomus/sshj.svg?branch=master[link="https://travis-ci.org/hierynomus/sshj"]
+image:https://maven-badges.herokuapp.com/maven-central/com.hierynomus/sshj/badge.svg["Maven Central",link="https://maven-badges.herokuapp.com/maven-central/com.hierynomus/sshj"]
+image:https://javadoc-emblem.rhcloud.com/doc/com.hierynomus/sshj/badge.svg["Javadoc",link="http://www.javadoc.io/doc/com.hierynomus/sshj"]
 
 To get started, have a look at one of the examples. Hopefully you will find the API pleasant to work with :)
 
@@ -96,6 +98,11 @@ Google Group: http://groups.google.com/group/sshj-users
 Fork away!
 
 == Release history
+SSHJ 0.17.0 (2016-07-05)::
+* *Introduced breaking change in SFTP copy behaviour*: Previously an SFTP copy operation would behave differently if both source and target were folders with different names.
+  In this case instead of copying the contents of the source into the target directory, the directory itself was copied as a sub directory of the target directory.
+  This behaviour has been removed in favour of the default behaviour which is to copy the contents of the source into the target. Bringing the behaviour in line with how SCP works.
+* Fixed https://github.com/hierynomus/sshj/issues/252[#252] (via: https://github.com/hierynomus/sshj/pulls/253[#253]): Same name subdirs are no longer merged by accident
 SSHJ 0.16.0 (2016-04-11)::
 * Fixed https://github.com/hierynomus/sshj/issues/239[#239]: Remote port forwards did not work if you used the empty string as address, or a catch-all address.
 * Fixed https://github.com/hierynomus/sshj/issues/242[#242]: Added OSGI headers to sources jar manifest

build.gradle

@@ -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"
@@ -103,7 +105,16 @@ task javadocJar(type: Jar) {
 task sourcesJar(type: Jar) {
     classifier = 'sources'
     from sourceSets.main.allSource
-    manifest = project.tasks.jar.manifest
+    manifest {
+        attributes(
+            // Add the needed OSGI attributes
+            "Bundle-ManifestVersion": "2",
+            "Bundle-Name": "${project.jar.manifest.name} Source",
+            "Bundle-Version": project.jar.manifest.version,
+            "Eclipse-SourceBundle": "${project.jar.manifest.symbolicName};version=\"${project.jar.manifest.version}\";roots:=\".\"",
+            "Bundle-SymbolicName": "${project.jar.manifest.symbolicName}.source"
+        )
+    }
 }
 
 artifacts {
@@ -195,4 +206,5 @@ uploadArchives {
     }
 }
 
+tasks.compileGroovy.onlyIf { false }
 tasks.release.dependsOn 'build', 'uploadArchives'

src/main/java/net/schmizz/sshj/DefaultConfig.java

@@ -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);

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

@@ -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
         }
 
     }
-
 }

src/main/java/net/schmizz/sshj/xfer/scp/SCPDownloadClient.java

@@ -25,8 +25,8 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
-/** Support for uploading files over a connected link using SCP. */
-public final class SCPDownloadClient extends AbstractSCPClient {
+/** Support for downloading files over a connected link using SCP. */
+public class SCPDownloadClient extends AbstractSCPClient {
 
     private boolean recursiveMode = true;
 

src/main/java/net/schmizz/sshj/xfer/scp/SCPUploadClient.java

@@ -25,7 +25,7 @@ import java.io.IOException;
 import java.io.InputStream;
 
 /** Support for uploading files over a connected link using SCP. */
-public final class SCPUploadClient extends AbstractSCPClient {
+public class SCPUploadClient extends AbstractSCPClient {
 
     private LocalFileFilter uploadFilter;
 

src/test/groovy/com/hierynomus/sshj/sftp/SFTPClientTest.groovy

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

src/test/java/com/hierynomus/sshj/sftp/SFTPClientTest.java

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