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.

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