[GH-328] Added configurable timeout(s) to DefaultSftpClient

apache · Oct 5, 2023 · 6dade0c · 6dade0c
1 parent 1283515
commit 6dade0c
Show file tree

Hide file tree

Showing 7 changed files with 28 additions and 10 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -26,6 +26,7 @@
 
 ## Bug Fixes
 
+* [GH-328](https://github.com/apache/mina-sshd/issues/328) Added configurable timeout(s) to `DefaultSftpClient`.
 * [GH-370](https://github.com/apache/mina-sshd/issues/370) Also compare file keys in `ModifiableFileWatcher`.
 * [GH-371](https://github.com/apache/mina-sshd/issues/371) Fix channel pool in `SftpFileSystem`.
 * [GH-383](https://github.com/apache/mina-sshd/issues/383) Use correct default `OpenOption`s in `SftpFileSystemProvider.newFileChannel()`.

diff --git a/sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java b/sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java
@@ -655,7 +655,7 @@ public void operationComplete(OpenFuture future) {
                                     @Override
                                     public void operationComplete(IoReadFuture future) {
                                         try {
-                                            future.verify();
+                                            future.verify(OPEN_TIMEOUT);
                                             Buffer buffer = future.getBuffer();
                                             baosOut.write(buffer.array(), buffer.rpos(), buffer.available());
                                             buffer.rpos(buffer.rpos() + buffer.available());
@@ -675,7 +675,7 @@ public void operationComplete(IoReadFuture future) {
                                     @Override
                                     public void operationComplete(IoReadFuture future) {
                                         try {
-                                            future.verify();
+                                            future.verify(OPEN_TIMEOUT);
                                             Buffer buffer = future.getBuffer();
                                             baosErr.write(buffer.array(), buffer.rpos(), buffer.available());
                                             buffer.rpos(buffer.rpos() + buffer.available());

diff --git a/sshd-core/src/test/java/org/apache/sshd/common/forward/Sshd1033Test.java b/sshd-core/src/test/java/org/apache/sshd/common/forward/Sshd1033Test.java
@@ -106,9 +106,10 @@ protected void doTest(boolean testLocal, boolean testDynamic) throws IOException
             client.setServerKeyVerifier(AcceptAllServerKeyVerifier.INSTANCE);
             client.start();
 
-            try (ClientSession session = client.connect("temp", "localhost", sshPort).verify().getClientSession()) {
+            try (ClientSession session
+                    = client.connect("temp", "localhost", sshPort).verify(CONNECT_TIMEOUT).getClientSession()) {
                 session.addPasswordIdentity("temp");
-                session.auth().verify();
+                session.auth().verify(AUTH_TIMEOUT);
 
                 if (testLocal) {
                     LOGGER.info("================== Local ==================");

diff --git a/sshd-core/src/test/java/org/apache/sshd/util/test/BaseTestSupport.java b/sshd-core/src/test/java/org/apache/sshd/util/test/BaseTestSupport.java
@@ -46,8 +46,8 @@ public abstract class BaseTestSupport extends JUnitTestSupport {
     public static final String TEST_LOCALHOST
             = System.getProperty("org.apache.sshd.test.localhost", SshdSocketAddress.LOCALHOST_IPV4);
 
-    public static final Duration CONNECT_TIMEOUT = CoreTestSupportUtils.getTimeout("connect", Duration.ofSeconds(7));
-    public static final Duration AUTH_TIMEOUT = CoreTestSupportUtils.getTimeout("auth", Duration.ofSeconds(5));
+    public static final Duration CONNECT_TIMEOUT = CoreTestSupportUtils.getTimeout("connect", Duration.ofSeconds(10));
+    public static final Duration AUTH_TIMEOUT = CoreTestSupportUtils.getTimeout("auth", Duration.ofSeconds(8));
     public static final Duration OPEN_TIMEOUT = CoreTestSupportUtils.getTimeout("open", Duration.ofSeconds(9));
     public static final Duration DEFAULT_TIMEOUT = CoreTestSupportUtils.getTimeout("default", Duration.ofSeconds(5));
     public static final Duration CLOSE_TIMEOUT = CoreTestSupportUtils.getTimeout("close", Duration.ofSeconds(15));

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
@@ -24,6 +24,7 @@
 import java.nio.channels.FileChannel;
 import java.nio.charset.Charset;
 import java.nio.file.attribute.FileTime;
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -36,6 +37,7 @@
 
 import org.apache.sshd.client.channel.ClientChannel;
 import org.apache.sshd.client.subsystem.AbstractSubsystemClient;
+import org.apache.sshd.common.Property;
 import org.apache.sshd.common.SshConstants;
 import org.apache.sshd.common.SshException;
 import org.apache.sshd.common.channel.Channel;
@@ -61,6 +63,12 @@ public abstract class AbstractSftpClient
         extends AbstractSubsystemClient
         implements FullAccessSftpClient, SftpErrorDataHandler {
     public static final int INIT_COMMAND_SIZE = Byte.BYTES /* command */ + Integer.BYTES /* version */;
+    /**
+     * Property that can be used on the {@link org.apache.sshd.common.FactoryManager} to control the internal timeout
+     * used by the client to complete the buffer sending in {@link #send(int, Buffer)}.
+     */
+    public static final Property<Duration> SFTP_CLIENT_CMD_TIMEOUT
+            = Property.duration("sftp-client-cmd-timeout", Duration.ofSeconds(30L));
 
     protected final SftpErrorDataHandler errorDataHandler;
 

diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/DefaultSftpClient.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/DefaultSftpClient.java
@@ -298,7 +298,8 @@ public int send(int cmd, Buffer buffer) throws IOException {
         ClientChannel clientChannel = getClientChannel();
         IoOutputStream asyncIn = clientChannel.getAsyncIn();
         IoWriteFuture writeFuture = asyncIn.writeBuffer(buf);
-        writeFuture.verify();
+        Duration cmdTimeout = SFTP_CLIENT_CMD_TIMEOUT.getRequired(clientChannel);
+        writeFuture.verify(cmdTimeout);
         return id;
     }
 
@@ -377,8 +378,9 @@ protected void init(ClientSession session, SftpVersionSelector initialVersionSel
         if (traceEnabled) {
             log.trace("init({}) send SSH_FXP_INIT - initial version={}", clientChannel, initialVersion);
         }
+
         IoWriteFuture writeFuture = asyncIn.writeBuffer(buf);
-        writeFuture.verify();
+        writeFuture.verify(initializationTimeout);
 
         if (traceEnabled) {
             log.trace("init({}) wait for SSH_FXP_INIT respose (timeout={})", clientChannel, initializationTimeout);

diff --git a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpPerformanceTest.java b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpPerformanceTest.java
@@ -27,6 +27,7 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.nio.file.StandardOpenOption;
+import java.time.Duration;
 import java.util.Arrays;
 
 import eu.rekawek.toxiproxy.model.ToxicDirection;
@@ -38,6 +39,7 @@
 import org.apache.sshd.common.keyprovider.KeyIdentityProvider;
 import org.apache.sshd.sftp.client.SftpClient.OpenMode;
 import org.apache.sshd.sftp.client.fs.SftpFileSystem;
+import org.apache.sshd.util.test.CoreTestSupportUtils;
 import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
@@ -48,6 +50,8 @@
 
 @Ignore("Special class used for development only - not really a test just useful to run as such")
 public class SftpPerformanceTest {
+    public static final Duration SFTP_CONNECT_TIMEOUT = CoreTestSupportUtils.getTimeout("sftp.connect", Duration.ofSeconds(30));
+    public static final Duration SFTP_AUTH_TIMEOUT = CoreTestSupportUtils.getTimeout("sftp.auth", Duration.ofSeconds(15));
 
     public static final String USERNAME = "foo";
     public static final String PASSWORD = "pass";
@@ -125,9 +129,11 @@ public ClientSession createClientSession(SshClient client, ContainerProxy proxy)
         final String ipAddressViaToxiproxy = proxy.getContainerIpAddress();
         final int portViaToxiproxy = proxy.getProxyPort();
 
-        ClientSession session = client.connect(USERNAME, ipAddressViaToxiproxy, portViaToxiproxy).verify().getClientSession();
+        ClientSession session = client.connect(USERNAME, ipAddressViaToxiproxy, portViaToxiproxy)
+                .verify(SFTP_CONNECT_TIMEOUT)
+                .getClientSession();
         session.addPasswordIdentity(PASSWORD);
-        session.auth().verify();
+        session.auth().verify(SFTP_AUTH_TIMEOUT);
         return session;
     }