Harden proxy client reconnect and auth behavior

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)

shard.yml

@@ -1,5 +1,5 @@
 name: http_proxy
-version: 0.15.0
+version: 0.15.1
 
 authors:
   - Theo Li <bbtfrr@gmail.com>

spec/client_spec.cr

@@ -1,22 +1,37 @@
 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
 
 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)

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

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"