From 037994514bd068e2e81489f60e57a975ac976f38 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Mon, 17 Jan 2022 00:17:28 +0200 Subject: [PATCH] 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 | 4 +++- lib/httpx/resolver.rb | 21 ++++++++++++++++++++- lib/httpx/resolver/https.rb | 12 ++++++++---- lib/httpx/resolver/multi.rb | 17 +++++++++++++++++ lib/httpx/resolver/native.rb | 2 -- lib/httpx/resolver/resolver.rb | 27 ++++++--------------------- lib/httpx/resolver/system.rb | 2 -- sig/resolver.rbs | 11 +++++++++-- sig/resolver/multi.rbs | 1 + sig/resolver/resolver.rbs | 12 ++++-------- test/resolver_test.rb | 14 +++++++------- 11 files changed, 75 insertions(+), 48 deletions(-) diff --git a/lib/httpx/pool.rb b/lib/httpx/pool.rb index 1a021262..a0bcd59e 100644 --- a/lib/httpx/pool.rb +++ b/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 diff --git a/lib/httpx/resolver.rb b/lib/httpx/resolver.rb index afef45c5..aeb3f5a8 100644 --- a/lib/httpx/resolver.rb +++ b/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? diff --git a/lib/httpx/resolver/https.rb b/lib/httpx/resolver/https.rb index 8a876ae2..689d6b5f 100644 --- a/lib/httpx/resolver/https.rb +++ b/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 diff --git a/lib/httpx/resolver/multi.rb b/lib/httpx/resolver/multi.rb index 0a2b6eb6..7709a076 100644 --- a/lib/httpx/resolver/multi.rb +++ b/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) diff --git a/lib/httpx/resolver/native.rb b/lib/httpx/resolver/native.rb index f5aa3095..c7ed3f17 100644 --- a/lib/httpx/resolver/native.rb +++ b/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) diff --git a/lib/httpx/resolver/resolver.rb b/lib/httpx/resolver/resolver.rb index ae071620..8bda11d2 100644 --- a/lib/httpx/resolver/resolver.rb +++ b/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) diff --git a/lib/httpx/resolver/system.rb b/lib/httpx/resolver/system.rb index a17d21cb..5ae8e3a2 100644 --- a/lib/httpx/resolver/system.rb +++ b/lib/httpx/resolver/system.rb @@ -86,8 +86,6 @@ module HTTPX end def <<(connection) - return if early_resolve(connection) - @connections << connection resolve end diff --git a/sig/resolver.rbs b/sig/resolver.rbs index cb3fdb62..5b35302c 100644 --- a/sig/resolver.rbs +++ b/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 diff --git a/sig/resolver/multi.rbs b/sig/resolver/multi.rbs index 2cb663e6..3873cd75 100644 --- a/sig/resolver/multi.rbs +++ b/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 \ No newline at end of file diff --git a/sig/resolver/resolver.rbs b/sig/resolver/resolver.rbs index e8bd6a05..8b99464c 100644 --- a/sig/resolver/resolver.rbs +++ b/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 diff --git a/test/resolver_test.rb b/test/resolver_test.rb index a3f7704f..da37b97f 100644 --- a/test/resolver_test.rb +++ b/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