From 8dc8fe6876cd8414708816678c83ee2ab27b5608 Mon Sep 17 00:00:00 2001 From: Martin Cornejo Date: Mon, 8 May 2023 23:15:37 +0200 Subject: [PATCH] Now it's possible to configure NettyNioAsyncHttpClient in order to use a non blocking DNS resolver. --- .../bugfix-NettyNIOHTTPClient-35595eb.json | 6 + bom-internal/pom.xml | 10 + .../http/SdkHttpConfigurationOption.java | 13 ++ http-clients/netty-nio-client/pom.xml | 8 + .../nio/netty/NettyNioAsyncHttpClient.java | 19 ++ .../nio/netty/internal/BootstrapProvider.java | 18 ++ .../netty/internal/NettyConfiguration.java | 4 + .../nio/netty/NettyClientTlsAuthTest.java | 19 ++ ...yNioAsyncHttpClientNonBlockingDnsTest.java | 192 ++++++++++++++++++ .../NettyNioAsyncHttpClientTestUtils.java | 148 ++++++++++++++ .../NettyNioAsyncHttpClientWireMockTest.java | 143 ++----------- .../http/nio/netty/ProxyWireMockTest.java | 24 +++ .../netty/internal/BootstrapProviderTest.java | 22 ++ 13 files changed, 502 insertions(+), 124 deletions(-) create mode 100644 .changes/next-release/bugfix-NettyNIOHTTPClient-35595eb.json create mode 100644 http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClientNonBlockingDnsTest.java create mode 100644 http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClientTestUtils.java diff --git a/.changes/next-release/bugfix-NettyNIOHTTPClient-35595eb.json b/.changes/next-release/bugfix-NettyNIOHTTPClient-35595eb.json new file mode 100644 index 000000000000..c11c4d917556 --- /dev/null +++ b/.changes/next-release/bugfix-NettyNIOHTTPClient-35595eb.json @@ -0,0 +1,6 @@ +{ + "category": "Netty NIO HTTP Client", + "contributor": "martinKindall", + "type": "bugfix", + "description": "By default, Netty threads are blocked during dns resolution, namely InetAddress.getByName is used under the hood. Now, there's an option to configure the NettyNioAsyncHttpClient in order to use a non blocking dns resolution strategy." +} diff --git a/bom-internal/pom.xml b/bom-internal/pom.xml index cbb29dd54f46..c440267d88b2 100644 --- a/bom-internal/pom.xml +++ b/bom-internal/pom.xml @@ -134,6 +134,16 @@ netty-buffer ${netty.version} + + io.netty + netty-resolver + ${netty.version} + + + io.netty + netty-resolver-dns + ${netty.version} + org.reactivestreams reactive-streams diff --git a/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpConfigurationOption.java b/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpConfigurationOption.java index 42074fbe76dd..9e894227d3de 100644 --- a/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpConfigurationOption.java +++ b/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpConfigurationOption.java @@ -131,6 +131,16 @@ public final class SdkHttpConfigurationOption extends AttributeMap.Key { public static final SdkHttpConfigurationOption TLS_NEGOTIATION_TIMEOUT = new SdkHttpConfigurationOption<>("TlsNegotiationTimeout", Duration.class); + /** + * Configure whether to use a non-blocking dns resolver or not. False by default, as netty's default dns resolver is + * blocking; it namely calls java.net.InetAddress.getByName. + *

