Skip to content

Commit 17e9a4b

Browse files
dagnirdagnir
authored
Don't special case 0 TTL in Apache 5 (#6606)
* Define infinite TTL as <= 0 This commit updates the definition of the CONNECTION_TIME_TO_LIVE (TTL) so that non-positive values indicate an infinite TTL. The reason for this is that in contrast to Apache 4.x, Apache 5.x uses 0 to indicate immediate expiry. This updated definition allows us to use a new default value of -1 across all supported HTTP clients to indicate an infinite TTL. Note that in practice, this is already the case, as all clients other than the Apache 5.x client treat non-positive values as infinite: - For Apache 4.x, value of <= 0 is treated as infinite: https://github.com/apache/httpcomponents-core/blob/a5c117028b7c620974304636d52f06f172f1d08b/httpcore/src/main/java/org/apache/http/pool/PoolEntry.java#L87-L93 - For Netty, `OldConnectionReaperHandler` is only used when the TTL value > 0, otherwise the handler is not added to the channel's pipeline: https://github.com/aws/aws-sdk-java-v2/blob/90f17ab300e27bee5367e4eff30b49bfa1f06af0/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/ChannelPipelineInitializer.java#L213-L216 - CRT and URLConnection client does not support this configuration. * Don't special case 0 TTL in Apache 5 Shadow the global default CONNECTION_TIME_TO_LIVE value of 0 to -1 within the Apache 5 client instead of special casing the 0 value when building the connection config. * Revert "Define infinite TTL as <= 0" This reverts commit 86a120a. * Add tests * Add changelog * Remove check * Review comments * Update javadoc,changelog
1 parent 2392409 commit 17e9a4b

File tree

3 files changed

+215
-6
lines changed

3 files changed

+215
-6
lines changed

‎.changes/next-release/bugfix-Apache5HTTPClientPreview-7f5d968.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "bugfix",
3+
"category": "Apache 5 HTTP Client (Preview)",
4+
"contributor": "",
5+
"description": "Ignore negative values set `connectionTimeToLive`. There is no behavior change on the client as negative values have no meaning for Apache 5."
6+
}

‎http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/Apache5HttpClient.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -453,10 +453,8 @@ public interface Builder extends SdkHttpClient.Builder<Apache5HttpClient.Builder
453453

454454

455455
/**
456-
* The maximum amount of time that a connection should be allowed to remain open, regardless of usage frequency.
457-
*
458-
* <p>Note: A duration of 0 is treated as infinite to maintain backward compatibility with Apache 4.x behavior.
459-
* The SDK handles this internally by not setting the TTL when the value is 0.</p>
456+
* The maximum amount of time that a connection should be allowed to remain open, regardless of usage frequency. Only
457+
* positive values have an effect.
460458
*/
461459
Builder connectionTimeToLive(Duration connectionTimeToLive);
462460

