about summary refs log tree commit homepage
diff options
context:
space:
mode:
authorEric Wong <normalperson@yhbt.net>2009-03-24 02:35:26 -0700
committerEric Wong <normalperson@yhbt.net>2009-03-24 02:44:20 -0700
commit32b6e838c28b7948811a6470d8c0a49d5767ec69 (patch)
tree016c38119477d56a839be8afc8ee702446c02b6b
parent7110a0dc380c53433e0b76496356abe7ccfa9021 (diff)
downloadunicorn-32b6e838c28b7948811a6470d8c0a49d5767ec69.tar.gz
This cuts the HttpParser interface down to #execute and #reset
method.  HttpParser#execute will return true if it completes and
false if it is not.  http->nread state is kept internally so we
don't have to keep track of it in Ruby; removing one parameter
from #execute.

HttpParser#reset is unchanged.

All errors are handled through exceptions anyways, so the
HttpParser#error? method stopped being useful.

Also added some more unit tests to the HttpParser since I know
some folks are (rightfully) uncomfortable with changing stable C
code.  We now have tests for incremental parsing.

In summary, we have:

  * more test cases
  * less C code
  * simpler interfaces
  * small performance improvement

  => win \o/
-rw-r--r--ext/unicorn/http11/http11.c123
-rw-r--r--ext/unicorn/http11/http11_parser.c47
-rw-r--r--ext/unicorn/http11/http11_parser.h4
-rw-r--r--ext/unicorn/http11/http11_parser.rl3
-rw-r--r--lib/unicorn/http_request.rb8
-rw-r--r--test/unit/test_http_parser.rb113
6 files changed, 130 insertions, 168 deletions
diff --git a/ext/unicorn/http11/http11.c b/ext/unicorn/http11/http11.c
index 0b96099..f62dce7 100644
--- a/ext/unicorn/http11/http11.c
+++ b/ext/unicorn/http11/http11.c
@@ -1,4 +1,5 @@
 /**
+ * Copyright (c) 2009 Eric Wong (all bugs are Eric's fault)
  * Copyright (c) 2005 Zed A. Shaw
  * You can redistribute it and/or modify it under the same terms as Ruby.
  */
@@ -367,113 +368,37 @@ static VALUE HttpParser_reset(VALUE self)
 
 /**
  * call-seq:
- *    parser.finish -> true/false
+ *    parser.execute(req_hash, data) -> true/false
  *
- * Finishes a parser early which could put in a "good" or bad state.
- * You should call reset after finish it or bad things will happen.
- */
-static VALUE HttpParser_finish(VALUE self)
-{
-  http_parser *http = NULL;
-  DATA_GET(self, http_parser, http);
-  http_parser_finish(http);
-
-  return http_parser_is_finished(http) ? Qtrue : Qfalse;
-}
-
-
-/**
- * call-seq:
- *    parser.execute(req_hash, data, start) -> Integer
- *
- * Takes a Hash and a String of data, parses the String of data filling in the Hash
- * returning an Integer to indicate how much of the data has been read.  No matter
- * what the return value, you should call HttpParser#finished? and HttpParser#error?
- * to figure out if it's done parsing or there was an error.
+ * Takes a Hash and a String of data, parses the String of data filling
+ * in the Hash returning a boolean to indicate whether or not parsing
+ * is finished.
  *
- * This function now throws an exception when there is a parsing error.  This makes
- * the logic for working with the parser much easier.  You can still test for an
- * error, but now you need to wrap the parser with an exception handling block.
- *
- * The third argument allows for parsing a partial request and then continuing
- * the parsing from that position.  It needs all of the original data as well
- * so you have to append to the data buffer as you read.
+ * This function now throws an exception when there is a parsing error.
+ * This makes the logic for working with the parser much easier.  You
+ * will need to wrap the parser with an exception handling block.
  */
