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
The following commit(s) were added to refs/heads/master by this push:
new 6dade0cdf [GH-328] Added configurable timeout(s) to DefaultSftpClient
6dade0cdf is described below
commit 6dade0cdfe70428efae7dbf72f4976e360bbd7be
Author: Lyor Goldstein <lgoldst...@apache.org>
AuthorDate: Tue Sep 19 19:24:42 2023 +0300
[GH-328] Added configurable timeout(s) to DefaultSftpClient
---
CHANGES.md | 1 +
sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java | 4 ++--
.../test/java/org/apache/sshd/common/forward/Sshd1033Test.java | 5 +++--
.../test/java/org/apache/sshd/util/test/BaseTestSupport.java | 4 ++--
.../org/apache/sshd/sftp/client/impl/AbstractSftpClient.java | 8 ++++++++
.../org/apache/sshd/sftp/client/impl/DefaultSftpClient.java | 6 ++++--
.../java/org/apache/sshd/sftp/client/SftpPerformanceTest.java | 10 ++++++++--
7 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index aae82ad34..9c3b51f95 100644
--- 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
index 3266c67ec..a41c2e1fc 100644
--- 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 class ClientTest extends BaseTestSupport {
@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 class ClientTest extends BaseTestSupport {
@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
index 0160939ce..6f166fdf1 100644
--- 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 @@ public class Sshd1033Test extends BaseTestSupport {
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
index 01c2a9bb4..9400ddb0c 100644
--- 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
index 1c7b24cf6..ecc90ec48 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
@@ -24,6 +24,7 @@ import java.io.OutputStream;
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 java.util.concurrent.atomic.AtomicReference;
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
index 25792a048..07372de88 100644
---
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 class DefaultSftpClient extends AbstractSftpClient {
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 @@ public class DefaultSftpClient extends AbstractSftpClient {
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
index 1e2c7449f..488a52f7b 100644
---
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.Files;
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.client.session.ClientSession;
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 @@ import
org.testcontainers.containers.ToxiproxyContainer.ContainerProxy;
@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 class SftpPerformanceTest {
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;
}
