diff --git a/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientUriSanitizationTest.java b/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientUriSanitizationTest.java new file mode 100644 index 000000000000..d22e0be807a7 --- /dev/null +++ b/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientUriSanitizationTest.java @@ -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(); + } +} diff --git a/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientWireMockTest.java b/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientWireMockTest.java index 27c09184a035..8bcd978b4538 100644 --- a/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientWireMockTest.java +++ b/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientWireMockTest.java @@ -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; @@ -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; @@ -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"); + } } 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 c5037be562bf..8dbb74292b55 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 @@ -65,6 +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.SdkPreviewApi; import software.amazon.awssdk.annotations.SdkPublicApi; import software.amazon.awssdk.annotations.SdkTestInternalApi; import software.amazon.awssdk.http.AbortableInputStream; @@ -94,7 +95,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, @@ -104,6 +104,7 @@ * *

This can be created via {@link #builder()}

*/ +@SdkPreviewApi @SdkPublicApi public final class Apache5HttpClient implements SdkHttpClient { @@ -255,7 +256,6 @@ public void abort() { public void close() { HttpClientConnectionManager cm = httpClient.getHttpClientConnectionManager(); IdleConnectionReaper.getInstance().deregisterConnectionManager(cm); - // TODO : need to add test cases for this cm.close(CloseMode.IMMEDIATE); } @@ -409,8 +409,12 @@ public interface Builder extends SdkHttpClient.BuilderNote: 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.

*/ Builder connectionTimeToLive(Duration connectionTimeToLive); diff --git a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/Apache5SdkHttpService.java b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/Apache5SdkHttpService.java index b3fad1617efe..c724c0bc9483 100644 --- a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/Apache5SdkHttpService.java +++ b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/Apache5SdkHttpService.java @@ -15,6 +15,7 @@ package software.amazon.awssdk.http.apache5; +import software.amazon.awssdk.annotations.SdkPreviewApi; import software.amazon.awssdk.annotations.SdkPublicApi; import software.amazon.awssdk.http.SdkHttpClient; import software.amazon.awssdk.http.SdkHttpService; @@ -22,6 +23,7 @@ /** * Service binding for the Apache5 implementation. */ +@SdkPreviewApi @SdkPublicApi public class Apache5SdkHttpService implements SdkHttpService { @Override diff --git a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/ProxyConfiguration.java b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/ProxyConfiguration.java index 14bf452d80a6..712b2c42e149 100644 --- a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/ProxyConfiguration.java +++ b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/ProxyConfiguration.java @@ -22,6 +22,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.Set; +import software.amazon.awssdk.annotations.SdkPreviewApi; import software.amazon.awssdk.annotations.SdkPublicApi; import software.amazon.awssdk.utils.ProxyConfigProvider; import software.amazon.awssdk.utils.ProxySystemSetting; @@ -33,6 +34,7 @@ /** * Configuration that defines how to communicate via an HTTP or HTTPS proxy. */ +@SdkPreviewApi @SdkPublicApi public final class ProxyConfiguration implements ToCopyableBuilder { private final URI endpoint; @@ -437,7 +439,7 @@ public Builder scheme(String scheme) { return this; } - public void setuseEnvironmentVariableValues(Boolean useEnvironmentVariableValues) { + public void setUseEnvironmentVariableValues(Boolean useEnvironmentVariableValues) { useEnvironmentVariableValues(useEnvironmentVariableValues); } 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 42471e5a592e..fc3ecf5c1cd6 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 @@ -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 { @@ -59,7 +59,6 @@ public HttpUriRequestBase create(final HttpExecuteRequest request, final Apache5 return base; } - //TODO : check if this is still valid /** * * The Apache HTTP client doesn't allow consecutive slashes in the URI. For S3 diff --git a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientUriSanitizationTest.java b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientUriSanitizationTest.java new file mode 100644 index 000000000000..60b101d07763 --- /dev/null +++ b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientUriSanitizationTest.java @@ -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(); + } +} 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 c5f609bd9e7b..613280251d49 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 @@ -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; @@ -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; @@ -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"); + } } diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientUriSanitizationTestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientUriSanitizationTestSuite.java new file mode 100644 index 000000000000..36d34a328840 --- /dev/null +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientUriSanitizationTestSuite.java @@ -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 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(); + } +} \ No newline at end of file