Fix TCP fallback with multiple nameservers [Bug #8285]

lib/resolv.rb

@@ -513,35 +513,40 @@ def each_resource(name, typeclass, &proc)
 
     def fetch_resource(name, typeclass)
       lazy_initialize
-      begin
-        requester = make_udp_requester
+      truncated = {}
+      requesters = {}
+      udp_requester = begin
+        make_udp_requester
       rescue Errno::EACCES
         # fall back to TCP
       end
       senders = {}
+
       begin
-        @config.resolv(name) {|candidate, tout, nameserver, port|
-          requester ||= make_tcp_requester(nameserver, port)
+        @config.resolv(name) do |candidate, tout, nameserver, port|
           msg = Message.new
           msg.rd = 1
           msg.add_question(candidate, typeclass)
-          unless sender = senders[[candidate, nameserver, port]]
+
+          requester = requesters.fetch([nameserver, port]) do
+            if !truncated[candidate] && udp_requester
+              udp_requester
+            else
+              requesters[[nameserver, port]] = make_tcp_requester(nameserver, port)
+            end
+          end
+
+          unless sender = senders[[candidate, requester, nameserver, port]]
             sender = requester.sender(msg, candidate, nameserver, port)
             next if !sender
-            senders[[candidate, nameserver, port]] = sender
+            senders[[candidate, requester, nameserver, port]] = sender
           end
           reply, reply_name = requester.request(sender, tout)
           case reply.rcode
           when RCode::NoError
             if reply.tc == 1 and not Requester::TCP === requester
-              requester.close
               # Retry via TCP:
-              requester = make_tcp_requester(nameserver, port)
-              senders = {}
-              # This will use TCP for all remaining candidates (assuming the
-              # current candidate does not already respond successfully via
-              # TCP).  This makes sense because we already know the full
-              # response will not fit in an untruncated UDP packet.
+              truncated[candidate] = true
               redo
             else
               yield(reply, reply_name)
@@ -552,9 +557,10 @@ def fetch_resource(name, typeclass)
           else
             raise Config::OtherResolvError.new(reply_name.to_s)
           end
-        }
+        end
       ensure
-        requester&.close
+        udp_requester&.close
+        requesters.each_value { |requester| requester&.close }
       end
     end
 
@@ -569,6 +575,11 @@ def make_udp_requester # :nodoc:
 
     def make_tcp_requester(host, port) # :nodoc:
       return Requester::TCP.new(host, port)
+    rescue Errno::ECONNREFUSED
+      # Treat a refused TCP connection attempt to a nameserver like a timeout,
+      # as Resolv::DNS::Config#resolv considers ResolvTimeout exceptions as a
+      # hint to try the next nameserver:
+      raise ResolvTimeout
     end
 
     def extract_resources(msg, name, typeclass) # :nodoc:
@@ -1800,7 +1811,6 @@ def canonical_key(key) # :nodoc:
       end
     end
 
-
     ##
     # Base class for SvcParam. [RFC9460]
 
@@ -2499,7 +2509,6 @@ def initialize(version, ssize, hprecision, vprecision, latitude, longitude, alti
 
         attr_reader :altitude
 
-
         def encode_rdata(msg) # :nodoc:
           msg.put_bytes(@version)
           msg.put_bytes(@ssize.scalar)
@@ -3439,4 +3448,3 @@ def DefaultResolver.replace_resolvers new_resolvers
   AddressRegex = /(?:#{IPv4::Regex})|(?:#{IPv6::Regex})/
 
 end
-

test/resolv/test_dns.rb

@@ -698,4 +698,117 @@ def test_unreachable_server
   ensure
     sock&.close
   end
+
+  def test_multiple_servers_with_timeout_and_truncated_tcp_fallback
+    begin
+      OpenSSL
+    rescue LoadError
+      skip 'autoload problem. see [ruby-dev:45021][Bug #5786]'
+    end if defined?(OpenSSL)
+
+    num_records = 50
+
+    with_udp_and_tcp('127.0.0.1', 0) do |u1, t1|
+      with_tcp('0.0.0.0', 0) do |t2|
+        _, server1_port, _, server1_address = u1.addr
+        _, server2_port, _, server2_address = t2.addr
+
+        client_thread = Thread.new do
+          Resolv::DNS.open(nameserver_port: [[server1_address, server1_port], [server2_address, server2_port]]) do |dns|
+            dns.timeouts = [0.1, 0.2]
+            dns.getresources('foo.example.org', Resolv::DNS::Resource::IN::A)
+          end
+        end
+
+        udp_server1_thread = Thread.new do
+          msg, (_, client_port, _, client_address) = Timeout.timeout(5) { u1.recvfrom(4096) }
+          id, word2, _qdcount, _ancount, _nscount, _arcount = msg.unpack('nnnnnn')
+          opcode = (word2 & 0x7800) >> 11
+          rd = (word2 & 0x0100) >> 8
+          name = [3, 'foo', 7, 'example', 3, 'org', 0].pack('Ca*Ca*Ca*C')
+          qr = 1
+          aa = 0
+          tc = 1
+          ra = 1
+          z = 0
+          rcode = 0
+          qdcount = 0
+          ancount = num_records
+          nscount = 0
+          arcount = 0
+          word2 = (qr << 15) |
+                  (opcode << 11) |
+                  (aa << 10) |
+                  (tc << 9) |
+                  (rd << 8) |
+                  (ra << 7) |
+                  (z << 4) |
+                  rcode
+          msg = [id, word2, qdcount, ancount, nscount, arcount].pack('nnnnnn')
+          type = 1
+          klass = 1
+          ttl = 3600
+          rdlength = 4
+          num_records.times do |i|
+            rdata = [192, 0, 2, i].pack('CCCC') # 192.0.2.x (TEST-NET address) RFC 3330
+            rr = [name, type, klass, ttl, rdlength, rdata].pack('a*nnNna*')
+            msg << rr
+          end
+          u1.send(msg[0...512], 0, client_address, client_port)
+        end
+
+        tcp_server1_thread = Thread.new { t1.accept }
+
+        tcp_server2_thread = Thread.new do
+          ct = t2.accept
+          msg = ct.recv(512)
+          msg.slice!(0..1) # Size (only for TCP)
+          id, word2, _qdcount, _ancount, _nscount, _arcount = msg.unpack('nnnnnn')
+          rd = (word2 & 0x0100) >> 8
+          opcode = (word2 & 0x7800) >> 11
+          name = [3, 'foo', 7, 'example', 3, 'org', 0].pack('Ca*Ca*Ca*C')
+          qr = 1
+          aa = 0
+          tc = 0
+          ra = 1
+          z = 0
+          rcode = 0
+          qdcount = 0
+          ancount = num_records
+          nscount = 0
+          arcount = 0
+          word2 = (qr << 15) |
+                  (opcode << 11) |
+                  (aa << 10) |
+                  (tc << 9) |
+                  (rd << 8) |
+                  (ra << 7) |
+                  (z << 4) |
+                  rcode
+          msg = [id, word2, qdcount, ancount, nscount, arcount].pack('nnnnnn')
+          type = 1
+          klass = 1
+          ttl = 3600
+          rdlength = 4
+          num_records.times do |i|
+            rdata = [192, 0, 2, i].pack('CCCC') # 192.0.2.x (TEST-NET address) RFC 3330
+            rr = [name, type, klass, ttl, rdlength, rdata].pack('a*nnNna*')
+            msg << rr
+          end
+          msg = "#{[msg.bytesize].pack('n')}#{msg}" # Prefix with size
+          ct.send(msg, 0)
+          ct.close
+        end
+        result, = assert_join_threads([client_thread, udp_server1_thread, tcp_server1_thread, tcp_server2_thread])
+        assert_instance_of(Array, result)
+        assert_equal(50, result.length)
+        result.each_with_index do |rr, i|
+          assert_instance_of(Resolv::DNS::Resource::IN::A, rr)
+          assert_instance_of(Resolv::IPv4, rr.address)
+          assert_equal("192.0.2.#{i}", rr.address.to_s)
+          assert_equal(3600, rr.ttl)
+        end
+      end
+    end
+  end
 end