diff --git a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/Apache5HttpClient.java b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/Apache5HttpClient.java index 135b76e05a0d..85f4de74dee7 100644 --- a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/Apache5HttpClient.java +++ b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/Apache5HttpClient.java @@ -44,6 +44,7 @@ import org.apache.hc.client5.http.auth.AuthSchemeFactory; import org.apache.hc.client5.http.auth.CredentialsProvider; import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase; +import org.apache.hc.client5.http.config.ConnectionConfig; import org.apache.hc.client5.http.impl.DefaultSchemePortResolver; import org.apache.hc.client5.http.impl.classic.HttpClientBuilder; import org.apache.hc.client5.http.impl.classic.HttpClients; @@ -70,6 +71,7 @@ import org.apache.hc.core5.pool.PoolStats; import org.apache.hc.core5.ssl.SSLInitializationException; import org.apache.hc.core5.util.TimeValue; +import org.apache.hc.core5.util.Timeout; import software.amazon.awssdk.annotations.SdkPreviewApi; import software.amazon.awssdk.annotations.SdkPublicApi; import software.amazon.awssdk.annotations.SdkTestInternalApi; @@ -343,7 +345,6 @@ private Apache5HttpRequestConfig createRequestConfig(DefaultBuilder builder, AttributeMap resolvedOptions) { return Apache5HttpRequestConfig.builder() .socketTimeout(resolvedOptions.get(SdkHttpConfigurationOption.READ_TIMEOUT)) - .connectionTimeout(resolvedOptions.get(SdkHttpConfigurationOption.CONNECTION_TIMEOUT)) .connectionAcquireTimeout( resolvedOptions.get(SdkHttpConfigurationOption.CONNECTION_ACQUIRE_TIMEOUT)) .proxyConfiguration(builder.proxyConfiguration) @@ -728,17 +729,28 @@ public PoolingHttpClientConnectionManager create(Apache5HttpClient.DefaultBuilde .setSSLSocketFactory(sslsf) .setSchemePortResolver(DefaultSchemePortResolver.INSTANCE) .setDnsResolver(configuration.dnsResolver); - Duration connectionTtl = standardOptions.get(SdkHttpConfigurationOption.CONNECTION_TIME_TO_LIVE); - if (!connectionTtl.isZero()) { - // Skip TTL=0 to maintain backward compatibility (infinite in 4.x vs immediate expiration in 5.x) - builder.setConnectionTimeToLive(TimeValue.of(connectionTtl.toMillis(), TimeUnit.MILLISECONDS)); - } builder.setMaxConnPerRoute(standardOptions.get(SdkHttpConfigurationOption.MAX_CONNECTIONS)); builder.setMaxConnTotal(standardOptions.get(SdkHttpConfigurationOption.MAX_CONNECTIONS)); builder.setDefaultSocketConfig(buildSocketConfig(standardOptions)); + builder.setDefaultConnectionConfig(getConnectionConfig(standardOptions)); return builder.build(); } + private static ConnectionConfig getConnectionConfig(AttributeMap standardOptions) { + ConnectionConfig.Builder connectionConfigBuilder = + ConnectionConfig.custom() + .setConnectTimeout(Timeout.ofMilliseconds( + standardOptions.get(SdkHttpConfigurationOption.CONNECTION_TIMEOUT).toMillis())) + .setSocketTimeout(Timeout.ofMilliseconds( + standardOptions.get(SdkHttpConfigurationOption.READ_TIMEOUT).toMillis())); + Duration connectionTtl = standardOptions.get(SdkHttpConfigurationOption.CONNECTION_TIME_TO_LIVE); + if (!connectionTtl.isZero()) { + // Skip TTL=0 to maintain backward compatibility (infinite in 4.x vs immediate expiration in 5.x) + connectionConfigBuilder.setTimeToLive(TimeValue.ofMilliseconds(connectionTtl.toMillis())); + } + return connectionConfigBuilder.build(); + } + private SSLConnectionSocketFactory getPreferredSocketFactory(Apache5HttpClient.DefaultBuilder configuration, AttributeMap standardOptions) { return Optional.ofNullable(configuration.socketFactory) diff --git a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/Apache5HttpRequestConfig.java b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/Apache5HttpRequestConfig.java index 92bf31c33bfa..159ae9aceca9 100644 --- a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/Apache5HttpRequestConfig.java +++ b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/Apache5HttpRequestConfig.java @@ -27,14 +27,12 @@ public final class Apache5HttpRequestConfig { private final Duration socketTimeout; - private final Duration connectionTimeout; private final Duration connectionAcquireTimeout; private final boolean expectContinueEnabled; private final ProxyConfiguration proxyConfiguration; private Apache5HttpRequestConfig(Builder builder) { this.socketTimeout = builder.socketTimeout; - this.connectionTimeout = builder.connectionTimeout; this.connectionAcquireTimeout = builder.connectionAcquireTimeout; this.expectContinueEnabled = builder.expectContinueEnabled; this.proxyConfiguration = builder.proxyConfiguration; @@ -44,10 +42,6 @@ public Duration socketTimeout() { return socketTimeout; } - public Duration connectionTimeout() { - return connectionTimeout; - } - public Duration connectionAcquireTimeout() { return connectionAcquireTimeout; } @@ -73,7 +67,6 @@ public static Builder builder() { public static final class Builder { private Duration socketTimeout; - private Duration connectionTimeout; private Duration connectionAcquireTimeout; private boolean expectContinueEnabled; private ProxyConfiguration proxyConfiguration; @@ -86,11 +79,6 @@ public Builder socketTimeout(Duration socketTimeout) { return this; } - public Builder connectionTimeout(Duration connectionTimeout) { - this.connectionTimeout = connectionTimeout; - return this; - } - public Builder connectionAcquireTimeout(Duration connectionAcquireTimeout) { this.connectionAcquireTimeout = connectionAcquireTimeout; return this; diff --git a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/impl/Apache5HttpRequestFactory.java b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/impl/Apache5HttpRequestFactory.java index fc3ecf5c1cd6..e9eaad4cc968 100644 --- a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/impl/Apache5HttpRequestFactory.java +++ b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/impl/Apache5HttpRequestFactory.java @@ -52,7 +52,7 @@ public class Apache5HttpRequestFactory { private static final List IGNORE_HEADERS = Arrays.asList(HttpHeaders.CONTENT_LENGTH, HttpHeaders.HOST, HttpHeaders.TRANSFER_ENCODING); - public HttpUriRequestBase create(final HttpExecuteRequest request, final Apache5HttpRequestConfig requestConfig) { + public HttpUriRequestBase create(HttpExecuteRequest request, Apache5HttpRequestConfig requestConfig) { HttpUriRequestBase base = createApacheRequest(request, sanitizeUri(request.httpRequest())); addHeadersToRequest(base, request.httpRequest()); addRequestConfig(base, request.httpRequest(), requestConfig); @@ -90,12 +90,10 @@ private URI sanitizeUri(SdkHttpRequest request) { private void addRequestConfig(HttpUriRequestBase base, SdkHttpRequest request, Apache5HttpRequestConfig requestConfig) { - int connectTimeout = saturatedCast(requestConfig.connectionTimeout().toMillis()); int connectAcquireTimeout = saturatedCast(requestConfig.connectionAcquireTimeout().toMillis()); RequestConfig.Builder requestConfigBuilder = RequestConfig .custom() .setConnectionRequestTimeout(connectAcquireTimeout, TimeUnit.MILLISECONDS) - .setConnectTimeout(connectTimeout, TimeUnit.MILLISECONDS) .setResponseTimeout(saturatedCast(requestConfig.socketTimeout().toMillis()), TimeUnit.MILLISECONDS); /* diff --git a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/utils/Apache5Utils.java b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/utils/Apache5Utils.java index 5bd726463206..2b6afe771775 100644 --- a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/utils/Apache5Utils.java +++ b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/utils/Apache5Utils.java @@ -79,6 +79,7 @@ public static CredentialsProvider newProxyCredentialsProvider(ProxyConfiguration * Returns a new instance of NTCredentials used for proxy authentication. */ private static Credentials newNtCredentials(ProxyConfiguration proxyConfiguration) { + // Deprecated NTCredentials is used to maintain backward compatibility with Apache4. return new NTCredentials( proxyConfiguration.username(), proxyConfiguration.password().toCharArray(), diff --git a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientWireMockTest.java b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientWireMockTest.java index 613280251d49..6eb0e946d33c 100644 --- a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientWireMockTest.java +++ b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientWireMockTest.java @@ -17,6 +17,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; import static com.github.tomakehurst.wiremock.client.WireMock.any; +import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor; import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo; import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; import static org.assertj.core.api.Assertions.assertThat; @@ -28,10 +29,15 @@ import com.github.tomakehurst.wiremock.junit.WireMockRule; import com.github.tomakehurst.wiremock.matching.RequestPatternBuilder; import java.io.IOException; +import java.net.ConnectException; import java.net.HttpURLConnection; import java.net.InetAddress; +import java.net.SocketTimeoutException; import java.net.URI; import java.net.UnknownHostException; +import java.time.Duration; +import java.util.stream.Stream; +import org.apache.hc.client5.http.ConnectTimeoutException; import org.apache.hc.client5.http.DnsResolver; import org.apache.hc.client5.http.HttpRoute; import org.apache.hc.client5.http.SystemDefaultDnsResolver; @@ -279,4 +285,71 @@ public void closeReleasesResources() throws Exception { IllegalStateException.class ).hasMessageContaining("Connection pool shut down"); } + + @Test + public void connectionTimeout_exceedsLimit_throwsException() throws Exception { + // Test connection timeout with a very short timeout and non-responsive address + try (SdkHttpClient client = Apache5HttpClient.builder() + .connectionTimeout(Duration.ofMillis(100)) + .build()) { + + // Use a non-routable address to simulate connection timeout + // 192.0.2.1 is a reserved test address + SdkHttpFullRequest request = SdkHttpFullRequest.builder() + .uri(URI.create("http://192.0.2.1:8080/test")) + .method(SdkHttpMethod.GET) + .putHeader("Host", "192.0.2.1:8080") + .build(); + + assertThatThrownBy(() -> + client.prepareRequest(HttpExecuteRequest.builder().request(request).build()).call()) + .isInstanceOfAny( + ConnectTimeoutException.class, + ConnectException.class, + IOException.class) + .satisfies(exception -> { + // message vary based on JVM + String message = exception.getMessage().toLowerCase(); + boolean hasTimeoutMessage = Stream.of("timeout", "timed out", "read timeout") + .anyMatch(message::contains); + assertThat(hasTimeoutMessage).isTrue(); + }); + } + } + + @Test + public void socketTimeout_exceedsLimit_throwsException() throws Exception { + // Configure WireMock to delay response longer than socket timeout + mockServer.stubFor(any(urlPathEqualTo("/delayed")) + .willReturn(aResponse() + .withStatus(200) + .withBody("delayed response") + .withFixedDelay(2000))); + + try (SdkHttpClient client = Apache5HttpClient.builder() + .socketTimeout(Duration.ofMillis(500)) + .build()) { + + SdkHttpFullRequest request = SdkHttpFullRequest.builder() + .uri(URI.create("http://localhost:" + mockServer.port() + "/delayed")) + .method(SdkHttpMethod.GET) + .putHeader("Host", "localhost:" + mockServer.port()) + .putHeader("User-Agent", "test-client") + .build(); + + assertThatThrownBy(() -> + client.prepareRequest(HttpExecuteRequest.builder().request(request).build()).call()) + .isInstanceOfAny( + SocketTimeoutException.class, + IOException.class) + .satisfies(exception -> { + String message = exception.getMessage().toLowerCase(); + boolean hasTimeoutMessage = Stream.of("timeout", "timed out", "read timeout") + .anyMatch(message::contains); + assertThat(hasTimeoutMessage).isTrue(); + + }); + mockServer.verify(1, getRequestedFor(urlPathEqualTo("/delayed"))); + } + } } diff --git a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/MetricReportingTest.java b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/MetricReportingTest.java index 3c2529d697f7..936f57658959 100644 --- a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/MetricReportingTest.java +++ b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/MetricReportingTest.java @@ -109,7 +109,6 @@ public void prepareRequest_connectionManagerNotPooling_callableCalled_metricsRep private Apache5HttpClient newClient() { Apache5HttpRequestConfig config = Apache5HttpRequestConfig.builder() .connectionAcquireTimeout(Duration.ofDays(1)) - .connectionTimeout(Duration.ofDays(1)) .socketTimeout(Duration.ofDays(1)) .proxyConfiguration(ProxyConfiguration.builder().build()) .build(); diff --git a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/impl/ApacheHttpRequestFactoryTest.java b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/impl/ApacheHttpRequestFactoryTest.java index dbc2cc79db7f..5dcc215db480 100644 --- a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/impl/ApacheHttpRequestFactoryTest.java +++ b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/impl/ApacheHttpRequestFactoryTest.java @@ -50,7 +50,6 @@ public void setup() { instance = new Apache5HttpRequestFactory(); requestConfig = Apache5HttpRequestConfig.builder() .connectionAcquireTimeout(Duration.ZERO) - .connectionTimeout(Duration.ZERO) .socketTimeout(Duration.ZERO) .build(); }