From 7987e1a4001491f8a494f3926037f8cbee713263 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Fri, 3 Sep 2010 01:48:24 +0000 Subject: enable HTTP keepalive support for all methods Yes, this means even POST/PUT bodies may be kept alive, but only if the body (and trailers) are fully-consumed. --- ext/unicorn_http/global_variables.h | 4 --- ext/unicorn_http/unicorn_http.rl | 65 +++++++++++++++---------------------- test/unit/test_http_parser.rb | 16 +++++++-- test/unit/test_http_parser_ng.rb | 37 +++++++++++++++------ 4 files changed, 67 insertions(+), 55 deletions(-) diff --git a/ext/unicorn_http/global_variables.h b/ext/unicorn_http/global_variables.h index 8377704..274f456 100644 --- a/ext/unicorn_http/global_variables.h +++ b/ext/unicorn_http/global_variables.h @@ -26,8 +26,6 @@ static VALUE g_http; static VALUE g_http_09; static VALUE g_http_10; static VALUE g_http_11; -static VALUE g_GET; -static VALUE g_HEAD; /** Defines common length and error messages for input length validation. */ #define DEF_MAX_LENGTH(N, length) \ @@ -82,8 +80,6 @@ static void init_globals(void) DEF_GLOBAL(http_11, "HTTP/1.1"); DEF_GLOBAL(http_10, "HTTP/1.0"); DEF_GLOBAL(http_09, "HTTP/0.9"); - DEF_GLOBAL(GET, "GET"); - DEF_GLOBAL(HEAD, "HEAD"); } #undef DEF_GLOBAL diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl index 236fbaa..fb99f9c 100644 --- a/ext/unicorn_http/unicorn_http.rl +++ b/ext/unicorn_http/unicorn_http.rl @@ -18,12 +18,12 @@ #define UH_FL_HASTRAILER 0x8 #define UH_FL_INTRAILER 0x10 #define UH_FL_INCHUNK 0x20 -#define UH_FL_KAMETHOD 0x40 +#define UH_FL_REQEOF 0x40 #define UH_FL_KAVERSION 0x80 #define UH_FL_HASHEADER 0x100 -/* both of these flags need to be set for keepalive to be supported */ -#define UH_FL_KEEPALIVE (UH_FL_KAMETHOD | UH_FL_KAVERSION) +/* all of these flags need to be set for keepalive to be supported */ +#define UH_FL_KEEPALIVE (UH_FL_KAVERSION | UH_FL_REQEOF | UH_FL_HASHEADER) /* keep this small for Rainbows! since every client has one */ struct http_parser { @@ -79,46 +79,29 @@ static void parser_error(const char *msg) */ static void hp_keepalive_connection(struct http_parser *hp, VALUE val) { - /* REQUEST_METHOD is always set before any headers */ - if (HP_FL_TEST(hp, KAMETHOD)) { - if (STR_CSTR_CASE_EQ(val, "keep-alive")) { - /* basically have HTTP/1.0 masquerade as HTTP/1.1+ */ - HP_FL_SET(hp, KAVERSION); - } else if (STR_CSTR_CASE_EQ(val, "close")) { - /* - * it doesn't matter what HTTP version or request method we have, - * if a client says "Connection: close", we disable keepalive - */ - HP_FL_UNSET(hp, KEEPALIVE); - } else { - /* - * client could've sent anything, ignore it for now. Maybe - * "HP_FL_UNSET(hp, KEEPALIVE);" just in case? - * Raising an exception might be too mean... - */ - } + if (STR_CSTR_CASE_EQ(val, "keep-alive")) { + /* basically have HTTP/1.0 masquerade as HTTP/1.1+ */ + HP_FL_SET(hp, KAVERSION); + } else if (STR_CSTR_CASE_EQ(val, "close")) { + /* + * it doesn't matter what HTTP version or request method we have, + * if a client says "Connection: close", we disable keepalive + */ + HP_FL_UNSET(hp, KAVERSION); + } else { + /* + * client could've sent anything, ignore it for now. Maybe + * "HP_FL_UNSET(hp, KAVERSION);" just in case? + * Raising an exception might be too mean... + */ } } static void request_method(struct http_parser *hp, const char *ptr, size_t len) { - VALUE v; + VALUE v = rb_str_new(ptr, len); - /* - * we only support keepalive for GET and HEAD requests for now other - * methods are too rarely seen to be worth optimizing. POST is unsafe - * since some clients send extra bytes after POST bodies. - */ - if (CONST_MEM_EQ("GET", ptr, len)) { - HP_FL_SET(hp, KAMETHOD); - v = g_GET; - } else if (CONST_MEM_EQ("HEAD", ptr, len)) { - HP_FL_SET(hp, KAMETHOD); - v = g_HEAD; - } else { - v = rb_str_new(ptr, len); - } rb_hash_aset(hp->env, g_request_method, v); } @@ -206,7 +189,8 @@ static void write_value(struct http_parser *hp, hp->len.content = parse_length(RSTRING_PTR(v), RSTRING_LEN(v)); if (hp->len.content < 0) parser_error("invalid Content-Length"); - HP_FL_SET(hp, HASBODY); + if (hp->len.content != 0) + HP_FL_SET(hp, HASBODY); hp_invalid_if_trailer(hp); } else if (f == g_http_transfer_encoding) { if (STR_CSTR_CASE_EQ(v, "chunked")) { @@ -305,6 +289,7 @@ static void write_value(struct http_parser *hp, if (HP_FL_TEST(hp, CHUNKED)) cs = http_parser_en_ChunkedBody; } else { + HP_FL_SET(hp, REQEOF); assert(!HP_FL_TEST(hp, CHUNKED) && "chunked encoding without body!"); } /* @@ -559,6 +544,8 @@ static VALUE HttpParser_parse(VALUE self) hp->cs == http_parser_en_ChunkedBody) { advance_str(data, hp->offset + 1); hp->offset = 0; + if (HP_FL_TEST(hp, INTRAILER)) + HP_FL_SET(hp, REQEOF); return hp->env; } @@ -710,8 +697,10 @@ static VALUE HttpParser_filter_body(VALUE self, VALUE buf, VALUE data) memcpy(RSTRING_PTR(buf), dptr, nr); hp->len.content -= nr; - if (hp->len.content == 0) + if (hp->len.content == 0) { + HP_FL_SET(hp, REQEOF); hp->cs = http_parser_first_final; + } advance_str(data, nr); rb_str_set_len(buf, nr); data = Qnil; diff --git a/test/unit/test_http_parser.rb b/test/unit/test_http_parser.rb index 222c227..31cb2cb 100644 --- a/test/unit/test_http_parser.rb +++ b/test/unit/test_http_parser.rb @@ -76,12 +76,22 @@ class HttpParserTest < Test::Unit::TestCase assert parser.keepalive? end - def test_connection_keep_alive_ka_bad_method + def test_connection_keep_alive_no_body parser = HttpParser.new req = {} tmp = "POST / HTTP/1.1\r\nConnection: keep-alive\r\n\r\n" assert_equal req.object_id, parser.headers(req, tmp).object_id - assert ! parser.keepalive? + assert parser.keepalive? + end + + def test_connection_keep_alive_no_body_empty + parser = HttpParser.new + req = {} + tmp = "POST / HTTP/1.1\r\n" \ + "Content-Length: 0\r\n" \ + "Connection: keep-alive\r\n\r\n" + assert_equal req.object_id, parser.headers(req, tmp).object_id + assert parser.keepalive? end def test_connection_keep_alive_ka_bad_version @@ -461,7 +471,7 @@ class HttpParserTest < Test::Unit::TestCase assert_equal 'page=1', req['QUERY_STRING'] assert_equal "", s assert_equal m, req['REQUEST_METHOD'] - assert ! parser.keepalive? # TODO: read HTTP/1.2 when it's final + assert parser.keepalive? # TODO: read HTTP/1.2 when it's final } end diff --git a/test/unit/test_http_parser_ng.rb b/test/unit/test_http_parser_ng.rb index 65b843e..699ee64 100644 --- a/test/unit/test_http_parser_ng.rb +++ b/test/unit/test_http_parser_ng.rb @@ -27,6 +27,12 @@ class HttpParserNgTest < Test::Unit::TestCase assert ! @parser.keepalive? assert @parser.headers? assert_equal 123, @parser.content_length + dst = "" + buf = '.' * 123 + @parser.filter_body(dst, buf) + assert_equal '.' * 123, dst + assert_equal "", buf + assert @parser.keepalive? end def test_identity_step_headers @@ -41,6 +47,12 @@ class HttpParserNgTest < Test::Unit::TestCase assert_equal 0, str.size assert ! @parser.keepalive? assert @parser.headers? + dst = "" + buf = '.' * 123 + @parser.filter_body(dst, buf) + assert_equal '.' * 123, dst + assert_equal "", buf + assert @parser.keepalive? end def test_identity_oneshot_header @@ -50,6 +62,12 @@ class HttpParserNgTest < Test::Unit::TestCase assert_equal '123', req['CONTENT_LENGTH'] assert_equal 0, str.size assert ! @parser.keepalive? + assert @parser.headers? + dst = "" + buf = '.' * 123 + @parser.filter_body(dst, buf) + assert_equal '.' * 123, dst + assert_equal "", buf end def test_identity_oneshot_header_with_body @@ -67,7 +85,7 @@ class HttpParserNgTest < Test::Unit::TestCase assert_equal 0, str.size assert_equal tmp, body assert_equal "", @parser.filter_body(tmp, str) - assert ! @parser.keepalive? + assert @parser.keepalive? end def test_identity_oneshot_header_with_body_partial @@ -85,7 +103,7 @@ class HttpParserNgTest < Test::Unit::TestCase assert_nil rv assert_equal "", str assert_equal str.object_id, @parser.filter_body(tmp, str).object_id - assert ! @parser.keepalive? + assert @parser.keepalive? end def test_identity_oneshot_header_with_body_slop @@ -99,7 +117,7 @@ class HttpParserNgTest < Test::Unit::TestCase assert_equal "G", @parser.filter_body(tmp, str) assert_equal 1, tmp.size assert_equal "a", tmp - assert ! @parser.keepalive? + assert @parser.keepalive? end def test_chunked @@ -122,6 +140,10 @@ class HttpParserNgTest < Test::Unit::TestCase assert_equal rv.object_id, @parser.filter_body(tmp, rv).object_id assert_equal "PUT", rv assert ! @parser.keepalive? + rv << "TY: FOO\r\n\r\n" + assert_equal req, @parser.trailers(req, rv) + assert_equal "FOO", req["HTTP_PUTTY"] + assert @parser.keepalive? end def test_two_chunks @@ -177,7 +199,7 @@ class HttpParserNgTest < Test::Unit::TestCase assert_equal req, @parser.trailers(req, moo = "\r\n") assert_equal "", moo assert @parser.body_eof? - assert ! @parser.keepalive? + assert @parser.keepalive? end def test_two_chunks_oneshot @@ -237,7 +259,7 @@ class HttpParserNgTest < Test::Unit::TestCase assert_nil @parser.trailers(req, str << "\r") assert_equal req, @parser.trailers(req, str << "\nGET / ") assert_equal "GET / ", str - assert ! @parser.keepalive? + assert @parser.keepalive? end def test_trailers_slowly @@ -297,14 +319,12 @@ class HttpParserNgTest < Test::Unit::TestCase assert_equal req, @parser.headers(req, str) assert_nil @parser.content_length assert_raise(HttpParserError) { @parser.filter_body('', str) } - assert ! @parser.keepalive? end def test_overflow_content_length n = HttpParser::LENGTH_MAX + 1 str = "PUT / HTTP/1.1\r\nContent-Length: #{n}\r\n\r\n" assert_raise(HttpParserError) { @parser.headers({}, str) } - assert ! @parser.keepalive? end def test_bad_chunk @@ -315,13 +335,11 @@ class HttpParserNgTest < Test::Unit::TestCase assert_equal req, @parser.headers(req, str) assert_nil @parser.content_length assert_raise(HttpParserError) { @parser.filter_body('', str) } - assert ! @parser.keepalive? end def test_bad_content_length str = "PUT / HTTP/1.1\r\nContent-Length: 7ff\r\n\r\n" assert_raise(HttpParserError) { @parser.headers({}, str) } - assert ! @parser.keepalive? end def test_bad_trailers @@ -338,7 +356,6 @@ class HttpParserNgTest < Test::Unit::TestCase assert_equal '', str str << "Transfer-Encoding: identity\r\n\r\n" assert_raise(HttpParserError) { @parser.trailers(req, str) } - assert ! @parser.keepalive? end def test_repeat_headers -- cgit v1.2.3-24-ge0c7