From 7f9e68ba3b6787c4d3af919684177e18c661bae4 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Thu, 26 Nov 2020 17:13:02 +0000 Subject: [PATCH] fixed: session hangs when HTTP/2 receives a 421 While adding a test, a bug was found, where an HTTP/2 connection receiving a 421 response wasn't redirecting the request well to an HTTP/1.1 one. This has been fixed by changing the way a new connection is instantiated. --- lib/httpx/connection.rb | 17 +++-------------- lib/httpx/session.rb | 12 +++++++++--- test/https_test.rb | 11 +++++++++++ 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/lib/httpx/connection.rb b/lib/httpx/connection.rb index d12bd7bc..04267d4d 100644 --- a/lib/httpx/connection.rb +++ b/lib/httpx/connection.rb @@ -120,8 +120,8 @@ module HTTPX end end - def create_idle - self.class.new(@type, @origin, @options) + def create_idle(options = {}) + self.class.new(@type, @origin, @options.merge(options)) end def merge(connection) @@ -131,17 +131,6 @@ module HTTPX end end - def unmerge(connection) - @origins -= connection.instance_variable_get(:@origins) - purge_pending do |request| - request.uri.origin == connection.origin && begin - request.transition(:idle) - connection.send(request) - true - end - end - end - def purge_pending(&block) pendings = [] if @parser @@ -399,7 +388,7 @@ module HTTPX parser.on(:error) do |request, ex| case ex when MisdirectedRequestError - emit(:uncoalesce, request.uri) + emit(:misdirected, request) else response = ErrorResponse.new(request, ex, @options) request.emit(:response, response) diff --git a/lib/httpx/session.rb b/lib/httpx/session.rb index 1291be69..1a245a59 100644 --- a/lib/httpx/session.rb +++ b/lib/httpx/session.rb @@ -77,10 +77,16 @@ module HTTPX end def set_connection_callbacks(connection, connections, options) - connection.on(:uncoalesce) do |uncoalesced_uri| - other_connection = build_connection(uncoalesced_uri, options) + connection.on(:misdirected) do |misdirected_request| + other_connection = connection.create_idle(ssl: { alpn_protocols: %w[http/1.1] }) + other_connection.merge(connection) + catch(:coalesced) do + pool.init_connection(other_connection, options) + end + set_connection_callbacks(other_connection, connections, options) connections << other_connection - connection.unmerge(other_connection) + misdirected_request.transition(:idle) + other_connection.send(misdirected_request) end connection.on(:altsvc) do |alt_origin, origin, alt_params| other_connection = build_altsvc_connection(connection, connections, alt_origin, origin, alt_params, options) diff --git a/test/https_test.rb b/test/https_test.rb index db77afb2..4fcc7a7f 100644 --- a/test/https_test.rb +++ b/test/https_test.rb @@ -85,6 +85,17 @@ class HTTPSTest < Minitest::Test end end + def test_http2_uncoalesce_on_misdirected + uri = build_uri("/status/421") + HTTPX.plugin(SessionWithPool).wrap do |http| + response = http.get(uri) + verify_status(response, 421) + connection_count = http.pool.connection_count + assert connection_count == 2, "expected to have 2 connections, instead have #{connection_count}" + assert response.version == "1.1", "request should have been retried with HTTP/1.1" + end + end + private def origin(orig = httpbin)