From a942f4f35af6d2764f05c3f76186b383142f7d3b Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Tue, 25 Sep 2012 18:04:37 -0700 Subject: response: propagate EOFError on remote errors When proxying remote requests, EOFError may get incorrectly discarded when a the remote server sets the Content-Length: header _and_ Connection:close to disconnect the connection. We will now raise errors correctly when Connection:close is give but we have not yet read the expected Content-Length. --- lib/kcar/response.rb | 14 ++++---------- test/test_response.rb | 29 +++++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/lib/kcar/response.rb b/lib/kcar/response.rb index dba84e6..f34971d 100644 --- a/lib/kcar/response.rb +++ b/lib/kcar/response.rb @@ -63,21 +63,14 @@ class Kcar::Response # body. It may only be called once (usually by a Rack server) as it streams # the response body off the our socket object. def each - if @parser.body_eof? - return - end + return if @parser.body_eof? if @unchunk @parser.chunked? ? each_unchunk { |x| yield x } : each_identity { |x| yield x } else - if @parser.keepalive? - @parser.chunked? ? each_rechunk { |x| yield x } : - each_identity { |x| yield x } - else - each_until_eof { |x| yield x } # fastest path - end + @parser.chunked? ? each_rechunk { |x| yield x } : + each_identity { |x| yield x } end - rescue EOFError end # :stopdoc: @@ -123,6 +116,7 @@ class Kcar::Response begin yield dst end while @sock.readpartial(READ_SIZE, dst) + rescue EOFError end def each_identity diff --git a/test/test_response.rb b/test/test_response.rb index a76c7c2..3feb5e9 100644 --- a/test/test_response.rb +++ b/test/test_response.rb @@ -485,7 +485,7 @@ class TestSession < Test::Unit::TestCase assert ! @c.closed? end - def test_rack_preserve_chunk_ary_no_keepalive + def test_rack_preserve_chunk_no_keepalive pid = fork do @s << "HTTP/1.1 200 OK\r\n" @s << "Connection: close\r\n" @@ -501,11 +501,36 @@ class TestSession < Test::Unit::TestCase tmp = [] assert ! body.parser.keepalive? assert_nothing_raised { body.each { |chunk| tmp << chunk.dup } } - assert_equal ["5\r\nhello\r\n0\r\nFoo: bar\r\n\r\n"], tmp + assert_equal "5\r\nhello\r\n0\r\nFoo: bar\r\n\r\n", tmp.join _, status = Process.waitpid2(pid) assert status.success? body.close assert @c.closed? end + def test_rack_preserve_chunk_no_keepalive + s = "HTTP/1.1 200 OK\r\n" + s << "Connection: close\r\n" + s << "Content-Length: 666\r\n" + s << "\r\n" + s << "hello" + @s.write(s) + @response = Kcar::Response.new(@c, []) + status, headers, body = @response.rack + assert_kind_of Array, headers + assert_equal status, "200 OK" + tmp = [] + assert ! body.parser.keepalive? + + closer = Thread.new do + Thread.pass until tmp[0] + @s.close + end + assert_raises(EOFError) { + body.each { |chunk| tmp << chunk.dup } + } + assert_nil @c.close + assert_equal "hello", tmp[0] + assert_nil closer.value + end end -- cgit v1.2.3-24-ge0c7