From 5adb27af998e213a26d6f5f055560b5c4bab3eae Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Fri, 25 Sep 2026 10:03:02 +0900 Subject: [PATCH] Apply the request deadline to the TCP connect Retrying a truncated reply over TCP connected with no timeout, so a nameserver that never completed the handshake held the query until the kernel gave up, regardless of `timeouts=`, and the resulting error escaped instead of moving on to the next nameserver. The connect now spends from the same interval as the request that follows it. `Addrinfo#connect` is used because `TCPSocket.new` accepts `connect_timeout:` only since Ruby 3.0, and it raises `Errno::ETIMEDOUT` on every supported version. Co-Authored-By: Claude Opus 5.5 --- lib/resolv.rb | 25 ++++++----- test/resolv/test_dns.rb | 92 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 10 deletions(-) diff --git a/lib/resolv.rb b/lib/resolv.rb index 174b913..df8f6e6 100644 --- a/lib/resolv.rb +++ b/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 diff --git a/test/resolv/test_dns.rb b/test/resolv/test_dns.rb index b8ecef2..77fc1a2 100644 --- a/test/resolv/test_dns.rb +++ b/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