Merge branch 'fix-ttl-expired'

lib/httpx/connection.rb

@@ -49,8 +49,8 @@ module HTTPX
     def initialize(uri, options)
       @current_session = @current_selector =
         @parser = @sibling = @coalesced_connection =
-                    @io = @ssl_session = @timeout =
+                    @family = @io = @ssl_session = @timeout =
-                            @connected_at = @response_received_at = nil
+                                      @connected_at = @response_received_at = nil
 
       @exhausted = @cloned = @main_sibling = false
 

lib/httpx/io/tcp.rb

@@ -59,8 +59,14 @@ module HTTPX
 
     # eliminates expired entries and returns whether there are still any left.
     def addresses?
+      prev_addr_size = @addresses.size
+
       @addresses.delete_if(&:expired?)
 
+      unless (decr = prev_addr_size - @addresses.size).zero?
+        @ip_index = @addresses.size - decr
+      end
+
       @addresses.any?
     end
 

lib/httpx/plugins/callbacks.rb

@@ -97,7 +97,7 @@ module HTTPX
             end
           end
           request.on(:response) do |res|
-            emit_or_callback_error(:response_completed, request, res)
+            emit_or_callback_error(:response_completed, request, res) if res.is_a?(Response)
           end
         end
 

lib/httpx/resolver.rb

@@ -63,10 +63,6 @@ module HTTPX
     end
 
     def cached_lookup_set(hostname, family, entries)
-      now = Utils.now
-      entries.each do |entry|
-        entry["TTL"] += now
-      end
       lookup_synchronize do |lookups|
         case family
         when Socket::AF_INET6
@@ -145,19 +141,20 @@ module HTTPX
 
       addresses = []
 
+      now = Utils.now
       message.each_answer do |question, _, value|
         case value
         when Resolv::DNS::Resource::IN::CNAME
           addresses << {
             "name" => question.to_s,
-            "TTL" => value.ttl,
+            "TTL" => (now + value.ttl),
             "alias" => value.name.to_s,
           }
         when Resolv::DNS::Resource::IN::A,
              Resolv::DNS::Resource::IN::AAAA
           addresses << {
             "name" => question.to_s,
-            "TTL" => value.ttl,
+            "TTL" => (now + value.ttl),
             "data" => value.address.to_s,
           }
         end

lib/httpx/resolver/https.rb

@@ -204,7 +204,7 @@ module HTTPX
           @queries.delete_if { |_, conn| connection == conn }
 
           Resolver.cached_lookup_set(hostname, @family, addresses) if @resolver_options[:cache]
-          catch(:coalesced) { emit_addresses(connection, @family, addresses.map { |addr| addr["data"] }) }
+          catch(:coalesced) { emit_addresses(connection, @family, addresses.map { |a| Resolver::Entry.new(a["data"], a["TTL"]) }) }
         end
       end
       return if @connections.empty?

lib/httpx/resolver/native.rb

@@ -375,7 +375,9 @@ module HTTPX
           @timeouts.delete(connection.peer.host)
           @connections.delete(connection)
           Resolver.cached_lookup_set(connection.peer.host, @family, addresses) if @resolver_options[:cache]
-          catch(:coalesced) { emit_addresses(connection, @family, addresses.map { |addr| addr["data"] }) }
+          catch(:coalesced) do
+            emit_addresses(connection, @family, addresses.map { |a| Resolver::Entry.new(a["data"], a["TTL"]) })
+          end
         end
       end
       close_or_resolve

lib/httpx/resolver/resolver.rb

@@ -71,7 +71,8 @@ module HTTPX
       addresses.map! { |address| address.is_a?(Resolver::Entry) ? address : Resolver::Entry.new(address) }
 
       # double emission check, but allow early resolution to work
-      return if !early_resolve && connection.addresses && !addresses.intersect?(connection.addresses)
+      conn_addrs = connection.addresses
+      return if !early_resolve && conn_addrs && (!conn_addrs.empty? && !addresses.intersect?(!conn_addrs))
 
       log do
         "resolver #{FAMILY_TYPES[RECORD_TYPES[family]]}: " \

lib/httpx/session.rb

@@ -198,7 +198,12 @@ module HTTPX
         end
       when :closing, :closed
         connection.idling
-        select_connection(connection, selector)
+        if connection.addresses?
+          select_connection(connection, selector)
+        else
+          # if addresses expired, resolve again
+          resolve_connection(connection, selector)
+        end
       else
         pin_connection(connection, selector)
       end

test/resolver_test.rb

@@ -6,28 +6,32 @@ class ResolverTest < Minitest::Test
   include HTTPX
 
   def test_cached_lookup
+    now = Process.clock_gettime(Process::CLOCK_MONOTONIC)
     assert_ips nil, Resolver.cached_lookup("test.com")
