Automatically decode DNS domain name to unicode, close AsyncHttpClient#1338

Motivation:

DnsNameResolver will return the domain / host name as ascii code using
punycode (https://tools.ietf.org/html/rfc3492). This is different to
what the JDK does which always convert it to unicode. We should do the
same by default but allow to also not do it.

Modifications:

- Add new builder method on DnsNameResolverBuilder which allow to
disable / enable converting. Default is to convert just like the JDK
does.
- Add unit tests for it.

Result:

DnsNameResolver and JDK impl behave the same way.

Author: slandelle

netty-bp/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java

@@ -39,13 +39,15 @@
 import io.netty.handler.codec.dns.DnsResponse;
 import io.netty.resolver.HostsFileEntriesResolver;
 import io.netty.resolver.InetNameResolver;
+import io.netty.util.NetUtil;
 import io.netty.util.NetUtil2;
 import io.netty.util.ReferenceCountUtil;
 import io.netty.util.concurrent.FastThreadLocal;
 import io.netty.util.concurrent.Future;
 import io.netty.util.concurrent.Promise;
 import io.netty.util.internal.EmptyArrays;
 import io.netty.util.internal.PlatformDependent;
+import io.netty.util.internal.StringUtil;
 import io.netty.util.internal.StringUtil2;
 import io.netty.util.internal.UnstableApi;
 import io.netty.util.internal.logging.InternalLogger;
@@ -81,19 +83,19 @@ public class DnsNameResolver extends InetNameResolver {
     static final String[] DEFAULT_SEACH_DOMAINS;
 
     static {
-        if (NetUtil2.isIpV4StackPreferred()) {
+        if (NetUtil.isIpV4StackPreferred()) {
             DEFAULT_RESOLVE_ADDRESS_TYPES = new InternetProtocolFamily2[] { InternetProtocolFamily2.IPv4 };
-            LOCALHOST_ADDRESS = NetUtil2.LOCALHOST4;
+            LOCALHOST_ADDRESS = NetUtil.LOCALHOST4;
         } else {
             DEFAULT_RESOLVE_ADDRESS_TYPES = new InternetProtocolFamily2[2];
             if (NetUtil2.isIpV6AddressesPreferred()) {
                 DEFAULT_RESOLVE_ADDRESS_TYPES[0] = InternetProtocolFamily2.IPv6;
                 DEFAULT_RESOLVE_ADDRESS_TYPES[1] = InternetProtocolFamily2.IPv4;
-                LOCALHOST_ADDRESS = NetUtil2.LOCALHOST6;
+                LOCALHOST_ADDRESS = NetUtil.LOCALHOST6;
             } else {
                 DEFAULT_RESOLVE_ADDRESS_TYPES[0] = InternetProtocolFamily2.IPv4;
                 DEFAULT_RESOLVE_ADDRESS_TYPES[1] = InternetProtocolFamily2.IPv6;
-                LOCALHOST_ADDRESS = NetUtil2.LOCALHOST4;
+                LOCALHOST_ADDRESS = NetUtil.LOCALHOST4;
             }
         }
     }
@@ -155,6 +157,7 @@ protected DnsServerAddressStream initialValue() throws Exception {
     private final boolean cnameFollowAAAARecords;
     private final InternetProtocolFamily2 preferredAddressType;
     private final DnsRecordType[] resolveRecordTypes;
+    private final boolean decodeIdn;
 
     /**
      * Creates a new DNS-based name resolver that communicates with the specified list of DNS servers.
@@ -175,7 +178,11 @@ protected DnsServerAddressStream initialValue() throws Exception {
      * @param hostsFileEntriesResolver the {@link HostsFileEntriesResolver} used to check for local aliases
      * @param searchDomains the list of search domain
      * @param ndots the ndots value
+     * @deprecated use {@link #DnsNameResolver(EventLoop, ChannelFactory, DnsServerAddresses, DnsCache, long,
+     *             InternetProtocolFamily2[], boolean, int, boolean, int,
+     *             boolean, HostsFileEntriesResolver, String[], int, boolean)}
      */
+    @Deprecated
     public DnsNameResolver(
             EventLoop eventLoop,
             ChannelFactory<? extends DatagramChannel> channelFactory,
@@ -191,6 +198,49 @@ public DnsNameResolver(
             HostsFileEntriesResolver hostsFileEntriesResolver,
             String[] searchDomains,
             int ndots) {
+        this(eventLoop, channelFactory, nameServerAddresses, resolveCache, queryTimeoutMillis, resolvedAddressTypes,
+                recursionDesired, maxQueriesPerResolve, traceEnabled, maxPayloadSize, optResourceEnabled,
+                hostsFileEntriesResolver, searchDomains, ndots, true);
+    }
+
+    /**
+     * Creates a new DNS-based name resolver that communicates with the specified list of DNS servers.
+     *
+     * @param eventLoop the {@link EventLoop} which will perform the communication with the DNS servers
+     * @param channelFactory the {@link ChannelFactory} that will create a {@link DatagramChannel}
+     * @param nameServerAddresses the addresses of the DNS server. For each DNS query, a new stream is created from
+     *                            this to determine which DNS server should be contacted for the next retry in case
+     *                            of failure.
+     * @param resolveCache the DNS resolved entries cache
+     * @param queryTimeoutMillis timeout of each DNS query in millis
+     * @param resolvedAddressTypes list of the protocol families
+     * @param recursionDesired if recursion desired flag must be set
+     * @param maxQueriesPerResolve the maximum allowed number of DNS queries for a given name resolution
+     * @param traceEnabled if trace is enabled
+     * @param maxPayloadSize the capacity of the datagram packet buffer
+     * @param optResourceEnabled if automatic inclusion of a optional records is enabled
+     * @param hostsFileEntriesResolver the {@link HostsFileEntriesResolver} used to check for local aliases
+     * @param searchDomains the list of search domain
+     * @param ndots the ndots value
+     * @param decodeIdn {@code true} if domain / host names should be decoded to unicode when received.
+     *                        See <a href="https://tools.ietf.org/html/rfc3492">rfc3492</a>.
+     */
+    public DnsNameResolver(
+            EventLoop eventLoop,
+            ChannelFactory<? extends DatagramChannel> channelFactory,
+            DnsServerAddresses nameServerAddresses,
+            final DnsCache resolveCache,
+            long queryTimeoutMillis,
+            InternetProtocolFamily2[] resolvedAddressTypes,
+            boolean recursionDesired,
+            int maxQueriesPerResolve,
+            boolean traceEnabled,
+            int maxPayloadSize,
+            boolean optResourceEnabled,
+            HostsFileEntriesResolver hostsFileEntriesResolver,
+            String[] searchDomains,
+            int ndots,
+            boolean decodeIdn) {
 
         super(eventLoop);
         checkNotNull(channelFactory, "channelFactory");
@@ -206,6 +256,7 @@ public DnsNameResolver(
         this.resolveCache = checkNotNull(resolveCache, "resolveCache");
         this.searchDomains = checkNotNull(searchDomains, "searchDomains").clone();
         this.ndots = checkPositiveOrZero(ndots, "ndots");
+        this.decodeIdn = decodeIdn;
 
         boolean cnameFollowARecords = false;
         boolean cnameFollowAAAARecords = false;
@@ -309,6 +360,10 @@ final DnsRecordType[] resolveRecordTypes() {
         return resolveRecordTypes;
     }
 
+    final boolean isDecodeIdn() {
+        return decodeIdn;
+    }
+
     /**
      * Returns {@code true} if and only if this resolver sends a DNS query with the RD (recursion desired) flag set.
      * The default value is {@code true}.
@@ -503,7 +558,7 @@ private static void validateAdditional(DnsRecord record, boolean validateType) {
     @Override
     protected final InetAddress loopbackAddress() {
         return preferredAddressType() == InternetProtocolFamily2.IPv4 ?
-                    NetUtil2.LOCALHOST4 : NetUtil2.LOCALHOST6;
+                    NetUtil.LOCALHOST4 : NetUtil.LOCALHOST6;
     }
 
     /**
@@ -514,7 +569,7 @@ protected void doResolve(String inetHost,
                              DnsRecord[] additionals,
                              Promise<InetAddress> promise,
                              DnsCache resolveCache) throws Exception {
-        final byte[] bytes = NetUtil2.createByteArrayFromIpAddressString(inetHost);
+        final byte[] bytes = NetUtil.createByteArrayFromIpAddressString(inetHost);
         if (bytes != null) {
             // The inetHost is actually an ipaddress.
             promise.setSuccess(InetAddress.getByAddress(bytes));
@@ -639,7 +694,7 @@ protected void doResolveAll(String inetHost,
                                 DnsRecord[] additionals,
                                 Promise<List<InetAddress>> promise,
                                 DnsCache resolveCache) throws Exception {
-        final byte[] bytes = NetUtil2.createByteArrayFromIpAddressString(inetHost);
+        final byte[] bytes = NetUtil.createByteArrayFromIpAddressString(inetHost);
         if (bytes != null) {
             // The unresolvedAddress was created via a String that contains an ipaddress.
             promise.setSuccess(Collections.singletonList(InetAddress.getByAddress(bytes)));

netty-bp/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverBuilder.java

@@ -15,23 +15,25 @@
  */
 package io.netty.resolver.dns;
 
-import static io.netty.util.internal.ObjectUtil2.intValue;
+import static io.netty.util.internal.ObjectUtil.intValue;
 
 import io.netty.bootstrap.ChannelFactory;
 import io.netty.channel.EventLoop;
 import io.netty.channel.ReflectiveChannelFactory;
 import io.netty.channel.socket.DatagramChannel;
 import io.netty.channel.socket.InternetProtocolFamily2;
 import io.netty.resolver.HostsFileEntriesResolver;
-import io.netty.util.internal.InternalThreadLocalMap;
+import io.netty.util.internal.UnstableApi;
 
+import java.util.ArrayList;
 import java.util.List;
 
 import static io.netty.util.internal.ObjectUtil.checkNotNull;
 
 /**
  * A {@link DnsNameResolver} builder.
  */
+@UnstableApi
 public final class DnsNameResolverBuilder {
 
     private final EventLoop eventLoop;
@@ -51,6 +53,7 @@ public final class DnsNameResolverBuilder {
     private HostsFileEntriesResolver hostsFileEntriesResolver = HostsFileEntriesResolver.DEFAULT;
     private String[] searchDomains = DnsNameResolver.DEFAULT_SEACH_DOMAINS;
     private int ndots = 1;
+    private boolean decodeIdn = true;
 
     /**
      * Creates a new builder.
@@ -158,8 +161,8 @@ public DnsNameResolverBuilder queryTimeoutMillis(long queryTimeoutMillis) {
     public DnsNameResolverBuilder resolvedAddressTypes(InternetProtocolFamily2... resolvedAddressTypes) {
         checkNotNull(resolvedAddressTypes, "resolvedAddressTypes");
 
-        final List<InternetProtocolFamily2> list =
-                InternalThreadLocalMap.get().arrayList(InternetProtocolFamily2.values().length);
+        final List<InternetProtocolFamily2> list = new ArrayList<InternetProtocolFamily2>(
+                InternetProtocolFamily2.values().length);
 
         for (InternetProtocolFamily2 f : resolvedAddressTypes) {
             if (f == null) {
@@ -195,8 +198,8 @@ public DnsNameResolverBuilder resolvedAddressTypes(InternetProtocolFamily2... re
     public DnsNameResolverBuilder resolvedAddressTypes(Iterable<InternetProtocolFamily2> resolvedAddressTypes) {
         checkNotNull(resolvedAddressTypes, "resolveAddressTypes");
 
-        final List<InternetProtocolFamily2> list =
-                InternalThreadLocalMap.get().arrayList(InternetProtocolFamily2.values().length);
+        final List<InternetProtocolFamily2> list = new ArrayList<InternetProtocolFamily2>(
+                InternetProtocolFamily2.values().length);
 
         for (InternetProtocolFamily2 f : resolvedAddressTypes) {
             if (f == null) {
@@ -297,8 +300,7 @@ public DnsNameResolverBuilder hostsFileEntriesResolver(HostsFileEntriesResolver
     public DnsNameResolverBuilder searchDomains(Iterable<String> searchDomains) {
         checkNotNull(searchDomains, "searchDomains");
 
-        final List<String> list =
-            InternalThreadLocalMap.get().arrayList(4);
+        final List<String> list = new ArrayList<String>(4);
 
         for (String f : searchDomains) {
             if (f == null) {
@@ -329,6 +331,18 @@ public DnsNameResolverBuilder ndots(int ndots) {
         return this;
     }
 
+    /**
+     * Set if domain / host names should be decoded to unicode when received.
+     * See <a href="https://tools.ietf.org/html/rfc3492">rfc3492</a>.
+     *
+     * @param decodeIdn if should get decoded
+     * @return {@code this}
+     */
+    public DnsNameResolverBuilder decodeIdn(boolean decodeIdn) {
+        this.decodeIdn = decodeIdn;
+        return this;
+    }
+
     /**
      * Returns a new {@link DnsNameResolver} instance.
      *
@@ -357,6 +371,7 @@ public DnsNameResolver build() {
                 optResourceEnabled,
                 hostsFileEntriesResolver,
                 searchDomains,
-                ndots);
+                ndots,
+                decodeIdn);
     }
 }

netty-bp/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverContext.java

@@ -37,6 +37,7 @@
 import io.netty.util.internal.StringUtil;
 import io.netty.util.internal.StringUtil2;
 
+import java.net.IDN;
 import java.net.Inet4Address;
 import java.net.Inet6Address;
 import java.net.InetAddress;
@@ -273,7 +274,8 @@ private void onResponseAorAAAA(
 
             final InetAddress resolved;
             try {
-                resolved = InetAddress.getByAddress(hostname, addrBytes);
+                resolved = InetAddress.getByAddress(
+                        parent.isDecodeIdn() ? IDN.toUnicode(hostname) : hostname, addrBytes);
             } catch (UnknownHostException e) {
                 // Should never reach here.
                 throw new Error(e);