https resolver: close the http-2 connection on error

the connection was being kept open beyond the scope of the session
scope.

also, the connection wasn't deactivated after use, lingering in the
selector.
This commit is contained in:
HoneyryderChuck 2025-11-03 23:48:01 +00:00
parent 3efca5b703
commit 7ca5d9febb
4 changed files with 49 additions and 17 deletions

View File

@ -31,7 +31,7 @@ module HTTPX
}.freeze }.freeze
def_delegators :@resolver_connection, :state, :connecting?, :to_io, :call, :close, def_delegators :@resolver_connection, :state, :connecting?, :to_io, :call, :close,
:terminate, :inflight?, :handle_socket_timeout :closed?, :deactivate, :terminate, :inflight?, :handle_socket_timeout
def initialize(_, options) def initialize(_, options)
super super
@ -109,6 +109,7 @@ module HTTPX
reset_hostname(hostname) reset_hostname(hostname)
throw(:resolve_error, e) if connection.pending.empty? throw(:resolve_error, e) if connection.pending.empty?
emit_resolve_error(connection, connection.peer.host, e) emit_resolve_error(connection, connection.peer.host, e)
close_or_resolve
end end
end end
@ -118,6 +119,7 @@ module HTTPX
hostname = @requests.delete(request) hostname = @requests.delete(request)
connection = reset_hostname(hostname) connection = reset_hostname(hostname)
emit_resolve_error(connection, connection.peer.host, e) emit_resolve_error(connection, connection.peer.host, e)
close_or_resolve
else else
# @type var response: HTTPX::Response # @type var response: HTTPX::Response
parse(request, response) parse(request, response)
@ -144,6 +146,7 @@ module HTTPX
unless @queries.value?(connection) unless @queries.value?(connection)
emit_resolve_error(connection) emit_resolve_error(connection)
close_or_resolve
return return
end end
@ -153,10 +156,12 @@ module HTTPX
connection = reset_hostname(host) connection = reset_hostname(host)
emit_resolve_error(connection) emit_resolve_error(connection)
close_or_resolve
when :decode_error when :decode_error
host = @requests.delete(request) host = @requests.delete(request)
connection = reset_hostname(host) connection = reset_hostname(host)
emit_resolve_error(connection, connection.peer.host, result) emit_resolve_error(connection, connection.peer.host, result)
close_or_resolve
end end
end end
@ -166,6 +171,7 @@ module HTTPX
host = @requests.delete(request) host = @requests.delete(request)
connection = reset_hostname(host) connection = reset_hostname(host)
emit_resolve_error(connection) emit_resolve_error(connection)
close_or_resolve
return return
else else
@ -204,9 +210,7 @@ module HTTPX
catch(:coalesced) { emit_addresses(connection, @family, addresses.map { |a| Resolver::Entry.new(a["data"], a["TTL"]) }) } catch(:coalesced) { emit_addresses(connection, @family, addresses.map { |a| Resolver::Entry.new(a["data"], a["TTL"]) }) }
end end
end end
return if @connections.empty? close_or_resolve(true)
resolve
end end
def build_request(hostname) def build_request(hostname)
@ -249,5 +253,20 @@ module HTTPX
connection connection
end end
def close_or_resolve(should_deactivate = false)
# drop already closed connections
@connections.shift until @connections.empty? || @connections.first.state != :closed
if (@connections - @queries.values).empty?
if should_deactivate
deactivate
else
disconnect
end
else
resolve
end
end
end end
end end

View File

@ -40,6 +40,8 @@ module HTTPX
def decode_response_body: (Response) -> dns_decoding_response def decode_response_body: (Response) -> dns_decoding_response
def reset_hostname: (String hostname, ?reset_candidates: bool) -> Connection? def reset_hostname: (String hostname, ?reset_candidates: bool) -> Connection?
def close_or_resolve: (?bool should_deactivate) -> void
end end
end end
end end

View File

@ -58,6 +58,8 @@ module HTTPX
def emit_resolve_error: (Connection connection, ?String hostname, ?StandardError) -> void def emit_resolve_error: (Connection connection, ?String hostname, ?StandardError) -> void
def resolve_error: (String hostname, ?StandardError?) -> (ResolveError | ResolveTimeoutError) def resolve_error: (String hostname, ?StandardError?) -> (ResolveError | ResolveTimeoutError)
def close_or_resolve: () -> void
end end
end end
end end

View File

@ -123,11 +123,16 @@ module Requests
when :https when :https
define_method :"test_resolver_#{resolver_type}_get_request" do define_method :"test_resolver_#{resolver_type}_get_request" do
session = HTTPX.plugin(SessionWithPool) HTTPX.plugin(SessionWithPool).wrap do |http|
uri = build_uri("/get") uri = build_uri("/get")
response = session.head(uri, resolver_class: resolver_type, resolver_options: options.merge(use_get: true)) response = http.head(uri, resolver_class: resolver_type, resolver_options: options.merge(use_get: true))
verify_status(response, 200) verify_status(response, 200)
response.close response.close
resolver_uri = URI(options[:uri])
resolver_conn = http.pool.connections.find { |c| c.origin.to_s == resolver_uri.origin }
assert resolver_conn, "https resolver connection should still be there"
assert resolver_conn.open?, "resolver connection should be kept around open"
end
end end
define_method :"test_resolver_#{resolver_type}_unresolvable_servername" do define_method :"test_resolver_#{resolver_type}_unresolvable_servername" do
@ -188,16 +193,20 @@ module Requests
end end
define_method :"test_resolver_#{resolver_type}_no_answers" do define_method :"test_resolver_#{resolver_type}_no_answers" do
session = HTTPX.plugin(SessionWithPool) HTTPX.plugin(SessionWithPool).wrap do |http|
uri = URI(build_uri("/get")) uri = URI(build_uri("/get"))
resolver_class = Class.new(HTTPX::Resolver::HTTPS) do resolver_class = Class.new(HTTPX::Resolver::HTTPS) do
def parse_addresses(_, request) def parse_addresses(_, request)
super([], request) super([], request)
end
end end
response = http.head(uri, resolver_class: resolver_class, resolver_options: options.merge(record_types: %w[]))
verify_error_response(response, HTTPX::ResolveError)
resolver_uri = URI(options[:uri])
resolver_conn = http.pool.connections.find { |c| c.origin.to_s == resolver_uri.origin }
assert resolver_conn, "https resolver connection should still be there"
assert resolver_conn.state == :closed, "resolver connection should have closed after error"
end end
response = session.head(uri, resolver_class: resolver_class, resolver_options: options.merge(record_types: %w[]))
verify_error_response(response, HTTPX::ResolveError)
assert session.pool.connections.size == 1, "https resolver connection should still be there"
end end
when :native when :native
define_method :"test_resolver_#{resolver_type}_tcp_request" do define_method :"test_resolver_#{resolver_type}_tcp_request" do