From 13ccab804e69a7f81358b73279d92049fc8136b4 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Tue, 23 Nov 2010 16:10:24 -0800 Subject: 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. --- ext/kcar/kcar.rl | 8 +++++++- lib/kcar/response.rb | 4 +++- test/test_parser.rb | 2 +- test/test_response.rb | 32 ++++++++++++++++++++++++++++++++ 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" -- cgit v1.2.3-24-ge0c7