From 9e8fc154a33dd15ca249d57ff3ed802174ae7308 Mon Sep 17 00:00:00 2001 From: John Viegas Date: Thu, 29 May 2025 08:43:46 -0700 Subject: [PATCH 01/14] Fix architecture test failures for apache5.x --- .../ApacheHttpClientUriNormalizationTest.java | 29 +++ http-clients/apache5-client/pom.xml | 2 +- .../impl/Apache5HttpRequestFactory.java | 2 - .../apache5/internal/utils/Apache5Utils.java | 55 ------ ...Apache5HttpClientUriNormalizationTest.java | 29 +++ .../4195d6e3-8849-4e5a-848d-04f810577cd3 | 3 + .../HttpClientUriNormalizationTestSuite.java | 169 ++++++++++++++++++ 7 files changed, 231 insertions(+), 58 deletions(-) create mode 100644 http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientUriNormalizationTest.java create mode 100644 http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientUriNormalizationTest.java create mode 100644 test/http-client-tests/src/main/java/software/amazon/awssdk/http/HttpClientUriNormalizationTestSuite.java diff --git a/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientUriNormalizationTest.java b/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientUriNormalizationTest.java new file mode 100644 index 000000000000..fc6f754b410d --- /dev/null +++ b/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientUriNormalizationTest.java @@ -0,0 +1,29 @@ +/* + * 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 software.amazon.awssdk.http.HttpClientUriNormalizationTestSuite; +import software.amazon.awssdk.http.SdkHttpClient; + +public class ApacheHttpClientUriNormalizationTest extends HttpClientUriNormalizationTestSuite { + + + @Override + protected SdkHttpClient createSdkHttpClient() { + return ApacheHttpClient.create(); + } +} + diff --git a/http-clients/apache5-client/pom.xml b/http-clients/apache5-client/pom.xml index dd7f37b16f81..587fba8253df 100644 --- a/http-clients/apache5-client/pom.xml +++ b/http-clients/apache5-client/pom.xml @@ -21,7 +21,7 @@ http-clients software.amazon.awssdk - 2.31.51-SNAPSHOT + 2.31.53-SNAPSHOT apache5-client 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 4a200df15fef..3219b271998f 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 @@ -100,8 +100,6 @@ private void addRequestConfig(HttpUriRequestBase base, .setResponseTimeout(saturatedCast(requestConfig.socketTimeout().toMillis()), TimeUnit.MILLISECONDS); // TODO as part of removed API : .setLocalAddress(requestConfig.localAddress()); - Apache5Utils.disableNormalizeUri(requestConfigBuilder); - /* * Enable 100-continue support for PUT operations, since this is * where we're potentially uploading large amounts of data and want 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 f5e78ea4a11e..a777b3d21d8c 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 @@ -29,29 +29,9 @@ import org.apache.hc.core5.http.io.entity.BufferedHttpEntity; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.http.apache5.ProxyConfiguration; -import software.amazon.awssdk.utils.Logger; -import software.amazon.awssdk.utils.ReflectionMethodInvoker; @SdkInternalApi public final class Apache5Utils { - private static final Logger logger = Logger.loggerFor(Apache5Utils.class); - private static final ReflectionMethodInvoker NORMALIZE_URI_INVOKER; - - static { - // Attempt to initialize the invoker once on class-load. If it fails, it will not be attempted again, but we'll - // use that opportunity to log a warning. - NORMALIZE_URI_INVOKER = - new ReflectionMethodInvoker<>(RequestConfig.Builder.class, - RequestConfig.Builder.class, - "setNormalizeUri", - boolean.class); - - try { - NORMALIZE_URI_INVOKER.initialize(); - } catch (NoSuchMethodException ignored) { - noSuchMethodThrownByNormalizeUriInvoker(); - } - } private Apache5Utils() { } @@ -79,37 +59,11 @@ public static HttpClientContext newClientContext(ProxyConfiguration proxyConfigu addPreemptiveAuthenticationProxy(clientContext, proxyConfiguration); RequestConfig.Builder builder = RequestConfig.custom(); - disableNormalizeUri(builder); - clientContext.setRequestConfig(builder.build()); return clientContext; } - /** - * From Apache v4.5.8, normalization should be disabled or AWS requests with special characters in URI path will fail - * with Signature Errors. - *

- * setNormalizeUri is added only in 4.5.8, so customers using the latest version of SDK with old versions (4.5.6 or less) - * of Apache httpclient will see NoSuchMethodError. Hence this method will suppress the error. - * - * Do not use Apache version 4.5.7 as it breaks URI paths with special characters and there is no option - * to disable normalization. - *

- * - * For more information, See https://github.com/aws/aws-sdk-java/issues/1919 - */ - public static void disableNormalizeUri(RequestConfig.Builder requestConfigBuilder) { - // For efficiency, do not attempt to call the invoker again if it failed to initialize on class-load - if (NORMALIZE_URI_INVOKER.isInitialized()) { - try { - NORMALIZE_URI_INVOKER.invoke(requestConfigBuilder, false); - } catch (NoSuchMethodException ignored) { - noSuchMethodThrownByNormalizeUriInvoker(); - } - } - } - /** * Returns a new Credentials Provider for use with proxy authentication. */ @@ -154,13 +108,4 @@ private static void addPreemptiveAuthenticationProxy(HttpClientContext clientCon } } - // Just log and then swallow the exception - private static void noSuchMethodThrownByNormalizeUriInvoker() { - // setNormalizeUri method was added in httpclient 4.5.8 - logger.warn(() -> "NoSuchMethodException was thrown when disabling normalizeUri. This indicates you are using " - + "an old version (< 4.5.8) of Apache http client. It is recommended to use http client " - + "version >= 4.5.9 to avoid the breaking change introduced in apache client 4.5.7 and " - + "the latency in exception handling. See https://github.com/aws/aws-sdk-java/issues/1919" - + " for more information"); - } } diff --git a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientUriNormalizationTest.java b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientUriNormalizationTest.java new file mode 100644 index 000000000000..592508ab1057 --- /dev/null +++ b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientUriNormalizationTest.java @@ -0,0 +1,29 @@ +/* + * 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 software.amazon.awssdk.http.HttpClientUriNormalizationTestSuite; +import software.amazon.awssdk.http.SdkHttpClient; + +public class Apache5HttpClientUriNormalizationTest extends HttpClientUriNormalizationTestSuite { + + + @Override + protected SdkHttpClient createSdkHttpClient() { + return Apache5HttpClient.create(); + } +} + diff --git a/test/architecture-tests/archunit_store/4195d6e3-8849-4e5a-848d-04f810577cd3 b/test/architecture-tests/archunit_store/4195d6e3-8849-4e5a-848d-04f810577cd3 index 8c3b1f284548..dd81179f8903 100644 --- a/test/architecture-tests/archunit_store/4195d6e3-8849-4e5a-848d-04f810577cd3 +++ b/test/architecture-tests/archunit_store/4195d6e3-8849-4e5a-848d-04f810577cd3 @@ -11,8 +11,11 @@ Method calls method in (FileStoreTlsKeyManagersProvider.java:52) Method calls method in (SystemPropertyTlsKeyManagersProvider.java:61) Method calls method in (ApacheHttpClient.java:699) +Method calls method in (Apache5HttpClient.java:741) Method calls method in (RepeatableInputStreamRequestEntity.java:113) Method calls method in (ApacheUtils.java:162) +Method calls method in (RepeatableInputStreamRequestEntity.java:131) +Method calls method in (RepeatableInputStreamRequestEntity.java:143) Method calls method in (NettyUtils.java:289) Method calls method in (UrlConnectionHttpClient.java:263) Method calls method in (CloudWatchMetricPublisher.java:293) diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/HttpClientUriNormalizationTestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/HttpClientUriNormalizationTestSuite.java new file mode 100644 index 000000000000..25eb91930767 --- /dev/null +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/HttpClientUriNormalizationTestSuite.java @@ -0,0 +1,169 @@ +/* + * 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.anyRequestedFor; +import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl; +import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options; +import static org.assertj.core.api.Assertions.assertThat; + +import com.github.tomakehurst.wiremock.WireMockServer; +import com.github.tomakehurst.wiremock.client.WireMock; +import com.github.tomakehurst.wiremock.verification.LoggedRequest; +import java.net.URI; +import java.util.List; +import java.util.stream.Stream; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +public abstract class HttpClientUriNormalizationTestSuite { + + protected static SdkHttpClient httpClient; + private static WireMockServer wireMockServer; + + @BeforeAll + static void setUp() { + wireMockServer = new WireMockServer(options().dynamicPort()); + wireMockServer.start(); + WireMock.configureFor("localhost", wireMockServer.port()); + } + + @BeforeEach + void prepare(){ + wireMockServer.stubFor(any(urlMatching(".*")) + .willReturn(aResponse() + .withStatus(200) + .withBody("success"))); + } + + @AfterEach + void reset(){ + wireMockServer.resetAll(); + } + + @AfterAll + static void tearDown() { + if (httpClient != null) { + httpClient.close(); + } + if (wireMockServer != null) { + wireMockServer.stop(); + } + } + + private static Stream uriTestCases() { + return Stream.of( + Arguments.of( + "Encoded spaces", + "/path/with%20spaces/file.txt", + "%20" + ), + Arguments.of( + "Encoded slashes", + "/path/with%2Fslash/file.txt", + "%2F" + ), + Arguments.of( + "Encoded plus", + "/path/with%2Bplus/file.txt", + "%2B" + ), + Arguments.of( + "Plus sign", + "/path/with+plus/file.txt", + "+" + ), + Arguments.of( + "Encoded question mark", + "/path/with%3Fquery/file.txt", + "%3F" + ), + Arguments.of( + "Encoded ampersand", + "/path/with%26ampersand/file.txt", + "%26" + ), + Arguments.of( + "Encoded equals", + "/path/with%3Dequals/file.txt", + "%3D" + ), + Arguments.of( + "AWS S3 style path", + "/my-bucket/folder%2Fsubfolder/file%20name.txt", + "%2F" + ) + ); + } + + @ParameterizedTest + @MethodSource("uriTestCases") + @DisplayName("Verify URI normalization is disabled (encoded characters are preserved)") + void testUriNormalizationDisabled(String testName, String path, String encodedChar) throws Exception { + httpClient = createSdkHttpClient(); + + // Create and execute request + HttpExecuteRequest request = createTestRequest(path); + ExecutableHttpRequest executableRequest = httpClient.prepareRequest(request); + HttpExecuteResponse response = executableRequest.call(); + + // Verify response was successful + assertThat(response.httpResponse().statusCode()).isEqualTo(200); + + // Capture the actual request sent to server + List requests = wireMockServer.findAll(anyRequestedFor(anyUrl())); + assertThat(requests).hasSize(1); + + String actualPathSent = requests.get(0).getUrl(); + assertThat(actualPathSent).contains(encodedChar); + } + + private HttpExecuteRequest createTestRequest(String path) { + String baseUrl = "http://localhost:" + wireMockServer.port(); + return HttpExecuteRequest.builder() + .request(SdkHttpRequest.builder() + .method(SdkHttpMethod.GET) + .uri(URI.create(baseUrl + path)) + .build()) + .build(); + } + + @ParameterizedTest + @MethodSource("uriTestCases") + @DisplayName("Test end-to-end execution flow with client context") + void testExecuteFlowWithClientContext(String testName, String path, String encodedChar) throws Exception { + httpClient = createSdkHttpClient(); + HttpExecuteRequest request = createTestRequest(path); + HttpExecuteResponse response = httpClient.prepareRequest(request).call(); + assertThat(response.httpResponse().statusCode()).isEqualTo(200); + List requests = wireMockServer.findAll(anyRequestedFor(anyUrl())); + assertThat(requests).hasSize(1); + + String actualUrl = requests.get(0).getUrl(); + assertThat(actualUrl).contains(encodedChar); + } + + protected abstract SdkHttpClient createSdkHttpClient(); +} \ No newline at end of file From 068cada6c77cc3f4086d865ae23872128aaaa0e3 Mon Sep 17 00:00:00 2001 From: John Viegas Date: Thu, 29 May 2025 10:29:19 -0700 Subject: [PATCH 02/14] Checkstyle issues --- .../awssdk/http/HttpClientUriNormalizationTestSuite.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/HttpClientUriNormalizationTestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/HttpClientUriNormalizationTestSuite.java index 25eb91930767..e09eaf3cb8f9 100644 --- a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/HttpClientUriNormalizationTestSuite.java +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/HttpClientUriNormalizationTestSuite.java @@ -51,7 +51,7 @@ static void setUp() { } @BeforeEach - void prepare(){ + void prepare() { wireMockServer.stubFor(any(urlMatching(".*")) .willReturn(aResponse() .withStatus(200) @@ -59,7 +59,7 @@ void prepare(){ } @AfterEach - void reset(){ + void reset() { wireMockServer.resetAll(); } From 4766e7479f93a6b843df94e9c8ddad6fbdfbea02 Mon Sep 17 00:00:00 2001 From: John Viegas Date: Fri, 30 May 2025 15:40:08 -0700 Subject: [PATCH 03/14] Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager --- .../http/apache5/Apache5HttpClient.java | 4 ++-- .../internal/conn/IdleConnectionReaper.java | 19 +++++++++---------- .../conn/IdleConnectionReaperTest.java | 9 +++++---- 3 files changed, 16 insertions(+), 16 deletions(-) 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 39f75a3866de..fd97ea17d05a 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 @@ -152,7 +152,7 @@ private ConnectionManagerAwareHttpClient createClient(Apache5HttpClient.DefaultB // Note that it is important we register the original connection manager with the // IdleConnectionReaper as it's required for the successful deregistration of managers // from the reaper. See https://github.com/aws/aws-sdk-java/issues/722. - HttpClientConnectionManager cm = cmFactory.create(configuration, standardOptions); + PoolingHttpClientConnectionManager cm = cmFactory.create(configuration, standardOptions); Registry authSchemeRegistry = configuration.authSchemeRegistry ; if (authSchemeRegistry != null) { @@ -689,7 +689,7 @@ public SdkHttpClient buildWithDefaults(AttributeMap serviceDefaults) { private static class ApacheConnectionManagerFactory { - public HttpClientConnectionManager create(Apache5HttpClient.DefaultBuilder configuration, + public PoolingHttpClientConnectionManager create(Apache5HttpClient.DefaultBuilder configuration, AttributeMap standardOptions) { // TODO : Deprecated method needs to be removed with new replacements SSLConnectionSocketFactory sslsf = getPreferredSocketFactory(configuration, standardOptions); diff --git a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/conn/IdleConnectionReaper.java b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/conn/IdleConnectionReaper.java index b7261fef2cc6..0edf47201d21 100644 --- a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/conn/IdleConnectionReaper.java +++ b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/conn/IdleConnectionReaper.java @@ -22,8 +22,9 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.function.Supplier; +import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager; import org.apache.hc.client5.http.io.HttpClientConnectionManager; -import org.apache.hc.core5.io.CloseMode; +import org.apache.hc.core5.util.TimeValue; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import software.amazon.awssdk.annotations.SdkInternalApi; @@ -38,7 +39,7 @@ public final class IdleConnectionReaper { private static final IdleConnectionReaper INSTANCE = new IdleConnectionReaper(); - private final Map connectionManagers; + private final Map connectionManagers; private final Supplier executorServiceSupplier; @@ -64,7 +65,7 @@ private IdleConnectionReaper() { } @SdkTestInternalApi - IdleConnectionReaper(Map connectionManagers, + IdleConnectionReaper(Map connectionManagers, Supplier executorServiceSupplier, long sleepPeriod) { @@ -81,7 +82,7 @@ private IdleConnectionReaper() { * @return {@code true} If the connection manager was not previously registered with this reaper, {@code false} * otherwise. */ - public synchronized boolean registerConnectionManager(HttpClientConnectionManager manager, long maxIdleTime) { + public synchronized boolean registerConnectionManager(PoolingHttpClientConnectionManager manager, long maxIdleTime) { boolean notPreviouslyRegistered = connectionManagers.put(manager, maxIdleTime) == null; setupExecutorIfNecessary(); return notPreviouslyRegistered; @@ -133,12 +134,12 @@ private void cleanupExecutorIfNecessary() { } private static final class ReaperTask implements Runnable { - private final Map connectionManagers; + private final Map connectionManagers; private final long sleepPeriod; private volatile boolean stopping = false; - private ReaperTask(Map connectionManagers, + private ReaperTask(Map connectionManagers, long sleepPeriod) { this.connectionManagers = connectionManagers; this.sleepPeriod = sleepPeriod; @@ -150,11 +151,9 @@ public void run() { try { Thread.sleep(sleepPeriod); - for (Map.Entry entry : connectionManagers.entrySet()) { + for (Map.Entry entry : connectionManagers.entrySet()) { try { - entry.getKey().close(CloseMode.GRACEFUL); - // Set idle connections - // entry.getKey().closeIdleConnections(entry.getValue(), TimeUnit.MILLISECONDS); + entry.getKey().closeIdle(TimeValue.ofMilliseconds(entry.getValue())); } catch (Exception t) { log.warn("Unable to close idle connections", t); } diff --git a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/conn/IdleConnectionReaperTest.java b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/conn/IdleConnectionReaperTest.java index fa4bf33ad1ad..9f0ef8d8667a 100644 --- a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/conn/IdleConnectionReaperTest.java +++ b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/conn/IdleConnectionReaperTest.java @@ -15,6 +15,7 @@ package software.amazon.awssdk.http.apache5.internal.conn; +import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager; import org.apache.hc.core5.io.CloseMode; import static org.mockito.ArgumentMatchers.any; @@ -28,6 +29,7 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import org.apache.hc.client5.http.io.HttpClientConnectionManager; +import org.apache.hc.core5.util.TimeValue; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -41,13 +43,13 @@ public class IdleConnectionReaperTest { private static final long SLEEP_PERIOD = 250; - private final Map connectionManagers = new HashMap<>(); + private final Map connectionManagers = new HashMap<>(); @Mock public ExecutorService executorService; @Mock - public HttpClientConnectionManager connectionManager; + public PoolingHttpClientConnectionManager connectionManager; private IdleConnectionReaper idleConnectionReaper; @@ -88,8 +90,7 @@ public void testReapsConnections() throws InterruptedException { reaper.registerConnectionManager(connectionManager, idleTime); try { Thread.sleep(SLEEP_PERIOD * 2); - // TODO : need to validate this in future PR - verify(connectionManager, atLeastOnce()).close(CloseMode.GRACEFUL); + verify(connectionManager, atLeastOnce()).closeIdle(any(TimeValue.class)); } finally { reaper.deregisterConnectionManager(connectionManager); } From 7f9e77f7680aec64ed81a0ebc76a855f327ca332 Mon Sep 17 00:00:00 2001 From: John Viegas Date: Tue, 3 Jun 2025 14:07:20 -0700 Subject: [PATCH 04/14] 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 --- ...epeatableInputStreamRequestEntityTest.java | 157 +++++++++++++++++ .../RepeatableInputStreamRequestEntity.java | 96 +++++----- ...epeatableInputStreamRequestEntityTest.java | 165 +++++++++++++++++- 3 files changed, 360 insertions(+), 58 deletions(-) diff --git a/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/internal/RepeatableInputStreamRequestEntityTest.java b/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/internal/RepeatableInputStreamRequestEntityTest.java index 274031651792..4f9286278b3e 100644 --- a/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/internal/RepeatableInputStreamRequestEntityTest.java +++ b/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/internal/RepeatableInputStreamRequestEntityTest.java @@ -16,6 +16,7 @@ package software.amazon.awssdk.http.apache.internal; import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; @@ -790,5 +791,161 @@ void multipleOperations_StatePreservation_WorksCorrectly() throws IOException { assertEquals(contentLength1, contentLength2); assertEquals(contentLength2, contentLength3); } + + @Test + @DisplayName("markSupported should be be called everytime") + void markSupported_NotCachedDuringConstruction() { + // Given + AtomicInteger markSupportedCalls = new AtomicInteger(0); + InputStream trackingStream = new ByteArrayInputStream("test".getBytes()) { + @Override + public boolean markSupported() { + markSupportedCalls.incrementAndGet(); + return true; + } + }; + + entity = createEntity(trackingStream); + assertEquals(0, markSupportedCalls.get()); + + // Multiple isRepeatable calls trigger new markSupported calls + assertTrue(entity.isRepeatable()); + assertTrue(entity.isRepeatable()); + assertEquals(2, markSupportedCalls.get()); + } + + @Test + @DisplayName("ContentStreamProvider.newStream() should only be called once") + void contentStreamProvider_NewStreamCalledOnce() { + AtomicInteger newStreamCalls = new AtomicInteger(0); + ContentStreamProvider provider = () -> { + if (newStreamCalls.incrementAndGet() > 1) { + throw new RuntimeException("Could not create new stream: Already created"); + } + return new ByteArrayInputStream("test".getBytes()); + }; + + entity = createEntity(provider); + + assertEquals(1, newStreamCalls.get()); + assertTrue(entity.isRepeatable()); + assertFalse(entity.isChunked()); + } + + @Test + @DisplayName("writeTo should use cached markSupported for reset decision") + void writeTo_UsesCachedMarkSupported() throws IOException { + // Given - Stream that changes markSupported behavior + AtomicInteger markSupportedCalls = new AtomicInteger(0); + ByteArrayInputStream baseStream = new ByteArrayInputStream("test".getBytes()); + InputStream stream = new InputStream() { + @Override + public int read() throws IOException { + return baseStream.read(); + } + + @Override + public boolean markSupported() { + return markSupportedCalls.incrementAndGet() == 1; // Only first call returns true + } + + @Override + public synchronized void reset() throws IOException { + baseStream.reset(); + } + }; + + entity = createEntity(stream); + + // Write twice + ByteArrayOutputStream output1 = new ByteArrayOutputStream(); + entity.writeTo(output1); + + ByteArrayOutputStream output2 = new ByteArrayOutputStream(); + entity.writeTo(output2); + + // Then - Both writes succeed using cached markSupported value + assertEquals("test", output1.toString()); + assertEquals("test", output2.toString()); + assertEquals(1, markSupportedCalls.get()); + } + + @Test + @DisplayName("Non-repeatable stream should not attempt reset") + void nonRepeatableStream_NoResetAttempt() throws IOException { + // Given + AtomicInteger resetCalls = new AtomicInteger(0); + InputStream nonRepeatableStream = new ByteArrayInputStream("test".getBytes()) { + @Override + public boolean markSupported() { + return false; + } + + @Override + public synchronized void reset() { + resetCalls.incrementAndGet(); + throw new RuntimeException("Reset not supported"); + } + }; + + entity = createEntity(nonRepeatableStream); + assertFalse(entity.isRepeatable()); + entity.writeTo(new ByteArrayOutputStream()); + entity.writeTo(new ByteArrayOutputStream()); + assertEquals(0, resetCalls.get()); + } + + @Test + @DisplayName("Stream should not be read during construction") + void constructor_DoesNotReadStream() { + // Given + InputStream nonReadableStream = new InputStream() { + @Override + public int read() throws IOException { + throw new IOException("Stream should not be read during construction"); + } + + @Override + public boolean markSupported() { + return true; + } + }; + assertDoesNotThrow(() -> entity = createEntity(nonReadableStream)); + assertTrue(entity.isRepeatable()); + } + + @Test + @DisplayName("getContent should reuse existing stream") + void getContent_ReusesExistingStream() throws IOException { + InputStream originalStream = new ByteArrayInputStream("content".getBytes()); + entity = createEntity(originalStream); + InputStream content1 = entity.getContent(); + InputStream content2 = entity.getContent(); + assertSame(content1, content2); + } + + @Test + @DisplayName("Empty stream should be repeatable") + void emptyStream_IsRepeatable() { + // Given - No content provider + HttpExecuteRequest request = HttpExecuteRequest.builder() + .request(httpRequestBuilder.build()) + .build(); + entity = new RepeatableInputStreamRequestEntity(request); + assertTrue(entity.isRepeatable()); + } + + // Helper methods + private RepeatableInputStreamRequestEntity createEntity(InputStream stream) { + return createEntity(() -> stream); + } + + private RepeatableInputStreamRequestEntity createEntity(ContentStreamProvider provider) { + HttpExecuteRequest request = HttpExecuteRequest.builder() + .request(httpRequestBuilder.build()) + .contentStreamProvider(provider) + .build(); + return new RepeatableInputStreamRequestEntity(request); + } } diff --git a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/RepeatableInputStreamRequestEntity.java b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/RepeatableInputStreamRequestEntity.java index a57e5f18d341..7239fa0c1f36 100644 --- a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/RepeatableInputStreamRequestEntity.java +++ b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/RepeatableInputStreamRequestEntity.java @@ -49,18 +49,11 @@ public class RepeatableInputStreamRequestEntity extends HttpEntityWrapper { /** * True if the "Transfer-Encoding:chunked" header is present */ - private boolean isChunked; - + private final boolean isChunked; /** - * The underlying InputStreamEntity being delegated to + * The underlying reference of content */ - private InputStreamEntity inputStreamRequestEntity; - - /** - * The InputStream containing the content to write out - */ - private InputStream content; - + private final InputStream content; /** * Record the original exception if we do attempt a retry, so that if the * retry fails, we can report the original exception. Otherwise, we're most @@ -70,18 +63,36 @@ public class RepeatableInputStreamRequestEntity extends HttpEntityWrapper { private IOException originalException; /** - * Creates a new RepeatableInputStreamRequestEntity using the information - * from the specified request. If the input stream containing the request's - * contents is repeatable, then this RequestEntity will report as being - * repeatable. - * - * @param request The details of the request being written out (content type, - * content length, and content). + * Helper class to capture both the created entity and the original content stream reference. + *

+ * We store the content stream reference to avoid calling {@code getContent()} on the wrapped + * entity multiple times, which could potentially create new stream instances or perform + * unnecessary operations. This ensures we consistently use the same stream instance for + * {@code markSupported()} checks and {@code reset()} operations throughout the entity's lifecycle. */ + + private static class EntityCreationResult { + final InputStreamEntity entity; + final InputStream content; + + EntityCreationResult(InputStreamEntity entity, InputStream content) { + this.entity = entity; + this.content = content; + } + } + public RepeatableInputStreamRequestEntity(HttpExecuteRequest request) { - super(createInputStreamEntity(request)); + this(createInputStreamEntityWithMetadata(request), request); + } + + private RepeatableInputStreamRequestEntity(EntityCreationResult result, HttpExecuteRequest request) { + super(result.entity); + this.content = result.content; + this.isChunked = request.httpRequest().matchingHeaders(TRANSFER_ENCODING).contains(CHUNKED); + } - isChunked = request.httpRequest().matchingHeaders(TRANSFER_ENCODING).contains(CHUNKED); + private static EntityCreationResult createInputStreamEntityWithMetadata(HttpExecuteRequest request) { + InputStream content = getContent(request.contentStreamProvider()); /* * If we don't specify a content length when we instantiate our @@ -93,35 +104,14 @@ public RepeatableInputStreamRequestEntity(HttpExecuteRequest request) { .map(RepeatableInputStreamRequestEntity::parseContentLength) .orElse(-1L); - content = getContent(request.contentStreamProvider()); - - // Create InputStreamEntity with proper ContentType handling for HttpClient 5.x - ContentType contentType = request.httpRequest().firstMatchingHeader("Content-Type") - .map(RepeatableInputStreamRequestEntity::parseContentType) - .orElse(null); - - if (contentLength >= 0) { - inputStreamRequestEntity = new InputStreamEntity(content, contentLength, contentType); - } else { - inputStreamRequestEntity = new InputStreamEntity(content, contentType); - } - } - - private static InputStreamEntity createInputStreamEntity(HttpExecuteRequest request) { - InputStream content = getContent(request.contentStreamProvider()); - - long contentLength = request.httpRequest().firstMatchingHeader("Content-Length") - .map(RepeatableInputStreamRequestEntity::parseContentLength) - .orElse(-1L); - ContentType contentType = request.httpRequest().firstMatchingHeader("Content-Type") .map(RepeatableInputStreamRequestEntity::parseContentType) .orElse(null); - if (contentLength >= 0) { - return new InputStreamEntity(content, contentLength, contentType); - } - return new InputStreamEntity(content, contentType); + InputStreamEntity entity = contentLength >= 0 + ? new InputStreamEntity(content, contentLength, contentType) + : new InputStreamEntity(content, contentType); + return new EntityCreationResult(entity, content); } private static long parseContentLength(String contentLength) { @@ -164,13 +154,9 @@ public boolean isChunked() { */ @Override public boolean isRepeatable() { - boolean markSupported = content.markSupported(); - boolean entityRepeatable = inputStreamRequestEntity.isRepeatable(); - boolean result = markSupported || entityRepeatable; - return result; + return content.markSupported() || super.isRepeatable(); } - /** * Resets the underlying InputStream if this isn't the first attempt to * write out the request, otherwise simply delegates to @@ -189,7 +175,7 @@ public void writeTo(OutputStream output) throws IOException { } firstAttempt = false; - inputStreamRequestEntity.writeTo(output); + super.writeTo(output); } catch (IOException ioe) { if (originalException == null) { originalException = ioe; @@ -200,12 +186,8 @@ public void writeTo(OutputStream output) throws IOException { @Override public void close() throws IOException { - try { - if (content != null) { - content.close(); - } - } finally { - super.close(); - } + // The InputStreamEntity handles closing the stream when it's closed + // We don't need to close our reference separately to avoid double-closing + super.close(); } } diff --git a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/RepeatableInputStreamRequestEntityTest.java b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/RepeatableInputStreamRequestEntityTest.java index 0cf37febec8c..73a8017e4cc1 100644 --- a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/RepeatableInputStreamRequestEntityTest.java +++ b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/RepeatableInputStreamRequestEntityTest.java @@ -16,6 +16,7 @@ package software.amazon.awssdk.http.apache5.internal; import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; @@ -790,4 +791,166 @@ void multipleOperations_StatePreservation_WorksCorrectly() throws IOException { assertEquals(contentLength1, contentLength2); assertEquals(contentLength2, contentLength3); } -} \ No newline at end of file + + @Test + @DisplayName("markSupported should be be called everytime") + void markSupported_NotCachedDuringConstruction() { + // Given + AtomicInteger markSupportedCalls = new AtomicInteger(0); + InputStream trackingStream = new ByteArrayInputStream("test".getBytes()) { + @Override + public boolean markSupported() { + markSupportedCalls.incrementAndGet(); + return true; + } + }; + + entity = createEntity(trackingStream); + assertEquals(0, markSupportedCalls.get()); + // Multiple isRepeatable calls trigger new markSupported calls + assertTrue(entity.isRepeatable()); + assertTrue(entity.isRepeatable()); + assertEquals(2, markSupportedCalls.get()); + } + + @Test + @DisplayName("ContentStreamProvider.newStream() should only be called once") + void contentStreamProvider_NewStreamCalledOnce() { + // Given + AtomicInteger newStreamCalls = new AtomicInteger(0); + ContentStreamProvider provider = () -> { + if (newStreamCalls.incrementAndGet() > 1) { + throw new RuntimeException("Could not create new stream: Already created"); + } + return new ByteArrayInputStream("test".getBytes()); + }; + entity = createEntity(provider); + assertEquals(1, newStreamCalls.get()); + assertTrue(entity.isRepeatable()); + assertFalse(entity.isChunked()); + } + + @Test + @DisplayName("writeTo should use cached markSupported for reset decision") + void writeTo_UsesCachedMarkSupported() throws IOException { + // Given - Stream that changes markSupported behavior + AtomicInteger markSupportedCalls = new AtomicInteger(0); + ByteArrayInputStream baseStream = new ByteArrayInputStream("test".getBytes()); + InputStream stream = new InputStream() { + @Override + public int read() throws IOException { + return baseStream.read(); + } + + @Override + public boolean markSupported() { + return markSupportedCalls.incrementAndGet() == 1; // Only first call returns true + } + + @Override + public synchronized void reset() throws IOException { + baseStream.reset(); + } + }; + + entity = createEntity(stream); + + // When - Write twice + ByteArrayOutputStream output1 = new ByteArrayOutputStream(); + entity.writeTo(output1); + + ByteArrayOutputStream output2 = new ByteArrayOutputStream(); + entity.writeTo(output2); + + // Then - Both writes succeed using cached markSupported value + assertEquals("test", output1.toString()); + assertEquals("test", output2.toString()); + assertEquals(1, markSupportedCalls.get()); + } + + @Test + @DisplayName("Non-repeatable stream should not attempt reset") + void nonRepeatableStream_NoResetAttempt() throws IOException { + // Given + AtomicInteger resetCalls = new AtomicInteger(0); + InputStream nonRepeatableStream = new ByteArrayInputStream("test".getBytes()) { + @Override + public boolean markSupported() { + return false; + } + + @Override + public synchronized void reset() { + resetCalls.incrementAndGet(); + throw new RuntimeException("Reset not supported"); + } + }; + + entity = createEntity(nonRepeatableStream); + assertFalse(entity.isRepeatable()); + + // Write twice + entity.writeTo(new ByteArrayOutputStream()); + entity.writeTo(new ByteArrayOutputStream()); + + // Reset never called + assertEquals(0, resetCalls.get()); + } + + @Test + @DisplayName("Stream should not be read during construction") + void constructor_DoesNotReadStream() { + + InputStream nonReadableStream = new InputStream() { + @Override + public int read() throws IOException { + throw new IOException("Stream should not be read during construction"); + } + + @Override + public boolean markSupported() { + return true; + } + }; + + // Should not throw exception + assertDoesNotThrow(() -> entity = createEntity(nonReadableStream)); + assertTrue(entity.isRepeatable()); + } + + @Test + @DisplayName("getContent should reuse existing stream") + void getContent_ReusesExistingStream() throws IOException { + InputStream originalStream = new ByteArrayInputStream("content".getBytes()); + entity = createEntity(originalStream); + + InputStream content1 = entity.getContent(); + InputStream content2 = entity.getContent(); + + assertSame(content1, content2); + } + + @Test + @DisplayName("Empty stream should be repeatable") + void emptyStream_IsRepeatable() { + // Given - No content provider + HttpExecuteRequest request = HttpExecuteRequest.builder() + .request(httpRequestBuilder.build()) + .build(); + entity = new RepeatableInputStreamRequestEntity(request); + assertTrue(entity.isRepeatable()); + } + + // Helper methods + private RepeatableInputStreamRequestEntity createEntity(InputStream stream) { + return createEntity(() -> stream); + } + + private RepeatableInputStreamRequestEntity createEntity(ContentStreamProvider provider) { + HttpExecuteRequest request = HttpExecuteRequest.builder() + .request(httpRequestBuilder.build()) + .contentStreamProvider(provider) + .build(); + return new RepeatableInputStreamRequestEntity(request); + } +} From 5fb2d35557167737248aa01f1904829368df7187 Mon Sep 17 00:00:00 2001 From: John Viegas Date: Tue, 3 Jun 2025 15:30:26 -0700 Subject: [PATCH 05/14] writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this --- ...epeatableInputStreamRequestEntityTest.java | 56 ++----------------- ...epeatableInputStreamRequestEntityTest.java | 52 +---------------- 2 files changed, 5 insertions(+), 103 deletions(-) diff --git a/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/internal/RepeatableInputStreamRequestEntityTest.java b/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/internal/RepeatableInputStreamRequestEntityTest.java index 4f9286278b3e..104204f1767c 100644 --- a/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/internal/RepeatableInputStreamRequestEntityTest.java +++ b/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/internal/RepeatableInputStreamRequestEntityTest.java @@ -36,12 +36,7 @@ import java.io.InputStream; import java.io.InterruptedIOException; import java.net.URI; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; import java.util.Random; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -344,7 +339,9 @@ public int read() throws IOException { return -1; } hasBeenRead = true; - return data[position++] & 0xFF; + int i = data[position] & 0xFF; + position++; + return i; } @Override @@ -671,51 +668,6 @@ void constructor_WithoutContentType_HandlesGracefully() { assertEquals(100L, entity.getContentLength()); } - @Test - @DisplayName("Entity should handle concurrent write attempts") - void writeTo_ConcurrentWrites_HandlesCorrectly() throws Exception { - // Given - String content = "Concurrent test content"; - ContentStreamProvider provider = () -> new ByteArrayInputStream(content.getBytes()); - SdkHttpRequest httpRequest = httpRequestBuilder.build(); - HttpExecuteRequest request = HttpExecuteRequest.builder() - .request(httpRequest) - .contentStreamProvider(provider) - .build(); - - entity = new RepeatableInputStreamRequestEntity(request); - - // Simulate concurrent writes - int threadCount = 5; - CountDownLatch latch = new CountDownLatch(threadCount); - List outputs = Collections.synchronizedList(new ArrayList<>()); - List exceptions = Collections.synchronizedList(new ArrayList<>()); - - for (int i = 0; i < threadCount; i++) { - new Thread(() -> { - try { - ByteArrayOutputStream output = new ByteArrayOutputStream(); - entity.writeTo(output); - outputs.add(output); - } catch (Exception e) { - exceptions.add(e); - } finally { - latch.countDown(); - } - }).start(); - } - - latch.await(5, TimeUnit.SECONDS); - - // At least one should succeed, others may fail due to stream state - assertFalse(outputs.isEmpty(), "At least one write should succeed"); - for (ByteArrayOutputStream output : outputs) { - if (output.size() > 0) { - assertEquals(content, output.toString()); - } - } - } - @Test @DisplayName("Entity should handle interrupted IO operations") void writeTo_InterruptedStream_ThrowsIOException() throws IOException { @@ -882,7 +834,7 @@ public boolean markSupported() { } @Override - public synchronized void reset() { + public synchronized void reset() { resetCalls.incrementAndGet(); throw new RuntimeException("Reset not supported"); } diff --git a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/RepeatableInputStreamRequestEntityTest.java b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/RepeatableInputStreamRequestEntityTest.java index 73a8017e4cc1..d69eb64bff13 100644 --- a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/RepeatableInputStreamRequestEntityTest.java +++ b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/RepeatableInputStreamRequestEntityTest.java @@ -36,12 +36,7 @@ import java.io.InputStream; import java.io.InterruptedIOException; import java.net.URI; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; import java.util.Random; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -671,51 +666,6 @@ void constructor_WithoutContentType_HandlesGracefully() { assertEquals(100L, entity.getContentLength()); } - @Test - @DisplayName("Entity should handle concurrent write attempts") - void writeTo_ConcurrentWrites_HandlesCorrectly() throws Exception { - // Given - String content = "Concurrent test content"; - ContentStreamProvider provider = () -> new ByteArrayInputStream(content.getBytes()); - SdkHttpRequest httpRequest = httpRequestBuilder.build(); - HttpExecuteRequest request = HttpExecuteRequest.builder() - .request(httpRequest) - .contentStreamProvider(provider) - .build(); - - entity = new RepeatableInputStreamRequestEntity(request); - - // Simulate concurrent writes - int threadCount = 5; - CountDownLatch latch = new CountDownLatch(threadCount); - List outputs = Collections.synchronizedList(new ArrayList<>()); - List exceptions = Collections.synchronizedList(new ArrayList<>()); - - for (int i = 0; i < threadCount; i++) { - new Thread(() -> { - try { - ByteArrayOutputStream output = new ByteArrayOutputStream(); - entity.writeTo(output); - outputs.add(output); - } catch (Exception e) { - exceptions.add(e); - } finally { - latch.countDown(); - } - }).start(); - } - - latch.await(5, TimeUnit.SECONDS); - - // At least one should succeed, others may fail due to stream state - assertFalse(outputs.isEmpty(), "At least one write should succeed"); - for (ByteArrayOutputStream output : outputs) { - if (output.size() > 0) { - assertEquals(content, output.toString()); - } - } - } - @Test @DisplayName("Entity should handle interrupted IO operations") void writeTo_InterruptedStream_ThrowsIOException() throws IOException { @@ -880,7 +830,7 @@ public boolean markSupported() { } @Override - public synchronized void reset() { + public synchronized void reset() { resetCalls.incrementAndGet(); throw new RuntimeException("Reset not supported"); } From 8b83790e3b7e8f93df708b69eb76a9389fae8eb6 Mon Sep 17 00:00:00 2001 From: John Viegas Date: Mon, 9 Jun 2025 09:55:31 -0700 Subject: [PATCH 06/14] 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 --- .../awssdk/http/apache5/Apache5HttpClient.java | 15 ++++++++------- .../apache5/Apache5HttpClientWireMockTest.java | 6 ------ 2 files changed, 8 insertions(+), 13 deletions(-) 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 fd97ea17d05a..eae37a32e411 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 @@ -694,16 +694,17 @@ public PoolingHttpClientConnectionManager create(Apache5HttpClient.DefaultBuilde // TODO : Deprecated method needs to be removed with new replacements SSLConnectionSocketFactory sslsf = getPreferredSocketFactory(configuration, standardOptions); - PoolingHttpClientConnectionManager cm = + PoolingHttpClientConnectionManagerBuilder builder = PoolingHttpClientConnectionManagerBuilder.create() .setSSLSocketFactory(sslsf) .setSchemePortResolver(DefaultSchemePortResolver.INSTANCE) - .setDnsResolver(configuration.dnsResolver) - .setConnectionTimeToLive( - TimeValue.of(standardOptions.get( - SdkHttpConfigurationOption.CONNECTION_TIME_TO_LIVE).toMillis(), - TimeUnit.MILLISECONDS)) - .build(); + .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)); + } + PoolingHttpClientConnectionManager cm = builder.build(); cm.setDefaultMaxPerRoute(standardOptions.get(SdkHttpConfigurationOption.MAX_CONNECTIONS)); 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 113090cab054..5f9ffaf2d3e3 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 @@ -153,12 +153,6 @@ public void credentialPlannerIsInvoked() throws Exception { mockProxyServer.verify(2, RequestPatternBuilder.allRequests()); } - - @Override - public void connectionPoolingWorks() throws Exception { - // TODO : future PR will handle this. - } - @Test public void overrideDnsResolver_WithDnsMatchingResolver_successful() throws Exception { overrideDnsResolver("magic.local.host"); From 14a2ead68de1a76caac747f8d7d533589ccbc816 Mon Sep 17 00:00:00 2001 From: John Viegas Date: Tue, 17 Jun 2025 14:55:58 -0700 Subject: [PATCH 07/14] disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json --- .brazil.json | 2 + .../http/apache5/Apache5HttpClient.java | 4 +- .../awssdk/http/SdkHttpClientTestSuite.java | 44 +++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/.brazil.json b/.brazil.json index d13213efd818..3b29aff0123a 100644 --- a/.brazil.json +++ b/.brazil.json @@ -141,7 +141,9 @@ "io.netty:netty-transport-classes-epoll": { "packageName": "Netty4", "packageVersion": "4.1" }, "io.netty:netty-transport-native-unix-common": { "packageName": "Netty4", "packageVersion": "4.1" }, "org.apache.httpcomponents:httpclient": { "packageName": "Apache-HttpComponents-HttpClient", "packageVersion": "4.5.x" }, + "org.apache.httpcomponents.client5:httpclient5": { "packageName": "Apache-HttpComponents-HttpClient5", "packageVersion": "5.0.x" }, "org.apache.httpcomponents:httpcore": { "packageName": "Apache-HttpComponents-HttpCore", "packageVersion": "4.4.x" }, + "org.apache.httpcomponents.core5:httpcore5": { "packageName": "Apache-HttpComponents-HttpCore5", "packageVersion": "5.0.x" }, "org.eclipse.jdt:org.eclipse.jdt.core": { "packageName": "AwsJavaSdk-Codegen-EclipseJdtDependencies", "packageVersion": "2.0" }, "org.eclipse.text:org.eclipse.text": { "packageName": "AwsJavaSdk-Codegen-EclipseJdtDependencies", "packageVersion": "2.0" }, "org.reactivestreams:reactive-streams": { "packageName": "Maven-org-reactivestreams_reactive-streams", "packageVersion": "1.x" }, 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 eae37a32e411..0af8b7bffb1b 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 @@ -166,7 +166,9 @@ private ConnectionManagerAwareHttpClient createClient(Apache5HttpClient.DefaultB .setUserAgent("") // SDK will set the user agent header in the pipeline. Don't let Apache waste time .setConnectionManager(ClientConnectionManagerFactory.wrap(cm)) //This is done to keep backward compatibility with Apache 4.x - .disableRedirectHandling(); + .disableRedirectHandling() + // SDK handles retries , we do not need additional retries on Http clients. + .disableAutomaticRetries(); addProxyConfig(builder, configuration); diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java index 71a8f5cc18ab..94b0e850e600 100644 --- a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java @@ -20,6 +20,8 @@ import static com.github.tomakehurst.wiremock.client.WireMock.any; import static com.github.tomakehurst.wiremock.client.WireMock.containing; import static com.github.tomakehurst.wiremock.client.WireMock.equalTo; +import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor; +import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching; import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo; import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; @@ -59,6 +61,7 @@ public abstract class SdkHttpClientTestSuite { private static final Logger LOG = Logger.loggerFor(SdkHttpClientTestSuite.class); private static final ConnectionCountingTrafficListener CONNECTION_COUNTER = new ConnectionCountingTrafficListener(); + private static final int HTTP_TOO_MANY_REQUESTS = 429; @Rule public WireMockRule mockServer = createWireMockRule(); @@ -218,6 +221,47 @@ public void testCustomTlsTrustManagerAndTrustAllFails() throws Exception { assertThatThrownBy(() -> createSdkHttpClient(httpClientOptions)).isInstanceOf(IllegalArgumentException.class); } + @Test + public void doesNotRetryOn429StatusCode() throws Exception { + SdkHttpClientOptions httpClientOptions = new SdkHttpClientOptions(); + httpClientOptions.trustAll(true); + try (SdkHttpClient client = createSdkHttpClient(httpClientOptions)) { + stubForMockRequest(HTTP_TOO_MANY_REQUESTS); + SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port(), SdkHttpMethod.POST); + HttpExecuteResponse response = client.prepareRequest(HttpExecuteRequest.builder() + .request(req) + .contentStreamProvider(req.contentStreamProvider().orElse(null)) + .build()) + .call(); + // Drain the response body if present + response.responseBody().ifPresent(IoUtils::drainInputStream); + + // Verify the response has 429 status code + assertThat(response.httpResponse().statusCode()).isEqualTo(429); + + // Verify that the request was made exactly once (no retries) + mockServer.verify(1, postRequestedFor(urlEqualTo("/")) + .withHeader("Host", containing("localhost"))); + + // Reset and make another request to ensure it's not a connection issue + mockServer.resetAll(); + stubForMockRequest(200); + HttpExecuteResponse successResponse = client.prepareRequest(HttpExecuteRequest.builder() + .request(req) + .contentStreamProvider(req.contentStreamProvider().orElse(null)) + .build()) + .call(); + successResponse.responseBody().ifPresent(IoUtils::drainInputStream); + + // Verify the second request succeeded + assertThat(successResponse.httpResponse().statusCode()).isEqualTo(200); + + // Verify only one request was made for each call (no retries) + mockServer.verify(1, postRequestedFor(urlEqualTo("/"))); + } + } + + protected void testForResponseCode(int returnCode) throws Exception { testForResponseCode(returnCode, SdkHttpMethod.POST); } From 5f33ad98dc48e3284e487bc9c8ffacd039c36f1c Mon Sep 17 00:00:00 2001 From: John Viegas Date: Wed, 18 Jun 2025 17:41:48 -0700 Subject: [PATCH 08/14] Added Test case for Async , handled review ocmments --- .../http/SdkAsyncHttpClientH1TestSuite.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkAsyncHttpClientH1TestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkAsyncHttpClientH1TestSuite.java index fc76f370ccf4..3345ee0d4153 100644 --- a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkAsyncHttpClientH1TestSuite.java +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkAsyncHttpClientH1TestSuite.java @@ -172,6 +172,26 @@ public void connectionsArePooledByHostAndPort() throws InterruptedException { } + @Test + public void doesNotRetryOn429StatusCode() throws InterruptedException { + server.return429OnFirstRequest = true; + server.closeConnection = false; + + HttpTestUtils.sendGetRequest(server.port(), client).join(); + // Wait to ensure no retries happen + Thread.sleep(100); + + // Verify only one request was made (no retries) + assertThat(server.requestCount).isEqualTo(1); + + // Send second request to verify connection reuse works after 429 + HttpTestUtils.sendGetRequest(server.port(), client).join(); + + // Verify connection was reused and total of 2 requests + assertThat(server.channels.size()).isEqualTo(1); + assertThat(server.requestCount).isEqualTo(2); + } + private static class Server extends ChannelInitializer { private static final byte[] CONTENT = "helloworld".getBytes(StandardCharsets.UTF_8); private ServerBootstrap bootstrap; @@ -181,6 +201,9 @@ private static class Server extends ChannelInitializer { private SslContext sslCtx; private boolean return500OnFirstRequest; private boolean closeConnection; + private boolean return429OnFirstRequest; + private volatile int requestCount = 0; + public void init() throws Exception { SelfSignedCertificate ssc = new SelfSignedCertificate(); @@ -218,10 +241,14 @@ private class BehaviorTestChannelHandler extends ChannelDuplexHandler { @Override public void channelRead(ChannelHandlerContext ctx, Object msg) { if (msg instanceof HttpRequest) { + requestCount++; HttpResponseStatus status; if (ctx.channel().equals(channels.get(0)) && return500OnFirstRequest) { status = INTERNAL_SERVER_ERROR; + } else if (ctx.channel().equals(channels.get(0)) && return429OnFirstRequest) { + status = HttpResponseStatus.TOO_MANY_REQUESTS; + return429OnFirstRequest = false; // Reset after first use } else { status = OK; } From 270c0f7c7f61c99cf0935a43d5e7fd49412a5592 Mon Sep 17 00:00:00 2001 From: John Viegas Date: Fri, 20 Jun 2025 18:30:20 -0700 Subject: [PATCH 09/14] Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x --- .../http/apache5/Apache5HttpClient.java | 68 ++++++-- .../http/apache5/MetricReportingTest.java | 8 +- .../awssdk/http/SdkHttpClientTestSuite.java | 162 +++++++++++++++++- 3 files changed, 208 insertions(+), 30 deletions(-) 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 0af8b7bffb1b..2965cae31d4a 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 @@ -23,7 +23,9 @@ import static software.amazon.awssdk.http.apache5.internal.conn.ClientConnectionRequestFactory.THREAD_LOCAL_REQUEST_METRIC_COLLECTOR; import static software.amazon.awssdk.utils.NumericUtils.saturatedCast; +import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.InputStream; import java.net.InetAddress; import java.security.KeyManagementException; import java.security.NoSuchAlgorithmException; @@ -56,10 +58,11 @@ import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory; import org.apache.hc.core5.http.ClassicHttpResponse; import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HttpEntity; +import org.apache.hc.core5.http.HttpResponse; import org.apache.hc.core5.http.config.Registry; import org.apache.hc.core5.http.impl.io.HttpRequestExecutor; import org.apache.hc.core5.http.io.SocketConfig; -import org.apache.hc.core5.http.io.entity.BufferedHttpEntity; import org.apache.hc.core5.io.CloseMode; import org.apache.hc.core5.pool.PoolStats; import org.apache.hc.core5.ssl.SSLInitializationException; @@ -80,6 +83,7 @@ import software.amazon.awssdk.http.apache5.internal.DefaultConfiguration; import software.amazon.awssdk.http.apache5.internal.SdkProxyRoutePlanner; import software.amazon.awssdk.http.apache5.internal.conn.ClientConnectionManagerFactory; +import software.amazon.awssdk.http.apache5.internal.conn.ConnectionAwareInputStream; import software.amazon.awssdk.http.apache5.internal.conn.IdleConnectionReaper; import software.amazon.awssdk.http.apache5.internal.conn.SdkConnectionKeepAliveStrategy; import software.amazon.awssdk.http.apache5.internal.conn.SdkTlsSocketFactory; @@ -90,6 +94,7 @@ import software.amazon.awssdk.metrics.MetricCollector; import software.amazon.awssdk.metrics.NoOpMetricCollector; import software.amazon.awssdk.utils.AttributeMap; +import software.amazon.awssdk.utils.IoUtils; import software.amazon.awssdk.utils.Logger; import software.amazon.awssdk.utils.Validate; @@ -262,20 +267,14 @@ private HttpExecuteResponse execute(HttpUriRequestBase apacheRequest, MetricColl HttpClientContext localRequestContext = Apache5Utils.newClientContext(requestConfig.proxyConfiguration()); THREAD_LOCAL_REQUEST_METRIC_COLLECTOR.set(metricCollector); try { - return httpClient.execute(apacheRequest, localRequestContext, response -> { - - // TODO : This is required since Apache5 closes streams immediately, check memory impacts because of this. - if (response.getEntity() != null) { - response.setEntity(new BufferedHttpEntity(response.getEntity())); - } - return createResponse(response, apacheRequest); - }); + HttpResponse httpResponse = httpClient.execute(apacheRequest, localRequestContext); + // Create a connection-aware input stream that closes the response when closed + return createResponse(httpResponse, apacheRequest); } finally { THREAD_LOCAL_REQUEST_METRIC_COLLECTOR.remove(); } } - private HttpUriRequestBase toApacheRequest(HttpExecuteRequest request) { return apacheHttpRequestFactory.create(request, requestConfig); } @@ -288,7 +287,7 @@ private HttpUriRequestBase toApacheRequest(HttpExecuteRequest request) { * @throws IOException If there were any problems getting any response information from the * HttpClient method object. */ - private HttpExecuteResponse createResponse(ClassicHttpResponse apacheHttpResponse, + private HttpExecuteResponse createResponse(HttpResponse apacheHttpResponse, HttpUriRequestBase apacheRequest) throws IOException { SdkHttpResponse.Builder responseBuilder = SdkHttpResponse.builder() @@ -302,17 +301,50 @@ private HttpExecuteResponse createResponse(ClassicHttpResponse apacheHttpRespons responseBuilder.appendHeader(header.getName(), header.getValue()); } - - AbortableInputStream responseBody = apacheHttpResponse.getEntity() != null ? - toAbortableInputStream(apacheHttpResponse, apacheRequest) : null; - + AbortableInputStream responseBody = getResponseBody(apacheHttpResponse, apacheRequest); return HttpExecuteResponse.builder().response(responseBuilder.build()).responseBody(responseBody).build(); } - private AbortableInputStream toAbortableInputStream(ClassicHttpResponse apacheHttpResponse, - HttpUriRequestBase apacheRequest) throws IOException { - return AbortableInputStream.create(apacheHttpResponse.getEntity().getContent(), apacheRequest::abort); + private AbortableInputStream getResponseBody(HttpResponse apacheHttpResponse, + HttpUriRequestBase apacheRequest) throws IOException { + AbortableInputStream responseBody = null; + if (apacheHttpResponse instanceof ClassicHttpResponse) { + ClassicHttpResponse classicResponse = (ClassicHttpResponse) apacheHttpResponse; + HttpEntity entity = classicResponse.getEntity(); + if (entity != null) { + if (entity.getContentLength() == 0) { + // Close immediately for empty responses + classicResponse.close(); + responseBody = AbortableInputStream.create(new ByteArrayInputStream(new byte[0])); + } else { + responseBody = createConnectionAwareStream(classicResponse, apacheRequest); + } + } else { + // No entity, close the response immediately + classicResponse.close(); + } + } + return responseBody; + } + + private AbortableInputStream createConnectionAwareStream(ClassicHttpResponse apacheResponse, + HttpUriRequestBase apacheRequest) throws IOException { + InputStream entityStream = null; + try { + entityStream = apacheResponse.getEntity().getContent(); + return AbortableInputStream.create( + new ConnectionAwareInputStream(entityStream, apacheResponse), + () -> { + apacheRequest.abort(); + IoUtils.closeQuietlyV2(apacheResponse, log); + } + ); + } catch (IOException e) { + // Ensure response is closed on error + IoUtils.closeQuietlyV2(apacheResponse, log); + throw e; + } } private Apache5HttpRequestConfig createRequestConfig(DefaultBuilder builder, 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 d2c3cb5147b5..8437f74e8859 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 @@ -64,12 +64,8 @@ public class MetricReportingTest { @Before public void methodSetup() throws IOException { - ClassicHttpResponse httpResponse = new BasicClassicHttpResponse(200, "OK"); - when(mockHttpClient.execute(any(HttpUriRequest.class), any(HttpContext.class), any(HttpClientResponseHandler.class))) - .thenAnswer(invocation -> { - HttpClientResponseHandler handler = invocation.getArgument(2); - return handler.handleResponse(httpResponse); - }); + when(mockHttpClient.execute(any(HttpUriRequest.class), any(HttpContext.class))) + .thenReturn(new BasicClassicHttpResponse(200, "OK")); when(mockHttpClient.getHttpClientConnectionManager()).thenReturn(cm); diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java index b2706354d335..6d473a9a16eb 100644 --- a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java @@ -20,6 +20,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.any; import static com.github.tomakehurst.wiremock.client.WireMock.containing; import static com.github.tomakehurst.wiremock.client.WireMock.equalTo; +import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor; import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor; import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching; @@ -27,6 +28,7 @@ import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.AssertionsForClassTypes.assertThatCode; import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder; @@ -236,6 +238,147 @@ public void doesNotRetryOn429StatusCode() throws Exception { } } + @Test + public void handlesNoContentResponse() throws Exception { + SdkHttpClient client = createSdkHttpClient(); + + mockServer.stubFor(any(urlPathEqualTo("/no-content")) + .willReturn(aResponse() + .withStatus(204))); + + SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + "/no-content", + SdkHttpMethod.DELETE); + HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder() + .request(req) + .build()) + .call(); + + assertThat(rsp.httpResponse().statusCode()).isEqualTo(204); + assertThat(rsp.responseBody()).isEmpty(); + } + + @Test + public void handlesLargeResponseBody() throws Exception { + SdkHttpClient client = createSdkHttpClient(); + // Create a large response body (1MB) + byte[] largeBody = new byte[1024 * 1024]; + for (int i = 0; i < largeBody.length; i++) { + largeBody[i] = (byte) (i % 256); + } + mockServer.stubFor(any(urlPathEqualTo("/large")) + .willReturn(aResponse() + .withStatus(200) + .withBody(largeBody))); + + SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + "/large", SdkHttpMethod.GET); + HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder() + .request(req) + .build()) + .call(); + + assertThat(rsp.httpResponse().statusCode()).isEqualTo(200); + assertThat(rsp.responseBody()).isPresent(); + + // Read the entire response and verify + byte[] readBuffer = IoUtils.toByteArray(rsp.responseBody().get()); + assertThat(readBuffer).isEqualTo(largeBody); + } + + @Test + public void testAbortResponseStream() throws Exception { + SdkHttpClient client = createSdkHttpClient(); + + mockServer.stubFor(any(urlPathEqualTo("/streaming")) + .willReturn(aResponse() + .withStatus(200) + .withBody("This is a streaming response that should be aborted"))); + + SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + "/streaming", SdkHttpMethod.POST); + ExecutableHttpRequest executableRequest = + client.prepareRequest(HttpExecuteRequest.builder() + .request(req) + .contentStreamProvider(req.contentStreamProvider().orElse(null)) + .build()); + HttpExecuteResponse rsp = executableRequest.call(); + + assertThat(rsp.httpResponse().statusCode()).isEqualTo(200); + assertThat(rsp.responseBody()).isPresent(); + + // Verify the stream is abortable + AbortableInputStream stream = rsp.responseBody().get(); + assertThat(stream).isInstanceOf(AbortableInputStream.class); + + // Read a few bytes + byte[] buffer = new byte[10]; + int bytesRead = stream.read(buffer); + assertThat(bytesRead).isGreaterThan(0); + assertThatCode(() -> stream.abort()).doesNotThrowAnyException(); + stream.close(); + } + + @Test + public void handlesMultipleSequentialRequests() throws Exception { + SdkHttpClient client = createSdkHttpClient(); + + mockServer.stubFor(any(urlPathEqualTo("/sequential")) + .willReturn(aResponse() + .withStatus(200) + .withBody("Response body"))); + + // Execute multiple requests sequentially + for (int i = 0; i < 5; i++) { + SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + "/sequential", SdkHttpMethod.GET); + HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder() + .request(req) + .build()) + .call(); + + assertThat(rsp.httpResponse().statusCode()).isEqualTo(200); + assertThat(IoUtils.toUtf8String(rsp.responseBody().orElse(null))).isEqualTo("Response body"); + } + mockServer.verify(5, getRequestedFor(urlEqualTo("/sequential"))); + } + + @Test + public void handlesVariousContentLengths() throws Exception { + SdkHttpClient client = createSdkHttpClient(); + int[] contentLengths = {0, 1, 100, 1024, 65536}; + + for (int length : contentLengths) { + String path = "/content-length-" + length; + byte[] body = new byte[length]; + for (int i = 0; i < length; i++) { + body[i] = (byte) ('A' + (i % 26)); + } + + mockServer.stubFor(any(urlPathEqualTo(path)) + .willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Length", String.valueOf(length)) + .withBody(body))); + + SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + path, SdkHttpMethod.GET); + HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder() + .request(req) + .build()) + .call(); + + assertThat(rsp.httpResponse().statusCode()).isEqualTo(200); + + if (length == 0) { + // Empty body should still have a response body present, but EOF immediately + if (rsp.responseBody().isPresent()) { + assertThat(rsp.responseBody().get().read()).isEqualTo(-1); + } + } else { + assertThat(rsp.responseBody()).isPresent(); + byte[] readBody = IoUtils.toByteArray(rsp.responseBody().get()); + assertThat(readBody).isEqualTo(body); + } + } + } + + private void validateStatusCodeWithRetryCheck(SdkHttpClient client, int expectedStatusCode, int expectedRequestCount) throws IOException { @@ -294,7 +437,6 @@ protected void testForResponseCodeUsingHttps(SdkHttpClient client, int returnCod .orElse(null)) .build()) .call(); - validateResponse(rsp, returnCode, sdkHttpMethod); } @@ -302,7 +444,6 @@ protected void stubForMockRequest(int returnCode) { ResponseDefinitionBuilder responseBuilder = aResponse().withStatus(returnCode) .withHeader("Some-Header", "With Value") .withBody("hello"); - if (returnCode >= 300 && returnCode <= 399) { responseBuilder.withHeader("Location", "Some New Location"); } @@ -316,7 +457,6 @@ private void validateResponse(HttpExecuteResponse response, int returnCode, SdkH RequestPatternBuilder patternBuilder = RequestPatternBuilder.newRequestPattern(requestMethod, urlMatching("/")) .withHeader("Host", containing("localhost")) .withHeader("User-Agent", equalTo("hello-world!")); - if (method == SdkHttpMethod.HEAD) { patternBuilder.withRequestBody(absent()); } else { @@ -359,13 +499,23 @@ private static SdkHttpFullRequest.Builder mockSdkRequestBuilder(String uriString return requestBuilder; } - protected SdkHttpFullRequest mockSdkRequest(String uriString, SdkHttpMethod method) { - SdkHttpFullRequest.Builder requestBuilder = mockSdkRequestBuilder(uriString, method); - if (method != SdkHttpMethod.HEAD) { + URI uri = URI.create(uriString); + SdkHttpFullRequest.Builder requestBuilder = SdkHttpFullRequest.builder() + .uri(uri) + .method(method) + .putHeader("Host", uri.getHost()) + .putHeader("User-Agent", "hello-world!"); + + // Only add body for methods that typically have a body + if (method != SdkHttpMethod.HEAD && method != SdkHttpMethod.GET && method != SdkHttpMethod.DELETE) { byte[] content = "Body".getBytes(StandardCharsets.UTF_8); requestBuilder.putHeader("Content-Length", Integer.toString(content.length)); requestBuilder.contentStreamProvider(() -> new ByteArrayInputStream(content)); + } else if (method == SdkHttpMethod.GET || method == SdkHttpMethod.DELETE) { + // For GET and DELETE, explicitly set Content-Length to 0 or don't set it at all + // Some clients like AWS CRT are strict about this + requestBuilder.contentStreamProvider(() -> new ByteArrayInputStream(new byte[0])); } return requestBuilder.build(); From e2d16adde7a8a19bc1f6853be60c82bc993005f6 Mon Sep 17 00:00:00 2001 From: John Viegas Date: Fri, 20 Jun 2025 21:13:52 -0700 Subject: [PATCH 10/14] Fix compilation issues --- .../http/apache5/Apache5HttpClient.java | 22 +-- .../awssdk/http/SdkHttpClientTestSuite.java | 158 +++++++++++++++++- 2 files changed, 159 insertions(+), 21 deletions(-) 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 2965cae31d4a..08f760a11839 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 @@ -318,7 +318,7 @@ private AbortableInputStream getResponseBody(HttpResponse apacheHttpResponse, classicResponse.close(); responseBody = AbortableInputStream.create(new ByteArrayInputStream(new byte[0])); } else { - responseBody = createConnectionAwareStream(classicResponse, apacheRequest); + responseBody = toAbortableInputStream(classicResponse, apacheRequest); } } else { // No entity, close the response immediately @@ -328,23 +328,9 @@ private AbortableInputStream getResponseBody(HttpResponse apacheHttpResponse, return responseBody; } - private AbortableInputStream createConnectionAwareStream(ClassicHttpResponse apacheResponse, - HttpUriRequestBase apacheRequest) throws IOException { - InputStream entityStream = null; - try { - entityStream = apacheResponse.getEntity().getContent(); - return AbortableInputStream.create( - new ConnectionAwareInputStream(entityStream, apacheResponse), - () -> { - apacheRequest.abort(); - IoUtils.closeQuietlyV2(apacheResponse, log); - } - ); - } catch (IOException e) { - // Ensure response is closed on error - IoUtils.closeQuietlyV2(apacheResponse, log); - throw e; - } + private AbortableInputStream toAbortableInputStream(ClassicHttpResponse apacheResponse, + HttpUriRequestBase apacheRequest) throws IOException { + return AbortableInputStream.create(apacheResponse.getEntity().getContent(), apacheRequest::abort); } private Apache5HttpRequestConfig createRequestConfig(DefaultBuilder builder, diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java index b2706354d335..ae62a951e8b3 100644 --- a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java @@ -20,6 +20,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.any; import static com.github.tomakehurst.wiremock.client.WireMock.containing; import static com.github.tomakehurst.wiremock.client.WireMock.equalTo; +import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor; import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor; import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching; @@ -27,6 +28,7 @@ import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.AssertionsForClassTypes.assertThatCode; import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder; @@ -236,6 +238,146 @@ public void doesNotRetryOn429StatusCode() throws Exception { } } + @Test + public void handlesNoContentResponse() throws Exception { + SdkHttpClient client = createSdkHttpClient(); + + mockServer.stubFor(any(urlPathEqualTo("/no-content")) + .willReturn(aResponse() + .withStatus(204))); + + SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + "/no-content", + SdkHttpMethod.DELETE); + HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder() + .request(req) + .build()) + .call(); + + assertThat(rsp.httpResponse().statusCode()).isEqualTo(204); + assertThat(rsp.responseBody()).isEmpty(); + } + + @Test + public void handlesLargeResponseBody() throws Exception { + SdkHttpClient client = createSdkHttpClient(); + // Create a large response body (1MB) + byte[] largeBody = new byte[1024 * 1024]; + for (int i = 0; i < largeBody.length; i++) { + largeBody[i] = (byte) (i % 256); + } + mockServer.stubFor(any(urlPathEqualTo("/large")) + .willReturn(aResponse() + .withStatus(200) + .withBody(largeBody))); + + SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + "/large", SdkHttpMethod.GET); + HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder() + .request(req) + .build()) + .call(); + + assertThat(rsp.httpResponse().statusCode()).isEqualTo(200); + assertThat(rsp.responseBody()).isPresent(); + + // Read the entire response and verify + byte[] readBuffer = IoUtils.toByteArray(rsp.responseBody().get()); + assertThat(readBuffer).isEqualTo(largeBody); + } + + @Test + public void testAbortResponseStream() throws Exception { + SdkHttpClient client = createSdkHttpClient(); + + mockServer.stubFor(any(urlPathEqualTo("/streaming")) + .willReturn(aResponse() + .withStatus(200) + .withBody("This is a streaming response that should be aborted"))); + + SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + "/streaming", SdkHttpMethod.POST); + ExecutableHttpRequest executableRequest = client.prepareRequest(HttpExecuteRequest.builder() + .request(req) + .contentStreamProvider(req.contentStreamProvider().orElse(null)) + .build()); + HttpExecuteResponse rsp = executableRequest.call(); + + assertThat(rsp.httpResponse().statusCode()).isEqualTo(200); + assertThat(rsp.responseBody()).isPresent(); + + // Verify the stream is abortable + AbortableInputStream stream = rsp.responseBody().get(); + assertThat(stream).isInstanceOf(AbortableInputStream.class); + + // Read a few bytes + byte[] buffer = new byte[10]; + int bytesRead = stream.read(buffer); + assertThat(bytesRead).isGreaterThan(0); + assertThatCode(() -> stream.abort()).doesNotThrowAnyException(); + stream.close(); + } + + @Test + public void handlesMultipleSequentialRequests() throws Exception { + SdkHttpClient client = createSdkHttpClient(); + + mockServer.stubFor(any(urlPathEqualTo("/sequential")) + .willReturn(aResponse() + .withStatus(200) + .withBody("Response body"))); + + // Execute multiple requests sequentially + for (int i = 0; i < 5; i++) { + SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + "/sequential", SdkHttpMethod.GET); + HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder() + .request(req) + .build()) + .call(); + + assertThat(rsp.httpResponse().statusCode()).isEqualTo(200); + assertThat(IoUtils.toUtf8String(rsp.responseBody().orElse(null))).isEqualTo("Response body"); + } + mockServer.verify(5, getRequestedFor(urlEqualTo("/sequential"))); + } + + @Test + public void handlesVariousContentLengths() throws Exception { + SdkHttpClient client = createSdkHttpClient(); + int[] contentLengths = {0, 1, 100, 1024, 65536}; + + for (int length : contentLengths) { + String path = "/content-length-" + length; + byte[] body = new byte[length]; + for (int i = 0; i < length; i++) { + body[i] = (byte) ('A' + (i % 26)); + } + + mockServer.stubFor(any(urlPathEqualTo(path)) + .willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Length", String.valueOf(length)) + .withBody(body))); + + SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + path, SdkHttpMethod.GET); + HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder() + .request(req) + .build()) + .call(); + + assertThat(rsp.httpResponse().statusCode()).isEqualTo(200); + + if (length == 0) { + // Empty body should still have a response body present, but EOF immediately + if (rsp.responseBody().isPresent()) { + assertThat(rsp.responseBody().get().read()).isEqualTo(-1); + } + } else { + assertThat(rsp.responseBody()).isPresent(); + byte[] readBody = IoUtils.toByteArray(rsp.responseBody().get()); + assertThat(readBody).isEqualTo(body); + } + } + } + + private void validateStatusCodeWithRetryCheck(SdkHttpClient client, int expectedStatusCode, int expectedRequestCount) throws IOException { @@ -359,13 +501,23 @@ private static SdkHttpFullRequest.Builder mockSdkRequestBuilder(String uriString return requestBuilder; } - protected SdkHttpFullRequest mockSdkRequest(String uriString, SdkHttpMethod method) { - SdkHttpFullRequest.Builder requestBuilder = mockSdkRequestBuilder(uriString, method); - if (method != SdkHttpMethod.HEAD) { + URI uri = URI.create(uriString); + SdkHttpFullRequest.Builder requestBuilder = SdkHttpFullRequest.builder() + .uri(uri) + .method(method) + .putHeader("Host", uri.getHost()) + .putHeader("User-Agent", "hello-world!"); + + // Only add body for methods that typically have a body + if (method != SdkHttpMethod.HEAD && method != SdkHttpMethod.GET && method != SdkHttpMethod.DELETE) { byte[] content = "Body".getBytes(StandardCharsets.UTF_8); requestBuilder.putHeader("Content-Length", Integer.toString(content.length)); requestBuilder.contentStreamProvider(() -> new ByteArrayInputStream(content)); + } else if (method == SdkHttpMethod.GET || method == SdkHttpMethod.DELETE) { + // For GET and DELETE, explicitly set Content-Length to 0 or don't set it at all + // Some clients like AWS CRT are strict about this + requestBuilder.contentStreamProvider(() -> new ByteArrayInputStream(new byte[0])); } return requestBuilder.build(); From 85efdaf1b665d1c76feeed10fb852ba4bf19a18b Mon Sep 17 00:00:00 2001 From: John Viegas Date: Fri, 20 Jun 2025 21:18:43 -0700 Subject: [PATCH 11/14] Fix checkstyle issues --- .../amazon/awssdk/http/apache5/Apache5HttpClient.java | 3 --- .../amazon/awssdk/http/SdkHttpClientTestSuite.java | 10 ++++++---- 2 files changed, 6 insertions(+), 7 deletions(-) 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 08f760a11839..8a21cbcb299d 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 @@ -25,7 +25,6 @@ import java.io.ByteArrayInputStream; import java.io.IOException; -import java.io.InputStream; import java.net.InetAddress; import java.security.KeyManagementException; import java.security.NoSuchAlgorithmException; @@ -83,7 +82,6 @@ import software.amazon.awssdk.http.apache5.internal.DefaultConfiguration; import software.amazon.awssdk.http.apache5.internal.SdkProxyRoutePlanner; import software.amazon.awssdk.http.apache5.internal.conn.ClientConnectionManagerFactory; -import software.amazon.awssdk.http.apache5.internal.conn.ConnectionAwareInputStream; import software.amazon.awssdk.http.apache5.internal.conn.IdleConnectionReaper; import software.amazon.awssdk.http.apache5.internal.conn.SdkConnectionKeepAliveStrategy; import software.amazon.awssdk.http.apache5.internal.conn.SdkTlsSocketFactory; @@ -94,7 +92,6 @@ import software.amazon.awssdk.metrics.MetricCollector; import software.amazon.awssdk.metrics.NoOpMetricCollector; import software.amazon.awssdk.utils.AttributeMap; -import software.amazon.awssdk.utils.IoUtils; import software.amazon.awssdk.utils.Logger; import software.amazon.awssdk.utils.Validate; diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java index ae62a951e8b3..20ce87ea8066 100644 --- a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java @@ -294,10 +294,12 @@ public void testAbortResponseStream() throws Exception { .withBody("This is a streaming response that should be aborted"))); SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + "/streaming", SdkHttpMethod.POST); - ExecutableHttpRequest executableRequest = client.prepareRequest(HttpExecuteRequest.builder() - .request(req) - .contentStreamProvider(req.contentStreamProvider().orElse(null)) - .build()); + ExecutableHttpRequest executableRequest = client.prepareRequest( + HttpExecuteRequest.builder() + .request(req) + .contentStreamProvider(req.contentStreamProvider() + .orElse(null)) + .build()); HttpExecuteResponse rsp = executableRequest.call(); assertThat(rsp.httpResponse().statusCode()).isEqualTo(200); From bb70f7dc7b8986c54b37af720c204c13d3a8cb78 Mon Sep 17 00:00:00 2001 From: John Viegas Date: Fri, 20 Jun 2025 21:26:54 -0700 Subject: [PATCH 12/14] Remove test which are specific to apache http --- .../apache/ApacheHttpClientWireMockTest.java | 42 +++++ .../Apache5HttpClientWireMockTest.java | 44 +++++ .../awssdk/http/SdkHttpClientTestSuite.java | 160 +----------------- 3 files changed, 89 insertions(+), 157 deletions(-) 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 d9eb7bdd80df..27c09184a035 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 @@ -19,6 +19,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.any; 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.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static software.amazon.awssdk.http.SdkHttpConfigurationOption.TRUST_ALL_CERTIFICATES; @@ -46,6 +47,7 @@ import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; import software.amazon.awssdk.http.HttpExecuteRequest; +import software.amazon.awssdk.http.HttpExecuteResponse; import software.amazon.awssdk.http.SdkHttpClient; import software.amazon.awssdk.http.SdkHttpClientTestSuite; import software.amazon.awssdk.http.SdkHttpFullRequest; @@ -53,6 +55,7 @@ import software.amazon.awssdk.http.apache.internal.ApacheHttpRequestConfig; import software.amazon.awssdk.http.apache.internal.impl.ConnectionManagerAwareHttpClient; import software.amazon.awssdk.utils.AttributeMap; +import software.amazon.awssdk.utils.IoUtils; @RunWith(MockitoJUnitRunner.class) public class ApacheHttpClientWireMockTest extends SdkHttpClientTestSuite { @@ -179,6 +182,45 @@ public void explicitNullDnsResolver_WithLocalhost_successful() throws Exception overrideDnsResolver("localhost", true); } + @Test + public void handlesVariousContentLengths() throws Exception { + SdkHttpClient client = createSdkHttpClient(); + int[] contentLengths = {0, 1, 100, 1024, 65536}; + + for (int length : contentLengths) { + String path = "/content-length-" + length; + byte[] body = new byte[length]; + for (int i = 0; i < length; i++) { + body[i] = (byte) ('A' + (i % 26)); + } + + mockServer.stubFor(any(urlPathEqualTo(path)) + .willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Length", String.valueOf(length)) + .withBody(body))); + + SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + path, SdkHttpMethod.GET); + HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder() + .request(req) + .build()) + .call(); + + assertThat(rsp.httpResponse().statusCode()).isEqualTo(200); + + if (length == 0) { + // Empty body should still have a response body present, but EOF immediately + if (rsp.responseBody().isPresent()) { + assertThat(rsp.responseBody().get().read()).isEqualTo(-1); + } + } else { + assertThat(rsp.responseBody()).isPresent(); + byte[] readBody = IoUtils.toByteArray(rsp.responseBody().get()); + assertThat(readBody).isEqualTo(body); + } + } + } + private void overrideDnsResolver(String hostName) throws IOException { overrideDnsResolver(hostName, false); } 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 5f9ffaf2d3e3..c5f609bd9e7b 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 @@ -19,6 +19,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.any; 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.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static software.amazon.awssdk.http.SdkHttpConfigurationOption.TRUST_ALL_CERTIFICATES; @@ -47,6 +48,7 @@ import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; import software.amazon.awssdk.http.HttpExecuteRequest; +import software.amazon.awssdk.http.HttpExecuteResponse; import software.amazon.awssdk.http.SdkHttpClient; import software.amazon.awssdk.http.SdkHttpClientTestSuite; import software.amazon.awssdk.http.SdkHttpFullRequest; @@ -54,6 +56,7 @@ import software.amazon.awssdk.http.apache5.internal.Apache5HttpRequestConfig; import software.amazon.awssdk.http.apache5.internal.impl.ConnectionManagerAwareHttpClient; import software.amazon.awssdk.utils.AttributeMap; +import software.amazon.awssdk.utils.IoUtils; @RunWith(MockitoJUnitRunner.class) public class Apache5HttpClientWireMockTest extends SdkHttpClientTestSuite { @@ -173,6 +176,47 @@ public void explicitNullDnsResolver_WithLocalhost_successful() throws Exception overrideDnsResolver("localhost", true); } + + + @Test + public void handlesVariousContentLengths() throws Exception { + SdkHttpClient client = createSdkHttpClient(); + int[] contentLengths = {0, 1, 100, 1024, 65536}; + + for (int length : contentLengths) { + String path = "/content-length-" + length; + byte[] body = new byte[length]; + for (int i = 0; i < length; i++) { + body[i] = (byte) ('A' + (i % 26)); + } + + mockServer.stubFor(any(urlPathEqualTo(path)) + .willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Length", String.valueOf(length)) + .withBody(body))); + + SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + path, SdkHttpMethod.GET); + HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder() + .request(req) + .build()) + .call(); + + assertThat(rsp.httpResponse().statusCode()).isEqualTo(200); + + if (length == 0) { + // Empty body should still have a response body present, but EOF immediately + if (rsp.responseBody().isPresent()) { + assertThat(rsp.responseBody().get().read()).isEqualTo(-1); + } + } else { + assertThat(rsp.responseBody()).isPresent(); + byte[] readBody = IoUtils.toByteArray(rsp.responseBody().get()); + assertThat(readBody).isEqualTo(body); + } + } + } + private void overrideDnsResolver(String hostName) throws IOException { overrideDnsResolver(hostName, false); } diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java index 20ce87ea8066..b2706354d335 100644 --- a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java @@ -20,7 +20,6 @@ import static com.github.tomakehurst.wiremock.client.WireMock.any; import static com.github.tomakehurst.wiremock.client.WireMock.containing; import static com.github.tomakehurst.wiremock.client.WireMock.equalTo; -import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor; import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor; import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching; @@ -28,7 +27,6 @@ import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.assertj.core.api.AssertionsForClassTypes.assertThatCode; import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder; @@ -238,148 +236,6 @@ public void doesNotRetryOn429StatusCode() throws Exception { } } - @Test - public void handlesNoContentResponse() throws Exception { - SdkHttpClient client = createSdkHttpClient(); - - mockServer.stubFor(any(urlPathEqualTo("/no-content")) - .willReturn(aResponse() - .withStatus(204))); - - SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + "/no-content", - SdkHttpMethod.DELETE); - HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder() - .request(req) - .build()) - .call(); - - assertThat(rsp.httpResponse().statusCode()).isEqualTo(204); - assertThat(rsp.responseBody()).isEmpty(); - } - - @Test - public void handlesLargeResponseBody() throws Exception { - SdkHttpClient client = createSdkHttpClient(); - // Create a large response body (1MB) - byte[] largeBody = new byte[1024 * 1024]; - for (int i = 0; i < largeBody.length; i++) { - largeBody[i] = (byte) (i % 256); - } - mockServer.stubFor(any(urlPathEqualTo("/large")) - .willReturn(aResponse() - .withStatus(200) - .withBody(largeBody))); - - SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + "/large", SdkHttpMethod.GET); - HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder() - .request(req) - .build()) - .call(); - - assertThat(rsp.httpResponse().statusCode()).isEqualTo(200); - assertThat(rsp.responseBody()).isPresent(); - - // Read the entire response and verify - byte[] readBuffer = IoUtils.toByteArray(rsp.responseBody().get()); - assertThat(readBuffer).isEqualTo(largeBody); - } - - @Test - public void testAbortResponseStream() throws Exception { - SdkHttpClient client = createSdkHttpClient(); - - mockServer.stubFor(any(urlPathEqualTo("/streaming")) - .willReturn(aResponse() - .withStatus(200) - .withBody("This is a streaming response that should be aborted"))); - - SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + "/streaming", SdkHttpMethod.POST); - ExecutableHttpRequest executableRequest = client.prepareRequest( - HttpExecuteRequest.builder() - .request(req) - .contentStreamProvider(req.contentStreamProvider() - .orElse(null)) - .build()); - HttpExecuteResponse rsp = executableRequest.call(); - - assertThat(rsp.httpResponse().statusCode()).isEqualTo(200); - assertThat(rsp.responseBody()).isPresent(); - - // Verify the stream is abortable - AbortableInputStream stream = rsp.responseBody().get(); - assertThat(stream).isInstanceOf(AbortableInputStream.class); - - // Read a few bytes - byte[] buffer = new byte[10]; - int bytesRead = stream.read(buffer); - assertThat(bytesRead).isGreaterThan(0); - assertThatCode(() -> stream.abort()).doesNotThrowAnyException(); - stream.close(); - } - - @Test - public void handlesMultipleSequentialRequests() throws Exception { - SdkHttpClient client = createSdkHttpClient(); - - mockServer.stubFor(any(urlPathEqualTo("/sequential")) - .willReturn(aResponse() - .withStatus(200) - .withBody("Response body"))); - - // Execute multiple requests sequentially - for (int i = 0; i < 5; i++) { - SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + "/sequential", SdkHttpMethod.GET); - HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder() - .request(req) - .build()) - .call(); - - assertThat(rsp.httpResponse().statusCode()).isEqualTo(200); - assertThat(IoUtils.toUtf8String(rsp.responseBody().orElse(null))).isEqualTo("Response body"); - } - mockServer.verify(5, getRequestedFor(urlEqualTo("/sequential"))); - } - - @Test - public void handlesVariousContentLengths() throws Exception { - SdkHttpClient client = createSdkHttpClient(); - int[] contentLengths = {0, 1, 100, 1024, 65536}; - - for (int length : contentLengths) { - String path = "/content-length-" + length; - byte[] body = new byte[length]; - for (int i = 0; i < length; i++) { - body[i] = (byte) ('A' + (i % 26)); - } - - mockServer.stubFor(any(urlPathEqualTo(path)) - .willReturn(aResponse() - .withStatus(200) - .withHeader("Content-Length", String.valueOf(length)) - .withBody(body))); - - SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + path, SdkHttpMethod.GET); - HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder() - .request(req) - .build()) - .call(); - - assertThat(rsp.httpResponse().statusCode()).isEqualTo(200); - - if (length == 0) { - // Empty body should still have a response body present, but EOF immediately - if (rsp.responseBody().isPresent()) { - assertThat(rsp.responseBody().get().read()).isEqualTo(-1); - } - } else { - assertThat(rsp.responseBody()).isPresent(); - byte[] readBody = IoUtils.toByteArray(rsp.responseBody().get()); - assertThat(readBody).isEqualTo(body); - } - } - } - - private void validateStatusCodeWithRetryCheck(SdkHttpClient client, int expectedStatusCode, int expectedRequestCount) throws IOException { @@ -503,23 +359,13 @@ private static SdkHttpFullRequest.Builder mockSdkRequestBuilder(String uriString return requestBuilder; } - protected SdkHttpFullRequest mockSdkRequest(String uriString, SdkHttpMethod method) { - URI uri = URI.create(uriString); - SdkHttpFullRequest.Builder requestBuilder = SdkHttpFullRequest.builder() - .uri(uri) - .method(method) - .putHeader("Host", uri.getHost()) - .putHeader("User-Agent", "hello-world!"); - // Only add body for methods that typically have a body - if (method != SdkHttpMethod.HEAD && method != SdkHttpMethod.GET && method != SdkHttpMethod.DELETE) { + protected SdkHttpFullRequest mockSdkRequest(String uriString, SdkHttpMethod method) { + SdkHttpFullRequest.Builder requestBuilder = mockSdkRequestBuilder(uriString, method); + if (method != SdkHttpMethod.HEAD) { byte[] content = "Body".getBytes(StandardCharsets.UTF_8); requestBuilder.putHeader("Content-Length", Integer.toString(content.length)); requestBuilder.contentStreamProvider(() -> new ByteArrayInputStream(content)); - } else if (method == SdkHttpMethod.GET || method == SdkHttpMethod.DELETE) { - // For GET and DELETE, explicitly set Content-Length to 0 or don't set it at all - // Some clients like AWS CRT are strict about this - requestBuilder.contentStreamProvider(() -> new ByteArrayInputStream(new byte[0])); } return requestBuilder.build(); From 602c6591c673015f96e9e5d6e5980e9588ed5489 Mon Sep 17 00:00:00 2001 From: John Viegas Date: Tue, 24 Jun 2025 14:09:14 -0700 Subject: [PATCH 13/14] Add benchmark for Apache5 and add Streaming Api test cases --- test/s3-benchmarks/pom.xml | 11 ++ test/sdk-benchmarks/pom.xml | 5 + .../httpclient/SdkHttpClientBenchmark.java | 35 ++++++ .../sync/ApacheHttpClientBenchmark.java | 111 ++++++++++++++++- .../awssdk/benchmark/utils/MockServer.java | 2 +- .../benchmark/utils/StreamingMockServlet.java | 112 ++++++++++++++++++ 6 files changed, 270 insertions(+), 6 deletions(-) create mode 100644 test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/utils/StreamingMockServlet.java diff --git a/test/s3-benchmarks/pom.xml b/test/s3-benchmarks/pom.xml index 4918b20eb69c..e304c85f1933 100644 --- a/test/s3-benchmarks/pom.xml +++ b/test/s3-benchmarks/pom.xml @@ -102,6 +102,17 @@ netty-nio-client ${awsjavasdk.version} + + apache-client + software.amazon.awssdk + ${awsjavasdk.version} + + + apache5-client + software.amazon.awssdk + ${awsjavasdk.version} + + software.amazon.awssdk aws-crt-client diff --git a/test/sdk-benchmarks/pom.xml b/test/sdk-benchmarks/pom.xml index 89774b720d89..209d6c148329 100644 --- a/test/sdk-benchmarks/pom.xml +++ b/test/sdk-benchmarks/pom.xml @@ -158,6 +158,11 @@ apache-client ${awsjavasdk.version} + + software.amazon.awssdk + apache5-client + ${awsjavasdk.version} + software.amazon.awssdk protocol-tests diff --git a/test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/apicall/httpclient/SdkHttpClientBenchmark.java b/test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/apicall/httpclient/SdkHttpClientBenchmark.java index c8448dfb37bc..8a130e2c83e9 100644 --- a/test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/apicall/httpclient/SdkHttpClientBenchmark.java +++ b/test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/apicall/httpclient/SdkHttpClientBenchmark.java @@ -40,4 +40,39 @@ public interface SdkHttpClientBenchmark { */ default void concurrentApiCall(Blackhole blackhole) { } + + + /** + * Benchmark for PUT operations with streaming + * + * @param blackhole the blackhole + */ + default void streamingPutOperation(Blackhole blackhole) { + } + + /** + * Benchmark for concurrent PUT operations + * + * @param blackhole the blackhole + */ + default void concurrentStreamingPutOperation(Blackhole blackhole) { + } + + /** + * Benchmark for GET operations with streaming response + * + * @param blackhole the blackhole + */ + default void streamingOutputOperation(Blackhole blackhole) { + } + + /** + * Benchmark for concurrent GET operations with streaming response + * + * @param blackhole the blackhole + */ + default void concurrentStreamingOutputOperation(Blackhole blackhole) { + } + + } diff --git a/test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/apicall/httpclient/sync/ApacheHttpClientBenchmark.java b/test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/apicall/httpclient/sync/ApacheHttpClientBenchmark.java index 232a892c23d5..2171c4467ba7 100644 --- a/test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/apicall/httpclient/sync/ApacheHttpClientBenchmark.java +++ b/test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/apicall/httpclient/sync/ApacheHttpClientBenchmark.java @@ -33,12 +33,14 @@ import org.openjdk.jmh.annotations.Measurement; import org.openjdk.jmh.annotations.Mode; import org.openjdk.jmh.annotations.OperationsPerInvocation; +import org.openjdk.jmh.annotations.Param; import org.openjdk.jmh.annotations.Scope; import org.openjdk.jmh.annotations.Setup; import org.openjdk.jmh.annotations.State; import org.openjdk.jmh.annotations.TearDown; import org.openjdk.jmh.annotations.Warmup; import org.openjdk.jmh.infra.Blackhole; +import org.openjdk.jmh.profile.GCProfiler; import org.openjdk.jmh.profile.StackProfiler; import org.openjdk.jmh.results.RunResult; import org.openjdk.jmh.runner.Runner; @@ -46,13 +48,20 @@ import org.openjdk.jmh.runner.options.OptionsBuilder; import software.amazon.awssdk.benchmark.apicall.httpclient.SdkHttpClientBenchmark; import software.amazon.awssdk.benchmark.utils.MockServer; +import software.amazon.awssdk.core.ResponseBytes; +import software.amazon.awssdk.core.sync.RequestBody; +import software.amazon.awssdk.core.sync.ResponseTransformer; import software.amazon.awssdk.http.SdkHttpClient; import software.amazon.awssdk.http.apache.ApacheHttpClient; +import software.amazon.awssdk.http.apache5.Apache5HttpClient; import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonClient; +import software.amazon.awssdk.services.protocolrestjson.model.StreamingInputOperationRequest; +import software.amazon.awssdk.services.protocolrestjson.model.StreamingOutputOperationRequest; +import software.amazon.awssdk.services.protocolrestjson.model.StreamingOutputOperationResponse; /** - * Benchmarking for running with different http clients. + * Benchmarking for running with different Apache HTTP clients. */ @State(Scope.Benchmark) @Warmup(iterations = 3, time = 15, timeUnit = TimeUnit.SECONDS) @@ -61,6 +70,19 @@ @BenchmarkMode(Mode.Throughput) public class ApacheHttpClientBenchmark implements SdkHttpClientBenchmark { + private static final int STREAM_SIZE = 1024 * 1024; // 1MB + private static final byte[] STREAM_DATA = new byte[STREAM_SIZE]; + + static { + // Initialize stream data + for (int i = 0; i < STREAM_SIZE; i++) { + STREAM_DATA[i] = (byte) (i % 256); + } + } + + @Param({"apache4", "apache5"}) + private String clientType; + private MockServer mockServer; private SdkHttpClient sdkHttpClient; private ProtocolRestJsonClient client; @@ -70,8 +92,21 @@ public class ApacheHttpClientBenchmark implements SdkHttpClientBenchmark { public void setup() throws Exception { mockServer = new MockServer(); mockServer.start(); - sdkHttpClient = ApacheHttpClient.builder() - .buildWithDefaults(trustAllTlsAttributeMapBuilder().build()); + + // Create HTTP client based on parameter + switch (clientType) { + case "apache4": + sdkHttpClient = ApacheHttpClient.builder() + .buildWithDefaults(trustAllTlsAttributeMapBuilder().build()); + break; + case "apache5": + sdkHttpClient = Apache5HttpClient.builder() + .buildWithDefaults(trustAllTlsAttributeMapBuilder().build()); + break; + default: + throw new IllegalArgumentException("Unknown client type: " + clientType); + } + client = ProtocolRestJsonClient.builder() .endpointOverride(mockServer.getHttpsUri()) .httpClient(sdkHttpClient) @@ -109,12 +144,78 @@ public void concurrentApiCall(Blackhole blackhole) { awaitCountdownLatchUninterruptibly(countDownLatch, 10, TimeUnit.SECONDS); } - public static void main(String... args) throws Exception { + @Benchmark + @Override + public void streamingPutOperation(Blackhole blackhole) { + StreamingInputOperationRequest request = StreamingInputOperationRequest.builder() + .build(); + RequestBody requestBody = RequestBody.fromBytes(STREAM_DATA); + + blackhole.consume(client.streamingInputOperation(request, requestBody)); + } + + @Benchmark + @Override + @OperationsPerInvocation(CONCURRENT_CALLS) + public void concurrentStreamingPutOperation(Blackhole blackhole) { + CountDownLatch countDownLatch = new CountDownLatch(CONCURRENT_CALLS); + for (int i = 0; i < CONCURRENT_CALLS; i++) { + CompletableFuture future = CompletableFuture.runAsync(() -> { + StreamingInputOperationRequest request = StreamingInputOperationRequest.builder() + .build(); + RequestBody requestBody = RequestBody.fromBytes(STREAM_DATA); + client.streamingInputOperation(request, requestBody); + }, executorService); + + countDownUponCompletion(blackhole, future, countDownLatch); + } + + awaitCountdownLatchUninterruptibly(countDownLatch, 10, TimeUnit.SECONDS); + } + + @Benchmark + @Override + public void streamingOutputOperation(Blackhole blackhole) { + StreamingOutputOperationRequest request = StreamingOutputOperationRequest.builder() + .build(); + + ResponseBytes responseBytes = + client.streamingOutputOperation(request, ResponseTransformer.toBytes()); + + blackhole.consume(responseBytes.asByteArray()); + } + + @Benchmark + @Override + @OperationsPerInvocation(CONCURRENT_CALLS) + public void concurrentStreamingOutputOperation(Blackhole blackhole) { + CountDownLatch countDownLatch = new CountDownLatch(CONCURRENT_CALLS); + + for (int i = 0; i < CONCURRENT_CALLS; i++) { + CompletableFuture future = CompletableFuture.runAsync(() -> { + StreamingOutputOperationRequest request = StreamingOutputOperationRequest.builder() + .build(); + + ResponseBytes responseBytes = + client.streamingOutputOperation(request, ResponseTransformer.toBytes()); + + blackhole.consume(responseBytes.asByteArray()); + }, executorService); + + countDownUponCompletion(blackhole, future, countDownLatch); + } + + awaitCountdownLatchUninterruptibly(countDownLatch, 10, TimeUnit.SECONDS); + } + + public static void main(String... args) throws Exception { Options opt = new OptionsBuilder() - .include(ApacheHttpClientBenchmark.class.getSimpleName() + ".concurrentApiCall") + .include(ApacheHttpClientBenchmark.class.getSimpleName()) .addProfiler(StackProfiler.class) + .addProfiler(GCProfiler.class) .build(); + Collection run = new Runner(opt).run(); } } diff --git a/test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/utils/MockServer.java b/test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/utils/MockServer.java index 15cca300789c..7a4e9a13df6a 100644 --- a/test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/utils/MockServer.java +++ b/test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/utils/MockServer.java @@ -59,7 +59,7 @@ public MockServer() throws IOException { server.setConnectors(new Connector[] {connector, sslConnector}); ServletContextHandler context = new ServletContextHandler(server, "/", ServletContextHandler.SESSIONS); - context.addServlet(new ServletHolder(new AlwaysSuccessServlet()), "/*"); + context.addServlet(new ServletHolder(new StreamingMockServlet()), "/*"); server.setHandler(context); } diff --git a/test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/utils/StreamingMockServlet.java b/test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/utils/StreamingMockServlet.java new file mode 100644 index 000000000000..f47573356918 --- /dev/null +++ b/test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/utils/StreamingMockServlet.java @@ -0,0 +1,112 @@ +/* + * 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.benchmark.utils; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.io.PrintWriter; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +public class StreamingMockServlet extends HttpServlet { + private static final byte[] STREAMING_RESPONSE_DATA = new byte[1024 * 1024]; // 1MB response + + static { + // Initialize response data + for (int i = 0; i < STREAMING_RESPONSE_DATA.length; i++) { + STREAMING_RESPONSE_DATA[i] = (byte) (i % 256); + } + } + + @Override + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException { + handleRequest(request, response); + } + + @Override + protected void doPost(HttpServletRequest request, HttpServletResponse response) throws IOException { + handleRequest(request, response); + } + + @Override + protected void doPut(HttpServletRequest request, HttpServletResponse response) throws IOException { + handleRequest(request, response); + } + + private void handleRequest(HttpServletRequest request, HttpServletResponse response) throws IOException { + // Check if this should be a streaming response + if (isStreamingOperation(request)) { + handleStreamingRequest(request, response); + } else { + handleJsonRequest(request, response); + } + } + + private boolean isStreamingOperation(HttpServletRequest request) { + String uri = request.getRequestURI(); + String contentType = request.getContentType(); + + return uri.contains("streaming") || + uri.contains("StreamingInput") || + uri.contains("StreamingOutput") || + "application/octet-stream".equals(contentType); + } + + private void handleStreamingRequest(HttpServletRequest request, HttpServletResponse response) throws IOException { + + // Consume input stream if present + try (InputStream inputStream = request.getInputStream()) { + byte[] buffer = new byte[8192]; + while (inputStream.read(buffer) != -1) { + // Just consume the data + } + } + + // Send streaming response + response.setStatus(HttpServletResponse.SC_OK); + response.setContentType("application/octet-stream"); + response.setContentLength(STREAMING_RESPONSE_DATA.length); + response.setHeader("x-amz-request-id", "streaming-" + System.currentTimeMillis()); + + try (OutputStream outputStream = response.getOutputStream()) { + outputStream.write(STREAMING_RESPONSE_DATA); + outputStream.flush(); + } + } + + private void handleJsonRequest(HttpServletRequest request, HttpServletResponse response) throws IOException { + + response.setStatus(HttpServletResponse.SC_OK); + response.setContentType("application/json"); + response.setCharacterEncoding("UTF-8"); + response.setHeader("x-amz-request-id", "json-" + System.currentTimeMillis()); + + String jsonResponse = "{" + + "\"status\":\"success\"," + + "\"message\":\"Mock operation completed\"," + + "\"ResponseMetadata\":{" + + "\"RequestId\":\"mock-request-id\"" + + "}" + + "}"; + + try (PrintWriter writer = response.getWriter()) { + writer.write(jsonResponse); + writer.flush(); + } + } +} \ No newline at end of file From 40f144bed0e9a67cdf824bf6bc15a4d0a71c0cd6 Mon Sep 17 00:00:00 2001 From: John Viegas Date: Thu, 26 Jun 2025 13:00:53 -0700 Subject: [PATCH 14/14] Update Apache5 to 5.5 --- http-clients/apache5-client/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/http-clients/apache5-client/pom.xml b/http-clients/apache5-client/pom.xml index d33a016b9cf2..ac8c0cf545c6 100644 --- a/http-clients/apache5-client/pom.xml +++ b/http-clients/apache5-client/pom.xml @@ -36,7 +36,7 @@ org.apache.httpcomponents.client5 httpclient5 - 5.4.4 + 5.5 org.apache.httpcomponents.core5