From 687a544a9a1790936ecc9ff2d294c591fc631d3c Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Fri, 10 Oct 2025 11:35:50 +0100 Subject: [PATCH] fix: on altsvc processing, defer termination to when no more requests a connection may be in the middle of processing multiple requests; once an alt-svc header was received, while routing of pending data was done, the current state of parser wasn't, but the connection has been terminated and unlinked, leaving it in a state where errors may happen if relying on attachment to a selector. this fixes it by deferring termination to the point where the loop is done with the current state of the connection; it does so by storing a link to the altsvc connection. --- lib/httpx/connection.rb | 36 +++++++++++++++++++++------------ sig/connection.rbs | 1 + test/support/requests/altsvc.rb | 14 +++++++------ 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/lib/httpx/connection.rb b/lib/httpx/connection.rb index 106080dd..8631dce7 100644 --- a/lib/httpx/connection.rb +++ b/lib/httpx/connection.rb @@ -48,9 +48,9 @@ module HTTPX def initialize(uri, options) @current_session = @current_selector = - @parser = @sibling = @coalesced_connection = - @family = @io = @ssl_session = @timeout = - @connected_at = @response_received_at = nil + @parser = @sibling = @coalesced_connection = @altsvc_connection = + @family = @io = @ssl_session = @timeout = + @connected_at = @response_received_at = nil @exhausted = @cloned = @main_sibling = false @@ -86,10 +86,6 @@ module HTTPX @current_session.deselect_connection(self, @current_selector, @cloned) end - on(:altsvc) do |alt_origin, origin, alt_params| - build_altsvc_connection(alt_origin, origin, alt_params) - end - self.addresses = @options.addresses if @options.addresses end @@ -410,7 +406,11 @@ module HTTPX # * the number of pending requests # * whether the write buffer has bytes (i.e. for close handshake) if @pending.empty? && @inflight.zero? && @write_buffer.empty? - log(level: 3) { "NO MORE REQUESTS..." } + log(level: 3) { "NO MORE REQUESTS..." } if @parser && @parser.pending.any? + + # terminate if an altsvc connection has been established + terminate if @altsvc_connection + return end @@ -455,7 +455,14 @@ module HTTPX break if @state == :closing || @state == :closed # exit #consume altogether if all outstanding requests have been dealt with - return if @pending.empty? && @inflight.zero? + if @pending.empty? && @inflight.zero? && @write_buffer.empty? # rubocop:disable Style/Next + log(level: 3) { "NO MORE REQUESTS..." } if @parser && @parser.pending.any? + + # terminate if an altsvc connection has been established + terminate if @altsvc_connection + + return + end end unless ((ints = interests).nil? || ints == :w || @state == :closing) && !epiped # @@ -557,7 +564,7 @@ module HTTPX def set_parser_callbacks(parser) parser.on(:response) do |request, response| AltSvc.emit(request, response) do |alt_origin, origin, alt_params| - emit(:altsvc, alt_origin, origin, alt_params) + build_altsvc_connection(alt_origin, origin, alt_params) end @response_received_at = Utils.now @inflight -= 1 @@ -565,7 +572,7 @@ module HTTPX request.emit(:response, response) end parser.on(:altsvc) do |alt_origin, origin, alt_params| - emit(:altsvc, alt_origin, origin, alt_params) + build_altsvc_connection(alt_origin, origin, alt_params) end parser.on(:pong, &method(:send_pending)) @@ -791,6 +798,8 @@ module HTTPX # returns an HTTPX::Connection for the negotiated Alternative Service (or none). def build_altsvc_connection(alt_origin, origin, alt_params) + return if @altsvc_connection + # do not allow security downgrades on altsvc negotiation return if @origin.scheme == "https" && alt_origin.scheme != "https" @@ -808,10 +817,11 @@ module HTTPX connection.extend(AltSvc::ConnectionMixin) unless connection.is_a?(AltSvc::ConnectionMixin) - log(level: 1) { "#{origin} alt-svc: #{alt_origin}" } + @altsvc_connection = connection + + log(level: 1) { "#{origin}: alt-svc connection##{connection.object_id} established to #{alt_origin}" } connection.merge(self) - terminate rescue UnsupportedSchemeError altsvc["noop"] = true nil diff --git a/sig/connection.rbs b/sig/connection.rbs index 6aded0c5..d1119bd7 100644 --- a/sig/connection.rbs +++ b/sig/connection.rbs @@ -46,6 +46,7 @@ module HTTPX @exhausted: bool @cloned: bool @coalesced_connection: instance? + @altsvc_connection: instance? @sibling: instance? @main_sibling: bool diff --git a/test/support/requests/altsvc.rb b/test/support/requests/altsvc.rb index b5fb6cfd..2a18562c 100644 --- a/test/support/requests/altsvc.rb +++ b/test/support/requests/altsvc.rb @@ -8,12 +8,14 @@ module Requests HTTPX.plugin(SessionWithPool).wrap do |http| altsvc_uri = build_uri("/get", altsvc_origin) - response = http.get(altsvc_uri) - verify_status(response, 200) - verify_header(response.headers, "alt-svc", "h2=\"nghttp2:443\"") - response2 = http.get(altsvc_uri) - verify_status(response2, 200) - verify_no_header(response2.headers, "alt-svc") + res1, res2 = http.get(altsvc_uri, altsvc_uri) + verify_status(res1, 200) + verify_header(res1.headers, "alt-svc", "h2=\"nghttp2:443\"") + verify_status(res2, 200) + verify_header(res2.headers, "alt-svc", "h2=\"nghttp2:443\"") + res3 = http.get(altsvc_uri) + verify_status(res3, 200) + verify_no_header(res3.headers, "alt-svc") # introspection time end end