Skip to content

Commit 0973e8c

Browse files
Copilotdoom369
andauthored
Add real domain tests, make getAddressResolverGroup() default method, lazy resolver init
- Added tests for real domains (google.com, example.com) with network availability check via assumeTrue - Made getAddressResolverGroup() a default method returning null in the interface to avoid breaking downstream implementations - Lazily obtain group resolver only in branches that need DNS resolution - Added fail() to unknownHost test for proper assertion - Documented resolver group lifecycle ownership in builder Javadoc Agent-Logs-Url: https://github.com/doom369/async-http-client/sessions/ef98f786-3665-4a5f-9005-60ebdba761b1 Co-authored-by: doom369 <1536494+doom369@users.noreply.github.com>
1 parent 2153b52 commit 0973e8c

File tree

4 files changed

+73
-10
lines changed

4 files changed

+73
-10
lines changed

client/src/main/java/org/asynchttpclient/AsyncHttpClientConfig.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,9 @@ default boolean isHttp2CleartextEnabled() {
393393
* @return the {@link AddressResolverGroup} or {@code null} to use per-request resolvers
394394
*/
395395
@Nullable
396-
AddressResolverGroup<InetSocketAddress> getAddressResolverGroup();
396+
default AddressResolverGroup<InetSocketAddress> getAddressResolverGroup() {
397+
return null;
398+
}
397399

398400
boolean isUseNativeTransport();
399401

client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClientConfig.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1531,6 +1531,11 @@ public Builder setEventLoopGroup(EventLoopGroup eventLoopGroup) {
15311531
* <p>
15321532
* When set, this resolver group is used instead of the per-request {@link io.netty.resolver.NameResolver}.
15331533
* Pass {@code null} (the default) to use per-request resolvers (legacy behavior).
1534+
* <p>
1535+
* <b>Lifecycle:</b> The client takes ownership of the provided resolver group and will
1536+
* {@linkplain AddressResolverGroup#close() close} it when the client is shut down.
1537+
* Do not pass a shared resolver group that is used by other clients unless you manage
1538+
* its lifecycle independently.
15341539
*
15351540
* @param addressResolverGroup the resolver group, or {@code null} to use per-request resolvers
15361541
* @return the same builder instance

client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import io.netty.handler.codec.http2.Http2StreamChannel;
3636
import io.netty.handler.codec.http2.Http2StreamChannelBootstrap;
3737
import io.netty.resolver.AddressResolver;
38+
import io.netty.resolver.AddressResolverGroup;
3839
import io.netty.util.ReferenceCountUtil;
3940
import io.netty.util.Timer;
4041
import io.netty.util.concurrent.Future;
@@ -73,6 +74,7 @@
7374
import org.asynchttpclient.resolver.RequestHostnameResolver;
7475
import org.asynchttpclient.uri.Uri;
7576
import org.asynchttpclient.ws.WebSocketUpgradeHandler;
77+
import org.jetbrains.annotations.Nullable;
7678
import org.slf4j.Logger;
7779
import org.slf4j.LoggerFactory;
7880

@@ -371,15 +373,11 @@ private <T> Future<List<InetSocketAddress>> resolveAddresses(Request request, Pr
371373
Uri uri = request.getUri();
372374
final Promise<List<InetSocketAddress>> promise = ImmediateEventExecutor.INSTANCE.newPromise();
373375

374-
// Use the client's AddressResolverGroup when configured for non-blocking async DNS.
375-
AddressResolver<InetSocketAddress> groupResolver = channelManager.getAddressResolverGroup() != null
376-
? channelManager.getAddressResolverGroup().getResolver(channelManager.getEventLoopGroup().next())
377-
: null;
378-
379376
if (proxy != null && !proxy.isIgnoredForHost(uri.getHost()) && proxy.getProxyType().isHttp()) {
380377
int port = ProxyType.HTTPS.equals(proxy.getProxyType()) || uri.isSecured() ? proxy.getSecuredPort() : proxy.getPort();
381378
InetSocketAddress unresolvedRemoteAddress = InetSocketAddress.createUnresolved(proxy.getHost(), port);
382379
scheduleRequestTimeout(future, unresolvedRemoteAddress);
380+
AddressResolver<InetSocketAddress> groupResolver = getGroupResolver();
383381
if (groupResolver != null) {
384382
return RequestHostnameResolver.INSTANCE.resolve(groupResolver, unresolvedRemoteAddress, asyncHandler);
385383
}
@@ -394,14 +392,21 @@ private <T> Future<List<InetSocketAddress>> resolveAddresses(Request request, Pr
394392
// bypass resolution
395393
InetSocketAddress inetSocketAddress = new InetSocketAddress(request.getAddress(), port);
396394
return promise.setSuccess(singletonList(inetSocketAddress));
397-
} else if (groupResolver != null) {
395+
}
396+
AddressResolver<InetSocketAddress> groupResolver = getGroupResolver();
397+
if (groupResolver != null) {
398398
return RequestHostnameResolver.INSTANCE.resolve(groupResolver, unresolvedRemoteAddress, asyncHandler);
399399
} else {
400400
return RequestHostnameResolver.INSTANCE.resolve(request.getNameResolver(), unresolvedRemoteAddress, asyncHandler);
401401
}
402402
}
403403
}
404404

405+
private @Nullable AddressResolver<InetSocketAddress> getGroupResolver() {
406+
AddressResolverGroup<InetSocketAddress> group = channelManager.getAddressResolverGroup();
407+
return group != null ? group.getResolver(channelManager.getEventLoopGroup().next()) : null;
408+
}
409+
405410
private <T> NettyResponseFuture<T> newNettyResponseFuture(Request request, AsyncHandler<T> asyncHandler, NettyRequest nettyRequest, ProxyServer proxyServer) {
406411
NettyResponseFuture<T> future = new NettyResponseFuture<>(
407412
request,

client/src/test/java/org/asynchttpclient/AddressResolverGroupTest.java

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,21 +25,37 @@
2525
import org.junit.jupiter.api.AfterEach;
2626
import org.junit.jupiter.api.BeforeEach;
2727

28+
import java.net.InetSocketAddress;
29+
import java.net.Socket;
2830
import java.util.Arrays;
2931
import java.util.concurrent.ExecutionException;
32+
import java.util.concurrent.TimeUnit;
3033

3134
import static java.util.concurrent.TimeUnit.SECONDS;
35+
import static org.asynchttpclient.Dsl.asyncHttpClient;
3236
import static org.asynchttpclient.Dsl.config;
3337
import static org.asynchttpclient.Dsl.get;
3438
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
3539
import static org.junit.jupiter.api.Assertions.assertEquals;
3640
import static org.junit.jupiter.api.Assertions.assertNotNull;
3741
import static org.junit.jupiter.api.Assertions.assertNull;
42+
import static org.junit.jupiter.api.Assertions.assertTrue;
43+
import static org.junit.jupiter.api.Assertions.fail;
44+
import static org.junit.jupiter.api.Assumptions.assumeTrue;
3845

3946
public class AddressResolverGroupTest extends HttpTest {
4047

4148
private HttpServer server;
4249

50+
private static boolean isExternalNetworkAvailable() {
51+
try (Socket socket = new Socket()) {
52+
socket.connect(new InetSocketAddress("www.google.com", 443), 3000);
53+
return true;
54+
} catch (Exception e) {
55+
return false;
56+
}
57+
}
58+
4359
@BeforeEach
4460
public void start() throws Throwable {
4561
server = new HttpServer();
@@ -117,12 +133,47 @@ public void unknownHostWithDnsResolverGroupFails() throws Throwable {
117133
withClient(config().setAddressResolverGroup(resolverGroup)).run(client -> {
118134
try {
119135
client.prepareGet("http://nonexistent.invalid/foo").execute().get(10, SECONDS);
136+
fail("Request to nonexistent host should have thrown an exception");
120137
} catch (ExecutionException e) {
121-
// DNS failures may surface as UnknownHostException or as a Netty
122-
// DnsErrorCauseException (since MiscUtils.getCause unwraps the chain).
123-
// Either way, the request must fail, never silently succeed.
124138
assertNotNull(e.getCause(), "Should have a cause for the DNS failure");
125139
}
126140
});
127141
}
142+
143+
@RepeatedIfExceptionsTest(repeats = 5)
144+
public void resolveRealDomainWithDnsResolverGroup() throws Throwable {
145+
assumeTrue(isExternalNetworkAvailable(), "External network not available - skipping test");
146+
147+
DnsAddressResolverGroup resolverGroup = new DnsAddressResolverGroup(
148+
NioDatagramChannel.class,
149+
DnsServerAddressStreamProviders.platformDefault());
150+
151+
try (AsyncHttpClient client = asyncHttpClient(config().setAddressResolverGroup(resolverGroup))) {
152+
Response response = client.prepareGet("https://www.google.com/").execute().get(20, TimeUnit.SECONDS);
153+
assertNotNull(response);
154+
assertTrue(response.getStatusCode() >= 200 && response.getStatusCode() < 400,
155+
"Expected successful HTTP status but got " + response.getStatusCode());
156+
}
157+
}
158+
159+
@RepeatedIfExceptionsTest(repeats = 5)
160+
public void resolveMultipleRealDomainsWithDnsResolverGroup() throws Throwable {
161+
assumeTrue(isExternalNetworkAvailable(), "External network not available - skipping test");
162+
163+
DnsAddressResolverGroup resolverGroup = new DnsAddressResolverGroup(
164+
NioDatagramChannel.class,
165+
DnsServerAddressStreamProviders.platformDefault());
166+
167+
try (AsyncHttpClient client = asyncHttpClient(config().setAddressResolverGroup(resolverGroup))) {
168+
Response response1 = client.prepareGet("https://www.google.com/").execute().get(20, TimeUnit.SECONDS);
169+
assertNotNull(response1);
170+
assertTrue(response1.getStatusCode() >= 200 && response1.getStatusCode() < 400,
171+
"Expected successful HTTP status for google.com but got " + response1.getStatusCode());
172+
173+
Response response2 = client.prepareGet("https://www.example.com/").execute().get(20, TimeUnit.SECONDS);
174+
assertNotNull(response2);
175+
assertTrue(response2.getStatusCode() >= 200 && response2.getStatusCode() < 400,
176+
"Expected successful HTTP status for example.com but got " + response2.getStatusCode());
177+
}
178+
}
128179
}

0 commit comments

Comments
 (0)