+ * When enabled, a non-blocking dns resolver will be used instead, by modifying netty's bootstrap configuration. + * See https://netty.io/news/2016/05/26/4-1-0-Final.html + */ + public static final SdkHttpConfigurationOption USE_NONBLOCKING_DNS_RESOLVER = + new SdkHttpConfigurationOption<>("UseNonBlockingDnsResolver", Boolean.class); + private static final Duration DEFAULT_SOCKET_READ_TIMEOUT = Duration.ofSeconds(30); private static final Duration DEFAULT_SOCKET_WRITE_TIMEOUT = Duration.ofSeconds(30); private static final Duration DEFAULT_CONNECTION_TIMEOUT = Duration.ofSeconds(2); @@ -152,6 +162,8 @@ public final class SdkHttpConfigurationOption extends AttributeMap.Key { private static final TlsTrustManagersProvider DEFAULT_TLS_TRUST_MANAGERS_PROVIDER = null; private static final TlsKeyManagersProvider DEFAULT_TLS_KEY_MANAGERS_PROVIDER = SystemPropertyTlsKeyManagersProvider.create(); + private static final Boolean DEFAULT_USE_NONBLOCKING_DNS_RESOLVER = Boolean.FALSE; + public static final AttributeMap GLOBAL_HTTP_DEFAULTS = AttributeMap .builder() .put(READ_TIMEOUT, DEFAULT_SOCKET_READ_TIMEOUT) @@ -169,6 +181,7 @@ public final class SdkHttpConfigurationOption extends AttributeMap.Key { .put(TLS_KEY_MANAGERS_PROVIDER, DEFAULT_TLS_KEY_MANAGERS_PROVIDER) .put(TLS_TRUST_MANAGERS_PROVIDER, DEFAULT_TLS_TRUST_MANAGERS_PROVIDER) .put(TLS_NEGOTIATION_TIMEOUT, DEFAULT_TLS_NEGOTIATION_TIMEOUT) + .put(USE_NONBLOCKING_DNS_RESOLVER, DEFAULT_USE_NONBLOCKING_DNS_RESOLVER) .build(); private final String name; diff --git a/http-clients/netty-nio-client/pom.xml b/http-clients/netty-nio-client/pom.xml index f5f03549a653..9b76dd91f81a 100644 --- a/http-clients/netty-nio-client/pom.xml +++ b/http-clients/netty-nio-client/pom.xml @@ -85,6 +85,14 @@ io.netty netty-transport-classes-epoll + + io.netty + netty-resolver + + + io.netty + netty-resolver-dns + diff --git a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClient.java b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClient.java index 78a3fa80fa87..559b09f66bc3 100644 --- a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClient.java +++ b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClient.java @@ -475,6 +475,15 @@ public interface Builder extends SdkAsyncHttpClient.Builder http2ConfigurationBuilderConsumer); + + /** + * Configure whether to use a non-blocking dns resolver or not. False by default, as netty's default dns resolver is + * blocking; it namely calls java.net.InetAddress.getByName. + *

