Track If Netty Builder Set Resolver and Properly Use Noop Resolver When Proxying (Azure#26611)

Track If Netty Builder Set Resolver and Properly Use Noop Resolver When Proxying

Author: alzimmermsft

sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/NettyAsyncHttpClientBuilder.java

@@ -128,12 +128,18 @@ public NettyAsyncHttpClientBuilder(HttpClient nettyHttpClient) {
      */
     public com.azure.core.http.HttpClient build() {
         HttpClient nettyHttpClient;
+
+        // Used to track if the builder set the DefaultAddressResolverGroup. If it did, when proxying it allows the
+        // no-op address resolver to be set.
+        boolean addressResolverWasSetByBuilder = false;
         if (this.baseHttpClient != null) {
             nettyHttpClient = baseHttpClient;
         } else if (this.connectionProvider != null) {
             nettyHttpClient = HttpClient.create(this.connectionProvider).resolver(DefaultAddressResolverGroup.INSTANCE);
+            addressResolverWasSetByBuilder = true;
         } else {
             nettyHttpClient = HttpClient.create().resolver(DefaultAddressResolverGroup.INSTANCE);
+            addressResolverWasSetByBuilder = true;
         }
 
         nettyHttpClient = nettyHttpClient
@@ -187,11 +193,6 @@ public com.azure.core.http.HttpClient build() {
                                 handler, proxyChallengeHolder));
                     }
                 });
-
-                AddressResolverGroup<?> resolver = nettyHttpClient.configuration().resolver();
-                if (resolver == null) {
-                    nettyHttpClient.resolver(NoopAddressResolverGroup.INSTANCE);
-                }
             } else {
                 nettyHttpClient = nettyHttpClient.proxy(proxy ->
                     proxy.type(toReactorNettyProxyType(buildProxyOptions.getType(), logger))
@@ -200,6 +201,11 @@ public com.azure.core.http.HttpClient build() {
                         .password(ignored -> buildProxyOptions.getPassword())
                         .nonProxyHosts(buildProxyOptions.getNonProxyHosts()));
             }
+
+            AddressResolverGroup<?> resolver = nettyHttpClient.configuration().resolver();
+            if (resolver == null || addressResolverWasSetByBuilder) {
+                nettyHttpClient = nettyHttpClient.resolver(NoopAddressResolverGroup.INSTANCE);
+            }
         }
 
         return new NettyAsyncHttpClient(nettyHttpClient, disableBufferCopy,

sdk/core/azure-core-http-netty/src/test/java/com/azure/core/http/netty/NettyAsyncHttpClientTests.java

@@ -21,6 +21,8 @@
 import com.github.tomakehurst.wiremock.WireMockServer;
 import com.github.tomakehurst.wiremock.http.Fault;
 import io.netty.handler.proxy.ProxyConnectException;
+import io.netty.resolver.DefaultAddressResolverGroup;
+import io.netty.resolver.NoopAddressResolverGroup;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeAll;
@@ -32,6 +34,7 @@
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 import reactor.core.scheduler.Schedulers;
+import reactor.netty.resources.ConnectionProvider;
 import reactor.test.StepVerifier;
 import reactor.test.StepVerifierOptions;
 
@@ -56,6 +59,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertLinesMatch;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 public class NettyAsyncHttpClientTests {
@@ -436,6 +440,41 @@ public void failedProxyAuthenticationReturnsCorrectError() {
         }
     }
 
+    @Test
+    public void httpClientWithDefaultResolverUsesNoopResolverWithProxy() {
+        try (MockProxyServer mockProxyServer = new MockProxyServer()) {
+            NettyAsyncHttpClient httpClient = (NettyAsyncHttpClient) new NettyAsyncHttpClientBuilder()
+                .proxy(new ProxyOptions(ProxyOptions.Type.HTTP, mockProxyServer.socketAddress()))
+                .build();
+
+            assertEquals(NoopAddressResolverGroup.INSTANCE, httpClient.nettyClient.configuration().resolver());
+        }
+    }
+
+    @Test
+    public void httpClientWithConnectionProviderUsesNoopResolverWithProxy() {
+        try (MockProxyServer mockProxyServer = new MockProxyServer()) {
+            NettyAsyncHttpClient httpClient = (NettyAsyncHttpClient) new NettyAsyncHttpClientBuilder()
+                .connectionProvider(ConnectionProvider.newConnection())
+                .proxy(new ProxyOptions(ProxyOptions.Type.HTTP, mockProxyServer.socketAddress()))
+                .build();
+
+            assertEquals(NoopAddressResolverGroup.INSTANCE, httpClient.nettyClient.configuration().resolver());
+        }
+    }
+
+    @Test
+    public void httpClientWithResolverUsesConfiguredResolverWithProxy() {
+        try (MockProxyServer mockProxyServer = new MockProxyServer()) {
+            NettyAsyncHttpClient httpClient = (NettyAsyncHttpClient) new NettyAsyncHttpClientBuilder(
+                reactor.netty.http.client.HttpClient.create().resolver(DefaultAddressResolverGroup.INSTANCE))
+                .proxy(new ProxyOptions(ProxyOptions.Type.HTTP, mockProxyServer.socketAddress()))
+                .build();
+
+            assertNotEquals(NoopAddressResolverGroup.INSTANCE, httpClient.nettyClient.configuration().resolver());
+        }
+    }
+
     private static Stream<Arguments> requestHeaderSupplier() {
         return Stream.of(
             Arguments.of(null, NettyAsyncHttpClientResponseTransformer.NULL_REPLACEMENT),