Prepped for v0.17.2 release

This ensures that any header lines sent before the identification
string do not break the identification parsing if they are longer
than the identification string should be.

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.17.2
 :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,7 +98,14 @@ Google Group: http://groups.google.com/group/sshj-users
 Fork away!
 
 == Release history
-SSHJ 0.16.0 (2016-??-??)::
+SSHJ 0.17.1 (2016-07-06)::
+* Improved parsing of the SSH Server identification. Too long header lines now no longer break the protocol.
+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
 * Fixed https://github.com/hierynomus/sshj/issues/236[#236]: Remote Port forwarding with dynamic port allocation fails with BufferUnderflowException

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"
 }
 
@@ -43,6 +44,7 @@ test {
         exceptionFormat = 'full'
     }
     include "**/*Test.*"
+    include "**/*Spec.*"
     if (!project.hasProperty("allTests")) {
         useJUnit {
             excludeCategories 'com.hierynomus.sshj.test.SlowTests'
@@ -72,6 +74,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 +106,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 +207,5 @@ uploadArchives {
     }
 }
 
+tasks.compileGroovy.onlyIf { false }
 tasks.release.dependsOn 'build', 'uploadArchives'

src/main/java/com/hierynomus/sshj/transport/IdentificationStringParser.java

@@ -0,0 +1,95 @@
+/*
+ * 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.transport;
+
+import net.schmizz.sshj.common.Buffer;
+import net.schmizz.sshj.common.ByteArrayUtils;
+import net.schmizz.sshj.transport.TransportException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Arrays;
+
+public class IdentificationStringParser {
+    private static final Logger logger = LoggerFactory.getLogger(IdentificationStringParser.class);
+    private final Buffer.PlainBuffer buffer;
+
+    private byte[] EXPECTED_START_BYTES = new byte[] {'S', 'S', 'H', '-'};
+
+    public IdentificationStringParser(Buffer.PlainBuffer buffer) {
+        this.buffer = buffer;
+    }
+
+    public String parseIdentificationString() throws IOException {
+        for (;;) {
+            Buffer.PlainBuffer lineBuffer = new Buffer.PlainBuffer();
+            int lineStartPos = buffer.rpos();
+            for (;;) {
+                if (buffer.available() == 0) {
+                    buffer.rpos(lineStartPos);
+                    return "";
+                }
+                byte b = buffer.readByte();
+                lineBuffer.putByte(b);
+                if (b == '\n') {
+                    if (checkForIdentification(lineBuffer)) {
+                        return readIdentification(lineBuffer);
+                    } else {
+                        logHeaderLine(lineBuffer);
+                    }
+                    break;
+                }
+            }
+        }
+    }
+
+    private void logHeaderLine(Buffer.PlainBuffer lineBuffer) {
+
+    }
+
+    private String readIdentification(Buffer.PlainBuffer lineBuffer) throws Buffer.BufferException, TransportException {
+        byte[] bytes = new byte[lineBuffer.available()];
+        lineBuffer.readRawBytes(bytes);
+        if (bytes.length > 255) {
+            logger.error("Incorrect identification String received, line was longer than expected: {}", new String(bytes));
+            logger.error("Just for good measure, bytes were: {}", ByteArrayUtils.printHex(bytes, 0, bytes.length));
+            throw new TransportException("Incorrect identification: line too long: " + ByteArrayUtils.printHex(bytes, 0, bytes.length));
+        }
+        if (bytes[bytes.length - 2] != '\r') {
+            String ident = new String(bytes, 0, bytes.length - 1);
+            logger.warn("Server identification has bad line ending, was expecting a '\\r\\n' however got: '{}' (hex: {})", (char) (bytes[bytes.length - 2] & 0xFF), Integer.toHexString(bytes[bytes.length - 2] & 0xFF));
+            logger.warn("Will treat the identification of this server '{}' leniently", ident);
+            return ident;
+            // logger.error("Data received up til here was: {}", new String(bytes));
+            // throw new TransportException("Incorrect identification: bad line ending: " + ByteArrayUtils.toHex(bytes, 0, bytes.length));
+        }
+
+        // Strip off the \r\n
+        return new String(bytes, 0, bytes.length - 2);
+    }
+
+    private boolean checkForIdentification(Buffer.PlainBuffer lineBuffer) throws Buffer.BufferException {
+        if (lineBuffer.available() < 4) {
+            return false;
+        }
+        byte[] buf = new byte[4];
+        lineBuffer.readRawBytes(buf);
+        // Reset
+        lineBuffer.rpos(0);
+        return Arrays.equals(EXPECTED_START_BYTES, buf);
+    }
+}

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_17_2";
 
     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);
+                upload(listener, f, engine.getPathHelper().adjustForParent(remote, f.getName()));
-            return adjusted;
+            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,
+            try (RemoteFile rf = engine.open(adjusted, EnumSet.of(OpenMode.WRITE, OpenMode.CREAT, OpenMode.TRUNC))) {
-                                                                   OpenMode.CREAT,
+                try (InputStream fis = local.getInputStream();
-                                                                   OpenMode.TRUNC));
+                     RemoteFile.RemoteFileOutputStream rfos = rf.new RemoteFileOutputStream(0, 16)) {
-            try {
-                final InputStream fis = local.getInputStream();
-                final RemoteFile.RemoteFileOutputStream rfos = rf.new RemoteFileOutputStream(0, 16);
-                try {
                     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)
+        private boolean makeDirIfNotExists(final String remote) throws IOException {
-                throws IOException {
-            final FileAttributes attrs;
             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;
+                    return true;
-                } 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;
                 } else {
-                    log.debug("probeDir: {} already exists, path adjusted for {}", remote, local.getName());
+                    throw e;
-                    return prepareDir(local, engine.getPathHelper().adjustForParent(remote, local.getName()));
                 }
-            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());
+                throw new IOException("Trying to upload file " + local.getName() + " to path " + remote + " but that is a directory");
-                return engine.getPathHelper().adjustForParent(remote, local.getName());
             } 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/transport/TransportImpl.java

@@ -15,18 +15,14 @@
  */
 package net.schmizz.sshj.transport;
 
+import com.hierynomus.sshj.transport.IdentificationStringParser;
 import net.schmizz.concurrent.ErrorDeliveryUtil;
 import net.schmizz.concurrent.Event;
 import net.schmizz.sshj.AbstractService;
 import net.schmizz.sshj.Config;
 import net.schmizz.sshj.SSHClient;
 import net.schmizz.sshj.Service;
-import net.schmizz.sshj.common.Buffer;
+import net.schmizz.sshj.common.*;
-import net.schmizz.sshj.common.DisconnectReason;
-import net.schmizz.sshj.common.IOUtils;
-import net.schmizz.sshj.common.Message;
-import net.schmizz.sshj.common.SSHException;
-import net.schmizz.sshj.common.SSHPacket;
 import net.schmizz.sshj.transport.verification.AlgorithmsVerifier;
 import net.schmizz.sshj.transport.verification.HostKeyVerifier;
 import org.slf4j.Logger;
@@ -207,38 +203,47 @@ public final class TransportImpl
      */
     private String readIdentification(Buffer.PlainBuffer buffer)
             throws IOException {
-        String ident;
+        String ident = new IdentificationStringParser(buffer).parseIdentificationString();
-
+        if (ident.isEmpty()) {
-        byte[] data = new byte[256];
+            return ident;
-        for (; ; ) {
-            int savedBufPos = buffer.rpos();
-            int pos = 0;
-            boolean needLF = false;
-            for (; ; ) {
-                if (buffer.available() == 0) {
-                    // Need more data, so undo reading and return null
-                    buffer.rpos(savedBufPos);
-                    return "";
-                }
-                byte b = buffer.readByte();
-                if (b == '\r') {
-                    needLF = true;
-                    continue;
-                }
-                if (b == '\n')
-                    break;
-                if (needLF)
-                    throw new TransportException("Incorrect identification: bad line ending");
-                if (pos >= data.length)
-                    throw new TransportException("Incorrect identification: line too long");
-                data[pos++] = b;
-            }
-            ident = new String(data, 0, pos);
-            if (ident.startsWith("SSH-"))
-                break;
-            if (buffer.rpos() > 16 * 1024)
-                throw new TransportException("Incorrect identification: too many header lines");
         }
+//
+//        byte[] data = new byte[256];
+//        for (; ; ) {
+//            int savedBufPos = buffer.rpos();
+//            int pos = 0;
+//            boolean needLF = false;
+//            for (; ; ) {
+//                if (buffer.available() == 0) {
+//                    // Need more data, so undo reading and return null
+//                    buffer.rpos(savedBufPos);
+//                    return "";
+//                }
+//                byte b = buffer.readByte();
+//                if (b == '\r') {
+//                    needLF = true;
+//                    continue;
+//                }
+//                if (b == '\n')
+//                    break;
+//                if (needLF) {
+//                    log.error("Incorrect identification, was expecting a '\n' after the '\r', got: '{}' (hex: {})", b, Integer.toHexString(b & 0xFF));
+//                    log.error("Data received up til here was: {}", new String(data, 0, pos));
+//                    throw new TransportException("Incorrect identification: bad line ending: " + ByteArrayUtils.toHex(data, 0, pos));
+//                }
+//                if (pos >= data.length) {
+//                    log.error("Incorrect identification String received, line was longer than expected: {}", new String(data, 0, pos));
+//                    log.error("Just for good measure, bytes were: {}", ByteArrayUtils.printHex(data, 0, pos));
+//                    throw new TransportException("Incorrect identification: line too long: " + ByteArrayUtils.printHex(data, 0, pos));
+//                }
+//                data[pos++] = b;
+//            }
+//            ident = new String(data, 0, pos);
+//            if (ident.startsWith("SSH-"))
+//                break;
+//            if (buffer.rpos() > 16 * 1024)
+//                throw new TransportException("Incorrect identification: too many header lines");
+//        }
 
         if (!ident.startsWith("SSH-2.0-") && !ident.startsWith("SSH-1.99-"))
             throw new TransportException(DisconnectReason.PROTOCOL_VERSION_NOT_SUPPORTED,

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. */
+/** Support for downloading files over a connected link using SCP. */
-public final class SCPDownloadClient extends AbstractSCPClient {
+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/SFTPClientSpec.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 SFTPClientSpec 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/groovy/com/hierynomus/sshj/transport/IdentificationStringParserSpec.groovy

@@ -0,0 +1,103 @@
+/*
+ * 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.transport
+
+import net.schmizz.sshj.common.Buffer
+import net.schmizz.sshj.transport.TransportException
+import spock.lang.Specification
+
+class IdentificationStringParserSpec extends Specification {
+
+    def "should parse simple identification string"() {
+        given:
+        def buffer = new Buffer.PlainBuffer()
+        buffer.putRawBytes("SSH-2.0-OpenSSH-6.13\r\n".bytes)
+
+        when:
+        def ident = new IdentificationStringParser(buffer).parseIdentificationString()
+
+        then:
+        ident == "SSH-2.0-OpenSSH-6.13"
+    }
+
+    def "should leniently parse identification string without carriage return"() {
+        given:
+        def buffer = new Buffer.PlainBuffer()
+        buffer.putRawBytes("SSH-2.0-OpenSSH-6.13\n".bytes)
+
+        when:
+        def ident = new IdentificationStringParser(buffer).parseIdentificationString()
+
+        then:
+        ident == "SSH-2.0-OpenSSH-6.13"
+    }
+
+    def "should not parse header lines as part of ident"() {
+        given:
+        def buffer = new Buffer.PlainBuffer()
+        buffer.putRawBytes("header1\nheader2\r\nSSH-2.0-OpenSSH-6.13\r\n".bytes)
+
+        when:
+        def ident = new IdentificationStringParser(buffer).parseIdentificationString()
+
+        then:
+        ident == "SSH-2.0-OpenSSH-6.13"
+    }
+
+    def "should fail on too long ident string"() {
+        given:
+        def buffer = new Buffer.PlainBuffer()
+        buffer.putRawBytes("SSH-2.0-OpenSSH-6.13 ".bytes)
+        byte[] bs = new byte[255 - buffer.wpos()]
+        Arrays.fill(bs, 'a'.bytes[0])
+        buffer.putRawBytes(bs).putRawBytes("\r\n".bytes)
+
+        when:
+        new IdentificationStringParser(buffer).parseIdentificationString()
+
+        then:
+        thrown(TransportException.class)
+    }
+
+    def "should not fail on too long header line"() {
+        given:
+        def buffer = new Buffer.PlainBuffer()
+        buffer.putRawBytes("header1 ".bytes)
+        byte[] bs = new byte[255 - buffer.wpos()]
+        new Random().nextBytes(bs)
+        buffer.putRawBytes(bs).putRawBytes("\r\n".bytes)
+        buffer.putRawBytes("SSH-2.0-OpenSSH-6.13\r\n".bytes)
+
+        when:
+        def ident = new IdentificationStringParser(buffer).parseIdentificationString()
+
+        then:
+        ident == "SSH-2.0-OpenSSH-6.13"
+    }
+
+    def "should not fail on very short header line"() {
+        given:
+        def buffer = new Buffer.PlainBuffer()
+        buffer.putRawBytes("h1\n".bytes)
+        buffer.putRawBytes("SSH-2.0-OpenSSH-6.13\r\n".bytes)
+
+        when:
+        def ident = new IdentificationStringParser(buffer).parseIdentificationString()
+
+        then:
+        ident == "SSH-2.0-OpenSSH-6.13"
+    }
+}

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