From f491e8d1010b7903d7eabf1f8ea5cfd0a48553ff Mon Sep 17 00:00:00 2001 From: Jeroen van Erp Date: Thu, 7 Jul 2016 11:01:43 +0200 Subject: [PATCH] Fixed bug that crept in 0edc4a5 --- build.gradle | 1 + .../transport/IdentificationStringParser.java | 18 ++++++++++--- ...lientTest.groovy => SFTPClientSpec.groovy} | 2 +- .../IdentificationStringParserSpec.groovy | 25 +++++++++++++++++++ 4 files changed, 42 insertions(+), 4 deletions(-) rename src/test/groovy/com/hierynomus/sshj/sftp/{SFTPClientTest.groovy => SFTPClientSpec.groovy} (99%) diff --git a/build.gradle b/build.gradle index d3162d06..47bae822 100644 --- a/build.gradle +++ b/build.gradle @@ -44,6 +44,7 @@ test { exceptionFormat = 'full' } include "**/*Test.*" + include "**/*Spec.*" if (!project.hasProperty("allTests")) { useJUnit { excludeCategories 'com.hierynomus.sshj.test.SlowTests' diff --git a/src/main/java/com/hierynomus/sshj/transport/IdentificationStringParser.java b/src/main/java/com/hierynomus/sshj/transport/IdentificationStringParser.java index 46ff1098..d1591561 100644 --- a/src/main/java/com/hierynomus/sshj/transport/IdentificationStringParser.java +++ b/src/main/java/com/hierynomus/sshj/transport/IdentificationStringParser.java @@ -48,6 +48,8 @@ public class IdentificationStringParser { if (b == '\n') { if (checkForIdentification(lineBuffer)) { return readIdentification(lineBuffer); + } else { + logHeaderLine(lineBuffer); } break; } @@ -55,6 +57,10 @@ public class IdentificationStringParser { } } + 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); @@ -64,9 +70,12 @@ public class IdentificationStringParser { throw new TransportException("Incorrect identification: line too long: " + ByteArrayUtils.printHex(bytes, 0, bytes.length)); } if (bytes[bytes.length - 2] != '\r') { - logger.error("Incorrect identification, was expecting a '\\r\\n' however got: '{}' (hex: {})", bytes[bytes.length - 2], Integer.toHexString(bytes[bytes.length - 2] & 0xFF)); - 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)); + 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 @@ -74,6 +83,9 @@ public class IdentificationStringParser { } private boolean checkForIdentification(Buffer.PlainBuffer lineBuffer) throws Buffer.BufferException { + if (lineBuffer.available() < 4) { + return false; + } byte[] buf = new byte[4]; lineBuffer.readRawBytes(buf); // Reset diff --git a/src/test/groovy/com/hierynomus/sshj/sftp/SFTPClientTest.groovy b/src/test/groovy/com/hierynomus/sshj/sftp/SFTPClientSpec.groovy similarity index 99% rename from src/test/groovy/com/hierynomus/sshj/sftp/SFTPClientTest.groovy rename to src/test/groovy/com/hierynomus/sshj/sftp/SFTPClientSpec.groovy index b972129f..09764c1b 100644 --- a/src/test/groovy/com/hierynomus/sshj/sftp/SFTPClientTest.groovy +++ b/src/test/groovy/com/hierynomus/sshj/sftp/SFTPClientSpec.groovy @@ -26,7 +26,7 @@ import spock.lang.Unroll import static org.codehaus.groovy.runtime.IOGroovyMethods.withCloseable -class SFTPClientTest extends Specification { +class SFTPClientSpec extends Specification { @Rule public SshFixture fixture = new SshFixture() diff --git a/src/test/groovy/com/hierynomus/sshj/transport/IdentificationStringParserSpec.groovy b/src/test/groovy/com/hierynomus/sshj/transport/IdentificationStringParserSpec.groovy index 72a401be..380d75c7 100644 --- a/src/test/groovy/com/hierynomus/sshj/transport/IdentificationStringParserSpec.groovy +++ b/src/test/groovy/com/hierynomus/sshj/transport/IdentificationStringParserSpec.groovy @@ -33,6 +33,18 @@ class IdentificationStringParserSpec extends Specification { 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() @@ -75,4 +87,17 @@ class IdentificationStringParserSpec extends Specification { 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" + } }