GH-8797: Fix DefSftpSessionFactory.timeout logic

Fixes https://github.com/spring-projects/spring-integration/issues/8797

After migration to Apache MINA we have missed to fix `DefaultSftpSessionFactory.timeout`
to be `0` by default as it states in its Javadocs and reference manual
It is `null` by default which really means an infinite wait.

* Fix `DefaultSftpSessionFactory.timeout` to be a reasonable 30 seconds by default
* Fix `setTimeout()` Javadocs and respective `session-factory.adoc`
* Propagate this `timeout` down to the `SftpClient` for its commands interactions
This commit is contained in:
Artem Bilan
2023-11-28 13:50:54 -05:00
committed by Christian Tzolov
parent 9f7acd7d9d
commit ba287aeb5b
3 changed files with 47 additions and 6 deletions

View File

@@ -28,11 +28,13 @@ import java.util.concurrent.locks.ReentrantLock;
import org.apache.sshd.client.SshClient;
import org.apache.sshd.client.auth.keyboard.UserInteraction;
import org.apache.sshd.client.auth.password.PasswordIdentityProvider;
import org.apache.sshd.client.channel.ChannelSubsystem;
import org.apache.sshd.client.config.hosts.HostConfigEntry;
import org.apache.sshd.client.keyverifier.AcceptAllServerKeyVerifier;
import org.apache.sshd.client.keyverifier.RejectAllServerKeyVerifier;
import org.apache.sshd.client.keyverifier.ServerKeyVerifier;
import org.apache.sshd.client.session.ClientSession;
import org.apache.sshd.common.PropertyResolverUtils;
import org.apache.sshd.common.SshConstants;
import org.apache.sshd.common.config.keys.FilePasswordProvider;
import org.apache.sshd.common.keyprovider.KeyIdentityProvider;
@@ -44,9 +46,11 @@ import org.apache.sshd.common.util.security.SecurityUtils;
import org.apache.sshd.sftp.client.SftpClient;
import org.apache.sshd.sftp.client.SftpErrorDataHandler;
import org.apache.sshd.sftp.client.SftpVersionSelector;
import org.apache.sshd.sftp.client.impl.AbstractSftpClient;
import org.apache.sshd.sftp.client.impl.DefaultSftpClient;
import org.springframework.core.io.Resource;
import org.springframework.integration.context.IntegrationContextUtils;
import org.springframework.integration.file.remote.session.SessionFactory;
import org.springframework.integration.file.remote.session.SharedSessionCapable;
import org.springframework.util.Assert;
@@ -107,7 +111,7 @@ public class DefaultSftpSessionFactory implements SessionFactory<SftpClient.DirE
private boolean allowUnknownKeys = false;
private Integer timeout;
private Integer timeout = (int) IntegrationContextUtils.DEFAULT_TIMEOUT;
private SftpVersionSelector sftpVersionSelector = SftpVersionSelector.CURRENT;
@@ -263,9 +267,9 @@ public class DefaultSftpSessionFactory implements SessionFactory<SftpClient.DirE
/**
* The timeout property is used as the socket timeout parameter, as well as
* the default connection timeout. Defaults to <code>0</code>, which means,
* that no timeout will occur.
* @param timeout The timeout.
* the default connection timeout. Defaults to {@code 30 seconds}.
* Setting to {@code 0} means no timeout; to {@code null} - infinite wait.
* @param timeout the timeout.
* @see org.apache.sshd.client.future.ConnectFuture#verify(Duration, org.apache.sshd.common.future.CancelOption...)
*/
public void setTimeout(Integer timeout) {
@@ -420,8 +424,10 @@ public class DefaultSftpSessionFactory implements SessionFactory<SftpClient.DirE
/**
* The {@link DefaultSftpClient} extension to lock the {@link #send(int, Buffer)}
* for concurrent interaction.
* <p>
* Also sets the provided {@link #timeout} as a {@link AbstractSftpClient#SFTP_CLIENT_CMD_TIMEOUT} property.
*/
protected static class ConcurrentSftpClient extends DefaultSftpClient {
protected class ConcurrentSftpClient extends DefaultSftpClient {
private final Lock sendLock = new ReentrantLock();
@@ -442,6 +448,14 @@ public class DefaultSftpSessionFactory implements SessionFactory<SftpClient.DirE
}
}
@Override
protected ChannelSubsystem createSftpChannelSubsystem(ClientSession clientSession) {
ChannelSubsystem sftpChannelSubsystem = super.createSftpChannelSubsystem(clientSession);
PropertyResolverUtils.updateProperty(sftpChannelSubsystem,
AbstractSftpClient.SFTP_CLIENT_CMD_TIMEOUT.getName(), DefaultSftpSessionFactory.this.timeout);
return sftpChannelSubsystem;
}
}
}

View File

@@ -28,11 +28,13 @@ import java.util.stream.IntStream;
import org.apache.sshd.client.SshClient;
import org.apache.sshd.client.auth.password.PasswordIdentityProvider;
import org.apache.sshd.client.channel.ClientChannel;
import org.apache.sshd.client.keyverifier.AcceptAllServerKeyVerifier;
import org.apache.sshd.common.SshException;
import org.apache.sshd.server.SshServer;
import org.apache.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider;
import org.apache.sshd.sftp.client.SftpClient;
import org.apache.sshd.sftp.client.impl.AbstractSftpClient;
import org.apache.sshd.sftp.server.SftpSubsystemFactory;
import org.junit.jupiter.api.Test;
@@ -48,6 +50,7 @@ import static org.awaitility.Awaitility.await;
* @author Gary Russell
* @author Artem Bilan
* @author Auke Zaaiman
*
* @since 3.0.2
*/
public class SftpSessionFactoryTests {
@@ -192,4 +195,27 @@ public class SftpSessionFactoryTests {
}
}
@Test
void customTimeoutIsApplied() throws IOException {
try (SshServer server = SshServer.setUpDefaultServer()) {
server.setPasswordAuthenticator((arg0, arg1, arg2) -> true);
server.setPort(0);
server.setKeyPairProvider(new SimpleGeneratorHostKeyProvider(new File("hostkey.ser").toPath()));
server.setSubsystemFactories(Collections.singletonList(new SftpSubsystemFactory()));
server.start();
DefaultSftpSessionFactory sftpSessionFactory = new DefaultSftpSessionFactory();
sftpSessionFactory.setHost("localhost");
sftpSessionFactory.setPort(server.getPort());
sftpSessionFactory.setUser("user");
sftpSessionFactory.setPassword("pass");
sftpSessionFactory.setAllowUnknownKeys(true);
sftpSessionFactory.setTimeout(15_000);
ClientChannel clientChannel = sftpSessionFactory.getSession().getClientInstance().getClientChannel();
assertThat(AbstractSftpClient.SFTP_CLIENT_CMD_TIMEOUT.getRequired(clientChannel)).hasSeconds(15);
}
}
}

View File

@@ -97,7 +97,8 @@ The passphrase is obtained from that object.
Optional.
`timeout`::The timeout property is used as the socket timeout parameter, as well as the default connection timeout.
Defaults to `0`, which means, that no timeout will occur.
Defaults to `30 seconds`.
Setting to `0` means no timeout; to `null` - infinite wait.
[[sftp-unk-keys]]
`allowUnknownKeys`::Set to `true` to allow connections to hosts with unknown (or changed) keys.