From c98d568e070619c245492387db8eaca9a2f0d41e Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Mon, 30 Nov 2020 17:54:57 +0000 Subject: [PATCH] fixed some bugs from the https resolver while improving coverage * it now fails hard if the dns server is a domain and it's not resolvable; * it properly handles retries for other record types (p.ex. when A query fails, and AAAA follows); --- lib/httpx/resolver/https.rb | 29 +++++++-------------- test/support/requests/resolvers.rb | 41 +++++++++++++++++++++++++++--- 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/lib/httpx/resolver/https.rb b/lib/httpx/resolver/https.rb index 688535b8..4402741d 100644 --- a/lib/httpx/resolver/https.rb +++ b/lib/httpx/resolver/https.rb @@ -21,6 +21,7 @@ module HTTPX DEFAULTS = { uri: NAMESERVER, use_get: false, + record_types: RECORD_TYPES.keys, }.freeze def_delegator :@connections, :empty? @@ -30,7 +31,7 @@ module HTTPX def initialize(options) @options = Options.new(options) @resolver_options = Resolver::Options.new(DEFAULTS.merge(@options.resolver_options || {})) - @_record_types = Hash.new { |types, host| types[host] = RECORD_TYPES.keys.dup } + @_record_types = Hash.new { |types, host| types[host] = @resolver_options.record_types.dup } @queries = {} @requests = {} @connections = [] @@ -43,13 +44,9 @@ module HTTPX @uri_addresses ||= Resolv.getaddresses(@uri.host) - if @uri_addresses.empty? - ex = ResolveError.new("Can't resolve #{connection.origin.host}") - ex.set_backtrace(caller) - emit(:error, connection, ex) - else - early_resolve(connection) || resolve(connection) - end + raise ResolveError, "Can't resolve DNS server #{@uri.host}" if @uri_addresses.empty? + + early_resolve(connection) || resolve(connection) end def timeout @@ -70,12 +67,6 @@ module HTTPX private - def connect - return if @queries.empty? - - resolver_connection.__send__(__method__) - end - def pool Thread.current[:httpx_connection_pool] ||= Pool.new end @@ -104,6 +95,8 @@ module HTTPX log { "resolver: query #{type} for #{hostname}" } begin request = build_request(hostname, type) + request.on(:response, &method(:on_response).curry[request]) + request.on(:promise, &method(:on_promise)) @requests[request] = connection resolver_connection.send(request) @queries[hostname] = connection @@ -118,9 +111,7 @@ module HTTPX rescue StandardError => e connection = @requests[request] hostname = @queries.key(connection) - error = ResolveError.new("Can't resolve #{hostname}: #{e.message}") - error.set_backtrace(e.backtrace) - emit(:error, connection, error) + emit_resolve_error(connection, hostname, e) else parse(response) ensure @@ -143,7 +134,7 @@ module HTTPX return end end - if answers.empty? + if answers.nil? || answers.empty? host, connection = @queries.first @_record_types[host].shift if @_record_types[host].empty? @@ -202,8 +193,6 @@ module HTTPX request.headers["content-type"] = "application/dns-message" end request.headers["accept"] = "application/dns-message" - request.on(:response, &method(:on_response).curry[request]) - request.on(:promise, &method(:on_promise)) request end diff --git a/test/support/requests/resolvers.rb b/test/support/requests/resolvers.rb index 42d54041..84ae150f 100644 --- a/test/support/requests/resolvers.rb +++ b/test/support/requests/resolvers.rb @@ -7,7 +7,7 @@ module Requests system: { cache: false }, https: { uri: ENV["HTTPX_RESOLVER_URI"], cache: false }, }.each do |resolver, options| - define_method :"test_multiple_#{resolver}_resolver_errors" do + define_method :"test_resolver_#{resolver}_multiple_errors" do 2.times do |i| session = HTTPX.plugin(SessionWithPool) unknown_uri = "http://www.sfjewjfwigiewpgwwg-native-#{i}.com" @@ -17,7 +17,7 @@ module Requests end end - define_method :"test_#{resolver}_resolver_request" do + define_method :"test_resolver_#{resolver}_request" do session = HTTPX.plugin(SessionWithPool) uri = build_uri("/get") response = session.head(uri, resolver_class: resolver, resolver_options: options) @@ -27,13 +27,48 @@ module Requests next unless resolver == :https - define_method :"test_#{resolver}_resolver_get_request" do + define_method :"test_resolver_#{resolver}_get_request" do session = HTTPX.plugin(SessionWithPool) uri = build_uri("/get") response = session.head(uri, resolver_class: resolver, resolver_options: options.merge(use_get: true)) verify_status(response, 200) response.close end + + define_method :"test_resolver_#{resolver}_unresolvable_servername" do + session = HTTPX.plugin(SessionWithPool) + uri = build_uri("/get") + ex = assert_raises(HTTPX::ResolveError) do + session.head(uri, resolver_class: resolver, resolver_options: options.merge(uri: "https://unexisting-doh/dns-query")) + end + assert ex.message =~ /Can't resolve DNS server/ + end + + define_method :"test_resolver_#{resolver}_server_error" do + session = HTTPX.plugin(SessionWithPool) + uri = URI(build_uri("/get")) + resolver_class = Class.new(HTTPX::Resolver::HTTPS) do + def build_request(_hostname, _type) + @options.request_class.new("POST", @uri) + end + end + response = session.head(uri, resolver_class: resolver_class, resolver_options: options) + assert response.is_a?(HTTPX::ErrorResponse), "should be a response error" + assert response.error.is_a?(HTTPX::ResolveError), "should be a resolving error" + end + + define_method :"test_resolver_#{resolver}_decoding_error" do + session = HTTPX.plugin(SessionWithPool) + uri = URI(build_uri("/get")) + resolver_class = Class.new(HTTPX::Resolver::HTTPS) do + def decode_response_body(_response) + raise Resolv::DNS::DecodeError + end + end + response = session.head(uri, resolver_class: resolver_class, resolver_options: options.merge(record_types: %w[A])) + assert response.is_a?(HTTPX::ErrorResponse), "should be a response error" + assert response.error.is_a?(HTTPX::ResolveError), "should be a resolving error" + end end end end