Repository: netty/netty
Issue: DnsNameResolverContext infinite loop
Labels: defect

Got stuck there : 
https://github.com/netty/netty/blob/netty-4.1.0.Beta8/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverContext.java#L280

I think I did receive a response to another query but with the same id.

```
"dns-3-1" #12 prio=5 os_prio=0 tid=0x00007f87c442d800 nid=0x1401 runnable [0x00007f87a9ca0000]
   java.lang.Thread.State: RUNNABLE
        at io.netty.resolver.dns.DnsNameResolverContext.onResponseCNAME(DnsNameResolverContext.java:288)
        at io.netty.resolver.dns.DnsNameResolverContext.onResponseAorAAAA(DnsNameResolverContext.java:264)
        at io.netty.resolver.dns.DnsNameResolverContext.onResponse(DnsNameResolverContext.java:164)
        at io.netty.resolver.dns.DnsNameResolverContext$2.operationComplete(DnsNameResolverContext.java:142)
        at io.netty.util.concurrent.DefaultPromise.notifyListener0(DefaultPromise.java:680)
        at io.netty.util.concurrent.DefaultPromise.notifyListeners(DefaultPromise.java:567)
        at io.netty.util.concurrent.DefaultPromise.setSuccess(DefaultPromise.java:397)
        at io.netty.resolver.dns.DnsQueryContext.setSuccess(DnsQueryContext.java:181)
        at io.netty.resolver.dns.DnsQueryContext.finish(DnsQueryContext.java:164)
        at io.netty.resolver.dns.DnsNameResolver$DnsResponseHandler.channelRead(DnsNameResolver.java:944)
        at io.netty.channel.ChannelHandlerInvokerUtil.invokeChannelReadNow(ChannelHandlerInvokerUtil.java:83)
        at io.netty.channel.DefaultChannelHandlerInvoker.invokeChannelRead(DefaultChannelHandlerInvoker.java:163)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:155)
        at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:103)
        at io.netty.channel.ChannelHandlerInvokerUtil.invokeChannelReadNow(ChannelHandlerInvokerUtil.java:83)
        at io.netty.channel.DefaultChannelHandlerInvoker.invokeChannelRead(DefaultChannelHandlerInvoker.java:163)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:155)
        at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:950)
        at io.netty.channel.nio.AbstractNioMessageChannel$NioMessageUnsafe.read(AbstractNioMessageChannel.java:94)
        at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:510)
        at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:467)
        at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:381)
        at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:353)
        at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:742)
        at io.netty.util.concurrent.DefaultThreadFactory$DefaultRunnableDecorator.run(DefaultThreadFactory.java:137)
        at java.lang.Thread.run(Thread.java:745)
```


Issue comments:
@trustin PTAL ....

I can help if @serphacker shares a reproducer.

@serphacker ping

Should be fixed by #4946 


Pull request review comments:

Patch that resolved the issue:
diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverContext.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverContext.java
index 15add823202..8d42d041bf2 100644
--- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverContext.java
+++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverContext.java
@@ -283,8 +283,10 @@ private void onResponseCNAME(
         final String name = question.name().toLowerCase(Locale.US);
         String resolved = name;
         boolean found = false;
-        for (;;) {
-            String next = cnames.get(resolved);
+        while (!cnames.isEmpty()) { // Do not attempt to call Map.remove() when the Map is empty
+                                    // because it can be Collections.emptyMap()
+                                    // whose remove() throws a UnsupportedOperationException.
+            final String next = cnames.remove(resolved);
             if (next != null) {
                 found = true;
                 resolved = next;


Test patch:
diff --git a/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java b/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java
index 9218a0ce688..aef6fa76090 100644
--- a/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java
+++ b/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java
@@ -265,7 +265,7 @@ public class DnsNameResolverTest {
     private static final TestDnsServer dnsServer = new TestDnsServer();
     private static final EventLoopGroup group = new NioEventLoopGroup(1);
 
-    private DnsNameResolverBuilder newResolver() {
+    private static DnsNameResolverBuilder newResolver() {
         return new DnsNameResolverBuilder(group.next())
                 .channelType(NioDatagramChannel.class)
                 .nameServerAddresses(DnsServerAddresses.singleton(dnsServer.localAddress()))
@@ -273,7 +273,7 @@ private DnsNameResolverBuilder newResolver() {
                 .optResourceEnabled(false);
     }
 
-    private DnsNameResolverBuilder newResolver(InternetProtocolFamily... resolvedAddressTypes) {
+    private static DnsNameResolverBuilder newResolver(InternetProtocolFamily... resolvedAddressTypes) {
         return newResolver()
                 .resolvedAddressTypes(resolvedAddressTypes);
     }
@@ -349,7 +349,7 @@ public void testResolveAAAA() throws Exception {
         }
     }
 
-    private Map<String, InetAddress> testResolve0(DnsNameResolver resolver, Set<String> excludedDomains)
+    private static Map<String, InetAddress> testResolve0(DnsNameResolver resolver, Set<String> excludedDomains)
             throws InterruptedException {
 
         assertThat(resolver.isRecursionDesired(), is(true));
@@ -482,7 +482,7 @@ public void run() {
         }
     }
 
-    private UnknownHostException resolveNonExistentDomain(DnsNameResolver resolver) {
+    private static UnknownHostException resolveNonExistentDomain(DnsNameResolver resolver) {
         try {
             resolver.resolve("non-existent.netty.io").sync();
             fail();
@@ -505,8 +505,7 @@ public void testResolveIp() {
         }
     }
 
-    private void resolve(DnsNameResolver resolver, Map<String, Future<InetAddress>> futures, String hostname) {
-
+    private static void resolve(DnsNameResolver resolver, Map<String, Future<InetAddress>> futures, String hostname) {
         futures.put(hostname, resolver.resolve(hostname));
     }
 
@@ -580,7 +579,7 @@ public void encode(IoSession session, Object message, ProtocolEncoderOutput out)
                             // This is a hack to allow to also test for AAAA resolution as DnsMessageEncoder
                             // does not support it and it is hard to extend, because the interesting methods
                             // are private...
-                            // In case of RecordType.AAAA we need to encode the RecordType by ourself.
+                            // In case of RecordType.AAAA we need to encode the RecordType by ourselves.
                             if (record.getRecordType() == RecordType.AAAA) {
                                 try {
                                     recordEncoder.put(buf, record);
@@ -639,10 +638,10 @@ private static String nextDomain() {
             }
 
             private static String nextIp() {
-                return ippart() + "." + ippart() + '.' + ippart() + '.' + ippart();
+                return ipPart() + "." + ipPart() + '.' + ipPart() + '.' + ipPart();
             }
 
-            private static int ippart() {
+            private static int ipPart() {
                 return NUMBERS[index(NUMBERS.length)];
             }
 
@@ -672,10 +671,10 @@ public Set<ResourceRecord> getRecords(QuestionRecord questionRecord) {
                         } while (ThreadLocalRandom.current().nextBoolean());
                         break;
                     case MX:
-                        int prioritity = 0;
+                        int priority = 0;
                         do {
                             rm.put(DnsAttribute.DOMAIN_NAME, nextDomain());
-                            rm.put(DnsAttribute.MX_PREFERENCE, String.valueOf(++prioritity));
+                            rm.put(DnsAttribute.MX_PREFERENCE, String.valueOf(++priority));
                         } while (ThreadLocalRandom.current().nextBoolean());
                         break;
                     default: