From c1ddb6fdc9420328a7afba8b20687d526143cdd5 Mon Sep 17 00:00:00 2001 From: Richard Peng Date: Thu, 21 Sep 2023 21:18:45 -0700 Subject: [PATCH] fix: close sockets on initialize timeout --- lib/http/connection.rb | 6 ++++-- lib/http/timeout/null.rb | 12 ++++++++---- spec/lib/http/connection_spec.rb | 22 +++++++++++++++++++++- 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/lib/http/connection.rb b/lib/http/connection.rb index 2f7caef3..e8779e3a 100644 --- a/lib/http/connection.rb +++ b/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 diff --git a/lib/http/timeout/null.rb b/lib/http/timeout/null.rb index a5ee19ca..ce7de212 100644 --- a/lib/http/timeout/null.rb +++ b/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) diff --git a/spec/lib/http/connection_spec.rb b/spec/lib/http/connection_spec.rb index 2ce54e3b..719cfdca 100644 --- a/spec/lib/http/connection_spec.rb +++ b/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)