From 4651cd8b7c10d18bc745b84ebc7a55aad07d6077 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 10 Jun 2009 16:00:08 -0700 Subject: Optimize body-less GET/HEAD requests (again) 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. --- ext/unicorn/http11/http11.c | 11 +++++++++-- lib/unicorn/http_request.rb | 23 +++++++++++++---------- test/unit/test_http_parser.rb | 30 ++++++++++++++++++++++++++++++ test/unit/test_server.rb | 11 ++++++++++- 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 -- cgit v1.2.3-24-ge0c7