Stop reusing connections after a proxy refuses CONNECT

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

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
 

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/",

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

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