Apply the request deadline to the TCP connect

lib/resolv.rb

@@ -549,11 +549,14 @@ def fetch_resource(name, typeclass)
           msg.rd = 1
           msg.add_question(candidate, typeclass)
 
+          # Connecting to the nameserver comes out of the same interval as the
+          # request itself.
+          timelimit = Process.clock_gettime(Process::CLOCK_MONOTONIC) + tout
           requester = requesters.fetch([nameserver, port]) do
             if !truncated[candidate] && udp_requester
               udp_requester
             else
-              requesters[[nameserver, port]] = make_tcp_requester(nameserver, port)
+              requesters[[nameserver, port]] = make_tcp_requester(nameserver, port, tout)
             end
           end
 
@@ -563,7 +566,8 @@ def fetch_resource(name, typeclass)
             senders[[candidate, requester, nameserver, port]] = sender
           end
           begin
-            reply, reply_name = requester.request(sender, tout)
+            remaining = timelimit - Process.clock_gettime(Process::CLOCK_MONOTONIC)
+            reply, reply_name = requester.request(sender, remaining)
           rescue ResolvTimeout
             # Giving up part way through a frame loses stream sync, and a peer
             # seen going away leaves the socket dead.  Either way the requester
@@ -606,12 +610,12 @@ def make_udp_requester # :nodoc:
       end
     end
 
-    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:
+    def make_tcp_requester(host, port, timeout = nil) # :nodoc:
+      return Requester::TCP.new(host, port, timeout)
+    rescue Errno::ECONNREFUSED, Errno::ETIMEDOUT
+      # Treat a refused or timed out 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
 
@@ -982,11 +986,12 @@ def sender_for(addr, msg)
       end
 
       class TCP < Requester # :nodoc:
-        def initialize(host, port=Port)
+        def initialize(host, port=Port, timeout=nil)
           super()
           @host = host
           @port = port
-          sock = TCPSocket.new(@host, @port)
+          # TCPSocket.new takes connect_timeout: only since Ruby 3.0.
+          sock = Addrinfo.tcp(@host, @port).connect(timeout: timeout)
           @socks = [sock]
           @senders = {}
           @reusable = true

test/resolv/test_dns.rb

@@ -1615,4 +1615,96 @@ def test_tcp_peer_lost_while_idle_is_not_used_again
       end
     end
   end
+
+  # Fills the accept queue of +t+ so that a further connection attempt to it
+  # is left waiting, as Linux does by dropping the SYN.  Omits the test where
+  # the kernel completes or refuses such an attempt instead.
+  def with_accept_queue_full(t)
+    _, port, _, address = t.addr
+    sockaddr = Socket.sockaddr_in(port, address)
+    queued = []
+    begin
+      8.times do
+        s = Socket.new(:INET, :STREAM)
+        queued << s
+        begin
+          next unless s.connect_nonblock(sockaddr, exception: false) == :wait_writable
+        rescue SystemCallError
+          next
+        end
+        return yield unless s.wait_writable(EnvUtil.apply_timeout_scale(0.5))
+      end
+      omit('cannot leave a connection attempt waiting')
+    ensure
+      queued.each(&:close)
+    end
+  end
+
+  def test_truncated_tcp_fallback_gives_up_connecting_after_the_timeout
+    with_udp_and_tcp('127.0.0.1', 0) do |u, t|
+      _, server_port, _, server_address = u.addr
+      with_accept_queue_full(t) do
+        client_thread = Thread.new do
+          Resolv::DNS.open(nameserver_port: [[server_address, server_port]],
+                           raise_timeout_errors: true) do |dns|
+            dns.timeouts = EnvUtil.apply_timeout_scale(0.5)
+            assert_raise(Resolv::ResolvError) do
+              Timeout.timeout(EnvUtil.apply_timeout_scale(10)) do
+                dns.getresources('foo.example.org', Resolv::DNS::Resource::IN::A)
+              end
+            end
+          end
+        end
+
+        udp_server_thread = Thread.new { answer_truncated(u) }
+
+        assert_join_threads([client_thread, udp_server_thread])
+      end
+    end
+  end
+
+  def test_truncated_tcp_fallback_moves_on_when_connecting_times_out
+    with_udp_and_tcp('127.0.0.1', 0) do |u1, t1|
+      with_udp_and_tcp('127.0.0.1', 0) do |u2, t2|
+        u2.close # only the TCP side of the second nameserver is used
+        _, server1_port, _, server1_address = u1.addr
+        _, server2_port, _, server2_address = t2.addr
+        with_accept_queue_full(t1) do
+          done = Thread::Queue.new
+
+          client_thread = Thread.new do
+            begin
+              Resolv::DNS.open(nameserver_port: [[server1_address, server1_port],
+                                                 [server2_address, server2_port]],
+                               raise_timeout_errors: true) do |dns|
+                dns.timeouts = [EnvUtil.apply_timeout_scale(0.5),
+                                EnvUtil.apply_timeout_scale(5)]
+                Timeout.timeout(EnvUtil.apply_timeout_scale(10)) do
+                  dns.getresources('foo.example.org', Resolv::DNS::Resource::IN::A)
+                end
+              end
+            ensure
+              done.push(true)
+            end
+          end
+
+          udp_server1_thread = Thread.new { answer_truncated(u1) }
+
+          tcp_server2_thread = Thread.new do
+            ct = accept_within_timeout(t2)
+            begin
+              ct.write(framed(reply_for_query(read_framed_query(ct), '192.0.2.1')))
+              done.pop
+            ensure
+              ct.close
+            end
+          end
+
+          result, = assert_join_threads([client_thread, udp_server1_thread,
+                                         tcp_server2_thread])
+          assert_equal(['192.0.2.1'], result.map {|rr| rr.address.to_s })
+        end
+      end
+    end
+  end
 end