From 81026ea66279695206ea53287427c05281662572 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 9 Aug 2009 03:02:54 -0700 Subject: Switch to Ragel/C-based chunk/trailer parser This should be more robust, faster and easier to deal with than the ugly proof-of-concept regexp-based ones. --- Manifest | 4 -- ext/unicorn_http/global_variables.h | 2 - ext/unicorn_http/unicorn_http.rl | 62 ++---------------- lib/unicorn.rb | 2 - lib/unicorn/chunked_reader.rb | 77 ---------------------- lib/unicorn/http_request.rb | 43 ++++--------- lib/unicorn/tee_input.rb | 71 +++++++++++---------- lib/unicorn/trailer_parser.rb | 52 --------------- test/unit/test_chunked_reader.rb | 123 ------------------------------------ test/unit/test_http_parser.rb | 105 ++++++++++++++++-------------- test/unit/test_server.rb | 9 ++- test/unit/test_tee_input.rb | 21 +++++- test/unit/test_trailer_parser.rb | 52 --------------- 13 files changed, 140 insertions(+), 483 deletions(-) delete mode 100644 lib/unicorn/chunked_reader.rb delete mode 100644 lib/unicorn/trailer_parser.rb delete mode 100644 test/unit/test_chunked_reader.rb delete mode 100644 test/unit/test_trailer_parser.rb diff --git a/Manifest b/Manifest index e081315..d5fb1d8 100644 --- a/Manifest +++ b/Manifest @@ -32,7 +32,6 @@ lib/unicorn/app/inetd.rb lib/unicorn/app/old_rails.rb lib/unicorn/app/old_rails/static.rb lib/unicorn/cgi_wrapper.rb -lib/unicorn/chunked_reader.rb lib/unicorn/configurator.rb lib/unicorn/const.rb lib/unicorn/http_request.rb @@ -40,7 +39,6 @@ lib/unicorn/http_response.rb lib/unicorn/launcher.rb lib/unicorn/socket_helper.rb lib/unicorn/tee_input.rb -lib/unicorn/trailer_parser.rb lib/unicorn/util.rb local.mk.sample setup.rb @@ -129,7 +127,6 @@ test/rails/app-2.3.3.1/public/404.html test/rails/app-2.3.3.1/public/500.html test/rails/test_rails.rb test/test_helper.rb -test/unit/test_chunked_reader.rb test/unit/test_configurator.rb test/unit/test_http_parser.rb test/unit/test_http_parser_ng.rb @@ -139,6 +136,5 @@ test/unit/test_server.rb test/unit/test_signals.rb test/unit/test_socket_helper.rb test/unit/test_tee_input.rb -test/unit/test_trailer_parser.rb test/unit/test_upload.rb test/unit/test_util.rb diff --git a/ext/unicorn_http/global_variables.h b/ext/unicorn_http/global_variables.h index 3437ee2..1383ed4 100644 --- a/ext/unicorn_http/global_variables.h +++ b/ext/unicorn_http/global_variables.h @@ -3,7 +3,6 @@ static VALUE mUnicorn; static VALUE cHttpParser; static VALUE eHttpParserError; -static VALUE sym_http_body; static VALUE g_rack_url_scheme; static VALUE g_request_method; @@ -82,7 +81,6 @@ void init_globals(void) eHttpParserError = rb_define_class_under(mUnicorn, "HttpParserError", rb_eIOError); cHttpParser = rb_define_class_under(mUnicorn, "HttpParser", rb_cObject); - sym_http_body = ID2SYM(rb_intern("http_body")); } #undef DEF_GLOBAL diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl index 08b9ef9..5a5c053 100644 --- a/ext/unicorn_http/unicorn_http.rl +++ b/ext/unicorn_http/unicorn_http.rl @@ -38,7 +38,7 @@ struct http_parser { } len; }; -static void header_done(VALUE req, const char *at, size_t length); +static void finalize_header(VALUE req); #define REMAINING (unsigned long)(pe - p) #define LEN(AT, FPC) (FPC - buffer - hp->AT) @@ -149,7 +149,8 @@ static void write_value(VALUE req, struct http_parser *hp, rb_raise(eHttpParserError, "invalid chunk size"); } action header_done { - header_done(req, fpc + 1, pe - fpc - 1); + finalize_header(req); + cs = http_parser_first_final; if (hp->flags & UH_FL_HASBODY) { hp->flags |= UH_FL_INBODY; @@ -253,7 +254,7 @@ static struct http_parser *data_get(VALUE self) return hp; } -static void set_server_params(VALUE req) +static void finalize_header(VALUE req) { VALUE temp = rb_hash_aref(req, g_rack_url_scheme); VALUE server_name = g_localhost; @@ -287,29 +288,11 @@ static void set_server_params(VALUE req) } rb_hash_aset(req, g_server_name, server_name); rb_hash_aset(req, g_server_port, server_port); -} - -/** Finalizes the request header to have a bunch of stuff that's needed. */ -static void header_done(VALUE req, const char *at, size_t length) -{ - VALUE temp; + rb_hash_aset(req, g_server_protocol, g_server_protocol_value); /* rack requires QUERY_STRING */ if (rb_hash_aref(req, g_query_string) == Qnil) rb_hash_aset(req, g_query_string, rb_str_new(NULL, 0)); - - set_server_params(req); - rb_hash_aset(req, g_server_protocol, g_server_protocol_value); - - /* grab the initial body and stuff it into the hash */ - temp = rb_hash_aref(req, g_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 VALUE HttpParser_alloc(VALUE klass) @@ -347,40 +330,6 @@ static VALUE HttpParser_reset(VALUE self) return Qnil; } - -/** - * call-seq: - * parser.execute(req, data) -> true/false - * - * 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 - * will need to wrap the parser with an exception handling block. - */ - -static VALUE HttpParser_execute(VALUE self, VALUE req, VALUE data) -{ - struct http_parser *hp = data_get(self); - char *dptr = RSTRING_PTR(data); - long dlen = RSTRING_LEN(data); - - if (hp->start.offset < dlen) { - http_parser_execute(hp, req, dptr, dlen); - - VALIDATE_MAX_LENGTH(hp->start.offset, HEADER); - - if (hp->cs != http_parser_error) - return ((hp->cs == http_parser_first_final) || - (hp->cs == http_parser_en_ChunkedBody)) ? Qtrue : Qfalse; - - rb_raise(eHttpParserError, "Invalid HTTP format, parsing fails."); - } - rb_raise(eHttpParserError, "Requested start is after data buffer end."); -} - static void advance_str(VALUE str, off_t nr) { long len = RSTRING_LEN(str); @@ -524,7 +473,6 @@ void Init_unicorn_http(void) 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, "execute", HttpParser_execute,2); rb_define_method(cHttpParser, "headers", HttpParser_headers, 2); rb_define_method(cHttpParser, "read_body", HttpParser_read_body, 2); rb_define_method(cHttpParser, "trailers", HttpParser_headers, 2); diff --git a/lib/unicorn.rb b/lib/unicorn.rb index 556aba8..b185b25 100644 --- a/lib/unicorn.rb +++ b/lib/unicorn.rb @@ -11,8 +11,6 @@ module Unicorn autoload :HttpResponse, 'unicorn/http_response' autoload :Configurator, 'unicorn/configurator' autoload :TeeInput, 'unicorn/tee_input' - autoload :ChunkedReader, 'unicorn/chunked_reader' - autoload :TrailerParser, 'unicorn/trailer_parser' autoload :Util, 'unicorn/util' Z = '' # the stock empty string we use everywhere... diff --git a/lib/unicorn/chunked_reader.rb b/lib/unicorn/chunked_reader.rb deleted file mode 100644 index b813da6..0000000 --- a/lib/unicorn/chunked_reader.rb +++ /dev/null @@ -1,77 +0,0 @@ -# Copyright (c) 2009 Eric Wong -# You can redistribute it and/or modify it under the same terms as Ruby. - -require 'unicorn' -require 'unicorn_http' - -module Unicorn - class ChunkedReader - - def initialize(env, input, buf) - @env, @input, @buf = env, input, buf - @chunk_left = 0 - parse_chunk_header - end - - def readpartial(max, buf = Z.dup) - while @input && @chunk_left <= 0 && ! parse_chunk_header - @buf << @input.readpartial(Const::CHUNK_SIZE, buf) - end - - if @input - begin - @buf << @input.read_nonblock(Const::CHUNK_SIZE, buf) - rescue Errno::EAGAIN, Errno::EINTR - end - end - - max = @chunk_left if max > @chunk_left - buf.replace(last_block(max) || Z) - @chunk_left -= buf.size - (0 == buf.size && @input.nil?) and raise EOFError - buf - end - - private - - def last_block(max = nil) - rv = @buf - if max && rv && max < rv.size - @buf = rv[max - rv.size, rv.size - max] - return rv[0, max] - end - @buf = Z.dup - rv - end - - def parse_chunk_header - buf = @buf - # ignoring chunk-extension info for now, I haven't seen any use for it - # (or any users, and TE:chunked sent by clients is rare already) - # if there was not enough data in buffer to parse length of the chunk - # then just return - if buf.sub!(/\A(?:\r\n)?([a-fA-F0-9]{1,8})[^\r]*?\r\n/, Z) - @chunk_left = $1.to_i(16) - if 0 == @chunk_left # EOF - buf.sub!(/\A\r\n(?:\r\n)?/, Z) # cleanup for future requests - if trailer = @env[Const::HTTP_TRAILER] - tp = TrailerParser.new(trailer) - while ! tp.execute!(@env, buf) - buf << @input.readpartial(Const::CHUNK_SIZE) - end - end - @input = nil - end - return @chunk_left - end - - buf.size > 256 and - raise HttpParserError, - "malformed chunk, chunk-length not found in buffer: " \ - "#{buf.inspect}" - nil - end - - end - -end diff --git a/lib/unicorn/http_request.rb b/lib/unicorn/http_request.rb index ec215ae..26eff1f 100644 --- a/lib/unicorn/http_request.rb +++ b/lib/unicorn/http_request.rb @@ -1,6 +1,4 @@ -require 'stringio' - -# compiled extension +# coding:binary require 'unicorn_http' module Unicorn @@ -19,18 +17,13 @@ module Unicorn "SERVER_SOFTWARE" => "Unicorn #{Const::UNICORN_VERSION}".freeze } - NULL_IO = StringIO.new(Z) LOCALHOST = '127.0.0.1'.freeze - def initialize - end - # Being explicitly single-threaded, we have certain advantages in # not having to worry about variables being clobbered :) - BUFFER = ' ' * Const::CHUNK_SIZE # initial size, may grow - BUFFER.force_encoding(Encoding::BINARY) if Z.respond_to?(:force_encoding) + BUF = ' ' * Const::CHUNK_SIZE # initial size, may grow PARSER = HttpParser.new - PARAMS = Hash.new + REQ = {} # Does the majority of the IO processing. It has been written in # Ruby using about 8 different IO processing strategies. @@ -46,7 +39,7 @@ module Unicorn # This does minimal exception trapping and it is up to the caller # to handle any socket errors (e.g. user aborted upload). def read(socket) - PARAMS.clear + REQ.clear PARSER.reset # From http://www.ietf.org/rfc/rfc3875: @@ -56,42 +49,30 @@ module Unicorn # identify the client for the immediate request to the server; # that client may be a proxy, gateway, or other intermediary # acting on behalf of the actual source client." - PARAMS[Const::REMOTE_ADDR] = + REQ[Const::REMOTE_ADDR] = TCPSocket === socket ? socket.peeraddr.last : LOCALHOST # short circuit the common case with small GET requests first - PARSER.execute(PARAMS, socket.readpartial(Const::CHUNK_SIZE, BUFFER)) and + PARSER.headers(REQ, socket.readpartial(Const::CHUNK_SIZE, BUF)) and return handle_body(socket) - data = BUFFER.dup # socket.readpartial will clobber BUFFER + data = BUF.dup # socket.readpartial will clobber data # Parser is not done, queue up more data to read and continue parsing # an Exception thrown from the PARSER will throw us out of the loop begin - data << socket.readpartial(Const::CHUNK_SIZE, BUFFER) - PARSER.execute(PARAMS, data) and return handle_body(socket) + BUF << socket.readpartial(Const::CHUNK_SIZE, data) + PARSER.headers(REQ, BUF) and return handle_body(socket) end while true end private # Handles dealing with the rest of the request - # returns a Rack environment if successful + # returns a # Rack environment if successful def handle_body(socket) - PARAMS[Const::RACK_INPUT] = if (body = PARAMS.delete(:http_body)) - length = PARAMS[Const::CONTENT_LENGTH].to_i - - if /\Achunked\z/i =~ PARAMS[Const::HTTP_TRANSFER_ENCODING] - socket = ChunkedReader.new(PARAMS, socket, body) - length = body = nil - end - - TeeInput.new(socket, length, body) - else - NULL_IO - end - - PARAMS.update(DEFAULTS) + REQ[Const::RACK_INPUT] = Unicorn::TeeInput.new(socket) + REQ.update(DEFAULTS) end end diff --git a/lib/unicorn/tee_input.rb b/lib/unicorn/tee_input.rb index bbc496b..07676a6 100644 --- a/lib/unicorn/tee_input.rb +++ b/lib/unicorn/tee_input.rb @@ -13,15 +13,23 @@ module Unicorn class TeeInput - def initialize(input, size, body) - @tmp = Unicorn::Util.tmpio - - if body - @tmp.write(body) + # it's so awesome to not have to care for thread safety... + + RAW = HttpRequest::BUF # :nodoc: + DST = RAW.dup # :nodoc: + PARSER = HttpRequest::PARSER # :nodoc: + REQ = HttpRequest::REQ # :nodoc: + + def initialize(socket) + @tmp = Util.tmpio + @size = PARSER.content_length + return(@input = nil) if 0 == @size + @input = socket + if RAW.size > 0 + PARSER.read_body(DST, RAW) and finalize_input + @tmp.write(DST) @tmp.seek(0) end - @input = input - @size = size # nil if chunked end # returns the size of the input. This is what the Content-Length @@ -32,10 +40,10 @@ module Unicorn @size and return @size if @input - buf = Z.dup - while tee(Const::CHUNK_SIZE, buf) + pos = @tmp.tell + while tee(Const::CHUNK_SIZE, DST) end - @tmp.rewind + @tmp.seek(pos) end @size = @tmp.stat.size @@ -47,13 +55,12 @@ module Unicorn length = args.shift if nil == length rv = @tmp.read || Z.dup - tmp = Z.dup - while tee(Const::CHUNK_SIZE, tmp) - rv << tmp + while tee(Const::CHUNK_SIZE, DST) + rv << DST end rv else - buf = args.shift || Z.dup + buf = args.shift || DST.dup diff = @tmp.stat.size - @tmp.pos if 0 == diff tee(length, buf) @@ -70,7 +77,7 @@ module Unicorn orig_size = @tmp.stat.size if @tmp.pos == orig_size - tee(Const::CHUNK_SIZE, Z.dup) or return nil + tee(Const::CHUNK_SIZE, DST) or return nil @tmp.seek(orig_size) end @@ -79,8 +86,8 @@ module Unicorn # unlikely, if we got here, then @tmp is at EOF begin - orig_size = @tmp.stat.size - tee(Const::CHUNK_SIZE, Z.dup) or break + orig_size = @tmp.pos + tee(Const::CHUNK_SIZE, DST) or break @tmp.seek(orig_size) line << @tmp.gets $/ == line[-$/.size, $/.size] and return line @@ -108,25 +115,23 @@ module Unicorn # backing store as well as returning it. +buf+ must be specified. # returns nil if reading from the input returns nil def tee(length, buf) - begin - if @size - left = @size - @tmp.stat.size - 0 == left and return nil - if length >= left - @input.readpartial(left, buf) == left and @input = nil - elsif @input.nil? - return nil - else - @input.readpartial(length, buf) + unless PARSER.body_eof? + begin + if PARSER.read_body(buf, @input.readpartial(length, RAW)).nil? + @tmp.write(buf) + return buf end - else # ChunkedReader#readpartial just raises EOFError when done - @input.readpartial(length, buf) + rescue EOFError end - rescue EOFError - return @input = nil end - @tmp.write(buf) - buf + finalize_input + end + + def finalize_input + while PARSER.trailers(REQ, RAW).nil? + RAW << @input.readpartial(Const::CHUNK_SIZE, DST) + end + @input = nil end end diff --git a/lib/unicorn/trailer_parser.rb b/lib/unicorn/trailer_parser.rb deleted file mode 100644 index 22f2e1d..0000000 --- a/lib/unicorn/trailer_parser.rb +++ /dev/null @@ -1,52 +0,0 @@ -# Copyright (c) 2009 Eric Wong -# You can redistribute it and/or modify it under the same terms as Ruby. -require 'unicorn' -require 'unicorn_http' - -# Eventually I should integrate this into HttpParser... -module Unicorn - class TrailerParser - - TR_FR = 'a-z-'.freeze - TR_TO = 'A-Z_'.freeze - - # initializes HTTP trailer parser with acceptable +trailer+ - def initialize(http_trailer) - @trailers = http_trailer.split(/\s*,\s*/).inject({}) { |hash, key| - hash[key.tr(TR_FR, TR_TO)] = true - hash - } - end - - # Executes our TrailerParser on +data+ and modifies +env+ This will - # shrink +data+ as it is being consumed. Returns true if it has - # parsed all trailers, false if not. It raises HttpParserError on - # parse failure or unknown headers. It has slightly smaller limits - # than the C-based HTTP parser but should not be an issue in practice - # since Content-MD5 is probably the only legitimate use for it. - def execute!(env, data) - data.size > 0xffff and - raise HttpParserError, "trailer buffer too large: #{data.size} bytes" - - begin - data.sub!(/\A([^\r]+)\r\n/, Z) or return false # need more data - - key, val = $1.split(/:\s*/, 2) - - key.size > 256 and - raise HttpParserError, "trailer key #{key.inspect} is too long" - val.size > 8192 and - raise HttpParserError, "trailer value #{val.inspect} is too long" - - key.tr!(TR_FR, TR_TO) - - @trailers.delete(key) or - raise HttpParserError, "unknown trailer: #{key.inspect}" - env["HTTP_#{key}"] = val - - @trailers.empty? and return true - end while true - end - - end -end diff --git a/test/unit/test_chunked_reader.rb b/test/unit/test_chunked_reader.rb deleted file mode 100644 index a1323e9..0000000 --- a/test/unit/test_chunked_reader.rb +++ /dev/null @@ -1,123 +0,0 @@ -require 'test/unit' -require 'unicorn' -require 'unicorn_http' -require 'tempfile' -require 'io/nonblock' -require 'digest/sha1' - -class TestChunkedReader < Test::Unit::TestCase - - def setup - @env = {} - @rd, @wr = IO.pipe - @rd.binmode - @wr.binmode - @rd.sync = @wr.sync = true - @start_pid = $$ - end - - def teardown - return if $$ != @start_pid - @rd.close rescue nil - @wr.close rescue nil - begin - Process.wait - rescue Errno::ECHILD - break - end while true - end - - def test_error - cr = bin_reader("8\r\nasdfasdf\r\n8\r\nasdfasdfa#{'a' * 1024}") - a = nil - assert_nothing_raised { a = cr.readpartial(8192) } - assert_equal 'asdfasdf', a - assert_nothing_raised { a = cr.readpartial(8192) } - assert_equal 'asdfasdf', a - assert_raises(Unicorn::HttpParserError) { cr.readpartial(8192) } - end - - def test_eof1 - cr = bin_reader("0\r\n") - assert_raises(EOFError) { cr.readpartial(8192) } - end - - def test_eof2 - cr = bin_reader("0\r\n\r\n") - assert_raises(EOFError) { cr.readpartial(8192) } - end - - def test_readpartial1 - cr = bin_reader("4\r\nasdf\r\n0\r\n") - assert_equal 'asdf', cr.readpartial(8192) - assert_raises(EOFError) { cr.readpartial(8192) } - end - - def test_dd - cr = bin_reader("6\r\nhello\n\r\n") - tmp = Tempfile.new('test_dd') - tmp.sync = true - - pid = fork { - crd, cwr = IO.pipe - crd.binmode - cwr.binmode - crd.sync = cwr.sync = true - - pid = fork { - STDOUT.reopen(cwr) - crd.close - cwr.close - exec('dd', 'if=/dev/urandom', 'bs=93390', 'count=16') - } - cwr.close - begin - buf = crd.readpartial(16384) - tmp.write(buf) - @wr.write("#{'%x' % buf.size}\r\n#{buf}\r\n") - rescue EOFError - @wr.write("0\r\n\r\n") - Process.waitpid(pid) - exit 0 - end while true - } - assert_equal "hello\n", cr.readpartial(6) - sha1 = Digest::SHA1.new - buf = Unicorn::Z.dup - begin - cr.readpartial(16384, buf) - sha1.update(buf) - rescue EOFError - break - end while true - - assert_nothing_raised { Process.waitpid(pid) } - sha1_file = Digest::SHA1.new - File.open(tmp.path, 'rb') { |fp| - while fp.read(16384, buf) - sha1_file.update(buf) - end - } - assert_equal sha1_file.hexdigest, sha1.hexdigest - end - - def test_trailer - @env['HTTP_TRAILER'] = 'Content-MD5' - pid = fork { @wr.syswrite("Content-MD5: asdf\r\n") } - cr = bin_reader("8\r\nasdfasdf\r\n8\r\nasdfasdf\r\n0\r\n") - assert_equal 'asdfasdf', cr.readpartial(4096) - assert_equal 'asdfasdf', cr.readpartial(4096) - assert_raises(EOFError) { cr.readpartial(4096) } - pid, status = Process.waitpid2(pid) - assert status.success? - assert_equal 'asdf', @env['HTTP_CONTENT_MD5'] - end - -private - - def bin_reader(buf) - buf.force_encoding(Encoding::BINARY) if buf.respond_to?(:force_encoding) - Unicorn::ChunkedReader.new(@env, @rd, buf) - end - -end diff --git a/test/unit/test_http_parser.rb b/test/unit/test_http_parser.rb index 560f8d4..7072571 100644 --- a/test/unit/test_http_parser.rb +++ b/test/unit/test_http_parser.rb @@ -14,7 +14,8 @@ class HttpParserTest < Test::Unit::TestCase parser = HttpParser.new req = {} http = "GET / HTTP/1.1\r\n\r\n" - assert parser.execute(req, http) + assert_equal req, parser.headers(req, http) + assert_equal '', http assert_equal 'HTTP/1.1', req['SERVER_PROTOCOL'] assert_equal '/', req['REQUEST_PATH'] @@ -23,17 +24,18 @@ 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 - assert ! parser.execute(req, "G") + http = "G" + assert_nil parser.headers(req, http) + assert_equal "G", http 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 parser.headers(req, http) assert_equal 'HTTP/1.1', req['SERVER_PROTOCOL'] assert_equal '/hello-world', req['REQUEST_PATH'] @@ -42,52 +44,56 @@ 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] + assert_equal '', http end def test_parse_server_host_default_port parser = HttpParser.new req = {} - assert parser.execute(req, "GET / HTTP/1.1\r\nHost: foo\r\n\r\n") + tmp = "GET / HTTP/1.1\r\nHost: foo\r\n\r\n" + assert_equal req, parser.headers(req, tmp) assert_equal 'foo', req['SERVER_NAME'] assert_equal '80', req['SERVER_PORT'] - assert_nil req[:http_body] + assert_equal '', tmp end def test_parse_server_host_alt_port parser = HttpParser.new req = {} - assert parser.execute(req, "GET / HTTP/1.1\r\nHost: foo:999\r\n\r\n") + tmp = "GET / HTTP/1.1\r\nHost: foo:999\r\n\r\n" + assert_equal req, parser.headers(req, tmp) assert_equal 'foo', req['SERVER_NAME'] assert_equal '999', req['SERVER_PORT'] - assert_nil req[:http_body] + assert_equal '', tmp end def test_parse_server_host_empty_port parser = HttpParser.new req = {} - assert parser.execute(req, "GET / HTTP/1.1\r\nHost: foo:\r\n\r\n") + tmp = "GET / HTTP/1.1\r\nHost: foo:\r\n\r\n" + assert_equal req, parser.headers(req, tmp) assert_equal 'foo', req['SERVER_NAME'] assert_equal '80', req['SERVER_PORT'] - assert_nil req[:http_body] + assert_equal '', tmp end def test_parse_server_host_xfp_https parser = HttpParser.new req = {} - assert parser.execute(req, "GET / HTTP/1.1\r\nHost: foo:\r\n" \ - "X-Forwarded-Proto: https\r\n\r\n") + tmp = "GET / HTTP/1.1\r\nHost: foo:\r\n" \ + "X-Forwarded-Proto: https\r\n\r\n" + assert_equal req, parser.headers(req, tmp) assert_equal 'foo', req['SERVER_NAME'] assert_equal '443', req['SERVER_PORT'] - assert_nil req[:http_body] + assert_equal '', tmp end def test_parse_strange_headers parser = HttpParser.new 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] + assert_equal req, parser.headers(req, should_be_good) + assert_equal '', should_be_good # 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, @@ -110,8 +116,9 @@ class HttpParserTest < Test::Unit::TestCase parser = HttpParser.new req = {} sorta_safe = %(GET #{path} HTTP/1.1\r\n\r\n) - assert parser.execute(req, sorta_safe) - assert_nil req[:http_body] + assert_equal req, parser.headers(req, sorta_safe) + assert_equal path, req['REQUEST_URI'] + assert_equal '', sorta_safe end end @@ -120,30 +127,32 @@ class HttpParserTest < Test::Unit::TestCase req = {} bad_http = "GET / SsUTF/1.1" - assert_raises(HttpParserError) { parser.execute(req, bad_http) } + assert_raises(HttpParserError) { parser.headers(req, bad_http) } + + # make sure we can recover parser.reset - assert(parser.execute({}, "GET / HTTP/1.0\r\n\r\n")) - assert_nil req[:http_body] + req.clear + assert_equal req, parser.headers(req, "GET / HTTP/1.0\r\n\r\n") end 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_nil parser.headers(req, http) + assert_nil parser.headers(req, http) + assert_nil parser.headers(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_nil parser.headers(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_nil parser.headers(req, http << "\r") + assert_equal req, parser.headers(req, http << "\n") assert_equal 'HTTP/1.1', req['SERVER_PROTOCOL'] assert_nil req['FRAGMENT'] assert_equal '', req['QUERY_STRING'] - assert_nil req[:http_body] + assert_equal "", http end # not common, but underscores do appear in practice @@ -151,7 +160,7 @@ class HttpParserTest < Test::Unit::TestCase parser = HttpParser.new req = {} http = "GET http://under_score.example.com/foo?q=bar HTTP/1.0\r\n\r\n" - assert parser.execute(req, http) + assert_equal req, parser.headers(req, http) assert_equal 'http', req['rack.url_scheme'] assert_equal '/foo?q=bar', req['REQUEST_URI'] assert_equal '/foo', req['REQUEST_PATH'] @@ -160,14 +169,14 @@ 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] + assert_equal "", http end def test_absolute_uri parser = HttpParser.new req = {} http = "GET http://example.com/foo?q=bar HTTP/1.0\r\n\r\n" - assert parser.execute(req, http) + assert_equal req, parser.headers(req, http) assert_equal 'http', req['rack.url_scheme'] assert_equal '/foo?q=bar', req['REQUEST_URI'] assert_equal '/foo', req['REQUEST_PATH'] @@ -176,6 +185,7 @@ class HttpParserTest < Test::Unit::TestCase assert_equal 'example.com', req['HTTP_HOST'] assert_equal 'example.com', req['SERVER_NAME'] assert_equal '80', req['SERVER_PORT'] + assert_equal "", http end # X-Forwarded-Proto is not in rfc2616, absolute URIs are, however... @@ -184,7 +194,7 @@ class HttpParserTest < Test::Unit::TestCase req = {} http = "GET https://example.com/foo?q=bar HTTP/1.1\r\n" \ "X-Forwarded-Proto: http\r\n\r\n" - assert parser.execute(req, http) + assert_equal req, parser.headers(req, http) assert_equal 'https', req['rack.url_scheme'] assert_equal '/foo?q=bar', req['REQUEST_URI'] assert_equal '/foo', req['REQUEST_PATH'] @@ -193,6 +203,7 @@ class HttpParserTest < Test::Unit::TestCase assert_equal 'example.com', req['HTTP_HOST'] assert_equal 'example.com', req['SERVER_NAME'] assert_equal '443', req['SERVER_PORT'] + assert_equal "", http end # Host: header should be ignored for absolute URIs @@ -201,7 +212,7 @@ class HttpParserTest < Test::Unit::TestCase req = {} http = "GET http://example.com:8080/foo?q=bar HTTP/1.2\r\n" \ "Host: bad.example.com\r\n\r\n" - assert parser.execute(req, http) + assert_equal req, parser.headers(req, http) assert_equal 'http', req['rack.url_scheme'] assert_equal '/foo?q=bar', req['REQUEST_URI'] assert_equal '/foo', req['REQUEST_PATH'] @@ -210,6 +221,7 @@ class HttpParserTest < Test::Unit::TestCase assert_equal 'example.com:8080', req['HTTP_HOST'] assert_equal 'example.com', req['SERVER_NAME'] assert_equal '8080', req['SERVER_PORT'] + assert_equal "", http end def test_absolute_uri_with_empty_port @@ -217,7 +229,7 @@ class HttpParserTest < Test::Unit::TestCase req = {} http = "GET https://example.com:/foo?q=bar HTTP/1.1\r\n" \ "Host: bad.example.com\r\n\r\n" - assert parser.execute(req, http) + assert_equal req, parser.headers(req, http) assert_equal 'https', req['rack.url_scheme'] assert_equal '/foo?q=bar', req['REQUEST_URI'] assert_equal '/foo', req['REQUEST_PATH'] @@ -226,32 +238,33 @@ class HttpParserTest < Test::Unit::TestCase assert_equal 'example.com:', req['HTTP_HOST'] assert_equal 'example.com', req['SERVER_NAME'] assert_equal '443', req['SERVER_PORT'] + assert_equal "", http 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, parser.headers(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] + assert_equal "abcde", http 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 req, parser.headers(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] + assert_equal "", http end def test_unknown_methods @@ -261,13 +274,13 @@ class HttpParserTest < Test::Unit::TestCase 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) + ok = parser.headers(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 "", s assert_equal m, req['REQUEST_METHOD'] } end @@ -278,13 +291,13 @@ class HttpParserTest < Test::Unit::TestCase get = "GET /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, get) + ok = parser.headers(req, get) 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_nil req[:http_body] + assert_equal '', get end # lame random garbage maker @@ -309,7 +322,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) + parser.headers({}, get) parser.reset end end @@ -318,7 +331,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) + parser.headers({}, get) parser.reset end end @@ -327,7 +340,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) + parser.headers({}, get) parser.reset end @@ -335,7 +348,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) + parser.headers({}, get) parser.reset end end diff --git a/test/unit/test_server.rb b/test/unit/test_server.rb index 22b9934..0b409ba 100644 --- a/test/unit/test_server.rb +++ b/test/unit/test_server.rb @@ -33,6 +33,8 @@ class WebServerTest < Test::Unit::TestCase def teardown redirect_test_io do + wait_workers_ready("test_stderr.#$$.log", 1) + File.truncate("test_stderr.#$$.log", 0) @server.stop(true) end end @@ -53,8 +55,10 @@ class WebServerTest < Test::Unit::TestCase end results = hit(["http://localhost:#@port/"]) worker_pid = results[0].to_i + assert worker_pid != 0 tmp.sysseek(0) loader_pid = tmp.sysread(4096).to_i + assert loader_pid != 0 assert_equal worker_pid, loader_pid teardown @@ -65,6 +69,7 @@ class WebServerTest < Test::Unit::TestCase end results = hit(["http://localhost:#@port/"]) worker_pid = results[0].to_i + assert worker_pid != 0 tmp.sysseek(0) loader_pid = tmp.sysread(4096).to_i assert_equal $$, loader_pid @@ -158,9 +163,9 @@ class WebServerTest < Test::Unit::TestCase do_test(long, Unicorn::Const::CHUNK_SIZE * 2 -400) end - def test_file_streamed_request_bad_method + def test_file_streamed_request_bad_body body = "a" * (Unicorn::Const::MAX_BODY * 2) - long = "GET /test HTTP/1.1\r\nContent-length: #{body.length}\r\n\r\n" + body + long = "GET /test HTTP/1.1\r\nContent-ength: #{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) diff --git a/test/unit/test_tee_input.rb b/test/unit/test_tee_input.rb index a6c61e6..3a6c998 100644 --- a/test/unit/test_tee_input.rb +++ b/test/unit/test_tee_input.rb @@ -26,7 +26,8 @@ class TestTeeInput < Test::Unit::TestCase end def test_gets_long - ti = Unicorn::TeeInput.new(@rd, nil, "hello") + init_parser("hello", 5 + (4096 * 4 * 3) + "#$/foo#$/".size) + ti = Unicorn::TeeInput.new(@rd) status = line = nil pid = fork { @rd.close @@ -46,7 +47,8 @@ class TestTeeInput < Test::Unit::TestCase end def test_gets_short - ti = Unicorn::TeeInput.new(@rd, nil, "hello") + init_parser("hello", 5 + "#$/foo".size) + ti = Unicorn::TeeInput.new(@rd) status = line = nil pid = fork { @rd.close @@ -63,4 +65,19 @@ class TestTeeInput < Test::Unit::TestCase assert status.success? end +private + + def init_parser(body, size = nil) + @parser = Unicorn::TeeInput::PARSER + @parser.reset + body = body.to_s.freeze + buf = "POST / HTTP/1.1\r\n" \ + "Host: localhost\r\n" \ + "Content-Length: #{size || body.size}\r\n" \ + "\r\n#{body}" + buf = Unicorn::TeeInput::RAW.replace(buf) + assert_equal @env, @parser.headers(@env, buf) + assert_equal body, buf + end + end diff --git a/test/unit/test_trailer_parser.rb b/test/unit/test_trailer_parser.rb deleted file mode 100644 index 5f3b16d..0000000 --- a/test/unit/test_trailer_parser.rb +++ /dev/null @@ -1,52 +0,0 @@ -require 'test/unit' -require 'unicorn' -require 'unicorn_http' -require 'unicorn/trailer_parser' - -class TestTrailerParser < Test::Unit::TestCase - - def test_basic - tp = Unicorn::TrailerParser.new('Content-MD5') - env = {} - assert ! tp.execute!(env, "Content-MD5: asdf") - assert env.empty? - assert tp.execute!(env, "Content-MD5: asdf\r\n") - assert_equal 'asdf', env['HTTP_CONTENT_MD5'] - assert_equal 1, env.size - end - - def test_invalid_trailer - tp = Unicorn::TrailerParser.new('Content-MD5') - env = {} - assert_raises(Unicorn::HttpParserError) { - tp.execute!(env, "Content-MD: asdf\r\n") - } - assert env.empty? - end - - def test_multiple_trailer - tp = Unicorn::TrailerParser.new('Foo,Bar') - env = {} - buf = "Bar: a\r\nFoo: b\r\n" - assert tp.execute!(env, buf) - assert_equal 'a', env['HTTP_BAR'] - assert_equal 'b', env['HTTP_FOO'] - end - - def test_too_big_key - tp = Unicorn::TrailerParser.new('Foo,Bar') - env = {} - buf = "Bar#{'a' * 1024}: a\r\nFoo: b\r\n" - assert_raises(Unicorn::HttpParserError) { tp.execute!(env, buf) } - assert env.empty? - end - - def test_too_big_value - tp = Unicorn::TrailerParser.new('Foo,Bar') - env = {} - buf = "Bar: #{'a' * (1024 * 1024)}: a\r\nFoo: b\r\n" - assert_raises(Unicorn::HttpParserError) { tp.execute!(env, buf) } - assert env.empty? - end - -end -- cgit v1.2.3-24-ge0c7