DefaultSftpSessionFactory timeout of 0 is no longer 'no timeout'

[SLepUbIn]

In what version(s) of Spring Integration are you seeing this issue?

spring integration core 6.1.4

Describe the bug

Before migrating to spring boot 3/ spring integration 6, setting 0 as the timeout on DefaultSftpSessionFactory meant no timeout, as per the documentation. The doc still states that in version 6.1.4 :

/**
	 * 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.
	 * @see org.apache.sshd.client.future.ConnectFuture#verify(long)
	 */
	public void setTimeout(Integer timeout) {
		this.timeout = timeout;
	}

But in the code in DefaultSftpSessionFactory#initClientSession we have :

Duration verifyTimeout = this.timeout != null ? Duration.ofMillis(this.timeout) : null;
		HostConfigEntry config = this.hostConfig;
		if (config == null) {
			config = new HostConfigEntry(SshdSocketAddress.isIPv6Address(this.host) ? "" : this.host, this.host,
					this.port, this.user);
		}
		ClientSession clientSession =
				this.sshClient.connect(config)
						.verify(verifyTimeout)
						.getSession();

		clientSession.auth().verify(verifyTimeout);

So I have the impression that to obtain the previous behaviour we should call setTimeout with null instead of 0 because actually the timeout is effectively set to 0.

I discovered that in my unit tests, this is the resulting exception :

Caused by: org.apache.sshd.common.SshException: DefaultConnectFuture[toto@localhost/127.0.0.1:39899]: Failed to get operation result within specified timeout: 0

To Reproduce

My tests use the Fake sftp server from com.github.stefanbirkner.fakesftpserver.

And then use a session like this :

DefaultSftpSessionFactory factory = new DefaultSftpSessionFactory(true);
factory.setTimeout(0);

Expected behavior

If this is confirmed, either change the docs to the new behaviour stating that users should pass null and not 0, or change the current code and check that if timeout is 0 the verifyTimeout should not timeout.

[artembilan]

We need to determine for ourselves what is "no timeout".
In my feeling 0 really means no time out: we are not going to wait for the operation to finish and just rely on its immediate result (if any).
Setting that option to null (which is currently default) means to wait for result indefinitely:

    default T verify(CancelOption... options) throws IOException {
        return verify(Long.MAX_VALUE, options);
    }

So, tell me, please, what your thoughts on the matter.
However from sensitivity perspective neither 0, nor Long.MAX_VALUE are sufficient.
With 0 we just don't wait for an async operation in Apache MINA.
With Long.MAX_VALUE we might be blocked forever not knowing what is going on.
Therefore when we come to the page for this timeout understanding, I'm suggesting to have that timeout as 30s by default as we do in many other places in Spring Integration:

	/**
	 * The default timeout for blocking operations like send and receive messages.
	 * @since 6.1
	 */
	public static final long DEFAULT_TIMEOUT = 30000L;

[SLepUbIn]

@artembilan your reasoning sounds good to me.
I'm ok with the idea of having to set the timeout to null to have an "inifinite" waiting time. It just needs to be more explicit in the javadoc.