+ * When enabled, a non-blocking dns resolver will be used instead, by modifying netty's bootstrap configuration. + * See https://netty.io/news/2016/05/26/4-1-0-Final.html + */ + Builder useNonBlockingDnsResolver(Boolean useNonBlockingDnsResolver); } /** @@ -716,6 +725,16 @@ public void setHttp2Configuration(Http2Configuration http2Configuration) { http2Configuration(http2Configuration); } + @Override + public Builder useNonBlockingDnsResolver(Boolean useNonBlockingDnsResolver) { + standardOptions.put(SdkHttpConfigurationOption.USE_NONBLOCKING_DNS_RESOLVER, useNonBlockingDnsResolver); + return this; + } + + public void setUseNonBlockingDnsResolver(Boolean useNonBlockingDnsResolver) { + useNonBlockingDnsResolver(useNonBlockingDnsResolver); + } + @Override public SdkAsyncHttpClient buildWithDefaults(AttributeMap serviceDefaults) { if (standardOptions.get(SdkHttpConfigurationOption.TLS_NEGOTIATION_TIMEOUT) == null) { diff --git a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/BootstrapProvider.java b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/BootstrapProvider.java index 5e1b1a227764..579c08d6cd04 100644 --- a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/BootstrapProvider.java +++ b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/BootstrapProvider.java @@ -17,6 +17,11 @@ import io.netty.bootstrap.Bootstrap; import io.netty.channel.ChannelOption; +import io.netty.channel.socket.nio.NioDatagramChannel; +import io.netty.resolver.AddressResolverGroup; +import io.netty.resolver.dns.DnsAddressResolverGroup; +import io.netty.resolver.dns.DnsNameResolverBuilder; +import io.netty.resolver.dns.NoopDnsCache; import java.net.InetSocketAddress; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.http.nio.netty.SdkEventLoopGroup; @@ -56,8 +61,21 @@ public Bootstrap createBootstrap(String host, int port) { .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, nettyConfiguration.connectTimeoutMillis()) .option(ChannelOption.SO_KEEPALIVE, nettyConfiguration.tcpKeepAlive()) .remoteAddress(InetSocketAddress.createUnresolved(host, port)); + + if (nettyConfiguration.isNonBlockingResolver()) { + bootstrap.resolver(nonBlockingResolverGroup()); + } + sdkChannelOptions.channelOptions().forEach(bootstrap::option); return bootstrap; } + + private AddressResolverGroup nonBlockingResolverGroup() { + DnsNameResolverBuilder builder = new DnsNameResolverBuilder() + .channelType(NioDatagramChannel.class) + .resolveCache(NoopDnsCache.INSTANCE); + + return new DnsAddressResolverGroup(builder); + } } diff --git a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/NettyConfiguration.java b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/NettyConfiguration.java index 4dd6d7219e72..27a3a2f39a91 100644 --- a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/NettyConfiguration.java +++ b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/NettyConfiguration.java @@ -107,4 +107,8 @@ public boolean tcpKeepAlive() { public Duration tlsHandshakeTimeout() { return configuration.get(SdkHttpConfigurationOption.TLS_NEGOTIATION_TIMEOUT); } + + public boolean isNonBlockingResolver() { + return configuration.get(SdkHttpConfigurationOption.USE_NONBLOCKING_DNS_RESOLVER); + } } diff --git a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyClientTlsAuthTest.java b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyClientTlsAuthTest.java index dc7c408c3c9f..50fecf729126 100644 --- a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyClientTlsAuthTest.java +++ b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyClientTlsAuthTest.java @@ -39,6 +39,7 @@ import software.amazon.awssdk.http.EmptyPublisher; import software.amazon.awssdk.http.FileStoreTlsKeyManagersProvider; import software.amazon.awssdk.http.HttpTestUtils; +import software.amazon.awssdk.http.SdkHttpConfigurationOption; import software.amazon.awssdk.http.SdkHttpFullRequest; import software.amazon.awssdk.http.SdkHttpMethod; import software.amazon.awssdk.http.TlsKeyManagersProvider; @@ -185,6 +186,24 @@ public void nonProxy_noKeyManagerGiven_shouldThrowException() { .hasRootCauseInstanceOf(SSLException.class); } + @Test + public void builderUsesProvidedKeyManagersProviderNonBlockingDns() { + TlsKeyManagersProvider mockKeyManagersProvider = mock(TlsKeyManagersProvider.class); + netty = NettyNioAsyncHttpClient.builder() + .proxyConfiguration(proxyCfg) + .tlsKeyManagersProvider(mockKeyManagersProvider) + .buildWithDefaults(AttributeMap.builder() + .put(TRUST_ALL_CERTIFICATES, true) + .put(SdkHttpConfigurationOption.USE_NONBLOCKING_DNS_RESOLVER, true) + .build()); + + try { + sendRequest(netty, new RecordingResponseHandler()); + } catch (Exception ignored) { + } + verify(mockKeyManagersProvider).keyManagers(); + } + private void sendRequest(SdkAsyncHttpClient client, SdkAsyncHttpResponseHandler responseHandler) { AsyncExecuteRequest req = AsyncExecuteRequest.builder() .request(testSdkRequest()) diff --git a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClientNonBlockingDnsTest.java b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClientNonBlockingDnsTest.java new file mode 100644 index 000000000000..b2a8208edb4b --- /dev/null +++ b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClientNonBlockingDnsTest.java @@ -0,0 +1,192 @@ +/* + * 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.nio.netty; + +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.equalTo; +import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor; +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; +import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static com.github.tomakehurst.wiremock.client.WireMock.verify; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import static java.util.Collections.singletonMap; +import static org.apache.commons.lang3.RandomStringUtils.randomAlphabetic; +import static org.apache.commons.lang3.StringUtils.reverse; +import static org.assertj.core.api.Assertions.assertThat; +import static software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClientTestUtils.assertCanReceiveBasicRequest; +import static software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClientTestUtils.createProvider; +import static software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClientTestUtils.createRequest; +import static software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClientTestUtils.makeSimpleRequest; + +import com.github.tomakehurst.wiremock.junit.WireMockRule; +import java.io.IOException; +import java.net.URI; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import org.assertj.core.api.Condition; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.junit.MockitoJUnitRunner; +import software.amazon.awssdk.http.SdkHttpConfigurationOption; +import software.amazon.awssdk.http.SdkHttpFullRequest; +import software.amazon.awssdk.http.SdkHttpMethod; +import software.amazon.awssdk.http.SdkHttpRequest; +import software.amazon.awssdk.http.async.AsyncExecuteRequest; +import software.amazon.awssdk.http.async.SdkAsyncHttpClient; +import software.amazon.awssdk.utils.AttributeMap; + +@RunWith(MockitoJUnitRunner.class) +public class NettyNioAsyncHttpClientNonBlockingDnsTest { + + private final RecordingNetworkTrafficListener wiremockTrafficListener = new RecordingNetworkTrafficListener(); + + private static final SdkAsyncHttpClient client = NettyNioAsyncHttpClient.builder() + .buildWithDefaults( + AttributeMap.builder() + .put(SdkHttpConfigurationOption.TRUST_ALL_CERTIFICATES, true) + .put(SdkHttpConfigurationOption.USE_NONBLOCKING_DNS_RESOLVER, true) + .build()); + + @Rule + public WireMockRule mockServer = new WireMockRule(wireMockConfig() + .dynamicPort() + .dynamicHttpsPort() + .networkTrafficListener(wiremockTrafficListener)); + + @Before + public void methodSetup() { + wiremockTrafficListener.reset(); + } + + @AfterClass + public static void tearDown() throws Exception { + client.close(); + } + + @Test + public void useNonBlockingDnsResolver_shouldHonor() { + try (NettyNioAsyncHttpClient client = (NettyNioAsyncHttpClient) NettyNioAsyncHttpClient.builder() + .build()) { + assertThat(client.configuration().isNonBlockingResolver()).isEqualTo(false); + } + + try (NettyNioAsyncHttpClient client = (NettyNioAsyncHttpClient) NettyNioAsyncHttpClient.builder() + .useNonBlockingDnsResolver(false) + .build()) { + assertThat(client.configuration().isNonBlockingResolver()).isEqualTo(false); + } + + try (NettyNioAsyncHttpClient client = (NettyNioAsyncHttpClient) NettyNioAsyncHttpClient.builder() + .useNonBlockingDnsResolver(true) + .build()) { + assertThat(client.configuration().isNonBlockingResolver()).isEqualTo(true); + } + } + + @Test + public void canSendContentAndGetThatContentBackNonBlockingDns() throws Exception { + String body = randomAlphabetic(50); + stubFor(any(urlEqualTo("/echo?reversed=true")) + .withRequestBody(equalTo(body)) + .willReturn(aResponse().withBody(reverse(body)))); + URI uri = URI.create("http://localhost:" + mockServer.port()); + + SdkHttpRequest request = createRequest(uri, "/echo", body, SdkHttpMethod.POST, singletonMap("reversed", "true")); + + RecordingResponseHandler recorder = new RecordingResponseHandler(); + + client.execute(AsyncExecuteRequest.builder().request(request).requestContentPublisher(createProvider(body)).responseHandler(recorder).build()); + + recorder.completeFuture.get(5, TimeUnit.SECONDS); + + verify(1, postRequestedFor(urlEqualTo("/echo?reversed=true"))); + + assertThat(recorder.fullResponseAsString()).isEqualTo(reverse(body)); + } + + @Test + public void defaultThreadFactoryUsesHelpfulName() throws Exception { + // Make a request to ensure a thread is primed + makeSimpleRequest(client, mockServer); + + String expectedPattern = "aws-java-sdk-NettyEventLoop-\\d+-\\d+"; + assertThat(Thread.getAllStackTraces().keySet()) + .areAtLeast(1, new Condition<>(t -> t.getName().matches(expectedPattern), + "Matches default thread pattern: `%s`", expectedPattern)); + } + + @Test + public void canMakeBasicRequestOverHttp() throws Exception { + String smallBody = randomAlphabetic(10); + URI uri = URI.create("http://localhost:" + mockServer.port()); + + assertCanReceiveBasicRequest(client, uri, smallBody); + } + + @Test + public void canMakeBasicRequestOverHttps() throws Exception { + String smallBody = randomAlphabetic(10); + URI uri = URI.create("https://localhost:" + mockServer.httpsPort()); + + assertCanReceiveBasicRequest(client, uri, smallBody); + } + + @Test + public void canHandleLargerPayloadsOverHttp() throws Exception { + String largishBody = randomAlphabetic(25000); + + URI uri = URI.create("http://localhost:" + mockServer.port()); + + assertCanReceiveBasicRequest(client, uri, largishBody); + } + + @Test + public void canHandleLargerPayloadsOverHttps() throws Exception { + String largishBody = randomAlphabetic(25000); + + URI uri = URI.create("https://localhost:" + mockServer.httpsPort()); + + assertCanReceiveBasicRequest(client, uri, largishBody); + } + + @Test + public void requestContentOnlyEqualToContentLengthHeaderFromProvider() throws InterruptedException, ExecutionException, TimeoutException, IOException { + final String content = randomAlphabetic(32); + final String streamContent = content + reverse(content); + stubFor(any(urlEqualTo("/echo?reversed=true")) + .withRequestBody(equalTo(content)) + .willReturn(aResponse().withBody(reverse(content)))); + URI uri = URI.create("http://localhost:" + mockServer.port()); + + SdkHttpFullRequest request = createRequest(uri, "/echo", streamContent, SdkHttpMethod.POST, singletonMap("reversed", "true")); + request = request.toBuilder().putHeader("Content-Length", Integer.toString(content.length())).build(); + RecordingResponseHandler recorder = new RecordingResponseHandler(); + + client.execute(AsyncExecuteRequest.builder().request(request).requestContentPublisher(createProvider(streamContent)).responseHandler(recorder).build()); + + recorder.completeFuture.get(5, TimeUnit.SECONDS); + + // HTTP servers will stop processing the request as soon as it reads + // bytes equal to 'Content-Length' so we need to inspect the raw + // traffic to ensure that there wasn't anything after that. + assertThat(wiremockTrafficListener.requests().toString()).endsWith(content); + } +} diff --git a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClientTestUtils.java b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClientTestUtils.java new file mode 100644 index 000000000000..04f9a906ee04 --- /dev/null +++ b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClientTestUtils.java @@ -0,0 +1,148 @@ +/* + * 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.nio.netty; + +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.getRequestedFor; +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; +import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching; +import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo; +import static com.github.tomakehurst.wiremock.client.WireMock.verify; +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Collections.emptyMap; +import static org.apache.commons.lang3.RandomStringUtils.randomAlphabetic; +import static org.apache.commons.lang3.StringUtils.isBlank; +import static org.assertj.core.api.Assertions.assertThat; + +import com.github.tomakehurst.wiremock.WireMockServer; +import java.net.URI; +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.TimeUnit; +import java.util.stream.Stream; +import org.reactivestreams.Subscriber; +import org.reactivestreams.Subscription; +import software.amazon.awssdk.http.SdkHttpFullRequest; +import software.amazon.awssdk.http.SdkHttpMethod; +import software.amazon.awssdk.http.SdkHttpRequest; +import software.amazon.awssdk.http.async.AsyncExecuteRequest; +import software.amazon.awssdk.http.async.SdkAsyncHttpClient; +import software.amazon.awssdk.http.async.SdkHttpContentPublisher; + +public class NettyNioAsyncHttpClientTestUtils { + + /** + * Make a simple async request and wait for it to fiish. + * + * @param client Client to make request with. + */ + public static void makeSimpleRequest(SdkAsyncHttpClient client, WireMockServer mockServer) throws Exception { + String body = randomAlphabetic(10); + URI uri = URI.create("http://localhost:" + mockServer.port()); + stubFor(any(urlPathEqualTo("/")).willReturn(aResponse().withBody(body))); + SdkHttpRequest request = createRequest(uri); + RecordingResponseHandler recorder = new RecordingResponseHandler(); + client.execute(AsyncExecuteRequest.builder().request(request).requestContentPublisher(createProvider("")).responseHandler(recorder).build()); + recorder.completeFuture.get(5, TimeUnit.SECONDS); + } + + public static SdkHttpContentPublisher createProvider(String body) { + Stream chunks = splitStringBySize(body).stream() + .map(chunk -> ByteBuffer.wrap(chunk.getBytes(UTF_8))); + return new SdkHttpContentPublisher() { + + @Override + public Optional contentLength() { + return Optional.of(Long.valueOf(body.length())); + } + + @Override + public void subscribe(Subscriber s) { + s.onSubscribe(new Subscription() { + @Override + public void request(long n) { + chunks.forEach(s::onNext); + s.onComplete(); + } + + @Override + public void cancel() { + + } + }); + } + }; + } + + public static SdkHttpFullRequest createRequest(URI uri) { + return createRequest(uri, "/", null, SdkHttpMethod.GET, emptyMap()); + } + + public static SdkHttpFullRequest createRequest(URI uri, + String resourcePath, + String body, + SdkHttpMethod method, + Map params) { + String contentLength = body == null ? null : String.valueOf(body.getBytes(UTF_8).length); + return SdkHttpFullRequest.builder() + .uri(uri) + .method(method) + .encodedPath(resourcePath) + .applyMutation(b -> params.forEach(b::putRawQueryParameter)) + .applyMutation(b -> { + b.putHeader("Host", uri.getHost()); + if (contentLength != null) { + b.putHeader("Content-Length", contentLength); + } + }).build(); + } + + public static void assertCanReceiveBasicRequest(SdkAsyncHttpClient client, URI uri, String body) throws Exception { + stubFor(any(urlPathEqualTo("/")).willReturn(aResponse().withHeader("Some-Header", "With Value").withBody(body))); + + SdkHttpRequest request = createRequest(uri); + + RecordingResponseHandler recorder = new RecordingResponseHandler(); + client.execute(AsyncExecuteRequest.builder().request(request).requestContentPublisher(createProvider("")).responseHandler(recorder).build()); + + recorder.completeFuture.get(5, TimeUnit.SECONDS); + + assertThat(recorder.responses).hasOnlyOneElementSatisfying( + headerResponse -> { + assertThat(headerResponse.headers()).containsKey("Some-Header"); + assertThat(headerResponse.statusCode()).isEqualTo(200); + }); + + assertThat(recorder.fullResponseAsString()).isEqualTo(body); + verify(1, getRequestedFor(urlMatching("/"))); + } + + private static Collection splitStringBySize(String str) { + if (isBlank(str)) { + return Collections.emptyList(); + } + ArrayList split = new ArrayList<>(); + for (int i = 0; i <= str.length() / 1000; i++) { + split.add(str.substring(i * 1000, Math.min((i + 1) * 1000, str.length()))); + } + return split; + } +} diff --git a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClientWireMockTest.java b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClientWireMockTest.java index 9a1121e201f5..7fa61791c345 100644 --- a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClientWireMockTest.java +++ b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClientWireMockTest.java @@ -18,19 +18,14 @@ 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.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.stubFor; 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.client.WireMock.verify; import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; -import static java.nio.charset.StandardCharsets.UTF_8; -import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; import static org.apache.commons.lang3.RandomStringUtils.randomAlphabetic; -import static org.apache.commons.lang3.StringUtils.isBlank; import static org.apache.commons.lang3.StringUtils.reverse; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -40,6 +35,10 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.when; +import static software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClientTestUtils.assertCanReceiveBasicRequest; +import static software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClientTestUtils.createProvider; +import static software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClientTestUtils.createRequest; +import static software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClientTestUtils.makeSimpleRequest; import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.http.Fault; @@ -54,20 +53,15 @@ import io.netty.util.AttributeKey; import java.io.IOException; import java.net.URI; -import java.nio.ByteBuffer; import java.time.Duration; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.List; -import java.util.Map; -import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; -import java.util.stream.Stream; import javax.net.ssl.TrustManagerFactory; import org.assertj.core.api.Condition; import org.junit.AfterClass; @@ -78,8 +72,6 @@ import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; import org.mockito.stubbing.Answer; -import org.reactivestreams.Subscriber; -import org.reactivestreams.Subscription; import software.amazon.awssdk.http.HttpMetric; import software.amazon.awssdk.http.HttpTestUtils; import software.amazon.awssdk.http.SdkHttpConfigurationOption; @@ -88,7 +80,6 @@ import software.amazon.awssdk.http.SdkHttpRequest; import software.amazon.awssdk.http.async.AsyncExecuteRequest; import software.amazon.awssdk.http.async.SdkAsyncHttpClient; -import software.amazon.awssdk.http.async.SdkHttpContentPublisher; import software.amazon.awssdk.http.nio.netty.internal.NettyConfiguration; import software.amazon.awssdk.http.nio.netty.internal.SdkChannelPool; import software.amazon.awssdk.http.nio.netty.internal.SdkChannelPoolMap; @@ -183,7 +174,8 @@ public void invalidMaxPendingConnectionAcquireConfig_shouldPropagateException() .maxConcurrency(1) .maxPendingConnectionAcquires(0) .build()) { - assertThatThrownBy(() -> makeSimpleRequest(customClient)).hasMessageContaining("java.lang.IllegalArgumentException: maxPendingAcquires: 0 (expected: >= 1)"); + assertThatThrownBy(() -> makeSimpleRequest(customClient, mockServer)).hasMessageContaining("java.lang" + + ".IllegalArgumentException: maxPendingAcquires: 0 (expected: >= 1)"); } } @@ -196,7 +188,7 @@ public void customFactoryIsUsed() throws Exception { .threadFactory(threadFactory)) .build(); - makeSimpleRequest(customClient); + makeSimpleRequest(customClient, mockServer); customClient.close(); Mockito.verify(threadFactory, atLeastOnce()).newThread(Mockito.any()); @@ -208,7 +200,7 @@ public void openSslBeingUsed() throws Exception { NettyNioAsyncHttpClient.builder() .sslProvider(SslProvider.OPENSSL) .build()) { - makeSimpleRequest(customClient); + makeSimpleRequest(customClient, mockServer); } } @@ -218,7 +210,7 @@ public void defaultJdkSslProvider() throws Exception { NettyNioAsyncHttpClient.builder() .sslProvider(SslProvider.JDK) .build()) { - makeSimpleRequest(customClient); + makeSimpleRequest(customClient, mockServer); customClient.close(); } } @@ -226,7 +218,7 @@ public void defaultJdkSslProvider() throws Exception { @Test public void defaultThreadFactoryUsesHelpfulName() throws Exception { // Make a request to ensure a thread is primed - makeSimpleRequest(client); + makeSimpleRequest(client, mockServer); String expectedPattern = "aws-java-sdk-NettyEventLoop-\\d+-\\d+"; assertThat(Thread.getAllStackTraces().keySet()) @@ -247,7 +239,7 @@ public void customThreadCountIsRespected() throws Exception { // Have to make enough requests to prime the threads for (int i = 0; i < threadCount + 1; i++) { - makeSimpleRequest(customClient); + makeSimpleRequest(customClient, mockServer); } customClient.close(); @@ -267,7 +259,7 @@ public void customEventLoopGroup_NotClosedWhenClientIsClosed() throws Exception .eventLoopGroup(SdkEventLoopGroup.create(eventLoopGroup, NioSocketChannel::new)) .build(); - makeSimpleRequest(customClient); + makeSimpleRequest(customClient, mockServer); customClient.close(); Mockito.verify(threadFactory, atLeastOnce()).newThread(Mockito.any()); @@ -287,7 +279,7 @@ public void customChannelFactoryIsUsed() throws Exception { .eventLoopGroup(SdkEventLoopGroup.create(customEventLoopGroup, channelFactory)) .build(); - makeSimpleRequest(customClient); + makeSimpleRequest(customClient, mockServer); customClient.close(); Mockito.verify(channelFactory, atLeastOnce()).newChannel(); @@ -335,7 +327,7 @@ public void responseConnectionReused_shouldReleaseChannel() throws Exception { .maxConcurrency(1) .build(); - makeSimpleRequest(customClient); + makeSimpleRequest(customClient, mockServer); verifyChannelRelease(channel); assertThat(channel.isShutdown()).isFalse(); @@ -446,27 +438,12 @@ public void builderUsesProvidedTrustManagersProvider() throws Exception { } } - /** - * Make a simple async request and wait for it to fiish. - * - * @param client Client to make request with. - */ - private void makeSimpleRequest(SdkAsyncHttpClient client) throws Exception { - String body = randomAlphabetic(10); - URI uri = URI.create("http://localhost:" + mockServer.port()); - stubFor(any(urlPathEqualTo("/")).willReturn(aResponse().withBody(body))); - SdkHttpRequest request = createRequest(uri); - RecordingResponseHandler recorder = new RecordingResponseHandler(); - client.execute(AsyncExecuteRequest.builder().request(request).requestContentPublisher(createProvider("")).responseHandler(recorder).build()); - recorder.completeFuture.get(5, TimeUnit.SECONDS); - } - @Test public void canMakeBasicRequestOverHttp() throws Exception { String smallBody = randomAlphabetic(10); URI uri = URI.create("http://localhost:" + mockServer.port()); - assertCanReceiveBasicRequest(uri, smallBody); + assertCanReceiveBasicRequest(client, uri, smallBody); } @Test @@ -474,7 +451,7 @@ public void canMakeBasicRequestOverHttps() throws Exception { String smallBody = randomAlphabetic(10); URI uri = URI.create("https://localhost:" + mockServer.httpsPort()); - assertCanReceiveBasicRequest(uri, smallBody); + assertCanReceiveBasicRequest(client, uri, smallBody); } @Test @@ -483,7 +460,7 @@ public void canHandleLargerPayloadsOverHttp() throws Exception { URI uri = URI.create("http://localhost:" + mockServer.port()); - assertCanReceiveBasicRequest(uri, largishBody); + assertCanReceiveBasicRequest(client, uri, largishBody); } @Test @@ -492,7 +469,7 @@ public void canHandleLargerPayloadsOverHttps() throws Exception { URI uri = URI.create("https://localhost:" + mockServer.httpsPort()); - assertCanReceiveBasicRequest(uri, largishBody); + assertCanReceiveBasicRequest(client, uri, largishBody); } @Test @@ -579,88 +556,6 @@ public ChannelFuture close() { assertThat(channelClosedFuture.get(5, TimeUnit.SECONDS)).isTrue(); } - private void assertCanReceiveBasicRequest(URI uri, String body) throws Exception { - stubFor(any(urlPathEqualTo("/")).willReturn(aResponse().withHeader("Some-Header", "With Value").withBody(body))); - - SdkHttpRequest request = createRequest(uri); - - RecordingResponseHandler recorder = new RecordingResponseHandler(); - client.execute(AsyncExecuteRequest.builder().request(request).requestContentPublisher(createProvider("")).responseHandler(recorder).build()); - - recorder.completeFuture.get(5, TimeUnit.SECONDS); - - assertThat(recorder.responses).hasOnlyOneElementSatisfying( - headerResponse -> { - assertThat(headerResponse.headers()).containsKey("Some-Header"); - assertThat(headerResponse.statusCode()).isEqualTo(200); - }); - - assertThat(recorder.fullResponseAsString()).isEqualTo(body); - verify(1, getRequestedFor(urlMatching("/"))); - } - - private SdkHttpContentPublisher createProvider(String body) { - Stream chunks = splitStringBySize(body).stream() - .map(chunk -> ByteBuffer.wrap(chunk.getBytes(UTF_8))); - return new SdkHttpContentPublisher() { - - @Override - public Optional contentLength() { - return Optional.of(Long.valueOf(body.length())); - } - - @Override - public void subscribe(Subscriber s) { - s.onSubscribe(new Subscription() { - @Override - public void request(long n) { - chunks.forEach(s::onNext); - s.onComplete(); - } - - @Override - public void cancel() { - - } - }); - } - }; - } - - private SdkHttpFullRequest createRequest(URI uri) { - return createRequest(uri, "/", null, SdkHttpMethod.GET, emptyMap()); - } - - private SdkHttpFullRequest createRequest(URI uri, - String resourcePath, - String body, - SdkHttpMethod method, - Map params) { - String contentLength = body == null ? null : String.valueOf(body.getBytes(UTF_8).length); - return SdkHttpFullRequest.builder() - .uri(uri) - .method(method) - .encodedPath(resourcePath) - .applyMutation(b -> params.forEach(b::putRawQueryParameter)) - .applyMutation(b -> { - b.putHeader("Host", uri.getHost()); - if (contentLength != null) { - b.putHeader("Content-Length", contentLength); - } - }).build(); - } - - private static Collection splitStringBySize(String str) { - if (isBlank(str)) { - return Collections.emptyList(); - } - ArrayList split = new ArrayList<>(); - for (int i = 0; i <= str.length() / 1000; i++) { - split.add(str.substring(i * 1000, Math.min((i + 1) * 1000, str.length()))); - } - return split; - } - // Needs to be a non-anon class in order to spy public static class CustomThreadFactory implements ThreadFactory { @Override @@ -719,7 +614,7 @@ public void createNettyClient_ReadWriteTimeoutCanBeZero() throws Exception { .writeTimeout(Duration.ZERO) .build(); - makeSimpleRequest(customClient); + makeSimpleRequest(customClient, mockServer); customClient.close(); } diff --git a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/ProxyWireMockTest.java b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/ProxyWireMockTest.java index f797a760fdf7..438d65e1f9fc 100644 --- a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/ProxyWireMockTest.java +++ b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/ProxyWireMockTest.java @@ -126,6 +126,30 @@ public void proxyConfigured_hostInNonProxySet_doesNotConnect() { assertThat(responseHandler.fullResponseAsString()).isEqualTo("hello"); } + @Test + public void proxyConfigured_hostInNonProxySet_nonBlockingDns_doesNotConnect() { + RecordingResponseHandler responseHandler = new RecordingResponseHandler(); + AsyncExecuteRequest req = AsyncExecuteRequest.builder() + .request(testSdkRequest()) + .responseHandler(responseHandler) + .requestContentPublisher(new EmptyPublisher()) + .build(); + + ProxyConfiguration cfg = proxyCfg.toBuilder() + .nonProxyHosts(Stream.of("localhost").collect(Collectors.toSet())) + .build(); + + client = NettyNioAsyncHttpClient.builder() + .proxyConfiguration(cfg) + .useNonBlockingDnsResolver(true) + .build(); + + client.execute(req).join(); + + responseHandler.completeFuture.join(); + assertThat(responseHandler.fullResponseAsString()).isEqualTo("hello"); + } + private SdkHttpFullRequest testSdkRequest() { return SdkHttpFullRequest.builder() .method(SdkHttpMethod.GET) diff --git a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/internal/BootstrapProviderTest.java b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/internal/BootstrapProviderTest.java index 337cb7ba2ec2..bbfd645d334d 100644 --- a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/internal/BootstrapProviderTest.java +++ b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/internal/BootstrapProviderTest.java @@ -26,6 +26,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.junit.MockitoJUnitRunner; +import software.amazon.awssdk.http.SdkHttpConfigurationOption; import software.amazon.awssdk.http.nio.netty.SdkEventLoopGroup; import software.amazon.awssdk.utils.AttributeMap; @@ -52,6 +53,27 @@ public void createBootstrap_usesUnresolvedInetSocketAddress() { assertThat(inetSocketAddress.isUnresolved()).isTrue(); } + @Test + public void createBootstrapNonBlockingDns_usesUnresolvedInetSocketAddress() { + AttributeMap config = GLOBAL_HTTP_DEFAULTS.toBuilder() + .put(SdkHttpConfigurationOption.USE_NONBLOCKING_DNS_RESOLVER, true) + .build(); + + BootstrapProvider bootstrapProvider = + new BootstrapProvider(SdkEventLoopGroup.builder().build(), + new NettyConfiguration(config), + new SdkChannelOptions()); + + Bootstrap bootstrap = bootstrapProvider.createBootstrap("some-awesome-service-1234.amazonaws.com", 443); + + SocketAddress socketAddress = bootstrap.config().remoteAddress(); + + assertThat(socketAddress).isInstanceOf(InetSocketAddress.class); + InetSocketAddress inetSocketAddress = (InetSocketAddress)socketAddress; + + assertThat(inetSocketAddress.isUnresolved()).isTrue(); + } + @Test public void createBootstrap_defaultConfiguration_tcpKeepAliveShouldBeFalse() { Bootstrap bootstrap = bootstrapProvider.createBootstrap("some-awesome-service-1234.amazonaws.com", 443);