Skip to content

Commit c3cb46f

Browse files
authored
Preview API annotation added for Public APIs and TODOs addressed (#6215)
* Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams * writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this * Fix connectionPoolingWorks by setting skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x * disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json * Added Test case for Async , handled review ocmments * Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x * Fix compilation issues * Fix checkstyle issues * Remove test which are specific to apache http * Add benchmark for Apache5 and add Streaming Api test cases * Update Apache5 to 5.5 * Preview ready , addressing open TODOs * Added PublicApi since checkstyle was failing
1 parent 67d6690 commit c3cb46f

File tree

9 files changed

+234
-6
lines changed

9 files changed

+234
-6
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
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.apache;
17+
18+
19+
import org.junit.jupiter.api.DisplayName;
20+
import software.amazon.awssdk.http.SdkHttpClient;
21+
import software.amazon.awssdk.http.SdkHttpClientUriSanitizationTestSuite;
22+
23+
@DisplayName("Apache HTTP Client - URI Sanitization Tests")
24+
class ApacheHttpClientUriSanitizationTest extends SdkHttpClientUriSanitizationTestSuite {
25+
26+
@Override
27+
protected SdkHttpClient createHttpClient() {
28+
return ApacheHttpClient.create();
29+
}
30+
}

http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientWireMockTest.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
2121
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
2222
import static org.assertj.core.api.Assertions.assertThat;
23+
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
2324
import static org.mockito.Mockito.verify;
2425
import static org.mockito.Mockito.when;
2526
import static software.amazon.awssdk.http.SdkHttpConfigurationOption.TRUST_ALL_CERTIFICATES;
@@ -40,6 +41,7 @@
4041
import org.apache.http.conn.DnsResolver;
4142
import org.apache.http.conn.HttpClientConnectionManager;
4243
import org.apache.http.conn.routing.HttpRoute;
44+
import org.apache.http.impl.conn.ConnectionShutdownException;
4345
import org.apache.http.impl.conn.SystemDefaultDnsResolver;
4446
import org.junit.Rule;
4547
import org.junit.Test;
@@ -265,4 +267,23 @@ public InetAddress[] resolve(final String host) throws UnknownHostException {
265267

266268
mockProxyServer.verify(1, RequestPatternBuilder.allRequests());
267269
}
270+
271+
@Test
272+
public void closeReleasesResources() throws Exception {
273+
SdkHttpClient client = createSdkHttpClient();
274+
// Make a successful request first
275+
stubForMockRequest(200);
276+
SdkHttpFullRequest request = mockSdkRequest("http://localhost:" + mockServer.port(), SdkHttpMethod.POST);
277+
HttpExecuteResponse response = client.prepareRequest(
278+
HttpExecuteRequest.builder().request(request).build()).call();
279+
response.responseBody().ifPresent(IoUtils::drainInputStream);
280+
// Close the client
281+
client.close();
282+
// Verify subsequent requests fail
283+
assertThatThrownBy(() -> {
284+
client.prepareRequest(HttpExecuteRequest.builder().request(request).build()).call();
285+
}).isInstanceOfAny(
286+
IllegalStateException.class
287+
).hasMessageContaining("Connection pool shut down");
288+
}
268289
}

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
import org.apache.hc.core5.pool.PoolStats;
6666
import org.apache.hc.core5.ssl.SSLInitializationException;
6767
import org.apache.hc.core5.util.TimeValue;
68+
import software.amazon.awssdk.annotations.SdkPreviewApi;
6869
import software.amazon.awssdk.annotations.SdkPublicApi;
6970
import software.amazon.awssdk.annotations.SdkTestInternalApi;
7071
import software.amazon.awssdk.http.AbortableInputStream;
@@ -94,7 +95,6 @@
9495
import software.amazon.awssdk.utils.Logger;
9596
import software.amazon.awssdk.utils.Validate;
9697

