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