From aa489a500938ee3a265e18f39c91ff23a529ed1a Mon Sep 17 00:00:00 2001 From: John Viegas Date: Wed, 25 Jun 2025 14:39:32 -0700 Subject: [PATCH 1/2] Clean up unused APIs and add test to make sure it can be handled with alternatives --- ...eHttpClientLocalAddressFunctionalTest.java | 33 ++++ .../http/apache5/Apache5HttpClient.java | 18 -- .../internal/Apache5HttpRequestConfig.java | 13 -- .../internal/conn/SdkTlsSocketFactory.java | 5 - .../impl/Apache5HttpRequestFactory.java | 1 - .../net/InputShutdownCheckingSslSocket.java | 83 --------- .../Apache5ClientTlsHalfCloseTest.java | 146 ++++++++++++++++ ...5HttpClientLocalAddressFunctionalTest.java | 51 ++++++ .../InputShutdownCheckingSslSocketTest.java | 117 ------------- .../impl/ApacheHttpRequestFactoryTest.java | 4 +- ...ClientLocalAddressFunctionalTestSuite.java | 163 ++++++++++++++++++ 11 files changed, 394 insertions(+), 240 deletions(-) create mode 100644 http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientLocalAddressFunctionalTest.java delete mode 100644 http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/net/InputShutdownCheckingSslSocket.java create mode 100644 http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5ClientTlsHalfCloseTest.java create mode 100644 http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientLocalAddressFunctionalTest.java delete mode 100644 http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/conn/InputShutdownCheckingSslSocketTest.java create mode 100644 test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientLocalAddressFunctionalTestSuite.java diff --git a/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientLocalAddressFunctionalTest.java b/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientLocalAddressFunctionalTest.java new file mode 100644 index 000000000000..839929cc8e4e --- /dev/null +++ b/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientLocalAddressFunctionalTest.java @@ -0,0 +1,33 @@ +/* + * 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 java.net.InetAddress; +import java.time.Duration; +import org.junit.jupiter.api.DisplayName; +import software.amazon.awssdk.http.SdkHttpClient; +import software.amazon.awssdk.http.SdkHttpClientLocalAddressFunctionalTestSuite; + +@DisplayName("Apache HTTP Client - Local Address Functional Tests") +class ApacheHttpClientLocalAddressFunctionalTest extends SdkHttpClientLocalAddressFunctionalTestSuite { + + @Override + protected SdkHttpClient createHttpClient(InetAddress localAddress, Duration connectionTimeout) { + return ApacheHttpClient.builder() + .localAddress(localAddress) + .connectionTimeout(connectionTimeout) + .build(); + } +} \ No newline at end of file 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 8a21cbcb299d..c5037be562bf 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.net.InetAddress; import java.security.KeyManagementException; import java.security.NoSuchAlgorithmException; import java.security.cert.CertificateException; @@ -338,7 +337,6 @@ private Apache5HttpRequestConfig createRequestConfig(DefaultBuilder builder, .connectionAcquireTimeout( resolvedOptions.get(SdkHttpConfigurationOption.CONNECTION_ACQUIRE_TIMEOUT)) .proxyConfiguration(builder.proxyConfiguration) - .localAddress(Optional.ofNullable(builder.localAddress).orElse(null)) .expectContinueEnabled(Optional.ofNullable(builder.expectContinueEnabled) .orElse(DefaultConfiguration.EXPECT_CONTINUE_ENABLED)) .build(); @@ -406,11 +404,6 @@ public interface Builder extends SdkHttpClient.Builder authSchemeRegistry; private ProxyConfiguration proxyConfiguration = ProxyConfiguration.builder().build(); - private InetAddress localAddress; private Boolean expectContinueEnabled; private HttpRoutePlanner httpRoutePlanner; private CredentialsProvider credentialsProvider; @@ -564,16 +556,6 @@ public void setProxyConfiguration(ProxyConfiguration proxyConfiguration) { proxyConfiguration(proxyConfiguration); } - @Override - public Builder localAddress(InetAddress localAddress) { - this.localAddress = localAddress; - return this; - } - - public void setLocalAddress(InetAddress localAddress) { - localAddress(localAddress); - } - @Override public Builder expectContinueEnabled(Boolean expectContinueEnabled) { this.expectContinueEnabled = expectContinueEnabled; diff --git a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/Apache5HttpRequestConfig.java b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/Apache5HttpRequestConfig.java index a494cfe220da..92bf31c33bfa 100644 --- a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/Apache5HttpRequestConfig.java +++ b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/Apache5HttpRequestConfig.java @@ -15,7 +15,6 @@ package software.amazon.awssdk.http.apache5.internal; -import java.net.InetAddress; import java.time.Duration; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.http.apache5.ProxyConfiguration; @@ -30,7 +29,6 @@ public final class Apache5HttpRequestConfig { private final Duration socketTimeout; private final Duration connectionTimeout; private final Duration connectionAcquireTimeout; - private final InetAddress localAddress; private final boolean expectContinueEnabled; private final ProxyConfiguration proxyConfiguration; @@ -38,7 +36,6 @@ private Apache5HttpRequestConfig(Builder builder) { this.socketTimeout = builder.socketTimeout; this.connectionTimeout = builder.connectionTimeout; this.connectionAcquireTimeout = builder.connectionAcquireTimeout; - this.localAddress = builder.localAddress; this.expectContinueEnabled = builder.expectContinueEnabled; this.proxyConfiguration = builder.proxyConfiguration; } @@ -55,10 +52,6 @@ public Duration connectionAcquireTimeout() { return connectionAcquireTimeout; } - public InetAddress localAddress() { - return localAddress; - } - public boolean expectContinueEnabled() { return expectContinueEnabled; } @@ -82,7 +75,6 @@ public static final class Builder { private Duration socketTimeout; private Duration connectionTimeout; private Duration connectionAcquireTimeout; - private InetAddress localAddress; private boolean expectContinueEnabled; private ProxyConfiguration proxyConfiguration; @@ -104,11 +96,6 @@ public Builder connectionAcquireTimeout(Duration connectionAcquireTimeout) { return this; } - public Builder localAddress(InetAddress localAddress) { - this.localAddress = localAddress; - return this; - } - public Builder expectContinueEnabled(boolean expectContinueEnabled) { this.expectContinueEnabled = expectContinueEnabled; return this; diff --git a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/conn/SdkTlsSocketFactory.java b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/conn/SdkTlsSocketFactory.java index 5ac61570f6f1..8f2a0ef44406 100644 --- a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/conn/SdkTlsSocketFactory.java +++ b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/conn/SdkTlsSocketFactory.java @@ -27,9 +27,7 @@ import org.apache.hc.core5.http.protocol.HttpContext; import org.apache.hc.core5.util.TimeValue; import software.amazon.awssdk.annotations.SdkInternalApi; -import software.amazon.awssdk.http.apache5.internal.net.InputShutdownCheckingSslSocket; import software.amazon.awssdk.http.apache5.internal.net.SdkSocket; -import software.amazon.awssdk.http.apache5.internal.net.SdkSslSocket; import software.amazon.awssdk.utils.Logger; @SdkInternalApi @@ -62,9 +60,6 @@ public Socket connectSocket(TimeValue connectTimeout, log.trace(() -> String.format("Connecting to %s:%s", remoteAddress.getAddress(), remoteAddress.getPort())); Socket connectSocket = super.connectSocket(connectTimeout, socket, host, remoteAddress, localAddress, context); - if (connectSocket instanceof SSLSocket) { - return new InputShutdownCheckingSslSocket(new SdkSslSocket((SSLSocket) connectSocket)); - } return new SdkSocket(connectSocket); } } 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 3219b271998f..42471e5a592e 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 @@ -98,7 +98,6 @@ private void addRequestConfig(HttpUriRequestBase base, .setConnectionRequestTimeout(connectAcquireTimeout, TimeUnit.MILLISECONDS) .setConnectTimeout(connectTimeout, TimeUnit.MILLISECONDS) .setResponseTimeout(saturatedCast(requestConfig.socketTimeout().toMillis()), TimeUnit.MILLISECONDS); - // TODO as part of removed API : .setLocalAddress(requestConfig.localAddress()); /* * Enable 100-continue support for PUT operations, since this is diff --git a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/net/InputShutdownCheckingSslSocket.java b/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/net/InputShutdownCheckingSslSocket.java deleted file mode 100644 index 725c8a8aad9f..000000000000 --- a/http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/net/InputShutdownCheckingSslSocket.java +++ /dev/null @@ -1,83 +0,0 @@ -/* - * 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.internal.net; - -import java.io.FilterOutputStream; -import java.io.IOException; -import java.io.OutputStream; -import javax.net.ssl.SSLSocket; -import software.amazon.awssdk.annotations.SdkInternalApi; - -// TODO : This class will be removed in further PR , keeping it now so that we have a clear baseleine to compare -/** - * Wrapper socket that ensures the read end of the socket is still open before performing a {@code write()}. In TLS 1.3, it is - * permitted for the connection to be in a half-closed state, which is dangerous for the Apache5 client because it can get stuck - * in a state where it continues to write to the socket and potentially end up a blocked state writing to the socket - * indefinitely. - */ -@SdkInternalApi -public final class InputShutdownCheckingSslSocket extends DelegateSslSocket { - - public InputShutdownCheckingSslSocket(SSLSocket sock) { - super(sock); - } - - @Override - public OutputStream getOutputStream() throws IOException { - return new InputShutdownCheckingOutputStream(sock.getOutputStream(), sock); - } - - private static class InputShutdownCheckingOutputStream extends FilterOutputStream { - private final SSLSocket sock; - - InputShutdownCheckingOutputStream(OutputStream out, SSLSocket sock) { - super(out); - this.sock = sock; - } - - @Override - public void write(int b) throws IOException { - checkInputShutdown(); - out.write(b); - } - - @Override - public void write(byte[] b) throws IOException { - checkInputShutdown(); - out.write(b); - } - - @Override - public void write(byte[] b, int off, int len) throws IOException { - checkInputShutdown(); - out.write(b, off, len); - } - - private void checkInputShutdown() throws IOException { - if (sock.isInputShutdown()) { - throw new IOException("Remote end is closed."); - } - - try { - sock.getInputStream(); - } catch (IOException inputStreamException) { - IOException e = new IOException("Remote end is closed."); - e.addSuppressed(inputStreamException); - throw e; - } - } - } -} diff --git a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5ClientTlsHalfCloseTest.java b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5ClientTlsHalfCloseTest.java new file mode 100644 index 000000000000..334eb2ac6b5b --- /dev/null +++ b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5ClientTlsHalfCloseTest.java @@ -0,0 +1,146 @@ +/* + * 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 static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIf; +import software.amazon.awssdk.http.ContentStreamProvider; +import software.amazon.awssdk.http.FileStoreTlsKeyManagersProvider; +import software.amazon.awssdk.http.HttpExecuteRequest; +import software.amazon.awssdk.http.HttpExecuteResponse; +import software.amazon.awssdk.http.SdkHttpClient; +import software.amazon.awssdk.http.SdkHttpFullRequest; +import software.amazon.awssdk.http.SdkHttpMethod; +import software.amazon.awssdk.http.SdkHttpRequest; +import software.amazon.awssdk.http.TlsKeyManagersProvider; +import software.amazon.awssdk.http.server.MockServer; + +public class Apache5ClientTlsHalfCloseTest extends ClientTlsAuthTestBase { + + private static TlsKeyManagersProvider tlsKeyManagersProvider; + private static MockServer mockServer; + private SdkHttpClient httpClient; + + private static final int TWO_MB = 2 * 1024 * 1024; + private static final byte[] CONTENT = new byte[TWO_MB]; + + @Test + @EnabledIf("halfCloseSupported") + public void errorWhenServerHalfClosesSocketWhileStreamIsOpened() { + + mockServer = MockServer.createMockServer(MockServer.ServerBehavior.HALF_CLOSE); + mockServer.startServer(tlsKeyManagersProvider); + + httpClient = Apache5HttpClient.builder() + .tlsKeyManagersProvider(tlsKeyManagersProvider) + .build(); + IOException exception = assertThrows(IOException.class, () -> { + executeHttpRequest(httpClient); + }); + assertThat(exception.getMessage()) + .containsIgnoringCase("broken pipe"); + } + + + @Test + public void errorWhenServerFullClosesSocketWhileStreamIsOpened() throws IOException { + mockServer = MockServer.createMockServer(MockServer.ServerBehavior.FULL_CLOSE_IN_BETWEEN); + mockServer.startServer(tlsKeyManagersProvider); + + httpClient = Apache5HttpClient.builder() + .tlsKeyManagersProvider(tlsKeyManagersProvider) + .build(); + + IOException exception = assertThrows(IOException.class, () -> { + executeHttpRequest(httpClient); + }); + + if(halfCloseSupported()){ + assertEquals("Connection or outbound has closed", exception.getMessage()); + + }else { + assertEquals("Socket is closed", exception.getMessage()); + + } + } + + @Test + public void successfulRequestForFullCloseSocketAtTheEnd() throws IOException { + mockServer = MockServer.createMockServer(MockServer.ServerBehavior.FULL_CLOSE_AT_THE_END); + mockServer.startServer(tlsKeyManagersProvider); + + httpClient = Apache5HttpClient.builder() + .tlsKeyManagersProvider(tlsKeyManagersProvider) + .build(); + + HttpExecuteResponse response = executeHttpRequest(httpClient); + + assertThat(response.httpResponse().isSuccessful()).isTrue(); + } + + @AfterEach + void tearDown() { + if (mockServer != null) { + mockServer.stopServer(); + } + } + + @BeforeAll + public static void setUp() throws IOException { + ClientTlsAuthTestBase.setUp(); + System.setProperty("javax.net.ssl.trustStore", serverKeyStore.toAbsolutePath().toString()); + System.setProperty("javax.net.ssl.trustStorePassword", STORE_PASSWORD); + System.setProperty("javax.net.ssl.trustStoreType", "jks"); + tlsKeyManagersProvider = FileStoreTlsKeyManagersProvider.create(clientKeyStore, CLIENT_STORE_TYPE, STORE_PASSWORD); + } + + @AfterAll + public static void clear() throws IOException { + System.clearProperty("javax.net.ssl.trustStore"); + System.clearProperty("javax.net.ssl.trustStorePassword"); + System.clearProperty("javax.net.ssl.trustStoreType"); + ClientTlsAuthTestBase.teardown(); + + } + + private static HttpExecuteResponse executeHttpRequest(SdkHttpClient client) throws IOException { + ContentStreamProvider contentStreamProvider = () -> new ByteArrayInputStream(CONTENT); + SdkHttpRequest httpRequest = SdkHttpFullRequest.builder() + .method(SdkHttpMethod.PUT) + .protocol("https") + .host("localhost:" + mockServer.getPort()) + .build(); + HttpExecuteRequest request = HttpExecuteRequest.builder() + .request(httpRequest) + .contentStreamProvider(contentStreamProvider) + .build(); + + return client.prepareRequest(request).call(); + } + + public static boolean halfCloseSupported(){ + return MockServer.isTlsHalfCloseSupported(); + } +} \ No newline at end of file diff --git a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientLocalAddressFunctionalTest.java b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientLocalAddressFunctionalTest.java new file mode 100644 index 000000000000..f5f948574066 --- /dev/null +++ b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientLocalAddressFunctionalTest.java @@ -0,0 +1,51 @@ +/* + * 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 java.net.InetAddress; +import java.time.Duration; +import org.apache.hc.client5.http.impl.DefaultSchemePortResolver; +import org.apache.hc.client5.http.impl.routing.DefaultRoutePlanner; +import org.apache.hc.client5.http.routing.HttpRoutePlanner; +import org.apache.hc.core5.http.HttpException; +import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.http.protocol.HttpContext; +import org.junit.jupiter.api.DisplayName; +import software.amazon.awssdk.http.SdkHttpClient; +import software.amazon.awssdk.http.SdkHttpClientLocalAddressFunctionalTestSuite; + +@DisplayName("Apache5 HTTP Client - Local Address Functional Tests") +class Apache5HttpClientLocalAddressFunctionalTest extends SdkHttpClientLocalAddressFunctionalTestSuite { + + @Override + protected SdkHttpClient createHttpClient(InetAddress localAddress, Duration connectionTimeout) { + HttpRoutePlanner routePlanner = createLocalAddressRoutePlanner(localAddress); + + return Apache5HttpClient.builder() + .httpRoutePlanner(routePlanner) + .connectionTimeout(connectionTimeout) + .build(); + } + + private HttpRoutePlanner createLocalAddressRoutePlanner(InetAddress localAddress) { + return new DefaultRoutePlanner(DefaultSchemePortResolver.INSTANCE) { + @Override + protected InetAddress determineLocalAddress(HttpHost firstHop, HttpContext context) throws HttpException { + return localAddress != null ? localAddress : super.determineLocalAddress(firstHop, context); + } + }; + } +} diff --git a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/conn/InputShutdownCheckingSslSocketTest.java b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/conn/InputShutdownCheckingSslSocketTest.java deleted file mode 100644 index d9076549b1a1..000000000000 --- a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/conn/InputShutdownCheckingSslSocketTest.java +++ /dev/null @@ -1,117 +0,0 @@ -/* - * 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.internal.conn; - - -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import java.io.IOException; -import java.io.OutputStream; -import javax.net.ssl.SSLSocket; -import org.junit.jupiter.api.Test; -import software.amazon.awssdk.http.apache5.internal.net.InputShutdownCheckingSslSocket; - -public class InputShutdownCheckingSslSocketTest { - - @Test - public void outputStreamChecksInputShutdown() throws IOException { - SSLSocket mockSocket = mock(SSLSocket.class); - when(mockSocket.isInputShutdown()).thenReturn(true); - InputShutdownCheckingSslSocket socket = new InputShutdownCheckingSslSocket(mockSocket); - OutputStream os = socket.getOutputStream(); - assertThrows(IOException.class, () -> os.write(1)); - } - - @Test - public void outputStreamWritesNormallyWhenInputNotShutdown() throws IOException { - SSLSocket mockSocket = mock(SSLSocket.class); - OutputStream mockOutputStream = mock(OutputStream.class); - when(mockSocket.isInputShutdown()).thenReturn(false); - when(mockSocket.getOutputStream()).thenReturn(mockOutputStream); - InputShutdownCheckingSslSocket socket = new InputShutdownCheckingSslSocket(mockSocket); - OutputStream os = socket.getOutputStream(); - os.write(1); - verify(mockOutputStream).write(1); - } - - @Test - public void writeByteArrayThrowsIOExceptionWhenInputIsShutdown() throws IOException { - SSLSocket mockSocket = mock(SSLSocket.class); - when(mockSocket.isInputShutdown()).thenReturn(true); - InputShutdownCheckingSslSocket socket = new InputShutdownCheckingSslSocket(mockSocket); - OutputStream os = socket.getOutputStream(); - assertThrows(IOException.class, () -> os.write(new byte[10])); - } - - @Test - public void writeByteArraySucceedsWhenInputNotShutdown() throws IOException { - SSLSocket mockSocket = mock(SSLSocket.class); - OutputStream mockOutputStream = mock(OutputStream.class); - when(mockSocket.isInputShutdown()).thenReturn(false); - when(mockSocket.getOutputStream()).thenReturn(mockOutputStream); - - InputShutdownCheckingSslSocket socket = new InputShutdownCheckingSslSocket(mockSocket); - OutputStream os = socket.getOutputStream(); - - byte[] data = new byte[10]; - os.write(data); - verify(mockOutputStream).write(data); - } - - @Test - public void writeByteArrayWithOffsetThrowsIOExceptionWhenInputIsShutdown() throws IOException { - SSLSocket mockSocket = mock(SSLSocket.class); - when(mockSocket.isInputShutdown()).thenReturn(true); - - InputShutdownCheckingSslSocket socket = new InputShutdownCheckingSslSocket(mockSocket); - OutputStream os = socket.getOutputStream(); - - assertThrows(IOException.class, () -> os.write(new byte[10], 0, 10)); - } - - @Test - public void writeByteArrayWithOffsetSucceedsWhenInputNotShutdown() throws IOException { - SSLSocket mockSocket = mock(SSLSocket.class); - OutputStream mockOutputStream = mock(OutputStream.class); - when(mockSocket.isInputShutdown()).thenReturn(false); - when(mockSocket.getOutputStream()).thenReturn(mockOutputStream); - - InputShutdownCheckingSslSocket socket = new InputShutdownCheckingSslSocket(mockSocket); - OutputStream os = socket.getOutputStream(); - - byte[] data = new byte[10]; - os.write(data, 0, 10); - verify(mockOutputStream).write(data, 0, 10); - } - - @Test - public void checkInputShutdownThrowsIOExceptionWithSuppressed() throws IOException { - SSLSocket mockSocket = mock(SSLSocket.class); - when(mockSocket.isInputShutdown()).thenReturn(false); - when(mockSocket.getInputStream()).thenThrow(new IOException("InputStream exception")); - - InputShutdownCheckingSslSocket socket = new InputShutdownCheckingSslSocket(mockSocket); - OutputStream os = socket.getOutputStream(); - - IOException thrown = assertThrows(IOException.class, () -> os.write(1)); - assertTrue(thrown.getMessage().contains("Remote end is closed.")); - assertTrue(thrown.getSuppressed()[0].getMessage().contains("InputStream exception")); - } -} \ No newline at end of file diff --git a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/impl/ApacheHttpRequestFactoryTest.java b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/impl/ApacheHttpRequestFactoryTest.java index 7001aa45eb9c..dbc2cc79db7f 100644 --- a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/impl/ApacheHttpRequestFactoryTest.java +++ b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/internal/impl/ApacheHttpRequestFactoryTest.java @@ -24,7 +24,6 @@ import java.io.ByteArrayInputStream; import java.io.InputStream; -import java.net.InetAddress; import java.net.URI; import java.nio.charset.StandardCharsets; import java.time.Duration; @@ -41,7 +40,7 @@ import software.amazon.awssdk.http.apache5.internal.Apache5HttpRequestConfig; import software.amazon.awssdk.http.apache5.internal.RepeatableInputStreamRequestEntity; -public class ApacheHttpRequestFactoryTest { +class ApacheHttpRequestFactoryTest { private Apache5HttpRequestConfig requestConfig; private Apache5HttpRequestFactory instance; @@ -52,7 +51,6 @@ public void setup() { requestConfig = Apache5HttpRequestConfig.builder() .connectionAcquireTimeout(Duration.ZERO) .connectionTimeout(Duration.ZERO) - .localAddress(InetAddress.getLoopbackAddress()) .socketTimeout(Duration.ZERO) .build(); } diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientLocalAddressFunctionalTestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientLocalAddressFunctionalTestSuite.java new file mode 100644 index 000000000000..658036750012 --- /dev/null +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientLocalAddressFunctionalTestSuite.java @@ -0,0 +1,163 @@ +/* + * 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.urlPathEqualTo; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; + +import com.github.tomakehurst.wiremock.junit5.WireMockExtension; +import com.github.tomakehurst.wiremock.junit5.WireMockTest; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.net.BindException; +import java.net.InetAddress; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.stream.Stream; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; +import software.amazon.awssdk.utils.IoUtils; + +/** + * Abstract test suite for testing local address functionality across different HTTP client implementations. + * Subclasses must implement the {@link #createHttpClient(InetAddress, Duration)} method to provide + * their specific client implementation. + */ +@WireMockTest +public abstract class SdkHttpClientLocalAddressFunctionalTestSuite { + + @RegisterExtension + static WireMockExtension wireMock = WireMockExtension.newInstance() + .options(wireMockConfig().dynamicPort()) + .build(); + + private static final Duration CONNECTION_TIMEOUT = Duration.ofSeconds(2); + private static final String TEST_BODY = "test body"; + private static final String SUCCESS_RESPONSE = "success"; + + private SdkHttpClient client; + + /** + * Creates an HTTP client with the specified local address configuration. + * + * @param localAddress the local address to bind to, or null for default behavior + * @param connectionTimeout the connection timeout + * @return the configured HTTP client + */ + protected abstract SdkHttpClient createHttpClient(InetAddress localAddress, Duration connectionTimeout); + + @BeforeEach + void setUp() { + wireMock.stubFor(any(urlPathEqualTo("/")) + .willReturn(aResponse() + .withStatus(200) + .withBody(SUCCESS_RESPONSE))); + } + + @AfterEach + void tearDown() { + if (client != null) { + client.close(); + } + } + + @ParameterizedTest(name = "Invalid local address {0} should fail with BindException") + @ValueSource(strings = { + "192.0.2.1", // TEST-NET-1 reserved range + "198.51.100.1", // TEST-NET-2 + "203.0.113.1" // TEST-NET-3 + }) + @DisplayName("Invalid local addresses should fail with BindException") + void invalidLocalAddressesShouldFailWithBindexception(String invalidIpAddress) throws Exception { + InetAddress invalidAddress = InetAddress.getByName(invalidIpAddress); + client = createHttpClient(invalidAddress, CONNECTION_TIMEOUT); + assertThatExceptionOfType(BindException.class) + .isThrownBy(this::executeRequest); + } + + @ParameterizedTest(name = "Valid local address: {1}") + @MethodSource("provideValidLocalAddresses") + @DisplayName("Valid local addresses should succeed") + void validLocalAddressesShouldSucceed(InetAddress address, String description) throws Exception { + client = createHttpClient(address, CONNECTION_TIMEOUT); + HttpExecuteResponse response = executeRequest(); + assertThat(response.httpResponse().statusCode()) + .as("Request with %s should succeed", description) + .isEqualTo(200); + assertThat(readResponseBody(response)) + .isEqualTo(SUCCESS_RESPONSE); + } + + @Test + @DisplayName("Client without local address configuration should use system default") + void withoutLocalAddressConfigurationShouldSucceed() throws Exception { + client = createHttpClient(null, CONNECTION_TIMEOUT); + HttpExecuteResponse response = executeRequest(); + assertThat(response.httpResponse().statusCode()).isEqualTo(200); + assertThat(readResponseBody(response)).isEqualTo(SUCCESS_RESPONSE); + } + + private static Stream provideValidLocalAddresses() throws Exception { + return Stream.of( + Arguments.of(InetAddress.getLoopbackAddress(), "loopback address"), + Arguments.of(InetAddress.getByName("127.0.0.1"), "explicit localhost") + ); + } + + private HttpExecuteResponse executeRequest() throws Exception { + SdkHttpFullRequest request = createTestRequest(); + + return client.prepareRequest( + HttpExecuteRequest.builder() + .request(request) + .contentStreamProvider(request.contentStreamProvider().orElse(null)) + .build()) + .call(); + } + + private SdkHttpFullRequest createTestRequest() { + URI uri = URI.create("http://localhost:" + wireMock.getPort()); + byte[] content = TEST_BODY.getBytes(StandardCharsets.UTF_8); + + return SdkHttpFullRequest.builder() + .uri(uri) + .method(SdkHttpMethod.POST) + .putHeader("Host", uri.getHost()) + .putHeader("User-Agent", "test-client") + .putHeader("Content-Length", Integer.toString(content.length)) + .contentStreamProvider(() -> new ByteArrayInputStream(content)) + .build(); + } + + private String readResponseBody(HttpExecuteResponse response) throws IOException { + if (!response.responseBody().isPresent()) { + return ""; + } + return IoUtils.toUtf8String(response.responseBody().get()); + } +} From 4ea443c9a2e763787fed6b7ea41b0cfa44d9e682 Mon Sep 17 00:00:00 2001 From: John Viegas Date: Wed, 25 Jun 2025 16:31:28 -0700 Subject: [PATCH 2/2] Added NTCredentials to keep backward compatibilty with Apache4.x --- .../apache5/internal/utils/Apache5Utils.java | 43 ++++++++++--------- 1 file changed, 23 insertions(+), 20 deletions(-) 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 a777b3d21d8c..5bd726463206 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 @@ -18,7 +18,10 @@ import java.io.IOException; import java.io.UncheckedIOException; import org.apache.hc.client5.http.auth.AuthCache; +import org.apache.hc.client5.http.auth.AuthScope; +import org.apache.hc.client5.http.auth.Credentials; import org.apache.hc.client5.http.auth.CredentialsProvider; +import org.apache.hc.client5.http.auth.NTCredentials; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.client5.http.impl.auth.BasicAuthCache; import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider; @@ -37,8 +40,7 @@ private Apache5Utils() { } /** - * Utility function for creating a new BufferedEntity and wrapping any errors - * as a SdkClientException. + * Utility function for creating a new BufferedEntity and wrapping any errors as a SdkClientException. * * @param entity The HTTP entity to wrap with a buffered HTTP entity. * @return A new BufferedHttpEntity wrapping the specified entity. @@ -69,27 +71,29 @@ public static HttpClientContext newClientContext(ProxyConfiguration proxyConfigu */ public static CredentialsProvider newProxyCredentialsProvider(ProxyConfiguration proxyConfiguration) { BasicCredentialsProvider provider = new BasicCredentialsProvider(); - // TODO : NTCredentials is deprecated. - // provider.setCredentials(newAuthScope(proxyConfiguration), newNtCredentials(proxyConfiguration)); + provider.setCredentials(newAuthScope(proxyConfiguration), newNtCredentials(proxyConfiguration)); return provider; } - // /** - // * Returns a new instance of NTCredentials used for proxy authentication. - // */ - // private static Credentials newNtCredentials(ProxyConfiguration proxyConfiguration) { - // return new NTCredentials(proxyConfiguration.username(), - // proxyConfiguration.password(), - // proxyConfiguration.ntlmWorkstation(), - // proxyConfiguration.ntlmDomain()); - // } + /** + * Returns a new instance of NTCredentials used for proxy authentication. + */ + private static Credentials newNtCredentials(ProxyConfiguration proxyConfiguration) { + return new NTCredentials( + proxyConfiguration.username(), + proxyConfiguration.password().toCharArray(), + proxyConfiguration.ntlmWorkstation(), + proxyConfiguration.ntlmDomain() + ); + } + + /** + * Returns a new instance of AuthScope used for proxy authentication. + */ + private static AuthScope newAuthScope(ProxyConfiguration proxyConfiguration) { + return new AuthScope(proxyConfiguration.host(), proxyConfiguration.port()); + } - // /** - // * Returns a new instance of AuthScope used for proxy authentication. - // */ - // private static AuthScope newAuthScope(ProxyConfiguration proxyConfiguration) { - // return new AuthScope(proxyConfiguration.host(), proxyConfiguration.port()); - // } private static void addPreemptiveAuthenticationProxy(HttpClientContext clientContext, ProxyConfiguration proxyConfiguration) { @@ -107,5 +111,4 @@ private static void addPreemptiveAuthenticationProxy(HttpClientContext clientCon clientContext.setAuthCache(authCache); } } - }