97-
// TODO: All the Java Doc will be updated to consider the reference of Apache4.x if required
9898
/**
9999
* An implementation of {@link SdkHttpClient} that uses Apache5 HTTP client to communicate with the service. This is the most
100100
* powerful synchronous client that adds an extra dependency and additional startup latency in exchange for more functionality,
@@ -104,6 +104,7 @@
104104
*
105105
* <p>This can be created via {@link #builder()}</p>
106106
*/
107+
@SdkPreviewApi
107108
@SdkPublicApi
108109
public final class Apache5HttpClient implements SdkHttpClient {
109110

@@ -255,7 +256,6 @@ public void abort() {
255256
public void close() {
256257
HttpClientConnectionManager cm = httpClient.getHttpClientConnectionManager();
257258
IdleConnectionReaper.getInstance().deregisterConnectionManager(cm);
258-
// TODO : need to add test cases for this
259259
cm.close(CloseMode.IMMEDIATE);
260260
}
261261

@@ -409,8 +409,12 @@ public interface Builder extends SdkHttpClient.Builder<Apache5HttpClient.Builder
409409
*/
410410
Builder expectContinueEnabled(Boolean expectContinueEnabled);
411411

412+
412413
/**
413414
* The maximum amount of time that a connection should be allowed to remain open, regardless of usage frequency.
415+
*
416+
* <p>Note: A duration of 0 is treated as infinite to maintain backward compatibility with Apache 4.x behavior.
417+
* The SDK handles this internally by not setting the TTL when the value is 0.</p>
414418
*/
415419
Builder connectionTimeToLive(Duration connectionTimeToLive);
416420

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,15 @@
1515

1616
package software.amazon.awssdk.http.apache5;
1717

18+
import software.amazon.awssdk.annotations.SdkPreviewApi;
1819
import software.amazon.awssdk.annotations.SdkPublicApi;
1920
import software.amazon.awssdk.http.SdkHttpClient;
2021
import software.amazon.awssdk.http.SdkHttpService;
2122

2223
/**
2324
* Service binding for the Apache5 implementation.
2425
*/
26+
@SdkPreviewApi
2527
@SdkPublicApi
2628
public class Apache5SdkHttpService implements SdkHttpService {
2729
@Override

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.Collections;
2323
import java.util.HashSet;
2424
import java.util.Set;
25+
import software.amazon.awssdk.annotations.SdkPreviewApi;
2526
import software.amazon.awssdk.annotations.SdkPublicApi;
2627
import software.amazon.awssdk.utils.ProxyConfigProvider;
2728
import software.amazon.awssdk.utils.ProxySystemSetting;
@@ -33,6 +34,7 @@
3334
/**
3435
* Configuration that defines how to communicate via an HTTP or HTTPS proxy.
3536
*/
37+
@SdkPreviewApi
3638
@SdkPublicApi
3739
public final class ProxyConfiguration implements ToCopyableBuilder<ProxyConfiguration.Builder, ProxyConfiguration> {
3840
private final URI endpoint;
@@ -437,7 +439,7 @@ public Builder scheme(String scheme) {
437439
return this;
438440
}
439441

440-
public void setuseEnvironmentVariableValues(Boolean useEnvironmentVariableValues) {
442+
public void setUseEnvironmentVariableValues(Boolean useEnvironmentVariableValues) {
441443
useEnvironmentVariableValues(useEnvironmentVariableValues);
442444
}
443445

http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/impl/Apache5HttpRequestFactory.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
import software.amazon.awssdk.utils.http.SdkHttpUtils;
4545

4646
/**
47-
* Responsible for creating Apache HttpClient 4 request objects.
47+
* Responsible for creating Apache HttpClient 5 request objects.
4848
*/
4949
@SdkInternalApi
5050
public class Apache5HttpRequestFactory {
@@ -59,7 +59,6 @@ public HttpUriRequestBase create(final HttpExecuteRequest request, final Apache5
5959
return base;
6060
}
6161

62-
//TODO : check if this is still valid
6362
/**
6463
*
6564
* The Apache HTTP client doesn't allow consecutive slashes in the URI. For S3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
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+
19+
import org.junit.jupiter.api.DisplayName;
20+
import software.amazon.awssdk.http.SdkHttpClient;
21+
import software.amazon.awssdk.http.SdkHttpClientUriSanitizationTestSuite;
22+
23+
@DisplayName("Apache5 HTTP Client - URI Sanitization Tests")
24+
class Apache5HttpClientUriSanitizationTest extends SdkHttpClientUriSanitizationTestSuite {
25+
26+
@Override
27+
protected SdkHttpClient createHttpClient() {
28+
return Apache5HttpClient.create();
29+
}
30+
}

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
2121
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
2222
import static org.assertj.core.api.Assertions.assertThat;
23+
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
2324
import static org.mockito.Mockito.verify;
2425
import static org.mockito.Mockito.when;
2526
import static software.amazon.awssdk.http.SdkHttpConfigurationOption.TRUST_ALL_CERTIFICATES;
@@ -41,7 +42,6 @@
4142
import org.apache.hc.client5.http.io.HttpClientConnectionManager;
4243
import org.apache.hc.core5.http.HttpHost;
4344
import org.apache.hc.core5.io.CloseMode;
44-
import org.junit.Ignore;
4545
import org.junit.Rule;
4646
import org.junit.Test;
4747
import org.junit.runner.RunWith;
@@ -260,4 +260,23 @@ public InetAddress[] resolve(String host) throws UnknownHostException {
260260

261261
mockProxyServer.verify(1, RequestPatternBuilder.allRequests());
262262
}
263+
264+
@Test
265+
public void closeReleasesResources() throws Exception {
266+
SdkHttpClient client = createSdkHttpClient();
267+
// Make a successful request first
268+
stubForMockRequest(200);
269+
SdkHttpFullRequest request = mockSdkRequest("http://localhost:" + mockServer.port(), SdkHttpMethod.POST);
270+
HttpExecuteResponse response = client.prepareRequest(
271+
HttpExecuteRequest.builder().request(request).build()).call();
272+
response.responseBody().ifPresent(IoUtils::drainInputStream);
273+
// Close the client
274+
client.close();
275+
// Verify subsequent requests fail
276+
assertThatThrownBy(() -> {
277+
client.prepareRequest(HttpExecuteRequest.builder().request(request).build()).call();
278+
}).isInstanceOfAny(
279+
IllegalStateException.class
280+
).hasMessageContaining("Connection pool shut down");
281+
}
263282
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
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;
17+
18+
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
19+
import static com.github.tomakehurst.wiremock.client.WireMock.any;
20+
import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl;
21+
import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor;
22+
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
23+
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
24+
import static org.assertj.core.api.Assertions.assertThat;
25+
26+
import com.github.tomakehurst.wiremock.junit5.WireMockExtension;
27+
import com.github.tomakehurst.wiremock.junit5.WireMockTest;
28+
import java.net.URI;
29+
import java.util.stream.Stream;
30+
import org.junit.jupiter.api.AfterEach;
31+
import org.junit.jupiter.api.BeforeEach;
32+
import org.junit.jupiter.api.DisplayName;
33+
import org.junit.jupiter.api.extension.RegisterExtension;
34+
import org.junit.jupiter.params.ParameterizedTest;
35+
import org.junit.jupiter.params.provider.Arguments;
36+
import org.junit.jupiter.params.provider.MethodSource;
37+
38+
/**
39+
* Abstract test suite for testing URI sanitization functionality across different HTTP client implementations.
40+
* Verifies that consecutive slashes in URIs are properly encoded.
41+
*/
42+
@WireMockTest
43+
public abstract class SdkHttpClientUriSanitizationTestSuite {
44+
45+
@RegisterExtension
46+
static WireMockExtension wireMock = WireMockExtension.newInstance()
47+
.options(wireMockConfig().dynamicPort())
48+
.build();
49+
50+
private SdkHttpClient client;
51+
52+
protected abstract SdkHttpClient createHttpClient();
53+
54+
@BeforeEach
55+
void setUp() {
56+
client = createHttpClient();
57+
// Generic stub for all requests
58+
wireMock.stubFor(any(anyUrl())
59+
.willReturn(aResponse()
60+
.withStatus(200)
61+
.withBody("success")));
62+
}
63+
64+
@AfterEach
65+
void tearDown() {
66+
if (client != null) {
67+
client.close();
68+
}
69+
}
70+
71+
@ParameterizedTest(name = "URI path: ''{0}'' should become ''{1}''")
72+
@MethodSource("provideUriSanitizationTestCases")
73+
@DisplayName("URI paths should be properly sanitized")
74+
void uriPathsShouldBeProperlySanitized(String inputPath, String expectedPath) throws Exception {
75+
SdkHttpFullRequest request = createRequestWithPath(inputPath);
76+
HttpExecuteResponse response = executeRequest(request);
77+
78+
assertThat(response.httpResponse().statusCode()).isEqualTo(200);
79+
wireMock.verify(getRequestedFor(urlPathEqualTo(expectedPath)));
80+
}
81+
82+
private static Stream<Arguments> provideUriSanitizationTestCases() {
83+
return Stream.of(
84+
// Normal paths should remain unchanged
85+
Arguments.of("/normal/path", "/normal/path"),
86+
Arguments.of("/api/v1/users/123", "/api/v1/users/123"),
87+
Arguments.of("/single/slash/only", "/single/slash/only"),
88+
89+
// Consecutive slashes should be encoded
90+
Arguments.of("/path//to//resource", "/path/%2Fto/%2Fresource"),
91+
Arguments.of("/folder//file.txt", "/folder/%2Ffile.txt"),
92+
Arguments.of("//leading//double", "/%2Fleading/%2Fdouble"),
93+
Arguments.of("/trailing//", "/trailing/%2F"),
94+
Arguments.of("/multiple///slashes", "/multiple/%2F/slashes"),
95+
Arguments.of("/four////slashes", "/four/%2F/%2Fslashes"),
96+
97+
// Edge cases
98+
Arguments.of("//", "/%2F"),
99+
Arguments.of("///", "/%2F/"),
100+
Arguments.of("////", "/%2F/%2F")
101+
);
102+
}
103+
104+
private SdkHttpFullRequest createRequestWithPath(String path) {
105+
URI uri = URI.create("http://localhost:" + wireMock.getPort() + path);
106+
107+
return SdkHttpFullRequest.builder()
108+
.uri(uri)
109+
.method(SdkHttpMethod.GET)
110+
.putHeader("Host", uri.getHost() + ":" + uri.getPort())
111+
.build();
112+
}
113+
114+
private HttpExecuteResponse executeRequest(SdkHttpFullRequest request) throws Exception {
115+
return client.prepareRequest(
116+
HttpExecuteRequest.builder()
117+
.request(request)
118+
.build())
119+
.call();
120+
}
121+
}

0 commit comments

Comments
 (0)