Skip to content

Commit 238edf3

Browse files
committed
HTTPCLIENT-2386: Classic transport to use connect timeout as a default if TLS timeout has not been explicitly set
1 parent 2bd05c7 commit 238edf3

File tree

3 files changed

+28
-29
lines changed

3 files changed

+28
-29
lines changed

httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultHttpClientConnectionOperator.java

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
*/
2727
package org.apache.hc.client5.http.impl.io;
2828

29-
import javax.net.ssl.SSLSocket;
3029
import java.io.IOException;
3130
import java.net.InetSocketAddress;
3231
import java.net.Proxy;
@@ -36,13 +35,14 @@
3635
import java.util.Collections;
3736
import java.util.List;
3837

39-
import jdk.net.ExtendedSocketOptions;
40-
import jdk.net.Sockets;
38+
import javax.net.ssl.SSLSocket;
39+
4140
import org.apache.hc.client5.http.ConnectExceptionSupport;
4241
import org.apache.hc.client5.http.DnsResolver;
4342
import org.apache.hc.client5.http.SchemePortResolver;
4443
import org.apache.hc.client5.http.SystemDefaultDnsResolver;
4544
import org.apache.hc.client5.http.UnsupportedSchemeException;
45+
import org.apache.hc.client5.http.config.TlsConfig;
4646
import org.apache.hc.client5.http.impl.ConnPoolSupport;
4747
import org.apache.hc.client5.http.impl.DefaultSchemePortResolver;
4848
import org.apache.hc.client5.http.io.DetachedSocketFactory;
@@ -67,6 +67,9 @@
6767
import org.slf4j.Logger;
6868
import org.slf4j.LoggerFactory;
6969

