Merge branch 'gh-52' into 'master'

native resolver: moved timeouts reset out of idle transition, retry alias

See merge request os85/httpx!342

lib/httpx/resolver/native.rb

@@ -65,6 +65,7 @@ module HTTPX
       if nameserver && @ns_index < nameserver.size
         log { "resolver: failed resolving on nameserver #{@nameserver[@ns_index - 1]} (#{e.message})" }
         transition(:idle)
+        @timeouts.clear
       else
         handle_error(e)
       end
@@ -143,13 +144,16 @@ module HTTPX
 
       if !@timeouts[host].empty?
         log { "resolver: timeout after #{timeout}s, retry(#{@timeouts[host].first}) #{host}..." }
-        resolve(connection)
+        # must downgrade to tcp AND retry on same host as last
+        downgrade_socket
+        resolve(connection, h)
       elsif @ns_index + 1 < @nameserver.size
         # try on the next nameserver
         @ns_index += 1
         log { "resolver: failed resolving #{host} on nameserver #{@nameserver[@ns_index - 1]} (timeout error)" }
         transition(:idle)
-        resolve(connection)
+        @timeouts.clear
+        resolve(connection, h)
       else
 
         @timeouts.delete(host)
@@ -187,10 +191,9 @@ module HTTPX
             next unless @large_packet.full?
 
             parse(@large_packet.to_s)
-            @socket_type = @resolver_options.fetch(:socket_type, :udp)
             @large_packet = nil
-            transition(:idle)
-            transition(:open)
+            # downgrade to udp again
+            downgrade_socket
             return
           else
             size = @read_buffer[0, 2].unpack1("n")
@@ -304,13 +307,21 @@ module HTTPX
         end
 
         if address.key?("alias") # CNAME
+          hostname_alias = address["alias"]
           # clean up intermediate queries
           @timeouts.delete(name) unless connection.origin.host == name
 
-          if catch(:coalesced) { early_resolve(connection, hostname: address["alias"]) }
+          if catch(:coalesced) { early_resolve(connection, hostname: hostname_alias) }
             @connections.delete(connection)
           else
-            resolve(connection, address["alias"])
+            if @socket_type == :tcp
+              # must downgrade to udp if tcp
+              @socket_type = @resolver_options.fetch(:socket_type, :udp)
+              transition(:idle)
+              transition(:open)
+            end
+            log { "resolver: ALIAS #{hostname_alias} for #{name}" }
+            resolve(connection, hostname_alias)
             return
           end
         else
@@ -386,6 +397,14 @@ module HTTPX
       end
     end
 
+    def downgrade_socket
+      return unless @socket_type == :tcp
+
+      @socket_type = @resolver_options.fetch(:socket_type, :udp)
+      transition(:idle)
+      transition(:open)
+    end
+
     def transition(nextstate)
       case nextstate
       when :idle
@@ -393,7 +412,6 @@ module HTTPX
           @io.close
           @io = nil
         end
-        @timeouts.clear
       when :open
         return unless @state == :idle
 

sig/resolver/native.rbs

@@ -59,6 +59,8 @@ module HTTPX
 
       def build_socket: () -> (UDP | TCP)
 
+      def downgrade_socket: () -> void
+
       def transition: (Symbol nextstate) -> void
 
       def handle_error: (NativeResolveError | StandardError) -> void

test/support/assertion_helpers.rb

@@ -99,8 +99,8 @@ module ResponseHelpers
     File.join("test", "support", "fixtures", fixture_file_name)
   end
 
-  def start_test_servlet(servlet_class, *args)
-    server = servlet_class.new(*args)
+  def start_test_servlet(servlet_class, *args, **kwargs)
+    server = servlet_class.new(*args, **kwargs)
     th = Thread.new { server.start }
     begin
       yield server

test/support/requests/resolvers.rb

@@ -131,7 +131,7 @@ module Requests
               uri = build_uri("/get")
 
               before_time = Process.clock_gettime(Process::CLOCK_MONOTONIC, :second)
-              response = session.get(uri, resolver_class: resolver_type, resolver_options: resolver_opts)
+              response = session.get(uri, resolver_class: resolver_type, resolver_options: options.merge(resolver_opts))
               after_time = Process.clock_gettime(Process::CLOCK_MONOTONIC, :second)
               total_time = after_time - before_time
 
@@ -160,7 +160,7 @@ module Requests
               HTTPX.plugin(SessionWithPool).wrap do |session|
                 uri = build_uri("/get")
 
-                response = session.get(uri, resolver_class: resolver_type, resolver_options: resolver_opts)
+                response = session.get(uri, resolver_class: resolver_type, resolver_options: options.merge(resolver_opts))
                 verify_status(response, 200)
 
                 resolver = session.pool.resolver.resolvers[0]
@@ -180,7 +180,7 @@ module Requests
               HTTPX.plugin(SessionWithPool).wrap do |session|
                 uri = build_uri("/get")
 
-                response = session.get(uri, resolver_class: resolver_type, resolver_options: resolver_opts)
+                response = session.get(uri, resolver_class: resolver_type, resolver_options: options.merge(resolver_opts))
                 verify_error_response(response, /unknown DNS error/)
               end
             end
@@ -231,6 +231,8 @@ module Requests
               attr_reader :ios
             end
 
+            private
+
             def build_socket
               io = super
               self.class.ios << io

test/support/servlets/slow_dns.rb

@@ -6,15 +6,30 @@ require_relative "test"
 # from https://gist.github.com/peterc/1425383
 
 class SlowDNSServer < TestDNSResolver
-  def initialize(timeout)
+  def initialize(timeout, *args, hostname: nil, als: nil)
     @timeout = timeout
-    super()
+    @hostname = hostname
+    @alias = als
+    super(*args)
   end
 
   private
 
-  def dns_response(*)
-    sleep(@timeout)
+  def dns_response(query)
+    if @alias
+      domain = extract_domain(query)
+      sleep(@timeout) if domain == @alias
+    else
+      sleep(@timeout)
+    end
     super
   end
+
+  def resolve(domain)
+    if domain == "#{@hostname}."
+      @alias
+    else
+      super
+    end
+  end
 end

test/support/servlets/test.rb

@@ -116,11 +116,12 @@ end
 class TestDNSResolver
   attr_reader :queries, :answers
 
-  def initialize
-    @port = next_available_port
+  def initialize(port = next_available_port, socket_type = :udp)
+    @port = port
     @can_log = ENV.key?("HTTPX_DEBUG")
     @queries = 0
     @answers = 0
+    @socket_type = socket_type
   end
 
   def nameserver
@@ -128,10 +129,31 @@ class TestDNSResolver
   end
 
   def start
-    Socket.udp_server_loop(@port) do |query, src|
-      @queries += 1
-      src.reply(dns_response(query))
-      @answers += 1
+    if @socket_type == :udp
+      Socket.udp_server_loop(@port) do |query, src|
+        puts "bang bang"
+        @queries += 1
+        src.reply(dns_response(query))
+        @answers += 1
+      end
+    elsif @socket_type == :tcp
+      Socket.tcp_server_loop(@port) do |sock, _addrinfo|
+        begin
+          loop do
+            query = sock.readpartial(2048)
+            size = query[0, 2].unpack1("n")
+            query = query.byteslice(2..-1)
+            query << sock.readpartial(size - query.size) while query.size < size
+            @queries += 1
+            answer = dns_response(query)
+
+            answer.prepend([answer.size].pack("n"))
+            sock.write(answer)
+            @answers += 1
+          end
+        rescue EOFError
+        end
+      end
     end
   end