Skip to content

Commit

Permalink
fix: close sockets on initialize timeout (#762)
Browse files Browse the repository at this point in the history
Co-authored-by: Richard Peng <richard@withpersona.com>
  • Loading branch information
rpeng and rpeng committed Sep 26, 2023
1 parent 4060ccd commit 3b7133c
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 7 deletions.
5 changes: 4 additions & 1 deletion lib/http/connection.rb
Expand Up @@ -46,6 +46,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)
Expand Down Expand Up @@ -126,7 +129,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
Expand Down
13 changes: 8 additions & 5 deletions lib/http/timeout/null.rb
@@ -1,15 +1,10 @@
# frozen_string_literal: true

require "forwardable"
require "io/wait"

module HTTP
module Timeout
class Null
extend Forwardable

def_delegators :@socket, :close, :closed?

attr_reader :options, :socket

def initialize(options = {})
Expand All @@ -27,6 +22,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)
Expand Down
22 changes: 21 additions & 1 deletion spec/lib/http/connection_spec.rb
Expand Up @@ -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)
Expand Down

0 comments on commit 3b7133c

Please sign in to comment.