70+
import jdk.net.ExtendedSocketOptions;
71+
import jdk.net.Sockets;
72+
7073
/**
7174
* Default implementation of {@link HttpClientConnectionOperator} used as default in Http client,
7275
* when no instance provided by user to {@link BasicHttpClientConnectionManager} or {@link
@@ -178,11 +181,10 @@ public void connect(
178181
Args.notNull(socketConfig, "Socket config");
179182
Args.notNull(context, "Context");
180183

181-
final Timeout soTimeout = socketConfig.getSoTimeout();
182184
final SocketAddress socksProxyAddress = socketConfig.getSocksProxyAddress();
183185
final Proxy socksProxy = socksProxyAddress != null ? new Proxy(Proxy.Type.SOCKS, socksProxyAddress) : null;
184186
if (unixDomainSocket != null) {
185-
connectToUnixDomainSocket(conn, endpointHost, endpointName, attachment, unixDomainSocket, connectTimeout, socketConfig, context, soTimeout);
187+
connectToUnixDomainSocket(conn, endpointHost, endpointName, attachment, unixDomainSocket, connectTimeout, socketConfig, context);
186188
return;
187189
}
188190

@@ -208,17 +210,16 @@ public void connect(
208210
socket.bind(localAddress);
209211
}
210212
conn.bind(socket);
211-
configureSocket(socket, socketConfig, soTimeout);
213+
configureSocket(socket, socketConfig);
212214
socket.connect(remoteAddress, TimeValue.isPositive(connectTimeout) ? connectTimeout.toMillisecondsIntBound() : 0);
213215
conn.bind(socket);
214216
onAfterSocketConnect(context, endpointHost);
215217
if (LOG.isDebugEnabled()) {
216218
LOG.debug("{} {} connected {}->{}", ConnPoolSupport.getId(conn), endpointHost, conn.getLocalAddress(), conn.getRemoteAddress());
217219
}
218-
conn.setSocketTimeout(soTimeout);
219220
final TlsSocketStrategy tlsSocketStrategy = tlsSocketStrategyLookup != null ? tlsSocketStrategyLookup.lookup(endpointHost.getSchemeName()) : null;
220221
if (tlsSocketStrategy != null) {
221-
upgradeToTls(conn, endpointHost, endpointName, attachment, context, tlsSocketStrategy, socket);
222+
upgradeToTls(conn, endpointHost, endpointName, connectTimeout, attachment, context, tlsSocketStrategy, socket);
222223
}
223224
return;
224225
} catch (final RuntimeException ex) {
@@ -240,15 +241,23 @@ public void connect(
240241
}
241242

242243
private void upgradeToTls(final ManagedHttpClientConnection conn, final HttpHost endpointHost,
243-
final NamedEndpoint endpointName, final Object attachment, final HttpContext context,
244-
final TlsSocketStrategy tlsSocketStrategy, final Socket socket) throws IOException {
244+
final NamedEndpoint endpointName, final Timeout connectTimeout, final Object attachment,
245+
final HttpContext context, final TlsSocketStrategy tlsSocketStrategy, final Socket socket)
246+
throws IOException {
245247
final NamedEndpoint tlsName = endpointName != null ? endpointName : endpointHost;
246248
onBeforeTlsHandshake(context, endpointHost);
247249
if (LOG.isDebugEnabled()) {
248250
LOG.debug("{} {} upgrading to TLS", ConnPoolSupport.getId(conn), tlsName);
249251
}
252+
final TlsConfig tlsConfig = attachment instanceof TlsConfig ? (TlsConfig) attachment : TlsConfig.DEFAULT;
253+
final int soTimeout = socket.getSoTimeout();
254+
final Timeout handshakeTimeout = tlsConfig.getHandshakeTimeout() != null ? tlsConfig.getHandshakeTimeout() : connectTimeout;
255+
if (handshakeTimeout != null) {
256+
socket.setSoTimeout(handshakeTimeout.toMillisecondsIntBound());
257+
}
250258
final SSLSocket sslSocket = tlsSocketStrategy.upgrade(socket, tlsName.getHostName(), tlsName.getPort(), attachment, context);
251259
conn.bind(sslSocket, socket);
260+
socket.setSoTimeout(soTimeout);
252261
onAfterTlsHandshake(context, endpointHost);
253262
if (LOG.isDebugEnabled()) {
254263
LOG.debug("{} {} upgraded to TLS", ConnPoolSupport.getId(conn), tlsName);
@@ -263,8 +272,7 @@ private void connectToUnixDomainSocket(
263272
final Path unixDomainSocket,
264273
final Timeout connectTimeout,
265274
final SocketConfig socketConfig,
266-
final HttpContext context,
267-
final Timeout soTimeout) throws IOException {
275+
final HttpContext context) throws IOException {
268276
onBeforeSocketConnect(context, endpointHost);
269277
if (LOG.isDebugEnabled()) {
270278
LOG.debug("{} connecting to {} ({})", endpointHost, unixDomainSocket, connectTimeout);
@@ -275,16 +283,15 @@ private void connectToUnixDomainSocket(
275283
final Socket socket = unixDomainSocketFactory.connectSocket(newSocket, unixDomainSocket,
276284
connectTimeout);
277285
conn.bind(socket);
278-
configureSocket(socket, socketConfig, soTimeout);
286+
configureSocket(socket, socketConfig);
279287
onAfterSocketConnect(context, endpointHost);
280288
if (LOG.isDebugEnabled()) {
281289
LOG.debug("{} {} connected to {}", ConnPoolSupport.getId(conn), endpointHost, unixDomainSocket);
282290
}
283-
conn.setSocketTimeout(soTimeout);
284291

285292
final TlsSocketStrategy tlsSocketStrategy = tlsSocketStrategyLookup != null ? tlsSocketStrategyLookup.lookup(endpointHost.getSchemeName()) : null;
286293
if (tlsSocketStrategy != null) {
287-
upgradeToTls(conn, endpointHost, endpointName, attachment, context, tlsSocketStrategy, socket);
294+
upgradeToTls(conn, endpointHost, endpointName, connectTimeout, attachment, context, tlsSocketStrategy, socket);
288295
}
289296
} catch (final RuntimeException ex) {
290297
Closer.closeQuietly(newSocket);
@@ -300,10 +307,10 @@ private void connectToUnixDomainSocket(
300307
}
301308

302309
@SuppressWarnings("Since15")
303-
private static void configureSocket(final Socket socket, final SocketConfig socketConfig,
304-
final Timeout soTimeout) throws IOException {
305-
if (soTimeout != null) {
306-
socket.setSoTimeout(soTimeout.toMillisecondsIntBound());
310+
private static void configureSocket(final Socket socket, final SocketConfig socketConfig) throws IOException {
311+
final Timeout socketTimeout = socketConfig.getSoTimeout();
312+
if (socketTimeout != null) {
313+
socket.setSoTimeout(socketTimeout.toMillisecondsIntBound());
307314
}
308315
socket.setReuseAddress(socketConfig.isSoReuseAddress());
309316
socket.setTcpNoDelay(socketConfig.isTcpNoDelay());

httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/DefaultAsyncClientConnectionOperator.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ public void completed(final IOSession session) {
165165
if (tlsStrategy != null) {
166166
try {
167167
final Timeout socketTimeout = connection.getSocketTimeout();
168-
final Timeout handshakeTimeout = tlsConfig.getHandshakeTimeout();
168+
final Timeout handshakeTimeout = tlsConfig.getHandshakeTimeout() != null ? tlsConfig.getHandshakeTimeout() : connectTimeout;
169169
final NamedEndpoint tlsName = endpointName != null ? endpointName : endpointHost;
170170
onBeforeTlsHandshake(context, endpointHost);
171171
if (LOG.isDebugEnabled()) {
@@ -175,7 +175,7 @@ public void completed(final IOSession session) {
175175
connection,
176176
tlsName,
177177
attachment,
178-
handshakeTimeout != null ? handshakeTimeout : connectTimeout,
178+
handshakeTimeout,
179179
new FutureContribution<TransportSecurityLayer>(future) {
180180

181181
@Override

httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,6 @@ private void executeHandshake(
220220
final SSLSocket upgradedSocket,
221221
final String target,
222222
final Object attachment) throws IOException {
223-
final TlsConfig tlsConfig = attachment instanceof TlsConfig ? (TlsConfig) attachment : TlsConfig.DEFAULT;
224-
225223
final SSLParameters sslParameters = upgradedSocket.getSSLParameters();
226224
if (supportedProtocols != null) {
227225
sslParameters.setProtocols(supportedProtocols);
@@ -238,17 +236,11 @@ private void executeHandshake(
238236
}
239237
upgradedSocket.setSSLParameters(sslParameters);
240238

241-
final Timeout handshakeTimeout = tlsConfig.getHandshakeTimeout();
242-
if (handshakeTimeout != null) {
243-
upgradedSocket.setSoTimeout(handshakeTimeout.toMillisecondsIntBound());
244-
}
245-
246239
initializeSocket(upgradedSocket);
247240

248241
if (LOG.isDebugEnabled()) {
249242
LOG.debug("Enabled protocols: {}", (Object) upgradedSocket.getEnabledProtocols());
250243
LOG.debug("Enabled cipher suites: {}", (Object) upgradedSocket.getEnabledCipherSuites());
251-
LOG.debug("Starting handshake ({})", handshakeTimeout);
252244
}
253245
upgradedSocket.startHandshake();
254246
verifySession(target, upgradedSocket.getSession());

0 commit comments

Comments
 (0)