From 2c9cbe383ce8049707eda87c632855995a65e032 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 4 Nov 2009 23:00:49 -0800 Subject: http: allow headers/trailers to be written byte-wise This allows clients to trickle headers and trailers. While Unicorn itself does not support slow clients for many reasons, this affects servers that depend on our parser like Rainbows!. This actually does affect Unicorn when handling trailers, but HTTP trailers are very ever rarely used in requests. Fortunately this stupid bug does not seem able to trigger out-of-bounds conditions. --- ext/unicorn_http/unicorn_http.rl | 23 ++++++++++---------- test/unit/test_http_parser_ng.rb | 46 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 11 deletions(-) diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl index 5cacb37..6232e2c 100644 --- a/ext/unicorn_http/unicorn_http.rl +++ b/ext/unicorn_http/unicorn_http.rl @@ -25,14 +25,15 @@ /* both of these flags need to be set for keepalive to be supported */ #define UH_FL_KEEPALIVE (UH_FL_KAMETHOD | UH_FL_KAVERSION) +/* keep this small for Rainbows! since every client has one */ struct http_parser { int cs; /* Ragel internal state */ unsigned int flags; size_t mark; - union { /* these 3 fields don't nest */ + size_t offset; + union { /* these 2 fields don't nest */ size_t field; size_t query; - size_t offset; } start; union { size_t field_len; /* only used during header processing */ @@ -349,7 +350,7 @@ static void http_parser_execute(struct http_parser *hp, { const char *p, *pe; int cs = hp->cs; - size_t off = hp->start.offset; + size_t off = hp->offset; if (cs == http_parser_first_final) return; @@ -369,10 +370,10 @@ static void http_parser_execute(struct http_parser *hp, post_exec: /* "_out:" also goes here */ if (hp->cs != http_parser_error) hp->cs = cs; - hp->start.offset = p - buffer; + hp->offset = p - buffer; assert(p <= pe && "buffer overflow after parsing execute"); - assert(hp->start.offset <= len && "start.offset longer than length"); + assert(hp->offset <= len && "offset longer than length"); } static struct http_parser *data_get(VALUE self) @@ -531,12 +532,12 @@ static VALUE HttpParser_headers(VALUE self, VALUE req, VALUE data) rb_str_update(data); http_parser_execute(hp, req, RSTRING_PTR(data), RSTRING_LEN(data)); - VALIDATE_MAX_LENGTH(hp->start.offset, HEADER); + VALIDATE_MAX_LENGTH(hp->offset, HEADER); if (hp->cs == http_parser_first_final || hp->cs == http_parser_en_ChunkedBody) { - advance_str(data, hp->start.offset + 1); - hp->start.offset = 0; + advance_str(data, hp->offset + 1); + hp->offset = 0; return req; } @@ -637,9 +638,9 @@ static VALUE HttpParser_filter_body(VALUE self, VALUE buf, VALUE data) if (hp->cs == http_parser_error) rb_raise(eHttpParserError, "Invalid HTTP format, parsing fails."); - assert(hp->s.dest_offset <= hp->start.offset && + assert(hp->s.dest_offset <= hp->offset && "destination buffer overflow"); - advance_str(data, hp->start.offset); + advance_str(data, hp->offset); rb_str_set_len(buf, hp->s.dest_offset); if (RSTRING_LEN(buf) == 0 && chunked_eof(hp)) { @@ -663,7 +664,7 @@ static VALUE HttpParser_filter_body(VALUE self, VALUE buf, VALUE data) data = Qnil; } } - hp->start.offset = 0; /* for trailer parsing */ + hp->offset = 0; /* for trailer parsing */ return data; } diff --git a/test/unit/test_http_parser_ng.rb b/test/unit/test_http_parser_ng.rb index 397b0b0..0167f8f 100644 --- a/test/unit/test_http_parser_ng.rb +++ b/test/unit/test_http_parser_ng.rb @@ -12,6 +12,24 @@ class HttpParserNgTest < Test::Unit::TestCase @parser = HttpParser.new end + def test_identity_byte_headers + req = {} + str = "PUT / HTTP/1.1\r\n" + str << "Content-Length: 123\r\n" + str << "\r" + hdr = "" + str.each_byte { |byte| + assert_nil @parser.headers(req, hdr << byte.chr) + } + hdr << "\n" + assert_equal req.object_id, @parser.headers(req, hdr).object_id + assert_equal '123', req['CONTENT_LENGTH'] + assert_equal 0, hdr.size + assert ! @parser.keepalive? + assert @parser.headers? + assert 123, @parser.content_length + end + def test_identity_step_headers req = {} str = "PUT / HTTP/1.1\r\n" @@ -199,6 +217,34 @@ class HttpParserNgTest < Test::Unit::TestCase assert ! @parser.keepalive? end + def test_trailers_slowly + str = "PUT / HTTP/1.1\r\n" \ + "Trailer: Content-MD5\r\n" \ + "transfer-Encoding: chunked\r\n\r\n" \ + "1\r\na\r\n2\r\n..\r\n0\r\n" + req = {} + assert_equal req, @parser.headers(req, str) + assert_equal 'Content-MD5', req['HTTP_TRAILER'] + assert_nil req['HTTP_CONTENT_MD5'] + tmp = '' + assert_nil @parser.filter_body(tmp, str) + assert_equal 'a..', tmp + md5_b64 = [ Digest::MD5.digest(tmp) ].pack('m').strip.freeze + rv = @parser.filter_body(tmp, str) + assert_equal rv.object_id, str.object_id + assert_equal '', str + assert_nil @parser.trailers(req, str) + md5_hdr = "Content-MD5: #{md5_b64}\r\n".freeze + md5_hdr.each_byte { |byte| + str << byte.chr + assert_nil @parser.trailers(req, str) + } + assert_equal md5_b64, req['HTTP_CONTENT_MD5'] + assert_equal "CONTENT_MD5: #{md5_b64}\r\n", str + assert_nil @parser.trailers(req, str << "\r") + assert_equal req, @parser.trailers(req, str << "\n") + end + def test_max_chunk str = "PUT / HTTP/1.1\r\n" \ "transfer-Encoding: chunked\r\n\r\n" \ -- cgit v1.2.3-24-ge0c7