This is an automated email from the ASF dual-hosted git repository. lgoldstein pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit 4c19ef04fd2dfb35699da39abe04111dbcdaa4ff Author: Lyor Goldstein <lgoldst...@apache.org> AuthorDate: Fri May 22 12:30:38 2020 +0300 Tighter timeout limit on wait time for SFTP server response to client message --- .../common/util/closeable/SequentialCloseable.java | 4 +- .../common/util/closeable/SimpleCloseable.java | 5 +++ .../subsystem/sftp/impl/DefaultSftpClient.java | 15 ++++++- .../client/subsystem/sftp/SftpTransferTest.java | 50 ++++++++++++---------- 4 files changed, 48 insertions(+), 26 deletions(-) diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/closeable/SequentialCloseable.java b/sshd-common/src/main/java/org/apache/sshd/common/util/closeable/SequentialCloseable.java index 10f18b7..2d4a65a 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/util/closeable/SequentialCloseable.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/util/closeable/SequentialCloseable.java @@ -51,7 +51,7 @@ public class SequentialCloseable extends SimpleCloseable { Closeable c = iterator.next(); if (c != null) { if (traceEnabled) { - log.trace("doClose(" + immediately + ") closing " + c); + log.trace("doClose({}) closing {} immediately={}", this, c, immediately); } CloseFuture nextFuture = c.close(immediately); nextFuture.addListener(this); @@ -60,7 +60,7 @@ public class SequentialCloseable extends SimpleCloseable { } if (!iterator.hasNext()) { if (log.isDebugEnabled()) { - log.debug("doClose(" + immediately + ") signal close complete"); + log.debug("doClose({}) signal close complete immediately={}", this, immediately); } future.setClosed(); } diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/closeable/SimpleCloseable.java b/sshd-common/src/main/java/org/apache/sshd/common/util/closeable/SimpleCloseable.java index f232251..c147f72 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/util/closeable/SimpleCloseable.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/util/closeable/SimpleCloseable.java @@ -68,4 +68,9 @@ public class SimpleCloseable extends IoBaseCloseable { protected void doClose(boolean immediately) { future.setClosed(); } + + @Override + public String toString() { + return getClass().getSimpleName() + "[" + future + "]"; + } } diff --git a/sshd-sftp/src/main/java/org/apache/sshd/client/subsystem/sftp/impl/DefaultSftpClient.java b/sshd-sftp/src/main/java/org/apache/sshd/client/subsystem/sftp/impl/DefaultSftpClient.java index 7070c8e..16c5f81 100644 --- a/sshd-sftp/src/main/java/org/apache/sshd/client/subsystem/sftp/impl/DefaultSftpClient.java +++ b/sshd-sftp/src/main/java/org/apache/sshd/client/subsystem/sftp/impl/DefaultSftpClient.java @@ -309,13 +309,26 @@ public class DefaultSftpClient extends AbstractSftpClient { throw new SshException("Channel is being closed"); } + long rcvStart = System.nanoTime(); Buffer buffer = receive(id, idleTimeout); + long rcvEnd = System.nanoTime(); if (buffer != null) { return buffer; } + long rcvDuration = TimeUnit.NANOSECONDS.toMillis(rcvEnd - rcvStart); + if (rcvDuration <= 0L) { + idleTimeout--; + } else { + idleTimeout -= rcvDuration; + } + + if (idleTimeout <= 0L) { + throw new SshException("Timeout expired while waiting for id=" + id); + } + if (traceEnabled) { - log.trace("receive({}) check iteration #{} for id={}", this, count, id); + log.trace("receive({}) check iteration #{} for id={} remain time={}", this, count, id, idleTimeout); } } } diff --git a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTransferTest.java b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTransferTest.java index 8653ee0..05f94b4 100644 --- a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTransferTest.java +++ b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTransferTest.java @@ -44,32 +44,36 @@ public class SftpTransferTest extends AbstractSftpClientTestSupport { @Test public void testTransferIntegrity() throws IOException { + Path localRoot = detectTargetFolder().resolve("sftp"); + Files.createDirectories(localRoot); + + Path local0 = localRoot.resolve("files-0.txt"); + Files.deleteIfExists(local0); + + String data = getClass().getName() + "#" + getCurrentTestName() + "(" + new Date() + ")" + System.lineSeparator(); + try (BufferedWriter bos = Files.newBufferedWriter(local0)) { + long count = 0L; + while (count < 1024L * 1024L * 10L) { // 10 MB + bos.append(data); + count += data.length(); + } + } + try (ClientSession session = createClientSession(); SftpFileSystem fs = SftpClientFactory.instance().createSftpFileSystem(session)) { - Path localRoot = detectTargetFolder().resolve("sftp"); Path remoteRoot = fs.getDefaultDir().resolve("target/sftp"); - - Path local0 = localRoot.resolve("files-0.txt"); Path remote0 = remoteRoot.resolve("files-1.txt"); - Path local1 = localRoot.resolve("files-2.txt"); - Path remote1 = remoteRoot.resolve("files-3.txt"); - Path local2 = localRoot.resolve("files-4.txt"); - Files.deleteIfExists(local0); Files.deleteIfExists(remote0); + + Path local1 = localRoot.resolve("files-2.txt"); Files.deleteIfExists(local1); + + Path remote1 = remoteRoot.resolve("files-3.txt"); Files.deleteIfExists(remote1); - Files.deleteIfExists(local2); - Files.createDirectories(localRoot); - String data = getClass().getName() + "#" + getCurrentTestName() + "(" + new Date() + ")\n"; - try (BufferedWriter bos = Files.newBufferedWriter(local0)) { - long count = 0; - while (count < 1024 * 1024 * 10) { // 10 MB - bos.append(data); - count += data.length(); - } - } + Path local2 = localRoot.resolve("files-4.txt"); + Files.deleteIfExists(local2); Files.copy(local0, remote0); Files.copy(remote0, local1); @@ -93,11 +97,12 @@ public class SftpTransferTest extends AbstractSftpClientTestSupport { } } - private boolean sameContent(Path path, Path path2) throws IOException { - byte[] buffer1 = new byte[BUFFER_SIZE]; - byte[] buffer2 = new byte[BUFFER_SIZE]; + private static boolean sameContent(Path path, Path path2) throws IOException { try (InputStream in1 = Files.newInputStream(path); InputStream in2 = Files.newInputStream(path2)) { + byte[] buffer1 = new byte[BUFFER_SIZE]; + byte[] buffer2 = new byte[BUFFER_SIZE]; + while (true) { int nRead1 = readNBytes(in1, buffer1); int nRead2 = readNBytes(in2, buffer2); @@ -119,17 +124,16 @@ public class SftpTransferTest extends AbstractSftpClientTestSupport { } } - private int readNBytes(InputStream is, byte[] b) throws IOException { + private static int readNBytes(InputStream is, byte[] b) throws IOException { int n = 0; int len = b.length; while (n < len) { int count = is.read(b, n, len - n); if (count < 0) { - break; + return n; } n += count; } return n; } - }