reworked early_resolve to work with dual-stack

for multi-backed resolvers, resolving is attempted before sending it to
    the resolver. in this way, cached, local or ip resolves get
    propagated to the proper resolver by ip family, instead of the
    previous mess.

    the system resolver doesn't do these shenanigans (trust getaddrinfo)

lib/httpx/pool.rb

@@ -216,10 +216,12 @@ module HTTPX
       end
 
       manager = @resolvers[resolver_type]
-      manager.resolvers.each do |resolver|
+
+      (manager.is_a?(Resolver::Multi) && manager.early_resolve(connection)) || manager.resolvers.each do |resolver|
         resolver.pool = self
         yield resolver
       end
+
       manager
     end
   end

lib/httpx/resolver.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "resolv"
+require "ipaddr"
 
 module HTTPX
   module Resolver
@@ -23,9 +24,27 @@ module HTTPX
 
     @identifier_mutex = Mutex.new
     @identifier = 1
+    @system_resolver = Resolv::Hosts.new
 
     module_function
 
+    def nolookup_resolve(hostname)
+      ip_resolve(hostname) || cached_lookup(hostname) || system_resolve(hostname)
+    end
+
+    def ip_resolve(hostname)
+      [IPAddr.new(hostname)]
+    rescue ArgumentError
+    end
+
+    def system_resolve(hostname)
+      ips = @system_resolver.getaddresses(hostname)
+      return if ips.empty?
+
+      ips.map { |ip| IPAddr.new(ip) }
+    rescue IOError
+    end
+
     def cached_lookup(hostname)
       now = Utils.now
       @lookup_mutex.synchronize do
@@ -69,7 +88,7 @@ module HTTPX
         if address.key?("alias")
           lookup(address["alias"], ttl)
         else
-          address["data"]
+          IPAddr.new(address["data"])
         end
       end
       ips unless ips.empty?

lib/httpx/resolver/https.rb

@@ -35,7 +35,7 @@ module HTTPX
     def <<(connection)
       return if @uri.origin == connection.origin.to_s
 
-      @uri_addresses ||= ip_resolve(@uri.host) || system_resolve(@uri.host) || @resolver.getaddresses(@uri.host)
+      @uri_addresses ||= HTTPX::Resolver.nolookup_resolve(@uri.host) || @resolver.getaddresses(@uri.host)
 
       if @uri_addresses.empty?
         ex = ResolveError.new("Can't resolve DNS server #{@uri.host}")
@@ -43,7 +43,7 @@ module HTTPX
         throw(:resolve_error, ex)
       end
 
-      early_resolve(connection) || resolve(connection)
+      resolve(connection)
     end
 
     def closed?
@@ -132,8 +132,12 @@ module HTTPX
               if alias_address.nil?
                 connection = @queries[hostname]
                 @queries.delete(address["name"])
-                resolve(connection, address["alias"])
-                return # rubocop:disable Lint/NonLocalExitFromIterator
+                if catch(:coalesced) { early_resolve(connection, hostname: address["alias"]) }
+                  @connections.delete(connection)
+                else
+                  resolve(connection, address["alias"])
+                  return # rubocop:disable Lint/NonLocalExitFromIterator
+                end
               else
                 alias_address
               end

lib/httpx/resolver/multi.rb

@@ -12,6 +12,7 @@ module HTTPX
 
     def initialize(resolver_type, options)
       @options = options
+      @resolver_options = @options.resolver_options
 
       @resolvers = options.ip_families.map do |ip_family|
         resolver = resolver_type.new(ip_family, options)
@@ -40,6 +41,22 @@ module HTTPX
       @resolvers.filter_map { |r| r.resolver_connection if r.respond_to?(:resolver_connection) }
     end
 
+    def early_resolve(connection)
+      hostname = connection.origin.host
+      addresses = @resolver_options[:cache] && (connection.addresses || HTTPX::Resolver.nolookup_resolve(hostname))
+      return unless addresses
+
+      addresses = addresses.group_by(&:family)
+
+      @resolvers.each do |resolver|
+        addrs = addresses[resolver.family]
+
+        next if !addrs || addrs.empty?
+
+        resolver.emit_addresses(connection, resolver.family, addrs)
+      end
+    end
+
     private
 
     def on_resolver_connection(connection)

lib/httpx/resolver/native.rb

@@ -98,8 +98,6 @@ module HTTPX
     end
 
     def <<(connection)
-      return if early_resolve(connection)
-
       if @nameserver.nil?
         ex = ResolveError.new("No available nameserver")
         ex.set_backtrace(caller)

lib/httpx/resolver/resolver.rb

@@ -53,8 +53,6 @@ module HTTPX
       true
     end
 
-    private
-
     def emit_addresses(connection, family, addresses)
       addresses.map! do |address|
         address.is_a?(IPAddr) ? address : IPAddr.new(address.to_s)
@@ -75,29 +73,16 @@ module HTTPX
       end
     end
 
+    private
+
     def early_resolve(connection, hostname: connection.origin.host)
-      addresses = connection.addresses ||
-                  ip_resolve(hostname) ||
-                  (@resolver_options[:cache] && HTTPX::Resolver.cached_lookup(hostname)) ||
-                  system_resolve(hostname)
+      addresses = @resolver_options[:cache] && (connection.addresses || HTTPX::Resolver.nolookup_resolve(hostname))
+
       return unless addresses
 
