Rfc6724 resolver ordering semantics #778

arturobernalg · 2026-01-09T17:54:45Z

Introduce Rfc6724AddressSelectingDnsResolver and unit tests.
Define INTERLEAVE as “no bias” (preserve RFC6724 order); keep ONLY/PREFER behavior unchanged.

Define INTERLEAVE as no-bias and align tests and debug output.

rschmitt

Main thing here is that I'd like to see a proper implementation of INTERLEAVE, more unit test coverage, and an example program we can use for manual testing.

...client5/src/test/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolverTest.java

rschmitt · 2026-01-12T17:52:46Z

httpclient5/src/main/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolver.java

+        final InetAddress DA = a.dst;
+        final InetAddress DB = b.dst;
+        final InetAddress SourceDA = a.src;
+        final InetAddress SourceDB = b.src;


These should be named something like aDst, bDst, aSrc, and aDst respectively, so it doesn't look like you're calling static methods later on

httpclient5/src/main/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolver.java

rschmitt · 2026-01-12T18:20:03Z

httpclient5/src/main/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolver.java

+        if (ip.isLinkLocalAddress()) {
+            return Scope.LINK_LOCAL;
+        }
+        if (ip.isMulticastAddress()) {


We're writing this for use in an HTTP client, not as a general-purpose resolver (e.g. JEP 418), and I'm pretty sure TCP connections can only be established to unicast addresses. Should we just filter out multicast addresses?

This should be dead code now, right? Don't these get filtered out now before this method is called?

httpclient5/src/main/java/org/apache/hc/client5/http/config/ProtocolFamilyPreference.java

rschmitt · 2026-01-12T18:30:15Z

httpclient5/src/main/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolver.java

+            InetAddress src = null;
+            try (final DatagramSocket s = new DatagramSocket()) {
+                s.connect(dest); // does not send packets; OS picks source addr/if
+                src = s.getLocalAddress();


You probably want to inject a thing here that you can mock for unit testing purposes, e.g. a Function<InetSocketAddress, InetAddress> (or an equivalent that throws IOException).

rschmitt · 2026-01-12T18:31:50Z

...client5/src/test/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolverTest.java

+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+class Rfc6724AddressSelectingDnsResolverTest {


I think we need more unit test coverage here. I'd also like to see an integration test or a runnable example (with debug/trace logging enabled) that resolves a given DNS name. That would afford some non-trivial manual testing using loopback addresses, /etc/hosts, Docker networking, etc.

Do we need any additional test coverage for the prefix matching stuff? That seems like quite a bit of non-trivial code

rschmitt · 2026-01-12T18:36:18Z

httpclient5/src/main/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolver.java

+                final boolean preferV6 = pref == ProtocolFamilyPreference.PREFER_IPV6;
+                final List<InetAddress> first = new ArrayList<>();
+                final List<InetAddress> second = new ArrayList<>();
+                for (final InetAddress a : rfcSorted) {
+                    final boolean isV6 = a instanceof Inet6Address;
+                    if (preferV6 && isV6 || !preferV6 && !isV6) {
+                        first.add(a);
+                    } else {
+                        second.add(a);
+                    }
+                }
+                final List<InetAddress> merged = new ArrayList<>(rfcSorted.size());
+                merged.addAll(first);
+                merged.addAll(second);


Since we're already relying on the JDK to provide a stable sort, you can write this as:

List<InetAddress> merged = rfcSorted.stream() .sorted(Comparator.comparing( a -> ((a instanceof Inet6Address) != preferV6) )) .toList();

Add manual gated IT to dump DEFAULT vs INTERLEAVE results. Expand unit coverage for scope mapping and core RFC comparison rules.

arturobernalg · 2026-01-13T12:03:11Z

@rschmitt Please do another pass. I think i solve all your remarks

rschmitt · 2026-01-14T17:37:03Z

httpclient5-testing/src/test/java/org/apache/hc/client5/testing/ManualRfc6724ResolverIT.java

+        assumeTrue(Boolean.getBoolean(PROP_ENABLE),
+                "Enable with -Dhttpclient.rfc6724.it=true");


I'd rather control this with JUnit 5 tags somehow. The advantage of what you've done here is that we don't have to define an entire Maven profile for this test, but we could add support in our build for controlling test selection via system properties.

rschmitt · 2026-01-14T17:38:49Z

httpclient5/src/main/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolver.java

+ * @since 5.7
+ */
+@Contract(threading = ThreadingBehavior.IMMUTABLE)
+public final class Rfc6724AddressSelectingDnsResolver implements DnsResolver {


I'm not crazy about the name -- for one thing, it's obviously cryptic; for another, this RFC could be superceded by a newer one (as has happened multiple times with Happy Eyeballs and HTTP itself), and we'll be stuck with the old RFC number as part of our public API symbols

rschmitt · 2026-01-14T17:44:40Z

httpclient5/src/main/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolver.java

+            if (LOG.isDebugEnabled()) {
+                LOG.debug("{} resolved '{}' -> null", simpleName(), host);
+            }


Why pass simpleName() here? The logger configuration should add the class name

rschmitt · 2026-01-14T17:45:12Z

httpclient5/src/main/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolver.java

+            if (LOG.isDebugEnabled()) {
+                LOG.debug("{} resolved '{}' -> []", simpleName(), host);
+            }
+            return new InetAddress[0];


Why an empty array here, but null on line 128?

rschmitt · 2026-01-14T17:48:33Z

httpclient5/src/main/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolver.java

+    private static List<InetAddress> filterCandidates(
+            final InetAddress[] resolved,
+            final ProtocolFamilyPreference pref) {


This could be nicely implemented using streaming operations: Arrays.stream(resolved).filter(a -> isUsableDestination(a)).filter(pref::shouldInclude).collect(toList())

rschmitt · 2026-01-14T18:04:10Z

httpclient5/src/main/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolver.java

+        int i = 0;
+        int j = 0;
+        while (i < first.size() || j < second.size()) {
+            if (i < first.size()) {
+                out.add(first.get(i++));
+            }
+            if (j < second.size()) {
+                out.add(second.get(j++));
+            }
+        }


I think this would be a little cleaner with Iterators. It saves you from having to index into arrays (I'm a little neurotic about that, I realize), and it may help optimize out runtime array bounds checks.

rschmitt · 2026-01-14T18:15:53Z

httpclient5/src/test/java/org/apache/hc/client5/http/examples/Rfc6724ResolverExample.java

+import org.apache.hc.client5.http.SystemDefaultDnsResolver;
+import org.apache.hc.client5.http.config.ProtocolFamilyPreference;
+
+public final class Rfc6724ResolverExample {


src/hc/client # ./run-example.sh Rfc6724ResolverExample www.amazon.com Host: www.amazon.com Preference: DEFAULT IPv6 2600:1409:9800:1680:0:0:0:3bd4 IPv6 2600:1409:9800:168d:0:0:0:3bd4 IPv4 184.27.192.135 src/hc/client # ./run-example.sh Rfc6724ResolverExample www.amazon.com INTERLEAVE Host: www.amazon.com Preference: INTERLEAVE IPv6 2600:1409:9800:168d:0:0:0:3bd4 IPv4 184.27.192.135 IPv6 2600:1409:9800:1680:0:0:0:3bd4

Neat! I'd be cool if I could see the before-and-after (maybe you could intercept the "before" by wrapping the system default resolver, then passing the wrapper in to the RFC 6724 resolver), but it's not essential.

rschmitt · 2026-01-14T18:50:38Z

...client5/src/test/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolverTest.java

+        final Method addr = resolverClass.getDeclaredMethod("addr", InetAddress.class);
+        addr.setAccessible(true);
+
+        final Method fmtArr = resolverClass.getDeclaredMethod("fmt", InetAddress[].class);
+        fmtArr.setAccessible(true);
+
+        final Method fmtList = resolverClass.getDeclaredMethod("fmt", List.class);
+        fmtList.setAccessible(true);


Just declare these methods package-private and call them directly!

rschmitt · 2026-01-14T18:51:06Z

...client5/src/test/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolverTest.java

+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+class Rfc6724AddressSelectingDnsResolverTest {


Do we need any additional test coverage for the prefix matching stuff? That seems like quite a bit of non-trivial code

rschmitt · 2026-01-14T18:51:51Z

httpclient5-testing/src/test/java/org/apache/hc/client5/testing/ManualRfc6724ResolverIT.java

+import org.apache.hc.client5.http.config.ProtocolFamilyPreference;
+import org.junit.jupiter.api.Test;
+
+class ManualRfc6724ResolverIT {


What does this actually test? There are no assertions. If this basically just repeats the examples class then I'd rather not add it.

rschmitt · 2026-01-14T18:53:15Z

There are some things from my first review that still haven't been addressed. I suggest going through the comments one by one and hitting "Resolve conversation" as you address each one, so you can track what is and isn't done.

arturobernalg requested a review from rschmitt January 9, 2026 17:54

arturobernalg mentioned this pull request Jan 9, 2026

[HTTPCLIENT-2178] Implement Happy Eyeballs V2 (RFC 8305) #693

Open

Add RFC6724-based DNS resolver with configurable family preference.

3a1bd12

Define INTERLEAVE as no-bias and align tests and debug output.

arturobernalg force-pushed the Rfc6724A branch from 0802ffe to 3a1bd12 Compare January 10, 2026 11:21

rschmitt requested changes Jan 12, 2026

View reviewed changes

Implement true INTERLEAVE ordering.

f6b6dbb

Add manual gated IT to dump DEFAULT vs INTERLEAVE results. Expand unit coverage for scope mapping and core RFC comparison rules.

arturobernalg requested a review from rschmitt January 13, 2026 11:02

rschmitt reviewed Jan 14, 2026

View reviewed changes

		assumeTrue(Boolean.getBoolean(PROP_ENABLE),
		"Enable with -Dhttpclient.rfc6724.it=true");

Rfc6724 resolver ordering semantics #778

Are you sure you want to change the base?

Rfc6724 resolver ordering semantics #778

Conversation

arturobernalg commented Jan 9, 2026

Uh oh!

rschmitt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arturobernalg commented Jan 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rschmitt commented Jan 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants