Skip to content

Commit 1474ea6

Browse files
committed
Fix timeout configuration bug
TransportImpl now uses timeout from Config instead of hardcoded value. Added getTimeoutMs/setTimeoutMs to Config interface so users can configure timeout before creating SSH connection. Fixes #1028
1 parent 4be5228 commit 1474ea6

File tree

5 files changed

+99
-1
lines changed

5 files changed

+99
-1
lines changed

src/main/java/net/schmizz/sshj/Config.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,4 +204,19 @@ public interface Config {
204204
int getMaxCircularBufferSize();
205205

206206
void setMaxCircularBufferSize(int maxCircularBufferSize);
207+
208+
/**
209+
* Returns the timeout in milliseconds for blocking operations (e.g., authentication, channel open, etc.).
210+
* The default value is 30000 milliseconds (30 seconds).
211+
*
212+
* @return the timeout in milliseconds
213+
*/
214+
int getTimeoutMs();
215+
216+
/**
217+
* Sets the timeout in milliseconds for blocking operations (e.g., authentication, channel open, etc.).
218+
*
219+
* @param timeoutMs the timeout in milliseconds
220+
*/
221+
void setTimeoutMs(int timeoutMs);
207222
}

src/main/java/net/schmizz/sshj/ConfigImpl.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ public class ConfigImpl
5151
private boolean verifyHostKeyCertificates = true;
5252
// HF-982: default to 16MB buffers.
5353
private int maxCircularBufferSize = 16 * 1024 * 1024;
54+
private int timeoutMs = 30 * 1000; // Default to 30 seconds
5455

5556
@Override
5657
public List<Factory.Named<Cipher>> getCipherFactories() {
@@ -202,6 +203,16 @@ public void setVerifyHostKeyCertificates(boolean value) {
202203
verifyHostKeyCertificates = value;
203204
}
204205

206+
@Override
207+
public int getTimeoutMs() {
208+
return timeoutMs;
209+
}
210+
211+
@Override
212+
public void setTimeoutMs(int timeoutMs) {
213+
this.timeoutMs = timeoutMs;
214+
}
215+
205216
/**
206217
* Modern servers neglect the key algorithm ssh-rsa. OpenSSH 8.8 even dropped its support by default in favour
207218
* of rsa-sha2-*. However, there are legacy servers like Apache SSHD that don't support the newer replacements

src/main/java/net/schmizz/sshj/transport/TransportImpl.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ static final class ConnInfo {
9898
*/
9999
private final String clientID;
100100

101-
private volatile int timeoutMs = 30 * 1000; // Crazy long, but it was the original default
101+
private volatile int timeoutMs;
102102

103103
private volatile boolean authed = false;
104104

@@ -142,6 +142,7 @@ public TransportImpl(Config config) {
142142
this.decoder = new Decoder(this);
143143
this.kexer = new KeyExchanger(this);
144144
this.clientID = String.format("SSH-2.0-%s", config.getVersion());
145+
this.timeoutMs = config.getTimeoutMs();
145146
}
146147

147148
@Override

src/test/groovy/net/schmizz/sshj/ConfigImplSpec.groovy

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,4 +84,23 @@ class ConfigImplSpec extends Specification {
8484
then:
8585
config.keyAlgorithms == [ED25519, SSH_RSA, RSA_SHA_512, ECDSA, RSA_SHA_256]
8686
}
87+
88+
def "default timeout should be 30 seconds"() {
89+
given:
90+
def config = new DefaultConfig()
91+
92+
expect:
93+
config.getTimeoutMs() == 30000
94+
}
95+
96+
def "timeout can be set and retrieved"() {
97+
given:
98+
def config = new DefaultConfig()
99+
100+
when:
101+
config.setTimeoutMs(60000)
102+
103+
then:
104+
config.getTimeoutMs() == 60000
105+
}
87106
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*
2+
* Copyright (C)2009 - SSHJ Contributors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package net.schmizz.sshj.transport;
17+
18+
import net.schmizz.sshj.Config;
19+
import net.schmizz.sshj.DefaultConfig;
20+
import org.junit.jupiter.api.Test;
21+
22+
import static org.assertj.core.api.Assertions.assertThat;
23+
24+
class TransportImplTimeoutTest {
25+
26+
@Test
27+
void shouldUseDefaultTimeoutFromConfig() {
28+
Config config = new DefaultConfig();
29+
Transport transport = new TransportImpl(config);
30+
31+
assertThat(transport.getTimeoutMs()).isEqualTo(30000);
32+
}
33+
34+
@Test
35+
void shouldUseCustomTimeoutFromConfig() {
36+
Config config = new DefaultConfig();
37+
config.setTimeoutMs(60000);
38+
Transport transport = new TransportImpl(config);
39+
40+
assertThat(transport.getTimeoutMs()).isEqualTo(60000);
41+
}
42+
43+
@Test
44+
void shouldAllowChangingTimeoutAfterCreation() {
45+
Config config = new DefaultConfig();
46+
Transport transport = new TransportImpl(config);
47+
48+
transport.setTimeoutMs(45000);
49+
50+
assertThat(transport.getTimeoutMs()).isEqualTo(45000);
51+
}
52+
}

0 commit comments

Comments
 (0)