-    dns_entry = { "data" => "::2", "TTL" => 2, "name" => "test.com" }
+    dns_entry = { "data" => "::2", "TTL" => now + 2, "name" => "test.com" }
     Resolver.cached_lookup_set("test.com", Socket::AF_INET6, [dns_entry])
     assert_ips ["::2"], Resolver.cached_lookup("test.com")
     sleep 2
     assert_ips nil, Resolver.cached_lookup("test.com")
-    alias_entry = { "alias" => "test.com", "TTL" => 2, "name" => "foo.com" }
+
+    now = Process.clock_gettime(Process::CLOCK_MONOTONIC)
+    dns_entry = { "data" => "::2", "TTL" => now + 2, "name" => "test.com" }
+    alias_entry = { "alias" => "test.com", "TTL" => now + 2, "name" => "foo.com" }
     Resolver.cached_lookup_set("test.com", Socket::AF_INET6, [dns_entry])
     Resolver.cached_lookup_set("foo.com", Socket::AF_INET6, [alias_entry])
     assert_ips ["::2"], Resolver.cached_lookup("foo.com")
 
-    Resolver.cached_lookup_set("test.com", Socket::AF_INET6, [{ "data" => "::3", "TTL" => 2, "name" => "test.com" }])
+    Resolver.cached_lookup_set("test.com", Socket::AF_INET6, [{ "data" => "::3", "TTL" => now + 2, "name" => "test.com" }])
     assert_ips %w[::2 ::3], Resolver.cached_lookup("test.com")
 
-    Resolver.cached_lookup_set("test.com", Socket::AF_INET, [{ "data" => "127.0.0.2", "TTL" => 2, "name" => "test.com" }])
+    Resolver.cached_lookup_set("test.com", Socket::AF_INET, [{ "data" => "127.0.0.2", "TTL" => now + 2, "name" => "test.com" }])
     assert_ips %w[127.0.0.2 ::2 ::3], Resolver.cached_lookup("test.com")
 
-    Resolver.cached_lookup_set("test2.com", Socket::AF_INET6, [{ "data" => "::4", "TTL" => 2, "name" => "test3.com" }])
+    Resolver.cached_lookup_set("test2.com", Socket::AF_INET6, [{ "data" => "::4", "TTL" => now + 2, "name" => "test3.com" }])
     assert_ips %w[::4], Resolver.cached_lookup("test2.com")
     assert_ips %w[::4], Resolver.cached_lookup("test3.com")
 
-    Resolver.cached_lookup_set("test2.com", Socket::AF_INET, [{ "data" => "127.0.0.3", "TTL" => 2, "name" => "test3.com" }])
+    Resolver.cached_lookup_set("test2.com", Socket::AF_INET, [{ "data" => "127.0.0.3", "TTL" => now + 2, "name" => "test3.com" }])
     assert_ips %w[127.0.0.3 ::4], Resolver.cached_lookup("test2.com")
     assert_ips %w[127.0.0.3 ::4], Resolver.cached_lookup("test3.com")
   end

test/support/requests/resolvers.rb

@@ -304,6 +304,34 @@ module Requests
             end
           end
         end
+
+        define_method :"test_resolver_#{resolver_type}_ttl_expired" do
+          start_test_servlet(TestDNSResolver, ttl: 4) do |short_ttl_dns_server|
+            nameservers = [short_ttl_dns_server.nameserver]
+
+            resolver_opts = options.merge(nameserver: nameservers)
+
+            session = HTTPX.plugin(SessionWithPool)
+
+            2.times do
+              uri = URI(build_uri("/get"))
+              response = session.head(uri, resolver_class: resolver_type, resolver_options: resolver_opts)
+              verify_status(response, 200)
+              response.close
+            end
+
+            # expire ttl
+            sleep 4
+            uri = URI(build_uri("/get"))
+            response = session.head(uri, resolver_class: resolver_type, resolver_options: resolver_opts)
+            verify_status(response, 200)
+            response.close
+
+            num_answers = short_ttl_dns_server.answers
+            assert num_answers == 2, "should have only answered 2 times for DNS queries, instead is #{num_answers}"
+          end
+        end
+
       end
     end
   end

test/support/servlets/test.rb

@@ -184,12 +184,13 @@ end
 class TestDNSResolver
   attr_reader :queries, :answers
 
-  def initialize(port = next_available_port, socket_type = :udp)
+  def initialize(port = next_available_port, socket_type = :udp, ttl: 120)
     @port = port
     @can_log = ENV.key?("HTTPX_DEBUG")
     @queries = 0
     @answers = 0
     @socket_type = socket_type
+    @ttl = ttl
   end
 
   def nameserver
@@ -264,7 +265,7 @@ class TestDNSResolver
     section << "\x00\x01".b
 
     # TTL in seconds
-    section << [120].pack("N").b
+    section << [@ttl].pack("N").b
 
     # Calculate RDATA - we need its length in advance
     rdata = if cname