-static VALUE HttpParser_execute(VALUE self, VALUE req_hash,
-                                VALUE data, VALUE start)
-{
-  http_parser *http = NULL;
-  int from = 0;
-  char *dptr = NULL;
-  long dlen = 0;
-
-  DATA_GET(self, http_parser, http);
-
-  from = FIX2INT(start);
-  dptr = RSTRING_PTR(data);
-  dlen = RSTRING_LEN(data);
-
-  if(from >= dlen) {
-    rb_raise(eHttpParserError, "Requested start is after data buffer end.");
-  } else {
-    http->data = (void *)req_hash;
-    http_parser_execute(http, dptr, dlen, from);
-
-    VALIDATE_MAX_LENGTH(http_parser_nread(http), HEADER);
-
-    if(http_parser_has_error(http)) {
-      rb_raise(eHttpParserError, "Invalid HTTP format, parsing fails.");
-    } else {
-      return INT2FIX(http_parser_nread(http));
-    }
-  }
-}
-
 
-
-/**
- * call-seq:
- *    parser.error? -> true/false
- *
- * Tells you whether the parser is in an error state.
- */
-static VALUE HttpParser_has_error(VALUE self)
+static VALUE HttpParser_execute(VALUE self, VALUE req_hash, VALUE data)
 {
-  http_parser *http = NULL;
-  DATA_GET(self, http_parser, http);
+  http_parser *http;
+  char *dptr = RSTRING_PTR(data);
+  long dlen = RSTRING_LEN(data);
 
-  return http_parser_has_error(http) ? Qtrue : Qfalse;
-}
-
-
-/**
- * call-seq:
- *    parser.finished? -> true/false
- *
- * Tells you whether the parser is finished or not and in a good state.
- */
-static VALUE HttpParser_is_finished(VALUE self)
-{
-  http_parser *http = NULL;
   DATA_GET(self, http_parser, http);
 
-  return http_parser_is_finished(http) ? Qtrue : Qfalse;
-}
+  if (http->nread < dlen) {
+    http->data = (void *)req_hash;
+    http_parser_execute(http, dptr, dlen);
 
+    VALIDATE_MAX_LENGTH(http->nread, HEADER);
 
-/**
- * call-seq:
- *    parser.nread -> Integer
- *
- * Returns the amount of data processed so far during this processing cycle.  It is
- * set to 0 on initialize or reset calls and is incremented each time execute is called.
- */
-static VALUE HttpParser_nread(VALUE self)
-{
-  http_parser *http = NULL;
-  DATA_GET(self, http_parser, http);
+    if (!http_parser_has_error(http))
+      return http_parser_is_finished(http) ? Qtrue : Qfalse;
 
-  return INT2FIX(http->nread);
+    rb_raise(eHttpParserError, "Invalid HTTP format, parsing fails.");
+  }
+  rb_raise(eHttpParserError, "Requested start is after data buffer end.");
 }
 
 void Init_http11()
@@ -504,10 +429,6 @@ void Init_http11()
   rb_define_alloc_func(cHttpParser, HttpParser_alloc);
   rb_define_method(cHttpParser, "initialize", HttpParser_init,0);
   rb_define_method(cHttpParser, "reset", HttpParser_reset,0);
-  rb_define_method(cHttpParser, "finish", HttpParser_finish,0);
-  rb_define_method(cHttpParser, "execute", HttpParser_execute,3);
-  rb_define_method(cHttpParser, "error?", HttpParser_has_error,0);
-  rb_define_method(cHttpParser, "finished?", HttpParser_is_finished,0);
-  rb_define_method(cHttpParser, "nread", HttpParser_nread,0);
+  rb_define_method(cHttpParser, "execute", HttpParser_execute,2);
   init_common_fields();
 }
