Repository: netty/netty
Issue: Incorrect UnixResolverDnsServerAddressStreamProvider
Labels: defect

### Minimal yet complete reproducer code (or URL to code)

```java
import static org.junit.Assert.assertNotNull;

import java.io.File;
import java.io.FileOutputStream;
import java.io.OutputStream;
import java.nio.charset.StandardCharsets;

import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;

import io.netty.resolver.dns.UnixResolverDnsServerAddressStreamProvider;

public class ResolverTest {

    @Rule
    public final TemporaryFolder folder = new TemporaryFolder();

    @Test
    public void test() throws Exception {
        File f = folder.newFile();
        try (OutputStream out = new FileOutputStream(f)) {
            out.write(("domain linecorp.local\n" +
                       "nameserver 1.2.3.4\n" +
                       "nameserver 5.6.7.8\n").getBytes(StandardCharsets.UTF_8));
        }
        UnixResolverDnsServerAddressStreamProvider p =
                new UnixResolverDnsServerAddressStreamProvider(f, null);

        assertNotNull(p.nameServerAddressStream("somehost"));
    }
}
```

### Expected behavior

`UnixResolverDnsServerAddressStreamProvider.nameServerAddressStream()` should not return `null`.

The expected behavior of `DnsNameResolver` is to try `somehost.` and then `somehost.linecorp.local.`

### Actual behavior

Attempting to resolve a host will trigger a NPE:

```
java.lang.NullPointerException: nameServerAddrs
 at io.netty.util.internal.ObjectUtil.checkNotNull(ObjectUtil.java:31)
 at io.netty.resolver.dns.DnsNameResolverContext.<init>(DnsNameResolverContext.java:120)
 at io.netty.resolver.dns.DnsNameResolver$SingleResolverContext.<init>(DnsNameResolver.java:659)
 at io.netty.resolver.dns.DnsNameResolver.doResolveUncached(DnsNameResolver.java:653)
 at io.netty.resolver.dns.DnsNameResolver.doResolve(DnsNameResolver.java:590)
 at io.netty.resolver.dns.DnsNameResolver.doResolve(DnsNameResolver.java:523)
 at io.netty.resolver.SimpleNameResolver.resolve(SimpleNameResolver.java:63)
 at io.netty.resolver.dns.InflightNameResolver.resolve(InflightNameResolver.java:100)
 at io.netty.resolver.dns.InflightNameResolver.resolve(InflightNameResolver.java:66)
 at io.netty.resolver.dns.InflightNameResolver.resolve(InflightNameResolver.java:51)
 at io.netty.resolver.InetSocketAddressResolver.doResolve(InetSocketAddressResolver.java:57)
 at io.netty.resolver.InetSocketAddressResolver.doResolve(InetSocketAddressResolver.java:32)
 at io.netty.resolver.AbstractAddressResolver.resolve(AbstractAddressResolver.java:108)
 at io.netty.bootstrap.Bootstrap.doResolveAndConnect0(Bootstrap.java:208)
 at io.netty.bootstrap.Bootstrap.doResolveAndConnect(Bootstrap.java:170)
 at io.netty.bootstrap.Bootstrap.connect(Bootstrap.java:145)
```
### Netty version

4.1.11

### Workaround

Use `DefaultDnsServerAddressStreamProvider.INSTANCE` instead of `DnsServerAddressStreamProviders.platformDefault()`.

Issue comments:
@trustin can you submit a pr with a fix ?
Can't promise it at the moment because of busyness... 😢 
> The expected behavior of DnsNameResolver is to try somehost and then somehost.linecorp.local.

