about summary refs log tree commit homepage
diff options
context:
space:
mode:
authorEric Wong <normalperson@yhbt.net>2009-06-10 16:00:08 -0700
committerEric Wong <normalperson@yhbt.net>2009-06-10 16:00:08 -0700
commit4651cd8b7c10d18bc745b84ebc7a55aad07d6077 (patch)
tree01ae0123891d16aa51ab18ff7c65d2dd1f3e733f
parentb0013b043a15d77d810d5965157766c1af364db2 (diff)
downloadunicorn-4651cd8b7c10d18bc745b84ebc7a55aad07d6077.tar.gz
No point in making syscalls to deal with empty bodies.
Reinstate usage of the NULL_IO object which allows us
to avoid allocating new objects.
-rw-r--r--ext/unicorn/http11/http11.c11
-rw-r--r--lib/unicorn/http_request.rb23
-rw-r--r--test/unit/test_http_parser.rb30
-rw-r--r--test/unit/test_server.rb11
4 files changed, 62 insertions, 13 deletions
diff --git a/ext/unicorn/http11/http11.c b/ext/unicorn/http11/http11.c
index cd7a8f7..f640a08 100644
--- a/ext/unicorn/http11/http11.c
+++ b/ext/unicorn/http11/http11.c
@@ -324,10 +324,17 @@ static void header_done(void *data, const char *at, size_t length)
   }
   rb_hash_aset(req, global_server_name, server_name);
   rb_hash_aset(req, global_server_port, server_port);
+  rb_hash_aset(req, global_server_protocol, global_server_protocol_value);
 
   /* grab the initial body and stuff it into the hash */
-  rb_hash_aset(req, sym_http_body, rb_str_new(at, length));
-  rb_hash_aset(req, global_server_protocol, global_server_protocol_value);
+  temp = rb_hash_aref(req, global_request_method);
+  if (temp != Qnil) {
+    long len = RSTRING_LEN(temp);
+    char *ptr = RSTRING_PTR(temp);
+
+    if (memcmp(ptr, "HEAD", len) && memcmp(ptr, "GET", len))
+      rb_hash_aset(req, sym_http_body, rb_str_new(at, length));
+  }
 }
 
 static void HttpParser_free(void *data) {
diff --git a/lib/unicorn/http_request.rb b/lib/unicorn/http_request.rb
index 58a5554..d9cb8b2 100644
--- a/lib/unicorn/http_request.rb
+++ b/lib/unicorn/http_request.rb
@@ -94,19 +94,22 @@ module Unicorn
     # Handles dealing with the rest of the request
     # returns a Rack environment if successful
     def handle_body(socket)
-      http_body = PARAMS.delete(:http_body)
-
-      length = PARAMS[Const::CONTENT_LENGTH].to_i
-      if te = PARAMS[Const::HTTP_TRANSFER_ENCODING]
-        if /chunked/i =~ te
-          socket = DECHUNKER.reopen(socket, http_body)
-          length = http_body = nil
+      PARAMS[Const::RACK_INPUT] = if (body = PARAMS.delete(:http_body))
+        length = PARAMS[Const::CONTENT_LENGTH].to_i
+
+        if te = PARAMS[Const::HTTP_TRANSFER_ENCODING]
+          if /chunked/i =~ te
+            socket = DECHUNKER.reopen(socket, body)
+            length = body = nil
+          end
         end
+
+        inp = TEE.reopen(socket, length, body)
+        DEFAULTS[Const::STREAM_INPUT] ? inp : inp.consume
+      else
+        NULL_IO.closed? ? NULL_IO.reopen(Z) : NULL_IO
       end
 
-      inp = TEE.reopen(socket, length, http_body)
-      PARAMS[Const::RACK_INPUT] =
-                          DEFAULTS[Const::STREAM_INPUT] ? inp : inp.consume
       PARAMS.update(DEFAULTS)
     end
 
diff --git a/test/unit/test_http_parser.rb b/test/unit/test_http_parser.rb
index a158ebb..560f8d4 100644
--- a/test/unit/test_http_parser.rb
+++ b/test/unit/test_http_parser.rb
@@ -23,6 +23,7 @@ class HttpParserTest < Test::Unit::TestCase
     assert_equal 'GET', req['REQUEST_METHOD']
     assert_nil req['FRAGMENT']
     assert_equal '', req['QUERY_STRING']
+    assert_nil req[:http_body]
 
     parser.reset
     req.clear
@@ -41,6 +42,7 @@ class HttpParserTest < Test::Unit::TestCase
     assert_equal 'GET', req['REQUEST_METHOD']
     assert_nil req['FRAGMENT']
     assert_equal '', req['QUERY_STRING']
+    assert_nil req[:http_body]
   end
 
   def test_parse_server_host_default_port
@@ -49,6 +51,7 @@ class HttpParserTest < Test::Unit::TestCase
     assert parser.execute(req, "GET / HTTP/1.1\r\nHost: foo\r\n\r\n")
     assert_equal 'foo', req['SERVER_NAME']
     assert_equal '80', req['SERVER_PORT']
+    assert_nil req[:http_body]
   end
 
   def test_parse_server_host_alt_port
@@ -57,6 +60,7 @@ class HttpParserTest < Test::Unit::TestCase
     assert parser.execute(req, "GET / HTTP/1.1\r\nHost: foo:999\r\n\r\n")
     assert_equal 'foo', req['SERVER_NAME']
     assert_equal '999', req['SERVER_PORT']
+    assert_nil req[:http_body]
   end
 
   def test_parse_server_host_empty_port
@@ -65,6 +69,7 @@ class HttpParserTest < Test::Unit::TestCase
     assert parser.execute(req, "GET / HTTP/1.1\r\nHost: foo:\r\n\r\n")
     assert_equal 'foo', req['SERVER_NAME']
     assert_equal '80', req['SERVER_PORT']
+    assert_nil req[:http_body]
   end
 
   def test_parse_server_host_xfp_https
@@ -74,6 +79,7 @@ class HttpParserTest < Test::Unit::TestCase
                           "X-Forwarded-Proto: https\r\n\r\n")
     assert_equal 'foo', req['SERVER_NAME']
     assert_equal '443', req['SERVER_PORT']
