fix: close sockets on initialize timeout

Author: rpeng

lib/http/connection.rb

@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require "forwardable"
-
 require "http/headers"
 
 module HTTP
@@ -46,6 +45,9 @@ def initialize(req, options)
       reset_timer
     rescue IOError, SocketError, SystemCallError => e
       raise ConnectionError, "failed to connect: #{e}", e.backtrace
+    rescue TimeoutError
+      close
+      raise
     end
 
     # @see (HTTP::Response::Parser#status_code)
@@ -126,7 +128,7 @@ def finish_response
     # Close the connection
     # @return [void]
     def close
-      @socket.close unless @socket.closed?
+      @socket.close unless @socket&.closed?
 
       @pending_response = false
       @pending_request  = false

lib/http/timeout/null.rb

@@ -6,10 +6,6 @@
 module HTTP
   module Timeout
     class Null
-      extend Forwardable
-
-      def_delegators :@socket, :close, :closed?
-
       attr_reader :options, :socket
 
       def initialize(options = {})
@@ -27,6 +23,14 @@ def connect_ssl
         @socket.connect
       end
 
+      def close
+        @socket&.close
+      end
+
+      def closed?
+        @socket&.closed?
+      end
+
       # Configures the SSL connection and starts the connection
       def start_tls(host, ssl_socket_class, ssl_context)
         @socket = ssl_socket_class.new(socket, ssl_context)

spec/lib/http/connection_spec.rb

@@ -8,11 +8,31 @@
       :headers => {}
     )
   end
-  let(:socket) { double(:connect => nil) }
+  let(:socket) { double(:connect => nil, :close => nil) }
   let(:timeout_class) { double(:new => socket) }
   let(:opts) { HTTP::Options.new(:timeout_class => timeout_class) }
   let(:connection) { HTTP::Connection.new(req, opts) }
 
+  describe "#initialize times out" do
+    let(:req) do
+      HTTP::Request.new(
+        :verb    => :get,
+        :uri     => "https://example.com/",
+        :headers => {}
+      )
+    end
+
+    before do
+      expect(socket).to receive(:start_tls).and_raise(HTTP::TimeoutError)
+      expect(socket).to receive(:closed?) { false }
+      expect(socket).to receive(:close)
+    end
+
+    it "closes the connection" do
+      expect { connection }.to raise_error(HTTP::TimeoutError)
+    end
+  end
+
   describe "#read_headers!" do
     before do
       connection.instance_variable_set(:@pending_response, true)