DnsNameResolver.resolveAll(...) should also contain non preferred addresses (#9044)

Motivation:

At the moment we basically drop all non prefered addresses when calling DnsNameResolver.resolveAll(...). This is just incorrect and was introduced by 4cd39cc4b3. More correct is to still retain these but sort the returned List to have the prefered addresses on the beginning of the List. This also ensures resolve(...) will return the correct return type.

Modifications:

- Introduce PreferredAddressTypeComperator which we use to sort the List so it will contain the preferred address type first.
- Add unit test to verify behaviour

Result:

Include not only preferred addresses in the List that is returned by resolveAll(...)

resolver-dns/src/main/java/io/netty/resolver/dns/DnsAddressResolveContext.java

@@ -19,7 +19,6 @@ import static io.netty.resolver.dns.DnsAddressDecoder.decodeAddress;
 
 import java.net.InetAddress;
 import java.net.UnknownHostException;
-import java.util.ArrayList;
 import java.util.List;
 
 import io.netty.channel.EventLoop;
@@ -56,27 +55,8 @@ final class DnsAddressResolveContext extends DnsResolveContext<InetAddress> {
 
     @Override
     List<InetAddress> filterResults(List<InetAddress> unfiltered) {
-        final Class<? extends InetAddress> inetAddressType = parent.preferredAddressType().addressType();
+        unfiltered.sort(PreferredAddressTypeComparator.comparator(parent.preferredAddressType()));
-        final int size = unfiltered.size();
+        return unfiltered;
-        int numExpected = 0;
-        for (int i = 0; i < size; i++) {
-            InetAddress address = unfiltered.get(i);
-            if (inetAddressType.isInstance(address)) {
-                numExpected++;
-            }
-        }
-        if (numExpected == size || numExpected == 0) {
-            // If all the results are the preferred type, or none of them are, then we don't need to do any filtering.
-            return unfiltered;
-        }
-        List<InetAddress> filtered = new ArrayList<>(numExpected);
-        for (int i = 0; i < size; i++) {
-            InetAddress address = unfiltered.get(i);
-            if (inetAddressType.isInstance(address)) {
-                filtered.add(address);
-            }
-        }
-        return filtered;
     }
 
     @Override

resolver-dns/src/main/java/io/netty/resolver/dns/PreferredAddressTypeComparator.java

@@ -0,0 +1,54 @@
+/*
+ * Copyright 2019 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.channel.socket.InternetProtocolFamily;
+
+import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.util.Comparator;
+
+final class PreferredAddressTypeComparator implements Comparator<InetAddress> {
+
+    private static final PreferredAddressTypeComparator IPv4 = new PreferredAddressTypeComparator(Inet4Address.class);
+    private static final PreferredAddressTypeComparator IPv6 = new PreferredAddressTypeComparator(Inet6Address.class);
+
+    static PreferredAddressTypeComparator comparator(InternetProtocolFamily family) {
+        switch (family) {
+            case IPv4:
+                return IPv4;
+            case IPv6:
+                return IPv6;
+            default:
+                throw new IllegalArgumentException();
+        }
+    }
+
+    private final Class<? extends InetAddress> preferredAddressType;
+
+    private PreferredAddressTypeComparator(Class<? extends InetAddress> preferredAddressType) {
+        this.preferredAddressType = preferredAddressType;
+    }
+
+    @Override
+    public int compare(InetAddress o1, InetAddress o2) {
+        if (o1.getClass() == o2.getClass()) {
+            return 0;
+        }
+        return preferredAddressType.isAssignableFrom(o1.getClass()) ? -1 : 1;
+    }
+}

resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java

@@ -1219,6 +1219,15 @@ public class DnsNameResolverTest {
             } else {
                 assertEquals(ipv6Address, resolved.getHostAddress());
             }
+            InetAddress ipv4InetAddress = InetAddress.getByAddress("netty.com",
+                    InetAddress.getByName(ipv4Address).getAddress());
+            InetAddress ipv6InetAddress = InetAddress.getByAddress("netty.com",
+                    InetAddress.getByName(ipv6Address).getAddress());
+
+            List<InetAddress> resolvedAll = resolver.resolveAll("netty.com").syncUninterruptibly().getNow();
+            List<InetAddress> expected = types == ResolvedAddressTypes.IPV4_PREFERRED ?
+                    Arrays.asList(ipv4InetAddress, ipv6InetAddress) :  Arrays.asList(ipv6InetAddress, ipv4InetAddress);
+            assertEquals(expected, resolvedAll);
         } finally {
             nonCompliantDnsServer.stop();
         }
@@ -2582,5 +2591,4 @@ public class DnsNameResolverTest {
             }
         }
     }
-
 }

resolver-dns/src/test/java/io/netty/resolver/dns/PreferredAddressTypeComparatorTest.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright 2019 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.channel.socket.InternetProtocolFamily;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+public class PreferredAddressTypeComparatorTest {
+
+    @Test
+    public void testIpv4() throws UnknownHostException {
+        InetAddress ipv4Address1 = InetAddress.getByName("10.0.0.1");
+        InetAddress ipv4Address2 = InetAddress.getByName("10.0.0.2");
+        InetAddress ipv4Address3 = InetAddress.getByName("10.0.0.3");
+        InetAddress ipv6Address1 = InetAddress.getByName("::1");
+        InetAddress ipv6Address2 = InetAddress.getByName("::2");
+        InetAddress ipv6Address3 = InetAddress.getByName("::3");
+
+        PreferredAddressTypeComparator ipv4 = PreferredAddressTypeComparator.comparator(InternetProtocolFamily.IPv4);
+
+        List<InetAddress> addressList = new ArrayList<InetAddress>();
+        Collections.addAll(addressList, ipv4Address1, ipv4Address2, ipv6Address1,
+                ipv6Address2, ipv4Address3, ipv6Address3);
+        Collections.sort(addressList, ipv4);
+
+        Assert.assertEquals(Arrays.asList(ipv4Address1, ipv4Address2, ipv4Address3, ipv6Address1,
+                ipv6Address2, ipv6Address3), addressList);
+    }
+
+    @Test
+    public void testIpv6() throws UnknownHostException {
+        InetAddress ipv4Address1 = InetAddress.getByName("10.0.0.1");
+        InetAddress ipv4Address2 = InetAddress.getByName("10.0.0.2");
+        InetAddress ipv4Address3 = InetAddress.getByName("10.0.0.3");
+        InetAddress ipv6Address1 = InetAddress.getByName("::1");
+        InetAddress ipv6Address2 = InetAddress.getByName("::2");
+        InetAddress ipv6Address3 = InetAddress.getByName("::3");
+
+        PreferredAddressTypeComparator ipv4 = PreferredAddressTypeComparator.comparator(InternetProtocolFamily.IPv6);
+
+        List<InetAddress> addressList = new ArrayList<InetAddress>();
+        Collections.addAll(addressList, ipv4Address1, ipv4Address2, ipv6Address1,
+                ipv6Address2, ipv4Address3, ipv6Address3);
+        Collections.sort(addressList, ipv4);
+
+        Assert.assertEquals(Arrays.asList(ipv6Address1,
+                ipv6Address2, ipv6Address3, ipv4Address1, ipv4Address2, ipv4Address3), addressList);
+    }
+}