-      emit_addresses(connection, nil, addresses)
-    end
+      addresses.select! { |addr| addr.family == @family }
 
-    def ip_resolve(hostname)
-      [IPAddr.new(hostname)]
-    rescue ArgumentError
-    end
-
-    def system_resolve(hostname)
-      @system_resolver ||= Resolv::Hosts.new
-      ips = @system_resolver.getaddresses(hostname)
-      return if ips.empty?
-
-      ips.map! { |ip| IPAddr.new(ip) }
-      ips.sort! { |ip1, ip2| ip1.ipv6? && ip2.ipv4? ? 1 : 0 }
-    rescue IOError
+      emit_addresses(connection, @family, addresses)
     end
 
     def emit_resolve_error(connection, hostname = connection.origin.host, ex = nil)

lib/httpx/resolver/system.rb

@@ -86,8 +86,6 @@ module HTTPX
     end
 
     def <<(connection)
-      return if early_resolve(connection)
-
       @connections << connection
       resolve
     end

sig/resolver.rbs

@@ -13,11 +13,18 @@ module HTTPX
     type dns_result = { "name" => String, "TTL" => Numeric, "alias" => String }
                     | { "name" => String, "TTL" => Numeric, "data" => String }
 
-    def self?.cached_lookup: (String hostname) -> Array[String]?
+
+    def nolookup_resolve: (String hostname) -> Array[IPAddr]
+
+    def ip_resolve: (String hostname) -> Array[IPAddr]?
+
+    def system_resolve: (String hostname) -> Array[IPAddr]?
+
+    def self?.cached_lookup: (String hostname) -> Array[IPAddr]?
 
     def self?.cached_lookup_set: (String hostname, ip_family family, Array[dns_result] addresses) -> void
 
-    def self?.lookup: (String hostname, Numeric ttl) -> Array[String]?
+    def self?.lookup: (String hostname, Numeric ttl) -> Array[IPAddr]?
 
     def self?.generate_id: () -> Integer
 

sig/resolver/multi.rbs

@@ -1,6 +1,7 @@
 module HTTPX
   module Resolver
     class Multi
+      def early_resolve: (Connection connection, ?hostname: String) -> void
     end
   end
 end

sig/resolver/resolver.rbs

@@ -21,19 +21,15 @@ module HTTPX
 
       def empty?: () -> bool
 
+      def emit_addresses: (Connection connection, ip_family family, Array[IPAddr]) -> void
+
       private
 
       def initialize: (ip_family? family, options options) -> void
 
-      def emit_addresses: (Connection, ip_family? family, Array[ipaddr | Resolv::DNS::ip_address]) -> void
+      def early_resolve: (Connection connection, ?hostname: String) -> void
 
-      def early_resolve: (Connection, ?hostname: String) -> void
-
-      def ip_resolve: (String hostname) -> Array[ipaddr]?
-
-      def system_resolve: (String hostname) -> Array[ipaddr]?
-
-      def emit_resolve_error: (Connection, ?String hostname, ?StandardError) -> void
+      def emit_resolve_error: (Connection connection, ?String hostname, ?StandardError) -> void
 
       def resolve_error: (String hostname, ?StandardError?) -> ResolveError
     end

test/resolver_test.rb

@@ -8,10 +8,10 @@ class ResolverTest < Minitest::Test
   def test_cached_lookup
     ips = Resolver.cached_lookup("test.com")
     assert ips.nil?
-    dns_entry = { "data" => "IPv6", "TTL" => 2, "name" => "test.com" }
+    dns_entry = { "data" => "::2", "TTL" => 2, "name" => "test.com" }
     Resolver.cached_lookup_set("test.com", Socket::AF_INET6, [dns_entry])
     ips = Resolver.cached_lookup("test.com")
-    assert ips == ["IPv6"]
+    assert ips == ["::2"]
     sleep 2
     ips = Resolver.cached_lookup("test.com")
     assert ips.nil?
@@ -19,14 +19,14 @@ class ResolverTest < Minitest::Test
     Resolver.cached_lookup_set("test.com", Socket::AF_INET6, [dns_entry])
     Resolver.cached_lookup_set("foo.com", Socket::AF_INET6, [alias_entry])
     ips = Resolver.cached_lookup("foo.com")
-    assert ips == ["IPv6"]
+    assert ips == ["::2"]
 
-    Resolver.cached_lookup_set("test.com", Socket::AF_INET6, [{ "data" => "IPv6_2", "TTL" => 2, "name" => "test.com" }])
+    Resolver.cached_lookup_set("test.com", Socket::AF_INET6, [{ "data" => "::3", "TTL" => 2, "name" => "test.com" }])
     ips = Resolver.cached_lookup("test.com")
-    assert ips == %w[IPv6 IPv6_2]
+    assert ips == %w[::2 ::3]
 
-    Resolver.cached_lookup_set("test.com", Socket::AF_INET, [{ "data" => "IPv4", "TTL" => 2, "name" => "test.com" }])
+    Resolver.cached_lookup_set("test.com", Socket::AF_INET, [{ "data" => "127.0.0.2", "TTL" => 2, "name" => "test.com" }])
     ips = Resolver.cached_lookup("test.com")
-    assert ips == %w[IPv4 IPv6 IPv6_2]
+    assert ips == %w[127.0.0.2 ::2 ::3]
   end
 end