about summary refs log tree commit homepage
diff options
context:
space:
mode:
authorEric Wong <normalperson@yhbt.net>2009-11-04 23:00:49 -0800
committerEric Wong <normalperson@yhbt.net>2009-11-04 23:00:59 -0800
commit2c9cbe383ce8049707eda87c632855995a65e032 (patch)
treedd595da82ff97d39c38551c0bca227fe6046cc2b
parentf6c63f950d033e28567fabfc47a57f938c4cbabf (diff)
downloadunicorn-2c9cbe383ce8049707eda87c632855995a65e032.tar.gz
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.
-rw-r--r--ext/unicorn_http/unicorn_http.rl23
-rw-r--r--test/unit/test_http_parser_ng.rb46
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" \