header/body-less pipelined HTTP/1.1 responses handled properly

Some HTTP servers do not send any response headers
for errors, only the HTTP status line.

4 files changed, 43 insertions, 3 deletions

diff --git a/ext/kcar/kcar.rl b/ext/kcar/kcar.rl
index 5f2a1c1..b86f8e1 100644
--- a/ext/kcar/kcar.rl
+++ b/ext/kcar/kcar.rl
@@ -42,6 +42,7 @@ DEF_MAX_LENGTH(REASON, 256);
 #define UH_FL_INTRAILER 0x10
 #define UH_FL_INCHUNK  0x20
 #define UH_FL_KEEPALIVE 0x40
+#define UH_FL_HASHEADER 0x80
 
 struct http_parser {
   int cs; /* Ragel internal state */
@@ -178,6 +179,8 @@ static void write_value(VALUE hdr, struct http_parser *hp,
   const char *vptr;
   long vlen;
 
+  HP_FL_SET(hp, HASHEADER);
+
   /* Rack does not like Status headers, so we never send them */
   if (CSTR_CASE_EQ(fptr, flen, "status")) {
     hp->cont = Qnil;
@@ -534,6 +537,9 @@ static VALUE body_eof(VALUE self)
 {
   struct http_parser *hp = data_get(self);
 
+  if (!HP_FL_TEST(hp, HASHEADER) && HP_FL_ALL(hp, KEEPALIVE))
+    return Qtrue;
+
   if (HP_FL_TEST(hp, CHUNKED))
     return chunked_eof(hp) ? Qtrue : Qfalse;
 
@@ -560,7 +566,7 @@ static VALUE keepalive(VALUE self)
   struct http_parser *hp = data_get(self);
 
   if (HP_FL_ALL(hp, KEEPALIVE)) {
-    if ( HP_FL_TEST(hp, HASBODY) ) {
+    if (HP_FL_TEST(hp, HASHEADER) && HP_FL_TEST(hp, HASBODY) ) {
       if (HP_FL_TEST(hp, CHUNKED) || (hp->len.content >= 0))
         return Qtrue;
 
diff --git a/lib/kcar/response.rb b/lib/kcar/response.rb
index 6f151e5..67e0286 100644
--- a/lib/kcar/response.rb
+++ b/lib/kcar/response.rb
@@ -62,7 +62,9 @@ class Response < Struct.new(:sock, :hdr, :unchunk, :buf, :parser)
   # 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(&block)
-    return if parser.body_eof?
+    if parser.body_eof?
+      return
+    end
     if unchunk
       parser.chunked? ? each_unchunk(&block) : each_identity(&block)
     else
diff --git a/test/test_parser.rb b/test/test_parser.rb
index 40c6e01..7eab684 100644
--- a/test/test_parser.rb
+++ b/test/test_parser.rb
@@ -32,7 +32,7 @@ class TestParser < Test::Unit::TestCase
     response = @hp.headers(hdr, buf)
     assert_equal(["200 OK", hdr], response)
     assert hdr.empty?
-    assert ! @hp.keepalive? # no content-length
+    assert @hp.keepalive? # no content-length
   end
 
   def test_parser_status_with_content_length
diff --git a/test/test_response.rb b/test/test_response.rb
index 8eb0736..d97fea8 100644
--- a/test/test_response.rb
+++ b/test/test_response.rb
@@ -4,12 +4,44 @@ require 'pp'
 require 'socket'
 require 'kcar'
 require 'digest/sha1'
+$stderr.sync = true
 
 class TestSession < Test::Unit::TestCase
   def setup
     @s, @c = UNIXSocket.pair
   end
 
+  def test_http_status_only_pipelined
+    resp = "HTTP/1.1 404 Not Found\r\n\r\n" \
+           "HTTP/1.1 404 Not Found\r\n\r\n"
+    pid = fork do
+      @s << resp
+      @s.close
+    end
+
+    @s.close
+    @response = Kcar::Response.new(@c)
+    status, headers, body = @response.rack
+    assert_equal status, "404 Not Found"
+    assert_equal({}, headers)
+    tmp = []
+    assert_nothing_raised { body.each { |chunk| tmp << chunk.dup } }
+    assert_equal [], tmp
+    assert @response.parser.keepalive?
+    body.close
+
+    status, headers, body = @response.rack
+    assert_equal status, "404 Not Found"
+    assert_equal({},headers)
+    tmp = []
+    assert_nothing_raised { body.each { |chunk| tmp << chunk.dup } }
+    assert_equal [], tmp
+
+    _, status = Process.waitpid2(pid)
+    assert status.success?
+    body.close
+  end
+
   def test_http_small_pipelined_identity
     resp = "HTTP/1.1 200 OK\r\nContent-Length: 12\r\n\r\nhello world\n" \
            "HTTP/1.1 200 OK\r\nContent-Length: 14\r\n\r\ngoodbye world\n"