Skip to content

Commit

Permalink
Fix handling of EOF in Client#readpartial
Browse files Browse the repository at this point in the history
  • Loading branch information
ixti committed Nov 12, 2014
1 parent bb0ef37 commit e32978f
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 16 deletions.
27 changes: 20 additions & 7 deletions lib/http/client.rb
Expand Up @@ -55,11 +55,7 @@ def perform(req, options)

req.stream @socket

begin
read_more BUFFER_SIZE until @parser.headers
rescue IOError, Errno::ECONNRESET, Errno::EPIPE => ex
raise IOError, "problem making HTTP request: #{ex}"
end
read_headers!

body = Response::Body.new(self)
res = Response.new(@parser.status_code, @parser.http_version, @parser.headers, body, uri)
Expand All @@ -70,13 +66,22 @@ def perform(req, options)
end

# Read a chunk of the body
#
# @return [String] data chunk
# @return [Nil] when no more data left
def readpartial(size = BUFFER_SIZE)
return unless @socket

read_more size
begin
read_more size
finished = @parser.finished?
rescue EOFError
finished = true
end

chunk = @parser.chunk

finish_response if @parser.finished?
finish_response if finished

chunk.to_s
end
Expand Down Expand Up @@ -133,6 +138,14 @@ def make_request_body(opts, headers)
end
end

# Reads data from socket up until headers
def read_headers!
read_more BUFFER_SIZE until @parser.headers
rescue IOError, Errno::ECONNRESET, Errno::EPIPE => ex
return if ex.is_a?(EOFError) && @parser.headers
raise IOError, "problem making HTTP request: #{ex}"
end

# Callback for when we've reached the end of a response
def finish_response
@socket.close if @socket && !@socket.closed?
Expand Down
62 changes: 57 additions & 5 deletions spec/http/client_spec.rb
Expand Up @@ -158,11 +158,6 @@ def simple_response(body, status = 200)
client.get(test_endpoint).to_s
end

it 'fails on unexpected eof' do
expect { client.get("#{test_endpoint}/eof").to_s }
.to raise_error(IOError)
end

context 'with HEAD request' do
it 'does not iterates through body' do
expect(client).to_not receive(:readpartial)
Expand All @@ -175,6 +170,63 @@ def simple_response(body, status = 200)
end
end

context 'when server closes connection unexpectedly' do
before do
socket_spy = double

allow(socket_spy).to receive(:close) { nil }
allow(socket_spy).to receive(:closed?) { true }
allow(socket_spy).to receive(:readpartial) { chunks.shift.call }
allow(socket_spy).to receive(:<<) { nil }

allow(TCPSocket).to receive(:open) { socket_spy }
end

context 'during headers reading' do
let :chunks do
[
proc { "HTTP/1.1 200 OK\r\n" },
proc { "Content-Type: text/html\r" },
proc { fail EOFError }
]
end

it 'raises IOError' do
expect { client.get test_endpoint }.to raise_error IOError
end
end

context 'after headers were flushed' do
let :chunks do
[
proc { "HTTP/1.1 200 OK\r\n" },
proc { "Content-Type: text/html\r\n\r\n" },
proc { 'unexpected end of f' },
proc { fail EOFError }
]
end

it 'reads partially arrived body' do
res = client.get(test_endpoint).to_s
expect(res).to eq 'unexpected end of f'
end
end

context 'when body and headers were flushed in one chunk' do
let :chunks do
[
proc { "HTTP/1.1 200 OK\r\nContent-Type: text/html\r\n\r\nunexpected end of f" },
proc { fail EOFError }
]
end

it 'reads partially arrived body' do
res = client.get(test_endpoint).to_s
expect(res).to eq 'unexpected end of f'
end
end
end

context 'when server fully flushes response in one chunk' do
before do
socket_spy = double
Expand Down
4 changes: 0 additions & 4 deletions spec/support/example_server/servlet.rb
Expand Up @@ -74,10 +74,6 @@ def do_#{method.upcase}(req, res)
res['Location'] = "http://#{ExampleServer::ADDR}/"
end

get '/eof' do |req, _res|
req.instance_variable_get('@socket').close
end

post '/form' do |req, res|
if 'testing-form' == req.query['example']
res.status = 200
Expand Down

0 comments on commit e32978f

Please sign in to comment.