Skip to content

Preview API annotation added for Public APIs and TODOs addressed #6215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
9e8fc15
Fix architecture test failures for apache5.x
joviegas May 29, 2025
70a120e
Merge branch 'feature/master/apache5x' into joviegas/apache-5-achitec…
joviegas May 29, 2025
c7b09a2
Merge issues resolved
joviegas May 29, 2025
068cada
Checkstyle issues
joviegas May 29, 2025
4766e74
Update to use PoolingHttpClientConnectionManager class reference that…
joviegas May 30, 2025
c81efdc
Merge branch 'master' into joviegas/apache-5-achitecture-test-fix
joviegas May 30, 2025
6b64589
Merge branch 'feature/master/apache5x' into joviegas/apache-5-achitec…
joviegas May 30, 2025
89debeb
Merge branch 'feature/master/apache5x' into joviegas/apache-5-achitec…
joviegas Jun 2, 2025
7f9e77f
Fix stream reset failure in RepeatableInputStreamRequestEntity by sto…
joviegas Jun 3, 2025
5fb2d35
writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even…
joviegas Jun 3, 2025
52fedbd
Merge branch 'feature/master/apache5x' into joviegas/apache-5-achitec…
joviegas Jun 9, 2025
8b83790
Fix connectionPoolingWorks by setting skipping setConnectionTimeToLi…
joviegas Jun 9, 2025
14a2ead
disableAutomaticRetries in Apache 5.x since SDK handles retries , als…
joviegas Jun 17, 2025
9431959
Merge branch 'feature/master/apache5x' into joviegas/apache-5-achitec…
joviegas Jun 17, 2025
5f33ad9
Added Test case for Async , handled review ocmments
joviegas Jun 19, 2025
270c0f7
Donot do buffer the response using BufferedHttpEntity since it might …
joviegas Jun 21, 2025
1a18785
merge from master
joviegas Jun 21, 2025
e2d16ad
Fix compilation issues
joviegas Jun 21, 2025
85efdaf
Fix checkstyle issues
joviegas Jun 21, 2025
bb70f7d
Remove test which are specific to apache http
joviegas Jun 21, 2025
8deb7d0
Merge branch 'feature/master/apache5x' into joviegas/apache-5-achitec…
joviegas Jun 24, 2025
ca832c7
Merge branch 'feature/master/apache5x' into joviegas/apache-5-achitec…
joviegas Jun 24, 2025
602c659
Add benchmark for Apache5 and add Streaming Api test cases
joviegas Jun 24, 2025
6e4332f
Merge branch 'feature/master/apache5x' into joviegas/apache-5-achitec…
joviegas Jun 26, 2025
40f144b
Update Apache5 to 5.5
joviegas Jun 26, 2025
f4a9af0
Preview ready , addressing open TODOs
joviegas Jun 27, 2025
676486b
Merge branch 'feature/master/apache5x' into joviegas/apache-5-achitec…
joviegas Jun 27, 2025
59f75fe
Added PublicApi since checkstyle was failing
joviegas Jun 27, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package software.amazon.awssdk.http.apache;


import org.junit.jupiter.api.DisplayName;
import software.amazon.awssdk.http.SdkHttpClient;
import software.amazon.awssdk.http.SdkHttpClientUriSanitizationTestSuite;