diff --git a/ext/unicorn/http11/http11_parser.c b/ext/unicorn/http11/http11_parser.c
index d33eed0..b6d55c8 100644
--- a/ext/unicorn/http11/http11_parser.c
+++ b/ext/unicorn/http11/http11_parser.c
@@ -63,9 +63,10 @@ int http_parser_init(http_parser *parser)  {
 
 
 /** exec **/
-size_t http_parser_execute(http_parser *parser, const char *buffer, size_t len, size_t off)  {
+size_t http_parser_execute(http_parser *parser, const char *buffer, size_t len)  {
   const char *p, *pe;
   int cs = parser->cs;
+  size_t off = parser->nread;
 
   assert(off <= len && "offset past end of buffer");
 
@@ -76,7 +77,7 @@ size_t http_parser_execute(http_parser *parser, const char *buffer, size_t len,
   assert(pe - p == len - off && "pointers aren't same distance");
 
   
-#line 80 "http11_parser.c"
+#line 81 "http11_parser.c"
         {
         if ( p == pe )
                 goto _test_eof;
@@ -107,7 +108,7 @@ st2:
         if ( ++p == pe )
                 goto _test_eof2;
 case 2:
-#line 111 "http11_parser.c"
+#line 112 "http11_parser.c"
         switch( (*p) ) {
                 case 32: goto tr2;
                 case 36: goto st38;
@@ -133,7 +134,7 @@ st3:
         if ( ++p == pe )
                 goto _test_eof3;
 case 3:
-#line 137 "http11_parser.c"
+#line 138 "http11_parser.c"
         switch( (*p) ) {
                 case 42: goto tr4;
                 case 43: goto tr5;
@@ -157,7 +158,7 @@ st4:
         if ( ++p == pe )
                 goto _test_eof4;
 case 4:
-#line 161 "http11_parser.c"
+#line 162 "http11_parser.c"
         switch( (*p) ) {
                 case 32: goto tr8;
                 case 35: goto tr9;
@@ -228,7 +229,7 @@ st5:
         if ( ++p == pe )
                 goto _test_eof5;
 case 5:
-#line 232 "http11_parser.c"
+#line 233 "http11_parser.c"
         if ( (*p) == 72 )
                 goto tr10;
         goto st0;
@@ -240,7 +241,7 @@ st6:
         if ( ++p == pe )
                 goto _test_eof6;
 case 6:
-#line 244 "http11_parser.c"
+#line 245 "http11_parser.c"
         if ( (*p) == 84 )
                 goto st7;
         goto st0;
@@ -326,7 +327,7 @@ st14:
         if ( ++p == pe )
                 goto _test_eof14;
 case 14:
-#line 330 "http11_parser.c"
+#line 331 "http11_parser.c"
         if ( (*p) == 10 )
                 goto st15;
         goto st0;
@@ -378,7 +379,7 @@ st57:
         if ( ++p == pe )
                 goto _test_eof57;
 case 57:
-#line 382 "http11_parser.c"
+#line 383 "http11_parser.c"
         goto st0;
 tr21:
 #line 37 "http11_parser.rl"
@@ -394,7 +395,7 @@ st17:
         if ( ++p == pe )
                 goto _test_eof17;
 case 17:
-#line 398 "http11_parser.c"
+#line 399 "http11_parser.c"
         switch( (*p) ) {
                 case 33: goto tr23;
                 case 58: goto tr24;
@@ -433,7 +434,7 @@ st18:
         if ( ++p == pe )
                 goto _test_eof18;
 case 18:
-#line 437 "http11_parser.c"
+#line 438 "http11_parser.c"
         switch( (*p) ) {
                 case 13: goto tr26;
                 case 32: goto tr27;
@@ -447,7 +448,7 @@ st19:
         if ( ++p == pe )
                 goto _test_eof19;
 case 19:
-#line 451 "http11_parser.c"
+#line 452 "http11_parser.c"
         if ( (*p) == 13 )
                 goto tr29;
         goto st19;
@@ -500,7 +501,7 @@ st20:
         if ( ++p == pe )
                 goto _test_eof20;
 case 20:
-#line 504 "http11_parser.c"
+#line 505 "http11_parser.c"
         switch( (*p) ) {
                 case 32: goto tr31;
                 case 35: goto st0;
@@ -518,7 +519,7 @@ st21:
         if ( ++p == pe )
                 goto _test_eof21;
 case 21:
-#line 522 "http11_parser.c"
+#line 523 "http11_parser.c"
         switch( (*p) ) {
                 case 32: goto tr34;
                 case 35: goto st0;
@@ -536,7 +537,7 @@ st22:
         if ( ++p == pe )
                 goto _test_eof22;
 case 22:
-#line 540 "http11_parser.c"
+#line 541 "http11_parser.c"
         if ( (*p) < 65 ) {
                 if ( 48 <= (*p) && (*p) <= 57 )
                         goto st23;
@@ -567,7 +568,7 @@ st24:
         if ( ++p == pe )
                 goto _test_eof24;
 case 24:
-#line 571 "http11_parser.c"
+#line 572 "http11_parser.c"
         switch( (*p) ) {
                 case 43: goto st24;
                 case 58: goto st25;
@@ -592,7 +593,7 @@ st25:
         if ( ++p == pe )
                 goto _test_eof25;
 case 25:
-#line 596 "http11_parser.c"
+#line 597 "http11_parser.c"
         switch( (*p) ) {
                 case 32: goto tr8;
                 case 35: goto tr9;
@@ -636,7 +637,7 @@ st28:
         if ( ++p == pe )
                 goto _test_eof28;
 case 28:
-#line 640 "http11_parser.c"
+#line 641 "http11_parser.c"
         switch( (*p) ) {
                 case 32: goto tr42;
                 case 35: goto tr43;
@@ -685,7 +686,7 @@ st31:
         if ( ++p == pe )
                 goto _test_eof31;
 case 31:
-#line 689 "http11_parser.c"
+#line 690 "http11_parser.c"
         switch( (*p) ) {
                 case 32: goto tr8;
                 case 35: goto tr9;
@@ -733,7 +734,7 @@ st34:
         if ( ++p == pe )
                 goto _test_eof34;
 case 34:
-#line 737 "http11_parser.c"
+#line 738 "http11_parser.c"
         switch( (*p) ) {
                 case 32: goto tr53;
                 case 35: goto tr54;
@@ -751,7 +752,7 @@ st35:
         if ( ++p == pe )
                 goto _test_eof35;
 case 35:
-#line 755 "http11_parser.c"
+#line 756 "http11_parser.c"
         switch( (*p) ) {
                 case 32: goto tr57;
                 case 35: goto tr58;
@@ -769,7 +770,7 @@ st36:
         if ( ++p == pe )
                 goto _test_eof36;
 case 36:
-#line 773 "http11_parser.c"
+#line 774 "http11_parser.c"
         if ( (*p) < 65 ) {
                 if ( 48 <= (*p) && (*p) <= 57 )
                         goto st37;
@@ -1184,7 +1185,7 @@ case 56:
         _test_eof: {}
         _out: {}
         }
-#line 121 "http11_parser.rl"
+#line 122 "http11_parser.rl"
 
   if (!http_parser_has_error(parser))
     parser->cs = cs;
diff --git a/ext/unicorn/http11/http11_parser.h b/ext/unicorn/http11/http11_parser.h
index c96b3a0..6c332fe 100644
--- a/ext/unicorn/http11/http11_parser.h
+++ b/ext/unicorn/http11/http11_parser.h
@@ -36,10 +36,8 @@ typedef struct http_parser {
 
 int http_parser_init(http_parser *parser);
 int http_parser_finish(http_parser *parser);
-size_t http_parser_execute(http_parser *parser, const char *data, size_t len, size_t off);
+size_t http_parser_execute(http_parser *parser, const char *data, size_t len);
 int http_parser_has_error(http_parser *parser);
 int http_parser_is_finished(http_parser *parser);
 
-#define http_parser_nread(parser) (parser)->nread
-
 #endif
diff --git a/ext/unicorn/http11/http11_parser.rl b/ext/unicorn/http11/http11_parser.rl
index c3c4b1f..1fad2ca 100644
--- a/ext/unicorn/http11/http11_parser.rl
+++ b/ext/unicorn/http11/http11_parser.rl
@@ -105,9 +105,10 @@ int http_parser_init(http_parser *parser)  {
 
 
 /** exec **/
-size_t http_parser_execute(http_parser *parser, const char *buffer, size_t len, size_t off)  {
+size_t http_parser_execute(http_parser *parser, const char *buffer, size_t len)  {
   const char *p, *pe;
   int cs = parser->cs;
+  size_t off = parser->nread;
 
   assert(off <= len && "offset past end of buffer");
 
diff --git a/lib/unicorn/http_request.rb b/lib/unicorn/http_request.rb
index ef6ce05..04386fc 100644
--- a/lib/unicorn/http_request.rb
+++ b/lib/unicorn/http_request.rb
@@ -57,8 +57,8 @@ module Unicorn
     # to handle any socket errors (e.g. user aborted upload).
     def read(socket)
       # short circuit the common case with small GET requests first
-      nparsed = @parser.execute(@params, read_socket(socket), 0)
-      @parser.finished? and return handle_body(socket)
+      @parser.execute(@params, read_socket(socket)) and
+          return handle_body(socket)
 
       data = @buffer.dup # read_socket will clobber @buffer
 
@@ -66,8 +66,8 @@ module Unicorn
       # an Exception thrown from the @parser will throw us out of the loop
       loop do
         data << read_socket(socket)
-        nparsed = @parser.execute(@params, data, nparsed)
-        @parser.finished? and return handle_body(socket)
+        @parser.execute(@params, data) and
+            return handle_body(socket)
       end
       rescue HttpParserError => e
         @logger.error "HTTP parse error, malformed request " \
diff --git a/test/unit/test_http_parser.rb b/test/unit/test_http_parser.rb
index fc75990..1deeaa2 100644
--- a/test/unit/test_http_parser.rb
+++ b/test/unit/test_http_parser.rb
@@ -14,33 +14,40 @@ class HttpParserTest < Test::Unit::TestCase
     parser = HttpParser.new
     req = {}
     http = "GET / HTTP/1.1\r\n\r\n"
-    nread = parser.execute(req, http, 0)
-
-    assert nread == http.length, "Failed to parse the full HTTP request"
-    assert parser.finished?, "Parser didn't finish"
-    assert !parser.error?, "Parser had error"
-    assert nread == parser.nread, "Number read returned from execute does not match"
+    assert parser.execute(req, http)
 
     assert_equal 'HTTP/1.1', req['SERVER_PROTOCOL']
     assert_equal '/', req['REQUEST_PATH']
     assert_equal 'HTTP/1.1', req['HTTP_VERSION']
     assert_equal '/', req['REQUEST_URI']
-    assert_equal 'GET', req['REQUEST_METHOD']    
+    assert_equal 'GET', req['REQUEST_METHOD']
     assert_nil req['FRAGMENT']
     assert_nil req['QUERY_STRING']
 
     parser.reset
-    assert parser.nread == 0, "Number read after reset should be 0"
+    req.clear
+
+    assert ! parser.execute(req, "G")
+    assert req.empty?
+
+    # try parsing again to ensure we were reset correctly
+    http = "GET /hello-world HTTP/1.1\r\n\r\n"
+    assert parser.execute(req, http)
+
+    assert_equal 'HTTP/1.1', req['SERVER_PROTOCOL']
+    assert_equal '/hello-world', req['REQUEST_PATH']
+    assert_equal 'HTTP/1.1', req['HTTP_VERSION']
+    assert_equal '/hello-world', req['REQUEST_URI']
+    assert_equal 'GET', req['REQUEST_METHOD']
+    assert_nil req['FRAGMENT']
+    assert_nil req['QUERY_STRING']
   end
-
+
   def test_parse_strange_headers
     parser = HttpParser.new
     req = {}
     should_be_good = "GET / HTTP/1.1\r\naaaaaaaaaaaaa:++++++++++\r\n\r\n"
-    nread = parser.execute(req, should_be_good, 0)
-    assert_equal should_be_good.length, nread
-    assert parser.finished?
-    assert !parser.error?
+    assert parser.execute(req, should_be_good)
 
     # ref: http://thread.gmane.org/gmane.comp.lang.ruby.Unicorn.devel/37/focus=45
     # (note we got 'pen' mixed up with 'pound' in that thread,
@@ -49,10 +56,7 @@ class HttpParserTest < Test::Unit::TestCase
     # nasty_pound_header = "GET / HTTP/1.1\r\nX-SSL-Bullshit:   -----BEGIN CERTIFICATE-----\r\n\tMIIFbTCCBFWgAwIBAgICH4cwDQYJKoZIhvcNAQEFBQAwcDELMAkGA1UEBhMCVUsx\r\n\tETAPBgNVBAoTCGVTY2llbmNlMRIwEAYDVQQLEwlBdXRob3JpdHkxCzAJBgNVBAMT\r\n\tAkNBMS0wKwYJKoZIhvcNAQkBFh5jYS1vcGVyYXRvckBncmlkLXN1cHBvcnQuYWMu\r\n\tdWswHhcNMDYwNzI3MTQxMzI4WhcNMDcwNzI3MTQxMzI4WjBbMQswCQYDVQQGEwJV\r\n\tSzERMA8GA1UEChMIZVNjaWVuY2UxEzARBgNVBAsTCk1hbmNoZXN0ZXIxCzAJBgNV\r\n\tBAcTmrsogriqMWLAk1DMRcwFQYDVQQDEw5taWNoYWVsIHBhcmQYJKoZIhvcNAQEB\r\n\tBQADggEPADCCAQoCggEBANPEQBgl1IaKdSS1TbhF3hEXSl72G9J+WC/1R64fAcEF\r\n\tW51rEyFYiIeZGx/BVzwXbeBoNUK41OK65sxGuflMo5gLflbwJtHBRIEKAfVVp3YR\r\n\tgW7cMA/s/XKgL1GEC7rQw8lIZT8RApukCGqOVHSi/F1SiFlPDxuDfmdiNzL31+sL\r\n\t0iwHDdNkGjy5pyBSB8Y79dsSJtCW/iaLB0/n8Sj7HgvvZJ7x0fr+RQjYOUUfrePP\r\n\tu2MSpFyf+9BbC/aXgaZuiCvSR+8Snv3xApQY+fULK/xY8h8Ua51iXoQ5jrgu2SqR\r\n\twgA7BUi3G8LFzMBl8FRCDYGUDy7M6QaHXx1ZWIPWNKsCAwEAAaOCAiQwggIgMAwG\r\n\tA1UdEwEB/wQCMAAwEQYJYIZIAYb4QgEBBAQDAgWgMA4GA1UdDwEB/wQEAwID6DAs\r\n\tBglghkgBhvhCAQ0EHxYdVUsgZS1TY2llbmNlIFVzZXIgQ2VydGlmaWNhdGUwHQYD\r\n\tVR0OBBYEFDTt/sf9PeMaZDHkUIldrDYMNTBZMIGaBgNVHSMEgZIwgY+AFAI4qxGj\r\n\tloCLDdMVKwiljjDastqooXSkcjBwMQswCQYDVQQGEwJVSzERMA8GA1UEChMIZVNj\r\n\taWVuY2UxEjAQBgNVBAsTCUF1dGhvcml0eTELMAkGA1UEAxMCQ0ExLTArBgkqhkiG\r\n\t9w0BCQEWHmNhLW9wZXJhdG9yQGdyaWQtc3VwcG9ydC5hYy51a4IBADApBgNVHRIE\r\n\tIjAggR5jYS1vcGVyYXRvckBncmlkLXN1cHBvcnQuYWMudWswGQYDVR0gBBIwEDAO\r\n\tBgwrBgEEAdkvAQEBAQYwPQYJYIZIAYb4QgEEBDAWLmh0dHA6Ly9jYS5ncmlkLXN1\r\n\tcHBvcnQuYWMudmT4sopwqlBWsvcHViL2NybC9jYWNybC5jcmwwPQYJYIZIAYb4QgEDBDAWLmh0\r\n\tdHA6Ly9jYS5ncmlkLXN1cHBvcnQuYWMudWsvcHViL2NybC9jYWNybC5jcmwwPwYD\r\n\tVR0fBDgwNjA0oDKgMIYuaHR0cDovL2NhLmdyaWQt5hYy51ay9wdWIv\r\n\tY3JsL2NhY3JsLmNybDANBgkqhkiG9w0BAQUFAAOCAQEAS/U4iiooBENGW/Hwmmd3\r\n\tXCy6Zrt08YjKCzGNjorT98g8uGsqYjSxv/hmi0qlnlHs+k/3Iobc3LjS5AMYr5L8\r\n\tUO7OSkgFFlLHQyC9JzPfmLCAugvzEbyv4Olnsr8hbxF1MbKZoQxUZtMVu29wjfXk\r\n\thTeApBv7eaKCWpSp7MCbvgzm74izKhu3vlDk9w6qVrxePfGgpKPqfHiOoGhFnbTK\r\n\twTC6o2xq5y0qZ03JonF7OJspEd3I5zKY3E+ov7/ZhW6DqT8UFvsAdjvQbXyhV8Eu\r\n\tYhixw1aKEPzNjNowuIseVogKOLXxWI5vAi5HgXdS0/ES5gDGsABo4fqovUKlgop3\r\n\tRA==\r\n\t-----END CERTIFICATE-----\r\n\r\n"
     # parser = HttpParser.new
     # req = {}
-    # nread = parser.execute(req, nasty_pound_header, 0)
-    # assert_equal nasty_pound_header.length, nread
-    # assert parser.finished?
-    # assert !parser.error?
+    # assert parser.execute(req, nasty_pound_header, 0)
   end
 
   def test_parse_ie6_urls
@@ -66,10 +70,7 @@ class HttpParserTest < Test::Unit::TestCase
       parser = HttpParser.new
       req = {}
       sorta_safe = %(GET #{path} HTTP/1.1\r\n\r\n)
-      nread = parser.execute(req, sorta_safe, 0)
-      assert_equal sorta_safe.length, nread
-      assert parser.finished?
-      assert !parser.error?
+      assert parser.execute(req, sorta_safe)
     end
   end
   
@@ -78,28 +79,68 @@ class HttpParserTest < Test::Unit::TestCase
     req = {}
     bad_http = "GET / SsUTF/1.1"
 
-    error = false
-    begin
-      nread = parser.execute(req, bad_http, 0)
-    rescue => details
-      error = true
-    end
+    assert_raises(HttpParserError) { parser.execute(req, bad_http) }
+    parser.reset
+    assert(parser.execute({}, "GET / HTTP/1.0\r\n\r\n"))
+  end
 
-    assert error, "failed to throw exception"
-    assert !parser.finished?, "Parser shouldn't be finished"
-    assert parser.error?, "Parser SHOULD have error"
+  def test_piecemeal
+    parser = HttpParser.new
+    req = {}
+    http = "GET"
+    assert ! parser.execute(req, http)
+    assert_raises(HttpParserError) { parser.execute(req, http) }
+    assert ! parser.execute(req, http << " / HTTP/1.0")
+    assert_equal '/', req['REQUEST_PATH']
+    assert_equal '/', req['REQUEST_URI']
+    assert_equal 'GET', req['REQUEST_METHOD']
+    assert ! parser.execute(req, http << "\r\n")
+    assert_equal 'HTTP/1.0', req['HTTP_VERSION']
+    assert ! parser.execute(req, http << "\r")
+    assert parser.execute(req, http << "\n")
+    assert_equal 'HTTP/1.1', req['SERVER_PROTOCOL']
+    assert_nil req['FRAGMENT']
+    assert_nil req['QUERY_STRING']
+  end
+
+  def test_put_body_oneshot
+    parser = HttpParser.new
+    req = {}
+    http = "PUT / HTTP/1.0\r\nContent-Length: 5\r\n\r\nabcde"
+    assert parser.execute(req, http)
+    assert_equal '/', req['REQUEST_PATH']
+    assert_equal '/', req['REQUEST_URI']
+    assert_equal 'PUT', req['REQUEST_METHOD']
+    assert_equal 'HTTP/1.0', req['HTTP_VERSION']
+    assert_equal 'HTTP/1.1', req['SERVER_PROTOCOL']
+    assert_equal "abcde", req['HTTP_BODY']
+  end
+
+  def test_put_body_later
+    parser = HttpParser.new
+    req = {}
+    http = "PUT /l HTTP/1.0\r\nContent-Length: 5\r\n\r\n"
+    assert parser.execute(req, http)
+    assert_equal '/l', req['REQUEST_PATH']
+    assert_equal '/l', req['REQUEST_URI']
+    assert_equal 'PUT', req['REQUEST_METHOD']
+    assert_equal 'HTTP/1.0', req['HTTP_VERSION']
+    assert_equal 'HTTP/1.1', req['SERVER_PROTOCOL']
+    assert_equal "", req['HTTP_BODY']
   end
 
   def test_fragment_in_uri
     parser = HttpParser.new
     req = {}
     get = "GET /forums/1/topics/2375?page=1#posts-17408 HTTP/1.1\r\n\r\n"
+    ok = false
     assert_nothing_raised do
-      parser.execute(req, get, 0)
+      ok = parser.execute(req, get)
     end
-    assert parser.finished?
+    assert ok
     assert_equal '/forums/1/topics/2375?page=1', req['REQUEST_URI']
     assert_equal 'posts-17408', req['FRAGMENT']
+    assert_equal 'page=1', req['QUERY_STRING']
   end
 
   # lame random garbage maker
@@ -124,7 +165,7 @@ class HttpParserTest < Test::Unit::TestCase
     10.times do |c|
       get = "GET /#{rand_data(10,120)} HTTP/1.1\r\nX-#{rand_data(1024, 1024+(c*1024))}: Test\r\n\r\n"
       assert_raises Unicorn::HttpParserError do
-        parser.execute({}, get, 0)
+        parser.execute({}, get)
         parser.reset
       end
     end
@@ -133,7 +174,7 @@ class HttpParserTest < Test::Unit::TestCase
     10.times do |c|
       get = "GET /#{rand_data(10,120)} HTTP/1.1\r\nX-Test: #{rand_data(1024, 1024+(c*1024), false)}\r\n\r\n"
       assert_raises Unicorn::HttpParserError do
-        parser.execute({}, get, 0)
+        parser.execute({}, get)
         parser.reset
       end
     end
@@ -142,7 +183,7 @@ class HttpParserTest < Test::Unit::TestCase
     get = "GET /#{rand_data(10,120)} HTTP/1.1\r\n"
     get << "X-Test: test\r\n" * (80 * 1024)
     assert_raises Unicorn::HttpParserError do
-      parser.execute({}, get, 0)
+      parser.execute({}, get)
       parser.reset
     end
 
@@ -150,7 +191,7 @@ class HttpParserTest < Test::Unit::TestCase
     10.times do |c|
       get = "GET #{rand_data(1024, 1024+(c*1024), false)} #{rand_data(1024, 1024+(c*1024), false)}\r\n\r\n"
       assert_raises Unicorn::HttpParserError do
-        parser.execute({}, get, 0)
+        parser.execute({}, get)
         parser.reset
       end
     end