diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/AbstractSftpClient.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/AbstractSftpClient.java index f501b4251..915aa34e6 100644 --- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/AbstractSftpClient.java +++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/AbstractSftpClient.java @@ -1172,7 +1172,7 @@ public InputStream read(String path, int bufferSize, Collection mode) if (bufferSize <= 0) { bufferSize = getReadBufferSize(); } - if (bufferSize < MIN_WRITE_BUFFER_SIZE) { + if (bufferSize < MIN_READ_BUFFER_SIZE) { throw new IllegalArgumentException("Insufficient read buffer size: " + bufferSize + ", min.=" + MIN_READ_BUFFER_SIZE); } @@ -1184,12 +1184,6 @@ public InputStream read(String path, int bufferSize, Collection mode) return new SftpInputStreamAsync(this, bufferSize, path, mode); } - @Override - public InputStream read(String path, Collection mode) throws IOException { - int packetSize = (int) getChannel().getRemoteWindow().getPacketSize(); - return read(path, packetSize, mode); - } - @Override public OutputStream write(String path, int bufferSize, Collection mode) throws IOException { if (bufferSize <= 0) { @@ -1207,18 +1201,12 @@ public OutputStream write(String path, int bufferSize, Collection mode return new SftpOutputStreamAsync(this, bufferSize, path, mode); } - @Override - public OutputStream write(String path, Collection mode) throws IOException { - int packetSize = (int) getChannel().getRemoteWindow().getPacketSize(); - return write(path, packetSize, mode); - } - protected int getReadBufferSize() { return (int) getClientChannel().getLocalWindow().getPacketSize() - 13; } protected int getWriteBufferSize() { - return (int) getClientChannel().getLocalWindow().getPacketSize() - 13; + return (int) getClientChannel().getRemoteWindow().getPacketSize() - 13; } } diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/SftpInputStreamAsync.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/SftpInputStreamAsync.java index 8be006ff8..06c24936e 100644 --- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/SftpInputStreamAsync.java +++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/SftpInputStreamAsync.java @@ -281,13 +281,15 @@ protected void fillData() throws IOException { SftpClient client = getClient(); int cur = 0; while (cur < nb) { - int dlen = client.read(handle, clientOffset, data, cur, nb - cur, eof); + int dlen = client.read(handle, clientOffset + cur, data, cur, nb - cur, eof); + if (dlen > 0) { + cur += dlen; + } Boolean eofSignal = eof.getAndSet(null); if ((dlen < 0) || ((eofSignal != null) && eofSignal.booleanValue())) { eofIndicator = true; break; } - cur += dlen; } if (traceEnabled) { diff --git a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpTest.java b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpTest.java index 9d1178182..b150e3aa0 100644 --- a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpTest.java +++ b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpTest.java @@ -233,17 +233,57 @@ public void testReadBufferLimit() throws Exception { for (int index = 0; index < readLen; index++) { byte expByte = expected[index]; byte actByte = actual[index]; - if (expByte != actByte) { - fail("Mismatched values at index=" + index - + ": expected=0x" + Integer.toHexString(expByte & 0xFF) - + ", actual=0x" + Integer.toHexString(actByte & 0xFF)); - } + assertEquals("Mismatched values at index=" + index, Integer.toHexString(expByte & 0xFF), + Integer.toHexString(actByte & 0xFF)); } } finally { SftpModuleProperties.MAX_READDATA_PACKET_LENGTH.remove(sshd); } } + @Test // see SSHD-1287 + public void testReadWithLargeBuffer() throws Exception { + Path targetPath = detectTargetFolder(); + Path parentPath = targetPath.getParent(); + Path lclSftp = CommonTestSupportUtils.resolve(targetPath, SftpConstants.SFTP_SUBSYSTEM_NAME, getClass().getSimpleName(), + getCurrentTestName()); + Path testFile = assertHierarchyTargetFolderExists(lclSftp).resolve("file.bin"); + byte[] expected = new byte[1024 * 1024]; + + Factory factory = sshd.getRandomFactory(); + Random rnd = factory.create(); + rnd.fill(expected); + Files.write(testFile, expected); + + String file = CommonTestSupportUtils.resolveRelativeRemotePath(parentPath, testFile); + try (SftpClient sftp = createSingleSessionClient()) { + byte[] actual = new byte[expected.length]; + try (InputStream in = sftp.read(file, + 2 * SftpModuleProperties.MAX_READDATA_PACKET_LENGTH.getRequiredDefault() + 2048)) { + int off = 0; + int n = 0; + while (off < actual.length) { + n = in.read(actual, off, actual.length - off); + if (n < 0) { + break; + } + off += n; + } + assertEquals("Short read", actual.length, off); + if (n >= 0) { + n = in.read(); + assertTrue("Stream not at eof", n < 0); + } + } + for (int index = 0; index < actual.length; index++) { + byte expByte = expected[index]; + byte actByte = actual[index]; + assertEquals("Mismatched values at index=" + index, Integer.toHexString(expByte & 0xFF), + Integer.toHexString(actByte & 0xFF)); + } + } + } + @Test // see SSHD-1288 public void testReadWriteDownload() throws Exception { Assume.assumeTrue("Not sure appending to a file opened for reading works on Windows", @@ -281,10 +321,8 @@ public void testReadWriteDownload() throws Exception { } byte expByte = expected[j]; byte actByte = actual[i]; - if (expByte != actByte) { - fail("Mismatched values at index=" + i + ": expected=0x" + Integer.toHexString(expByte & 0xFF) - + ", actual=0x" + Integer.toHexString(actByte & 0xFF)); - } + assertEquals("Mismatched values at index=" + i, Integer.toHexString(expByte & 0xFF), + Integer.toHexString(actByte & 0xFF)); } } }