+    assert_nil req[:http_body]
   end
 
   def test_parse_strange_headers
@@ -81,6 +87,7 @@ class HttpParserTest < Test::Unit::TestCase
     req = {}
     should_be_good = "GET / HTTP/1.1\r\naaaaaaaaaaaaa:++++++++++\r\n\r\n"
     assert parser.execute(req, should_be_good)
+    assert_nil req[:http_body]
 
     # ref: http://thread.gmane.org/gmane.comp.lang.ruby.mongrel.devel/37/focus=45
     # (note we got 'pen' mixed up with 'pound' in that thread,
@@ -104,6 +111,7 @@ class HttpParserTest < Test::Unit::TestCase
       req = {}
       sorta_safe = %(GET #{path} HTTP/1.1\r\n\r\n)
       assert parser.execute(req, sorta_safe)
+      assert_nil req[:http_body]
     end
   end
   
@@ -115,6 +123,7 @@ class HttpParserTest < Test::Unit::TestCase
     assert_raises(HttpParserError) { parser.execute(req, bad_http) }
     parser.reset
     assert(parser.execute({}, "GET / HTTP/1.0\r\n\r\n"))
+    assert_nil req[:http_body]
   end
 
   def test_piecemeal
@@ -134,6 +143,7 @@ class HttpParserTest < Test::Unit::TestCase
     assert_equal 'HTTP/1.1', req['SERVER_PROTOCOL']
     assert_nil req['FRAGMENT']
     assert_equal '', req['QUERY_STRING']
+    assert_nil req[:http_body]
   end
 
   # not common, but underscores do appear in practice
@@ -150,6 +160,7 @@ class HttpParserTest < Test::Unit::TestCase
     assert_equal 'under_score.example.com', req['HTTP_HOST']
     assert_equal 'under_score.example.com', req['SERVER_NAME']
     assert_equal '80', req['SERVER_PORT']
+    assert_nil req[:http_body]
   end
 
   def test_absolute_uri
@@ -243,6 +254,24 @@ class HttpParserTest < Test::Unit::TestCase
     assert_equal "", req[:http_body]
   end
 
+  def test_unknown_methods
+    %w(GETT HEADR XGET XHEAD).each { |m|
+      parser = HttpParser.new
+      req = {}
+      s = "#{m} /forums/1/topics/2375?page=1#posts-17408 HTTP/1.1\r\n\r\n"
+      ok = false
+      assert_nothing_raised do
+        ok = parser.execute(req, s)
+      end
+      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']
+      assert_equal "", req[:http_body]
+      assert_equal m, req['REQUEST_METHOD']
+    }
+  end
+
   def test_fragment_in_uri
     parser = HttpParser.new
     req = {}
@@ -255,6 +284,7 @@ class HttpParserTest < Test::Unit::TestCase
     assert_equal '/forums/1/topics/2375?page=1', req['REQUEST_URI']
     assert_equal 'posts-17408', req['FRAGMENT']
     assert_equal 'page=1', req['QUERY_STRING']
+    assert_nil req[:http_body]
   end
 
   # lame random garbage maker
diff --git a/test/unit/test_server.rb b/test/unit/test_server.rb
index 742b240..0ce373f 100644
--- a/test/unit/test_server.rb
+++ b/test/unit/test_server.rb
@@ -152,9 +152,18 @@ class WebServerTest < Test::Unit::TestCase
 
   def test_file_streamed_request
     body = "a" * (Unicorn::Const::MAX_BODY * 2)
-    long = "GET /test HTTP/1.1\r\nContent-length: #{body.length}\r\n\r\n" + body
+    long = "PUT /test HTTP/1.1\r\nContent-length: #{body.length}\r\n\r\n" + body
     do_test(long, Unicorn::Const::CHUNK_SIZE * 2 -400)
   end
 
+  def test_file_streamed_request_bad_method
+    body = "a" * (Unicorn::Const::MAX_BODY * 2)
+    long = "GET /test HTTP/1.1\r\nContent-length: #{body.length}\r\n\r\n" + body
+    assert_raises(EOFError,Errno::ECONNRESET,Errno::EPIPE,Errno::EINVAL,
+                  Errno::EBADF) {
+      do_test(long, Unicorn::Const::CHUNK_SIZE * 2 -400)
+    }
+  end
+
 end