Skip to content

Commit

Permalink
GH-8797: Fix DefSftpSessionFactory.timeout logic
Browse files Browse the repository at this point in the history
Fixes #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
  • Loading branch information
artembilan authored and tzolov committed Dec 4, 2023
1 parent 9f7acd7 commit ba287ae
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,13 @@
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;
Expand All @@ -44,9 +46,11 @@
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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -263,9 +267,9 @@ public void setAllowUnknownKeys(boolean allowUnknownKeys) {

/**
* 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) {
Expand Down Expand Up @@ -420,8 +424,10 @@ protected SftpClient createSftpClient(
/**
* 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();

Expand All @@ -442,6 +448,14 @@ public int send(int cmd, Buffer buffer) throws IOException {
}
}

@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;
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,13 @@

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;

Expand All @@ -48,6 +50,7 @@
* @author Gary Russell
* @author Artem Bilan
* @author Auke Zaaiman
*
* @since 3.0.2
*/
public class SftpSessionFactoryTests {
Expand Down Expand Up @@ -192,4 +195,27 @@ void concurrentSessionListDoesntCauseFailure() throws IOException {
}
}

@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);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit ba287ae

Please sign in to comment.