Repository: netty/netty
Issue: UnknownHostException mentions hostname with search domain added
Labels: defect

Netty version: 4.1.4.Final

Context:
While testing the dns resolution of vert.x I found the cosmetic issue that the exception for UnknownHost mentions the hostname with the last attempted search domain appended, which is kind of confusing. I would prefer to see the original hostname supplied to the method in the exception. The resolution function is based on the netty resolver, the exception text is created in DnsNameResolverContext.

I tried to reproduce the issue with a unit test in netty, which I didn't get quite right, however it is shown when doing the following:
1. add an unknown hostname to the list of hostnames in DnsNameResolverTest, e.g. "unknown.hostname"
2. remove the line .nameServerAddresses(DnsServerAddresses.singleton(dnsServer.localAddress())) in newResolver() to enable dns resolution with real dns
3. add at least one search domain (I used internal.example.com)
4. run the unit test testResolveA()
5. if should fail with the error

java.net.UnknownHostException: failed to resolve 'unknown.hostname.internal.example.com'

$ java -version
java version "1.8.0_71"
Java(TM) SE Runtime Environment (build 1.8.0_71-b15)
Java HotSpot(TM) Client VM (build 25.71-b15, mixed mode)

Operating system: Windows 10 Home 64 bit

Microsoft Windows [Version 10.0.10586]

(edit: I  choose a rather bad example as search domain before, I have changed the example to example.com now)


Issue comments:
To show the issue in vert.x the following unit test should show the error message when running it inside the current master of eclipse/vert.x

https://gist.github.com/alexlehm/d4d4ffdf95c97c78dff7ba91de1b0d09

Starting test: UnknownHostExceptionTest#testResolveExceptionHostname 
java.net.UnknownHostException: failed to resolve 'sdfsdfsdfsdsdffd.internal.example.com'. Exceeded max queries per resolve 3

@alexlehm makes sense... interested in proving a PR ?

https://github.com/netty/netty/pull/5608


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 08ba335a6ad..3579e0644ec 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
@@ -69,6 +69,7 @@ public void operationComplete(Future<AddressedEnvelope<DnsResponse, InetSocketAd
     private final DnsNameResolver parent;
     private final DnsServerAddressStream nameServerAddrs;
     private final String hostname;
+    protected String pristineHostname;
     private final DnsCache resolveCache;
     private final boolean traceEnabled;
     private final int maxAllowedQueries;
@@ -116,6 +117,7 @@ public void operationComplete(Future<T> future) throws Exception {
                         String nextHostname = DnsNameResolverContext.this.hostname + "." + searchDomain;
                         DnsNameResolverContext<T> nextContext = newResolverContext(parent,
                             nextHostname, resolveCache);
+                        nextContext.pristineHostname = hostname;
                         nextContext.internalResolve(nextPromise);
                         nextPromise.addListener(this);
                     } else {
@@ -449,8 +451,13 @@ private void finishResolve(Promise<T> promise) {
         final int tries = maxAllowedQueries - allowedQueries;
         final StringBuilder buf = new StringBuilder(64);
 
-        buf.append("failed to resolve '")
-           .append(hostname).append('\'');
+        buf.append("failed to resolve '");
+        if (pristineHostname != null) {
+          buf.append(pristineHostname);
+        } else {
+          buf.append(hostname);
+        }
+        buf.append('\'');
         if (tries > 1) {
             if (tries < maxAllowedQueries) {
                 buf.append(" after ")


Test patch:
diff --git a/resolver-dns/src/test/java/io/netty/resolver/dns/SearchDomainTest.java b/resolver-dns/src/test/java/io/netty/resolver/dns/SearchDomainTest.java
index da89173848c..77c652ff79f 100644
--- a/resolver-dns/src/test/java/io/netty/resolver/dns/SearchDomainTest.java
+++ b/resolver-dns/src/test/java/io/netty/resolver/dns/SearchDomainTest.java
@@ -24,6 +24,7 @@
 import org.junit.Test;
 
 import java.net.InetAddress;
+import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -34,7 +35,10 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.core.StringContains.containsString;
 
 public class SearchDomainTest {
 
@@ -265,4 +269,23 @@ private List<String> assertResolveAll(DnsNameResolver resolver, String inetHost)
         }
         return list;
     }
+
+    @Test
+    public void testExceptionMsgNoSearchDomain() throws Exception {
+        Set<String> domains = new HashSet<String>();
+
+        TestDnsServer.MapRecordStoreA store = new TestDnsServer.MapRecordStoreA(domains);
+        dnsServer = new TestDnsServer(store);
+        dnsServer.start();
+
+        resolver = newResolver().searchDomains(Collections.singletonList("foo.com")).build();
+
+        Future<InetAddress> fut = resolver.resolve("unknown.hostname");
+        assertTrue(fut.await(10, TimeUnit.SECONDS));
+        assertFalse(fut.isSuccess());
+        final Throwable cause = fut.cause();
+        assertEquals(UnknownHostException.class, cause.getClass());
+        assertThat("search domain is included in UnknownHostException", cause.getMessage(),
+            not(containsString("foo.com")));
+    }
 }