diff --git a/CHANGELOG.md b/CHANGELOG.md index a5ca6fd..551340f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## 0.15.1 + +* Harden proxy client auth state handling to avoid stale `Proxy-Authorization` headers after proxy reconfiguration ([#43](https://github.com/mamantoha/http_proxy/pull/43)) +* Close previous client connection when reassigning proxy +* Send CONNECT `Proxy-Authorization` only when both username and password are present +* Improve `without_openssl` compatibility in proxy client TLS getter typing + ## 0.15.0 * Keep proxy configured on `HTTP::Client` reconnect ([#42](https://github.com/mamantoha/http_proxy/pull/42)). Fixes [#40](https://github.com/mamantoha/http_proxy/issues/40) diff --git a/shard.yml b/shard.yml index 7a86063..5d24366 100644 --- a/shard.yml +++ b/shard.yml @@ -1,5 +1,5 @@ name: http_proxy -version: 0.15.0 +version: 0.15.1 authors: - Theo Li diff --git a/spec/client_spec.cr b/spec/client_spec.cr index 44b4004..30c8913 100644 --- a/spec/client_spec.cr +++ b/spec/client_spec.cr @@ -1,15 +1,26 @@ require "./spec_helper" +class TrackableIO < IO::Memory + getter? closed_flag = false + + def close + @closed_flag = true + end +end + class RecordingProxyClient < HTTP::Proxy::Client getter open_calls = 0 + getter opened_ios = [] of TrackableIO - def initialize - super("127.0.0.1", 8080) + def initialize(username : String? = nil, password : String? = nil) + super("127.0.0.1", 8080, username: username, password: password) end def open(host, port, tls = nil, *, dns_timeout, connect_timeout, read_timeout, write_timeout) : IO @open_calls += 1 - IO::Memory.new + io = TrackableIO.new + @opened_ios << io + io end end @@ -17,6 +28,10 @@ class HTTP::Client def proxy_io_for_spec io end + + def apply_proxy_authorization_for_spec(request : HTTP::Request) + apply_proxy_authorization(request) + end end describe HTTP::Proxy::Client do @@ -50,6 +65,37 @@ describe HTTP::Proxy::Client do proxy_client.open_calls.should eq(2) end + it "clears proxy auth header when proxy is reconfigured without credentials" do + authenticated_proxy = RecordingProxyClient.new(username: "user", password: "password") + unauthenticated_proxy = RecordingProxyClient.new + client = HTTP::Client.new("httpbingo.org") + + client.proxy = authenticated_proxy + + request = HTTP::Request.new("GET", "/") + client.apply_proxy_authorization_for_spec(request) + request.headers["Proxy-Authorization"]?.should_not be_nil + + client.proxy = unauthenticated_proxy + + request = HTTP::Request.new("GET", "/") + client.apply_proxy_authorization_for_spec(request) + request.headers["Proxy-Authorization"]?.should be_nil + end + + it "closes previous connection when reconfiguring proxy" do + first_proxy = RecordingProxyClient.new + second_proxy = RecordingProxyClient.new + client = HTTP::Client.new("httpbingo.org") + + client.proxy = first_proxy + first_proxy.opened_ios.last.closed_flag?.should be_false + + client.proxy = second_proxy + + first_proxy.opened_ios.last.closed_flag?.should be_true + end + it "should make HTTP request" do with_proxy_server do |host, port, _username, _password, wants_close| proxy_client = HTTP::Proxy::Client.new(host, port) diff --git a/src/ext/http/client.cr b/src/ext/http/client.cr index 8095a06..005ed87 100644 --- a/src/ext/http/client.cr +++ b/src/ext/http/client.cr @@ -4,7 +4,10 @@ module HTTP class Client getter proxy : HTTP::Proxy::Client? = nil + @proxy_basic_auth_header : String? = nil + def proxy=(proxy_client : HTTP::Proxy::Client) : Nil + close if @io @proxy = proxy_client begin @@ -21,8 +24,14 @@ module HTTP raise IO::Error.new("Failed to open TCP connection to #{@host}:#{@port} (#{ex.message})") end - if proxy_client.username && proxy_client.password - proxy_basic_auth(proxy_client.username, proxy_client.password) + if username = proxy_client.username + if password = proxy_client.password + @proxy_basic_auth_header = "Basic #{Base64.strict_encode("#{username}:#{password}")}" + else + @proxy_basic_auth_header = nil + end + else + @proxy_basic_auth_header = nil end end @@ -31,15 +40,17 @@ module HTTP !!@proxy end - # Configures this client to perform proxy basic authentication in every - # request. - private def proxy_basic_auth(username : String?, password : String?) : Nil - header = "Basic #{Base64.strict_encode("#{username}:#{password}")}" - before_request do |request| + private def apply_proxy_authorization(request : HTTP::Request) : Nil + if proxy? && (header = @proxy_basic_auth_header) request.headers["Proxy-Authorization"] = header end end + def_around_exec do |request| + apply_proxy_authorization(request) + yield + end + # Keep proxy behavior across reconnects by rebuilding @io via proxy as well. private def io current_io = @io diff --git a/src/http/proxy/client.cr b/src/http/proxy/client.cr index 7857720..5580b49 100644 --- a/src/http/proxy/client.cr +++ b/src/http/proxy/client.cr @@ -19,7 +19,11 @@ module HTTP property password : String? property headers : HTTP::Headers - getter tls : OpenSSL::SSL::Context::Client? + {% if flag?(:without_openssl) %} + getter tls : Nil = nil + {% else %} + getter tls : OpenSSL::SSL::Context::Client? + {% end %} # Creates a new socket factory that tunnels via the given host and port. # The following optional arguments are supported: @@ -28,10 +32,14 @@ module HTTP # * `:username` - the user name to use when authenticating to the proxy # * `:password` - the password to use when authenticating # * `:user_agent` - the User-Agent request header - def initialize(@host, @port, *, - headers : HTTP::Headers? = nil, - @username = nil, @password = nil, - user_agent = "Crystal, HTTP::Proxy/#{HTTP::Proxy::VERSION}") + def initialize( + @host, + @port, + *, + headers : HTTP::Headers? = nil, + @username = nil, @password = nil, + user_agent = "Crystal, HTTP::Proxy/#{HTTP::Proxy::VERSION}", + ) @headers = headers || HTTP::Headers.new @headers["User-Agent"] ||= user_agent end @@ -55,10 +63,11 @@ module HTTP socket << "Host: #{host}:#{port}\r\n" - if @username - credentials = Base64.strict_encode("#{@username}:#{@password}") - credentials = "#{credentials}\n".gsub(/\s/, "") - socket << "Proxy-Authorization: Basic #{credentials}\r\n" + if username = @username + if password = @password + credentials = Base64.strict_encode("#{username}:#{password}") + socket << "Proxy-Authorization: Basic #{credentials}\r\n" + end end socket << "\r\n"