From 56988e4ac35e3f04af52ec9461d72d3b0691d383 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Thu, 1 Oct 2026 15:24:16 +0200 Subject: [PATCH] Stop reusing connections after a proxy refuses CONNECT A proxy can refuse a CONNECT tunnel with a keep-alive response, such as a 407 with a Content-Length body. Connection records the failed tunnel and the client skips sending the request, but the connection stayed keep-alive, so a persistent client reused it. The flag is never cleared, so the next request skipped sending again and returned a response read from the same socket without contacting the proxy: the old refusal if its body was still unread, or a response with status 0 once it had been read, whose body read then blocks until the read timeout. The socket can't carry another request, since the proxy never opened the tunnel. Mark the connection as not keep-alive when CONNECT fails. The client then closes it before the next request, and finish_response closes it once the refusal's body has been read, so every request asks the proxy again on a new connection. AuthProxyServer closes the socket after its 407, so no test kept a refused tunnel open. RefusingProxyServer answers every CONNECT with a keep-alive 407 and counts connections. --- CHANGELOG.md | 5 +++++ lib/http/connection/internals.rb | 1 + test/http/connection_test.rb | 19 +++++++++++++++++++ test/http_test.rb | 32 ++++++++++++++++++++++++++++++++ test/support/proxy_server.rb | 32 ++++++++++++++++++++++++++++++++ 5 files changed, 89 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 954aa1ae..6790707f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- A connection whose proxy refused the `CONNECT` tunnel is no longer kept + alive. The next request on a persistent client returned the old refusal + without contacting the proxy, or a response with status 0 once the + refusal's body had been read. It now opens a new connection and asks the + proxy again. - Building a default `Host` header now raises `HTTP::RequestError` when the request URI has a nil host (previously `NoMethodError`) or an empty host (e.g. `https:///path` or `https://:123/path`, which previously produced diff --git a/lib/http/connection/internals.rb b/lib/http/connection/internals.rb index 3cb662eb..72b2470b 100644 --- a/lib/http/connection/internals.rb +++ b/lib/http/connection/internals.rb @@ -76,6 +76,7 @@ def handle_proxy_connect_response if @parser.status_code != 200 @failed_proxy_connect = true + @keep_alive = false return end diff --git a/test/http/connection_test.rb b/test/http/connection_test.rb index 8aae7f61..bf91fecd 100644 --- a/test/http/connection_test.rb +++ b/test/http/connection_test.rb @@ -1101,6 +1101,25 @@ def test_proxy_connect_non_200_marks_failed_and_stores_headers assert conn.instance_variable_get(:@pending_response) end + def test_proxy_connect_non_200_is_not_kept_alive + proxy_req = build_req( + uri: "https://example.com/", + proxy: { proxy_address: "proxy.example.com", proxy_port: 8080 } + ) + proxy_socket = fake( + connect: nil, + close: nil, + closed?: false, + write: lambda(&:bytesize), + readpartial: "HTTP/1.1 407 Proxy Authentication Required\r\nContent-Length: 0\r\n\r\n", + start_tls: ->(*) {} + ) + proxy_opts = HTTP::Options.new(timeout_class: fake(new: proxy_socket), persistent: "https://example.com") + conn = HTTP::Connection.new(proxy_req, proxy_opts) + + assert_same false, conn.keep_alive? + end + def test_proxy_connect_200_completes_successfully_and_resets_parser proxy_req = build_req( uri: "https://example.com/", diff --git a/test/http_test.rb b/test/http_test.rb index 7cea9c9e..b6d130b1 100644 --- a/test/http_test.rb +++ b/test/http_test.rb @@ -816,3 +816,35 @@ def test_auth_proxy_ssl_responds_with_407_if_no_credentials assert_equal 407, response.status.to_i end end + +class HTTPViaRefusingProxyTest < Minitest::Test + run_server(:dummy_ssl) { DummyServer.new(ssl: true) } + run_server(:proxy) { RefusingProxyServer.new } + + def setup + super + @session = HTTP.timeout(5).via(proxy.addr, proxy.port).persistent(dummy_ssl.endpoint) + end + + def teardown + @session.close + super + end + + def get + @session.get(dummy_ssl.endpoint, ssl_context: SSLHelper.client_context) + end + + def test_persistent_client_retries_refused_tunnel_on_new_connection + statuses = Array.new(2) { get.status.to_i } + + assert_equal [407, 407], statuses + assert_equal 2, proxy.connections + end + + def test_persistent_client_retries_refused_tunnel_after_reading_refusal + assert_equal "deny", get.to_s + assert_equal 407, get.status.to_i + assert_equal 2, proxy.connections + end +end diff --git a/test/support/proxy_server.rb b/test/support/proxy_server.rb index b4f0dfa6..f6616f86 100644 --- a/test/support/proxy_server.rb +++ b/test/support/proxy_server.rb @@ -205,3 +205,35 @@ def authenticate(headers) "\r\n" end end + +class RefusingProxyServer < ProxyServer + def initialize + super + @connections = Queue.new + end + + def connections + @connections.size + end + + def reset + @connections.clear + end + + private + + def handle_request(client) + @connections << true + while read_proxy_request(client) + client.write "HTTP/1.1 407 Proxy Authentication Required\r\n" \ + "Proxy-Authenticate: Basic realm=\"proxy\"\r\n" \ + "Content-Length: 4\r\n" \ + "\r\n" \ + "deny" + end + rescue IOError, SystemCallError + nil + ensure + client.close rescue nil + end +end