@DisplayName("Apache HTTP Client - URI Sanitization Tests")
class ApacheHttpClientUriSanitizationTest extends SdkHttpClientUriSanitizationTestSuite {

@Override
protected SdkHttpClient createHttpClient() {
return ApacheHttpClient.create();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
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;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static software.amazon.awssdk.http.SdkHttpConfigurationOption.TRUST_ALL_CERTIFICATES;
Expand All @@ -40,6 +41,7 @@
import org.apache.http.conn.DnsResolver;
import org.apache.http.conn.HttpClientConnectionManager;
import org.apache.http.conn.routing.HttpRoute;
import org.apache.http.impl.conn.ConnectionShutdownException;
import org.apache.http.impl.conn.SystemDefaultDnsResolver;
import org.junit.Rule;
import org.junit.Test;
Expand Down Expand Up @@ -265,4 +267,23 @@ public InetAddress[] resolve(final String host) throws UnknownHostException {

mockProxyServer.verify(1, RequestPatternBuilder.allRequests());
}

@Test
public void closeReleasesResources() throws Exception {
SdkHttpClient client = createSdkHttpClient();
// Make a successful request first
stubForMockRequest(200);
SdkHttpFullRequest request = mockSdkRequest("http://localhost:" + mockServer.port(), SdkHttpMethod.POST);
HttpExecuteResponse response = client.prepareRequest(
HttpExecuteRequest.builder().request(request).build()).call();
response.responseBody().ifPresent(IoUtils::drainInputStream);
// Close the client
client.close();
// Verify subsequent requests fail
assertThatThrownBy(() -> {
client.prepareRequest(HttpExecuteRequest.builder().request(request).build()).call();
}).isInstanceOfAny(
IllegalStateException.class
).hasMessageContaining("Connection pool shut down");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
import org.apache.hc.core5.pool.PoolStats;
import org.apache.hc.core5.ssl.SSLInitializationException;
import org.apache.hc.core5.util.TimeValue;
import software.amazon.awssdk.annotations.SdkPublicApi;
import software.amazon.awssdk.annotations.SdkPreviewApi;
import software.amazon.awssdk.annotations.SdkTestInternalApi;
import software.amazon.awssdk.http.AbortableInputStream;
import software.amazon.awssdk.http.ExecutableHttpRequest;
Expand Down Expand Up @@ -94,7 +94,6 @@
import software.amazon.awssdk.utils.Logger;
import software.amazon.awssdk.utils.Validate;

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

public static final String CLIENT_NAME = "Apache5";
Expand Down Expand Up @@ -255,7 +254,6 @@ public void abort() {
public void close() {
HttpClientConnectionManager cm = httpClient.getHttpClientConnectionManager();
IdleConnectionReaper.getInstance().deregisterConnectionManager(cm);
// TODO : need to add test cases for this
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added test cases

cm.close(CloseMode.IMMEDIATE);
}

Expand Down Expand Up @@ -409,8 +407,12 @@ public interface Builder extends SdkHttpClient.Builder<Apache5HttpClient.Builder
*/
Builder expectContinueEnabled(Boolean expectContinueEnabled);


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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@

package software.amazon.awssdk.http.apache5;

import software.amazon.awssdk.annotations.SdkPublicApi;
import software.amazon.awssdk.annotations.SdkPreviewApi;
import software.amazon.awssdk.http.SdkHttpClient;
import software.amazon.awssdk.http.SdkHttpService;

/**
* Service binding for the Apache5 implementation.
*/
@SdkPublicApi
@SdkPreviewApi
public class Apache5SdkHttpService implements SdkHttpService {
@Override
public SdkHttpClient.Builder createHttpClientBuilder() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import software.amazon.awssdk.annotations.SdkPublicApi;
import software.amazon.awssdk.annotations.SdkPreviewApi;
import software.amazon.awssdk.utils.ProxyConfigProvider;
import software.amazon.awssdk.utils.ProxySystemSetting;
import software.amazon.awssdk.utils.ToString;
Expand All @@ -33,7 +33,7 @@
/**
* Configuration that defines how to communicate via an HTTP or HTTPS proxy.
*/
@SdkPublicApi
@SdkPreviewApi
public final class ProxyConfiguration implements ToCopyableBuilder<ProxyConfiguration.Builder, ProxyConfiguration> {
private final URI endpoint;
private final String username;
Expand Down Expand Up @@ -437,7 +437,7 @@ public Builder scheme(String scheme) {
return this;
}

public void setuseEnvironmentVariableValues(Boolean useEnvironmentVariableValues) {
public void setUseEnvironmentVariableValues(Boolean useEnvironmentVariableValues) {
useEnvironmentVariableValues(useEnvironmentVariableValues);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
import software.amazon.awssdk.utils.http.SdkHttpUtils;

/**
* Responsible for creating Apache HttpClient 4 request objects.
* Responsible for creating Apache HttpClient 5 request objects.
*/
@SdkInternalApi
public class Apache5HttpRequestFactory {
Expand All @@ -59,7 +59,6 @@ public HttpUriRequestBase create(final HttpExecuteRequest request, final Apache5
return base;
}

//TODO : check if this is still valid
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added test cases

/**
*
* The Apache HTTP client doesn't allow consecutive slashes in the URI. For S3
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package software.amazon.awssdk.http.apache5;


import org.junit.jupiter.api.DisplayName;
import software.amazon.awssdk.http.SdkHttpClient;
import software.amazon.awssdk.http.SdkHttpClientUriSanitizationTestSuite;

@DisplayName("Apache5 HTTP Client - URI Sanitization Tests")
class Apache5HttpClientUriSanitizationTest extends SdkHttpClientUriSanitizationTestSuite {

@Override
protected SdkHttpClient createHttpClient() {
return Apache5HttpClient.create();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
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;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static software.amazon.awssdk.http.SdkHttpConfigurationOption.TRUST_ALL_CERTIFICATES;
Expand All @@ -41,7 +42,6 @@
import org.apache.hc.client5.http.io.HttpClientConnectionManager;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.io.CloseMode;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -260,4 +260,23 @@ public InetAddress[] resolve(String host) throws UnknownHostException {

mockProxyServer.verify(1, RequestPatternBuilder.allRequests());
}

@Test
public void closeReleasesResources() throws Exception {
SdkHttpClient client = createSdkHttpClient();
// Make a successful request first
stubForMockRequest(200);
SdkHttpFullRequest request = mockSdkRequest("http://localhost:" + mockServer.port(), SdkHttpMethod.POST);
HttpExecuteResponse response = client.prepareRequest(
HttpExecuteRequest.builder().request(request).build()).call();
response.responseBody().ifPresent(IoUtils::drainInputStream);
// Close the client
client.close();
// Verify subsequent requests fail
assertThatThrownBy(() -> {
client.prepareRequest(HttpExecuteRequest.builder().request(request).build()).call();
}).isInstanceOfAny(
IllegalStateException.class
).hasMessageContaining("Connection pool shut down");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package software.amazon.awssdk.http;

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.anyUrl;
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;

import com.github.tomakehurst.wiremock.junit5.WireMockExtension;
import com.github.tomakehurst.wiremock.junit5.WireMockTest;
import java.net.URI;
import java.util.stream.Stream;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

/**
* Abstract test suite for testing URI sanitization functionality across different HTTP client implementations.
* Verifies that consecutive slashes in URIs are properly encoded.
*/
@WireMockTest
public abstract class SdkHttpClientUriSanitizationTestSuite {

@RegisterExtension
static WireMockExtension wireMock = WireMockExtension.newInstance()
.options(wireMockConfig().dynamicPort())
.build();

private SdkHttpClient client;

protected abstract SdkHttpClient createHttpClient();

@BeforeEach
void setUp() {
client = createHttpClient();
// Generic stub for all requests
wireMock.stubFor(any(anyUrl())
.willReturn(aResponse()
.withStatus(200)
.withBody("success")));
}

@AfterEach
void tearDown() {
if (client != null) {
client.close();
}
}

@ParameterizedTest(name = "URI path: ''{0}'' should become ''{1}''")
@MethodSource("provideUriSanitizationTestCases")
@DisplayName("URI paths should be properly sanitized")
void uriPathsShouldBeProperlySanitized(String inputPath, String expectedPath) throws Exception {
SdkHttpFullRequest request = createRequestWithPath(inputPath);
HttpExecuteResponse response = executeRequest(request);

assertThat(response.httpResponse().statusCode()).isEqualTo(200);
wireMock.verify(getRequestedFor(urlPathEqualTo(expectedPath)));
}

private static Stream<Arguments> provideUriSanitizationTestCases() {
return Stream.of(
// Normal paths should remain unchanged
Arguments.of("/normal/path", "/normal/path"),
Arguments.of("/api/v1/users/123", "/api/v1/users/123"),
Arguments.of("/single/slash/only", "/single/slash/only"),

// Consecutive slashes should be encoded
Arguments.of("/path//to//resource", "/path/%2Fto/%2Fresource"),
Arguments.of("/folder//file.txt", "/folder/%2Ffile.txt"),
Arguments.of("//leading//double", "/%2Fleading/%2Fdouble"),
Arguments.of("/trailing//", "/trailing/%2F"),
Arguments.of("/multiple///slashes", "/multiple/%2F/slashes"),
Arguments.of("/four////slashes", "/four/%2F/%2Fslashes"),

// Edge cases
Arguments.of("//", "/%2F"),
Arguments.of("///", "/%2F/"),
Arguments.of("////", "/%2F/%2F")
);
}

private SdkHttpFullRequest createRequestWithPath(String path) {
URI uri = URI.create("http://localhost:" + wireMock.getPort() + path);

return SdkHttpFullRequest.builder()
.uri(uri)
.method(SdkHttpMethod.GET)
.putHeader("Host", uri.getHost() + ":" + uri.getPort())
.build();
}

private HttpExecuteResponse executeRequest(SdkHttpFullRequest request) throws Exception {
return client.prepareRequest(
HttpExecuteRequest.builder()
.request(request)
.build())
.call();
}
}
Loading