The behavior of modifying the hostname should be provided at a higher layer (e.g. [DnsNameResolverContext.java](https://github.com/netty/netty/blob/4.1/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverContext.java#L141)). However `UnixResolverDnsServerAddressStreamProvider` default value when no match was found needs some work.
See PR https://github.com/netty/netty/pull/6748

Pull request review comments:
this seems to be not related to the PR. can you revert ?
Yes this is not directly related to this change but it makes unit testing more difficult. If I pull this out I would also pull the unit tests out (and submit them together) ... is that preferred from your approach?

https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man5/resolver.5.html
```
If there are multiple servers,
     the resolver library queries them in the order listed.  The algorithm used is to try a name server, and
     if the query times out, try the next, until out of name servers, then repeat trying all the name
     servers until a maximum number of retries are made.
```
its fine then... Just was not sure why you did this.
I will update the commit message

Patch that resolved the issue:
diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/UnixResolverDnsServerAddressStreamProvider.java b/resolver-dns/src/main/java/io/netty/resolver/dns/UnixResolverDnsServerAddressStreamProvider.java
index d12c63749bc..c22f3ae6c4d 100644
--- a/resolver-dns/src/main/java/io/netty/resolver/dns/UnixResolverDnsServerAddressStreamProvider.java
+++ b/resolver-dns/src/main/java/io/netty/resolver/dns/UnixResolverDnsServerAddressStreamProvider.java
@@ -27,11 +27,13 @@
 import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
 import static io.netty.resolver.dns.DefaultDnsServerAddressStreamProvider.DNS_PORT;
+import static io.netty.util.internal.ObjectUtil.checkNotNull;
 import static io.netty.util.internal.StringUtil.indexOfNonWhiteSpace;
 
 /**
@@ -54,7 +56,7 @@ public final class UnixResolverDnsServerAddressStreamProvider implements DnsServ
      * Attempt to parse {@code /etc/resolv.conf} and files in the {@code /etc/resolver} directory by default.
      * A failure to parse will return {@link DefaultDnsServerAddressStreamProvider}.
      */
-    public static DnsServerAddressStreamProvider parseSilently() {
+    static DnsServerAddressStreamProvider parseSilently() {
         try {
             UnixResolverDnsServerAddressStreamProvider nameServerCache =
                     new UnixResolverDnsServerAddressStreamProvider("/etc/resolv.conf", "/etc/resolver");
@@ -80,21 +82,25 @@ public static DnsServerAddressStreamProvider parseSilently() {
      * @throws IOException If an error occurs while parsing the input files.
      */
     public UnixResolverDnsServerAddressStreamProvider(File etcResolvConf, File... etcResolverFiles) throws IOException {
-        if (etcResolvConf == null && (etcResolverFiles == null || etcResolverFiles.length == 0)) {
-            throw new IllegalArgumentException("no files to parse");
+        if (etcResolverFiles != null && etcResolverFiles.length == 0) {
+            throw new IllegalArgumentException("etcResolverFiles must either be null or non-empty");
         }
-        if (etcResolverFiles != null) {
-            domainToNameServerStreamMap = parse(etcResolverFiles);
-            if (etcResolvConf != null) {
-                Map<String, DnsServerAddresses> etcResolvConfMap = parse(etcResolvConf);
-                defaultNameServerAddresses = etcResolvConfMap.remove(etcResolvConf.getName());
-                domainToNameServerStreamMap.putAll(etcResolvConfMap);
-            } else {
-                defaultNameServerAddresses = null;
+        Map<String, DnsServerAddresses> etcResolvConfMap = parse(checkNotNull(etcResolvConf, "etcResolvConf"));
+        domainToNameServerStreamMap = etcResolverFiles != null ? parse(etcResolverFiles) : etcResolvConfMap;
+
+        DnsServerAddresses defaultNameServerAddresses = etcResolvConfMap.get(etcResolvConf.getName());
+        if (defaultNameServerAddresses == null) {
+            Collection<DnsServerAddresses> values = etcResolvConfMap.values();
+            if (values.isEmpty()) {
+                throw new IllegalArgumentException(etcResolvConf + " didn't provide any name servers");
             }
+            this.defaultNameServerAddresses = values.iterator().next();
         } else {
-            domainToNameServerStreamMap = parse(etcResolvConf);
-            defaultNameServerAddresses = domainToNameServerStreamMap.remove(etcResolvConf.getName());
+            this.defaultNameServerAddresses = defaultNameServerAddresses;
+        }
+
+        if (etcResolverFiles != null) {
+            domainToNameServerStreamMap.putAll(etcResolvConfMap);
         }
     }
 
@@ -113,7 +119,7 @@ public UnixResolverDnsServerAddressStreamProvider(File etcResolvConf, File... et
      */
     public UnixResolverDnsServerAddressStreamProvider(String etcResolvConf, String etcResolverDir) throws IOException {
         this(etcResolvConf == null ? null : new File(etcResolvConf),
-             etcResolverDir == null ? null :new File(etcResolverDir).listFiles());
+             etcResolverDir == null ? null : new File(etcResolverDir).listFiles());
     }
 
     @Override
@@ -121,7 +127,7 @@ public DnsServerAddressStream nameServerAddressStream(String hostname) {
         for (;;) {
             int i = hostname.indexOf('.', 1);
             if (i < 0 || i == hostname.length() - 1) {
-                return defaultNameServerAddresses != null ? defaultNameServerAddresses.stream() : null;
+                return defaultNameServerAddresses.stream();
             }
 
             DnsServerAddresses addresses = domainToNameServerStreamMap.get(hostname);
@@ -133,9 +139,8 @@ public DnsServerAddressStream nameServerAddressStream(String hostname) {
         }
     }
 
-    boolean mayOverrideNameServers() {
-        return !domainToNameServerStreamMap.isEmpty() ||
-                defaultNameServerAddresses != null && defaultNameServerAddresses.stream().next() != null;
+    private boolean mayOverrideNameServers() {
+        return !domainToNameServerStreamMap.isEmpty() || defaultNameServerAddresses.stream().next() != null;
     }
 
     private static Map<String, DnsServerAddresses> parse(File... etcResolverFiles) throws IOException {
@@ -176,7 +181,7 @@ private static Map<String, DnsServerAddresses> parse(File... etcResolverFiles) t
                             port = Integer.parseInt(maybeIP.substring(i + 1));
                             maybeIP = maybeIP.substring(0, i);
                         }
-                        addresses.add(new InetSocketAddress(SocketUtils.addressByName(maybeIP), port));
+                        addresses.add(SocketUtils.socketAddress(maybeIP, port));
                     } else if (line.startsWith(DOMAIN_ROW_LABEL)) {
                         int i = indexOfNonWhiteSpace(line, DOMAIN_ROW_LABEL.length());
                         if (i < 0) {
@@ -184,7 +189,7 @@ private static Map<String, DnsServerAddresses> parse(File... etcResolverFiles) t
                                     " in file " + etcResolverFile + " value: " + line);
                         }
                         domainName = line.substring(i);
-                        if (addresses != null && !addresses.isEmpty()) {
+                        if (!addresses.isEmpty()) {
                             putIfAbsent(domainToNameServerStreamMap, domainName, addresses);
                         }
                         addresses = new ArrayList<InetSocketAddress>(2);
@@ -199,7 +204,7 @@ private static Map<String, DnsServerAddresses> parse(File... etcResolverFiles) t
                         logger.info("row type {} not supported. ignoring line: {}", SORTLIST_ROW_LABEL, line);
                     }
                 }
-                if (addresses != null && !addresses.isEmpty()) {
+                if (!addresses.isEmpty()) {
                     putIfAbsent(domainToNameServerStreamMap, domainName, addresses);
                 }
             } finally {
@@ -217,7 +222,7 @@ private static void putIfAbsent(Map<String, DnsServerAddresses> domainToNameServ
                                     String domainName,
                                     List<InetSocketAddress> addresses) {
         // TODO(scott): sortlist is being ignored.
-        putIfAbsent(domainToNameServerStreamMap, domainName, DnsServerAddresses.shuffled(addresses));
+        putIfAbsent(domainToNameServerStreamMap, domainName, DnsServerAddresses.sequential(addresses));
     }
 
     private static void putIfAbsent(Map<String, DnsServerAddresses> domainToNameServerStreamMap,


Test patch:
diff --git a/resolver-dns/src/test/java/io/netty/resolver/dns/UnixResolverDnsServerAddressStreamProviderTest.java b/resolver-dns/src/test/java/io/netty/resolver/dns/UnixResolverDnsServerAddressStreamProviderTest.java
new file mode 100644
index 00000000000..bcb61e78d14
--- /dev/null
+++ b/resolver-dns/src/test/java/io/netty/resolver/dns/UnixResolverDnsServerAddressStreamProviderTest.java
@@ -0,0 +1,94 @@
+/*
+ * Copyright 2017 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License 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 io.netty.resolver.dns;
+
+import io.netty.util.CharsetUtil;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.net.InetSocketAddress;
+
+import static org.junit.Assert.assertEquals;
+
+public class UnixResolverDnsServerAddressStreamProviderTest {
+    @Rule
+    public final TemporaryFolder folder = new TemporaryFolder();
+
+    @Test
+    public void defaultLookupShouldReturnResultsIfOnlySingleFileSpecified() throws Exception {
+        File f = buildFile("domain linecorp.local\n" +
+                           "nameserver 127.0.0.2\n" +
+                           "nameserver 127.0.0.3\n");
+        UnixResolverDnsServerAddressStreamProvider p =
+                new UnixResolverDnsServerAddressStreamProvider(f, null);
+
+        DnsServerAddressStream stream = p.nameServerAddressStream("somehost");
+        assertHostNameEquals("127.0.0.2", stream.next());
+        assertHostNameEquals("127.0.0.3", stream.next());
+    }
+
+    @Test
+    public void defaultReturnedWhenNoBetterMatch() throws Exception {
+        File f = buildFile("domain linecorp.local\n" +
+                           "nameserver 127.0.0.2\n" +
+                           "nameserver 127.0.0.3\n");
+        File f2 = buildFile("domain squarecorp.local\n" +
+                            "nameserver 127.0.0.4\n" +
+                            "nameserver 127.0.0.5\n");
+        UnixResolverDnsServerAddressStreamProvider p =
+                new UnixResolverDnsServerAddressStreamProvider(f, f2);
+
+        DnsServerAddressStream stream = p.nameServerAddressStream("somehost");
+        assertHostNameEquals("127.0.0.2", stream.next());
+        assertHostNameEquals("127.0.0.3", stream.next());
+    }
+
+    @Test
+    public void moreRefinedSelectionReturnedWhenMatch() throws Exception {
+        File f = buildFile("domain linecorp.local\n" +
+                           "nameserver 127.0.0.2\n" +
+                           "nameserver 127.0.0.3\n");
+        File f2 = buildFile("domain dc1.linecorp.local\n" +
+                            "nameserver 127.0.0.4\n" +
+                            "nameserver 127.0.0.5\n");
+        UnixResolverDnsServerAddressStreamProvider p =
+                new UnixResolverDnsServerAddressStreamProvider(f, f2);
+
+        DnsServerAddressStream stream = p.nameServerAddressStream("myhost.dc1.linecorp.local");
+        assertHostNameEquals("127.0.0.4", stream.next());
+        assertHostNameEquals("127.0.0.5", stream.next());
+    }
+
+    private File buildFile(String contents) throws IOException {
+        File f = folder.newFile();
+        OutputStream out = new FileOutputStream(f);
+        try {
+            out.write(contents.getBytes(CharsetUtil.UTF_8));
+        } finally {
+            out.close();
+        }
+        return f;
+    }
+
+    private static void assertHostNameEquals(String expectedHostname, InetSocketAddress next) {
+        assertEquals("unexpected hostname: " + next, expectedHostname, next.getHostName());
+    }
+}