@@ -769,8 +767,9 @@ private static ConnectionConfig getConnectionConfig(AttributeMap standardOptions
769767
.setSocketTimeout(Timeout.ofMilliseconds(
770768
standardOptions.get(SdkHttpConfigurationOption.READ_TIMEOUT).toMillis()));
771769
Duration connectionTtl = standardOptions.get(SdkHttpConfigurationOption.CONNECTION_TIME_TO_LIVE);
772-
if (!connectionTtl.isZero()) {
773-
// Skip TTL=0 to maintain backward compatibility (infinite in 4.x vs immediate expiration in 5.x)
770+
// Only accept positive values.
771+
// Note: TTL=0 is infinite in 4.x vs immediate expiration in 5.x
772+
if (!connectionTtl.isNegative() && !connectionTtl.isZero()) {
774773
connectionConfigBuilder.setTimeToLive(TimeValue.ofMilliseconds(connectionTtl.toMillis()));
775774
}
776775
return connectionConfigBuilder.build();

‎http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/ConnectionTtlTest.java‎

Lines changed: 204 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
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+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.http.apache5;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
20+
import com.github.tomakehurst.wiremock.WireMockServer;
21+
import com.github.tomakehurst.wiremock.client.WireMock;
22+
import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
23+
import java.io.IOException;
24+
import java.net.Socket;
25+
import java.net.URI;
26+
import java.security.cert.X509Certificate;
27+
import java.time.Duration;
28+
import java.util.ArrayList;
29+
import java.util.List;
30+
import javax.net.ssl.KeyManager;
31+
import javax.net.ssl.SSLContext;
32+
import javax.net.ssl.SSLSocket;
33+
import javax.net.ssl.TrustManager;
34+
import javax.net.ssl.X509TrustManager;
35+
import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
36+
import org.apache.hc.core5.http.protocol.HttpContext;
37+
import org.junit.jupiter.api.AfterAll;
38+
import org.junit.jupiter.api.AfterEach;
39+
import org.junit.jupiter.api.BeforeAll;
40+
import org.junit.jupiter.api.Test;
41+
import software.amazon.awssdk.http.HttpExecuteRequest;
42+
import software.amazon.awssdk.http.HttpExecuteResponse;
43+
import software.amazon.awssdk.http.SdkHttpClient;
44+
import software.amazon.awssdk.http.SdkHttpFullRequest;
45+
import software.amazon.awssdk.http.SdkHttpMethod;
46+
import software.amazon.awssdk.http.SdkHttpRequest;
47+
import software.amazon.awssdk.http.SystemPropertyTlsKeyManagersProvider;
48+
import software.amazon.awssdk.http.apache5.internal.conn.SdkTlsSocketFactory;
49+
import software.amazon.awssdk.utils.IoUtils;
50+
51+
public class ConnectionTtlTest {
52+
private static WireMockServer wireMockServer;
53+
private SdkHttpClient apache5;
54+
55+
@BeforeAll
56+
public static void setup() {
57+
wireMockServer = new WireMockServer(WireMockConfiguration.options().dynamicPort().dynamicHttpsPort());
58+
wireMockServer.start();
59+
60+
wireMockServer.stubFor(WireMock.get(WireMock.anyUrl())
61+
.willReturn(WireMock.aResponse()
62+
.withStatus(200)
63+
.withBody("Hello there!")));
64+
}
65+
66+
@AfterEach
67+
public void methodTeardown() {
68+
if (apache5 != null) {
69+
apache5.close();
70+
apache5 = null;
71+
}
72+
}
73+
74+
@AfterAll
75+
public static void teardown() {
76+
wireMockServer.stop();
77+
}
78+
79+
@Test
80+
public void execute_ttlDefault_connectionNotClosed() throws Exception {
81+
TestTlsSocketStrategy socketStrategy = TestTlsSocketStrategy.create();
82+
83+
apache5 = Apache5HttpClient.builder()
84+
.connectionMaxIdleTime(Duration.ofDays(1))
85+
.tlsSocketStrategy(socketStrategy)
86+
.build();
87+
88+
doGetCall(apache5);
89+
doGetCall(apache5);
90+
91+
List<SSLSocket> sockets = socketStrategy.getCreatedSockets();
92+
assertThat(sockets).hasSize(1);
93+
}
94+
95+
@Test
96+
public void execute_ttlNegative_connectionNotClosed() throws Exception {
97+
TestTlsSocketStrategy socketStrategy = TestTlsSocketStrategy.create();
98+
99+
apache5 = Apache5HttpClient.builder()
100+
.connectionTimeToLive(Duration.ofMillis(-1))
101+
.connectionMaxIdleTime(Duration.ofDays(1))
102+
.tlsSocketStrategy(socketStrategy)
103+
.build();
104+
105+
doGetCall(apache5);
106+
doGetCall(apache5);
107+
108+
List<SSLSocket> sockets = socketStrategy.getCreatedSockets();
109+
assertThat(sockets).hasSize(1);
110+
}
111+
112+
@Test
113+
public void execute_ttlIsZero_connectionNotClosed() throws Exception {
114+
TestTlsSocketStrategy socketStrategy = TestTlsSocketStrategy.create();
115+
116+
apache5 = Apache5HttpClient.builder()
117+
.connectionTimeToLive(Duration.ZERO)
118+
.connectionMaxIdleTime(Duration.ofDays(1))
119+
.tlsSocketStrategy(socketStrategy)
120+
.build();
121+
122+
doGetCall(apache5);
123+
doGetCall(apache5);
124+
125+
List<SSLSocket> sockets = socketStrategy.getCreatedSockets();
126+
assertThat(sockets).hasSize(1);
127+
}
128+
129+
@Test
130+
public void execute_ttlIsShort_idleExceedsTtl_connectionClosed() throws Exception {
131+
TestTlsSocketStrategy socketStrategy = TestTlsSocketStrategy.create();
132+
133+
long ttlMs = 5;
134+
135+
apache5 = Apache5HttpClient.builder()
136+
.connectionTimeToLive(Duration.ofMillis(ttlMs))
137+
.connectionMaxIdleTime(Duration.ofDays(1))
138+
.tlsSocketStrategy(socketStrategy)
139+
.build();
140+
141+
doGetCall(apache5);
142+
Thread.sleep(ttlMs * 10);
143+
doGetCall(apache5);
144+
145+
List<SSLSocket> sockets = socketStrategy.getCreatedSockets();
146+
// second request should have created a second socket as the first goes over TTL
147+
assertThat(sockets).hasSize(2);
148+
}
149+
150+
private void doGetCall(SdkHttpClient apache) throws IOException {
151+
SdkHttpRequest sdkRequest = SdkHttpFullRequest.builder()
152+
.method(SdkHttpMethod.GET)
153+
.uri(URI.create("https://localhost:" + wireMockServer.httpsPort()))
154+
.build();
155+
156+
HttpExecuteRequest executeRequest = HttpExecuteRequest.builder().request(sdkRequest).build();
157+
158+
HttpExecuteResponse response = apache.prepareRequest(executeRequest).call();
159+
IoUtils.drainInputStream(response.responseBody().get());
160+
}
161+
162+
private static class TestTlsSocketStrategy extends SdkTlsSocketFactory {
163+
private List<SSLSocket> sslSockets = new ArrayList<>();
164+
165+
TestTlsSocketStrategy(SSLContext ctx) {
166+
super(ctx, NoopHostnameVerifier.INSTANCE);
167+
}
168+
169+
@Override
170+
public SSLSocket upgrade(Socket socket, String target, int port, Object attachment, HttpContext context) throws IOException {
171+
SSLSocket upgradedSocket = super.upgrade(socket, target, port, attachment, context);
172+
sslSockets.add(upgradedSocket);
173+
return upgradedSocket;
174+
}
175+
176+
List<SSLSocket> getCreatedSockets() {
177+
return sslSockets;
178+
}
179+
180+
static TestTlsSocketStrategy create() throws Exception {
181+
KeyManager[] keyManagers = SystemPropertyTlsKeyManagersProvider.create().keyManagers();
182+
183+
TrustManager[] trustManagers = {
184+
new X509TrustManager() {
185+
@Override
186+
public void checkClientTrusted(X509Certificate[] x509Certificates, String s) {
187+
}
188+
189+
@Override
190+
public void checkServerTrusted(X509Certificate[] x509Certificates, String s) {
191+
}
192+
193+
@Override
194+
public X509Certificate[] getAcceptedIssuers() {
195+
return new X509Certificate[0];
196+
}
197+
}
198+
};
199+
SSLContext ssl = SSLContext.getInstance("SSL");
200+
ssl.init(keyManagers, trustManagers, null);
201+
return new TestTlsSocketStrategy(ssl);
202+
}
203+
}
204+
}

0 commit comments

Comments
 (0)