diff options
-rwxr-xr-x | GIT-VERSION-GEN | 2 | ||||
-rw-r--r-- | TODO | 3 | ||||
-rw-r--r-- | ext/unicorn_http/global_variables.h | 4 | ||||
-rw-r--r-- | ext/unicorn_http/unicorn_http.rl | 86 | ||||
-rw-r--r-- | lib/unicorn.rb | 1 | ||||
-rw-r--r-- | lib/unicorn/configurator.rb | 32 | ||||
-rw-r--r-- | lib/unicorn/const.rb | 4 | ||||
-rw-r--r-- | lib/unicorn/http_request.rb | 13 | ||||
-rw-r--r-- | lib/unicorn/http_server.rb | 9 | ||||
-rw-r--r-- | lib/unicorn/preread_input.rb | 2 | ||||
-rw-r--r-- | lib/unicorn/stream_input.rb | 156 | ||||
-rw-r--r-- | lib/unicorn/tee_input.rb | 147 | ||||
-rwxr-xr-x | t/t0013-rewindable-input-false.sh | 24 | ||||
-rw-r--r-- | t/t0013.ru | 12 | ||||
-rwxr-xr-x | t/t0014-rewindable-input-true.sh | 24 | ||||
-rw-r--r-- | t/t0014.ru | 12 | ||||
-rw-r--r-- | test/unit/test_http_parser.rb | 16 | ||||
-rw-r--r-- | test/unit/test_http_parser_ng.rb | 55 | ||||
-rw-r--r-- | test/unit/test_stream_input.rb | 160 | ||||
-rw-r--r-- | test/unit/test_tee_input.rb | 52 |
20 files changed, 601 insertions, 213 deletions
diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 2ca4f94..b4784c4 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.0.1.GIT +DEF_VER=v3.0.0pre1.GIT LF=' ' @@ -7,6 +7,3 @@ * scalability to >= 1024 worker processes for crazy NUMA systems * Rack 2.x support (when Rack 2.x exists) - -* allow disabling "rack.input" rewindability for performance - (but violate the Rack 1.x SPEC) diff --git a/ext/unicorn_http/global_variables.h b/ext/unicorn_http/global_variables.h index 8377704..274f456 100644 --- a/ext/unicorn_http/global_variables.h +++ b/ext/unicorn_http/global_variables.h @@ -26,8 +26,6 @@ static VALUE g_http; static VALUE g_http_09; static VALUE g_http_10; static VALUE g_http_11; -static VALUE g_GET; -static VALUE g_HEAD; /** Defines common length and error messages for input length validation. */ #define DEF_MAX_LENGTH(N, length) \ @@ -82,8 +80,6 @@ static void init_globals(void) DEF_GLOBAL(http_11, "HTTP/1.1"); DEF_GLOBAL(http_10, "HTTP/1.0"); DEF_GLOBAL(http_09, "HTTP/0.9"); - DEF_GLOBAL(GET, "GET"); - DEF_GLOBAL(HEAD, "HEAD"); } #undef DEF_GLOBAL diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl index 236fbaa..6cc2958 100644 --- a/ext/unicorn_http/unicorn_http.rl +++ b/ext/unicorn_http/unicorn_http.rl @@ -18,12 +18,12 @@ #define UH_FL_HASTRAILER 0x8 #define UH_FL_INTRAILER 0x10 #define UH_FL_INCHUNK 0x20 -#define UH_FL_KAMETHOD 0x40 +#define UH_FL_REQEOF 0x40 #define UH_FL_KAVERSION 0x80 #define UH_FL_HASHEADER 0x100 -/* both of these flags need to be set for keepalive to be supported */ -#define UH_FL_KEEPALIVE (UH_FL_KAMETHOD | UH_FL_KAVERSION) +/* all of these flags need to be set for keepalive to be supported */ +#define UH_FL_KEEPALIVE (UH_FL_KAVERSION | UH_FL_REQEOF | UH_FL_HASHEADER) /* keep this small for Rainbows! since every client has one */ struct http_parser { @@ -79,46 +79,29 @@ static void parser_error(const char *msg) */ static void hp_keepalive_connection(struct http_parser *hp, VALUE val) { - /* REQUEST_METHOD is always set before any headers */ - if (HP_FL_TEST(hp, KAMETHOD)) { - if (STR_CSTR_CASE_EQ(val, "keep-alive")) { - /* basically have HTTP/1.0 masquerade as HTTP/1.1+ */ - HP_FL_SET(hp, KAVERSION); - } else if (STR_CSTR_CASE_EQ(val, "close")) { - /* - * it doesn't matter what HTTP version or request method we have, - * if a client says "Connection: close", we disable keepalive - */ - HP_FL_UNSET(hp, KEEPALIVE); - } else { - /* - * client could've sent anything, ignore it for now. Maybe - * "HP_FL_UNSET(hp, KEEPALIVE);" just in case? - * Raising an exception might be too mean... - */ - } + if (STR_CSTR_CASE_EQ(val, "keep-alive")) { + /* basically have HTTP/1.0 masquerade as HTTP/1.1+ */ + HP_FL_SET(hp, KAVERSION); + } else if (STR_CSTR_CASE_EQ(val, "close")) { + /* + * it doesn't matter what HTTP version or request method we have, + * if a client says "Connection: close", we disable keepalive + */ + HP_FL_UNSET(hp, KAVERSION); + } else { + /* + * client could've sent anything, ignore it for now. Maybe + * "HP_FL_UNSET(hp, KAVERSION);" just in case? + * Raising an exception might be too mean... + */ } } static void request_method(struct http_parser *hp, const char *ptr, size_t len) { - VALUE v; + VALUE v = rb_str_new(ptr, len); - /* - * we only support keepalive for GET and HEAD requests for now other - * methods are too rarely seen to be worth optimizing. POST is unsafe - * since some clients send extra bytes after POST bodies. - */ - if (CONST_MEM_EQ("GET", ptr, len)) { - HP_FL_SET(hp, KAMETHOD); - v = g_GET; - } else if (CONST_MEM_EQ("HEAD", ptr, len)) { - HP_FL_SET(hp, KAMETHOD); - v = g_HEAD; - } else { - v = rb_str_new(ptr, len); - } rb_hash_aset(hp->env, g_request_method, v); } @@ -206,7 +189,8 @@ static void write_value(struct http_parser *hp, hp->len.content = parse_length(RSTRING_PTR(v), RSTRING_LEN(v)); if (hp->len.content < 0) parser_error("invalid Content-Length"); - HP_FL_SET(hp, HASBODY); + if (hp->len.content != 0) + HP_FL_SET(hp, HASBODY); hp_invalid_if_trailer(hp); } else if (f == g_http_transfer_encoding) { if (STR_CSTR_CASE_EQ(v, "chunked")) { @@ -305,6 +289,7 @@ static void write_value(struct http_parser *hp, if (HP_FL_TEST(hp, CHUNKED)) cs = http_parser_en_ChunkedBody; } else { + HP_FL_SET(hp, REQEOF); assert(!HP_FL_TEST(hp, CHUNKED) && "chunked encoding without body!"); } /* @@ -559,6 +544,8 @@ static VALUE HttpParser_parse(VALUE self) hp->cs == http_parser_en_ChunkedBody) { advance_str(data, hp->offset + 1); hp->offset = 0; + if (HP_FL_TEST(hp, INTRAILER)) + HP_FL_SET(hp, REQEOF); return hp->env; } @@ -632,6 +619,25 @@ static VALUE HttpParser_keepalive(VALUE self) /** * call-seq: + * parser.next? => true or false + * + * Exactly like HttpParser#keepalive?, except it will reset the internal + * parser state if it returns true. + */ +static VALUE HttpParser_next(VALUE self) +{ + struct http_parser *hp = data_get(self); + + if (HP_FL_ALL(hp, KEEPALIVE)) { + http_parser_init(hp); + rb_funcall(hp->env, id_clear, 0); + return Qtrue; + } + return Qfalse; +} + +/** + * call-seq: * parser.headers? => true or false * * This should be used to detect if a request has headers (and if @@ -708,10 +714,13 @@ static VALUE HttpParser_filter_body(VALUE self, VALUE buf, VALUE data) if (hp->len.content > 0) { long nr = MIN(dlen, hp->len.content); + hp->buf = data; memcpy(RSTRING_PTR(buf), dptr, nr); hp->len.content -= nr; - if (hp->len.content == 0) + if (hp->len.content == 0) { + HP_FL_SET(hp, REQEOF); hp->cs = http_parser_first_final; + } advance_str(data, nr); rb_str_set_len(buf, nr); data = Qnil; @@ -747,6 +756,7 @@ void Init_unicorn_http(void) rb_define_method(cHttpParser, "body_eof?", HttpParser_body_eof, 0); rb_define_method(cHttpParser, "keepalive?", HttpParser_keepalive, 0); rb_define_method(cHttpParser, "headers?", HttpParser_has_headers, 0); + rb_define_method(cHttpParser, "next?", HttpParser_next, 0); rb_define_method(cHttpParser, "buf", HttpParser_buf, 0); rb_define_method(cHttpParser, "env", HttpParser_env, 0); diff --git a/lib/unicorn.rb b/lib/unicorn.rb index 622dc6c..7891d67 100644 --- a/lib/unicorn.rb +++ b/lib/unicorn.rb @@ -73,6 +73,7 @@ class Unicorn::ClientShutdown < EOFError; end require 'unicorn/const' require 'unicorn/socket_helper' +require 'unicorn/stream_input' require 'unicorn/tee_input' require 'unicorn/http_request' require 'unicorn/configurator' diff --git a/lib/unicorn/configurator.rb b/lib/unicorn/configurator.rb index dd515a7..2a83dea 100644 --- a/lib/unicorn/configurator.rb +++ b/lib/unicorn/configurator.rb @@ -39,6 +39,7 @@ class Unicorn::Configurator }, :pid => nil, :preload_app => false, + :rewindable_input => true, # for Rack 2.x: (Rack::VERSION[0] <= 1), } #:startdoc: @@ -373,12 +374,22 @@ class Unicorn::Configurator # cause the master process to exit with an error. def preload_app(bool) - case bool - when TrueClass, FalseClass - set[:preload_app] = bool - else - raise ArgumentError, "preload_app=#{bool.inspect} not a boolean" - end + set_bool(:preload_app, bool) + end + + # Toggles making <code>env["rack.input"]</code> rewindable. + # Disabling rewindability can improve performance by lowering + # I/O and memory usage for applications that accept uploads. + # Keep in mind that the Rack 1.x spec requires + # <code>env["rack.input"]</code> to be rewindable, so this allows + # intentionally violating the current Rack 1.x spec. + # + # +rewindable_input+ defaults to +true+ when used with Rack 1.x for + # Rack conformance. When Rack 2.x is finalized, this will most + # likely default to +false+ while still conforming to the newer + # (less demanding) spec. + def rewindable_input(bool) + set_bool(:rewindable_input, bool) end # Allow redirecting $stderr to a given path. Unlike doing this from @@ -469,6 +480,15 @@ private end end + def set_bool(var, bool) #:nodoc: + case bool + when true, false + set[var] = bool + else + raise ArgumentError, "#{var}=#{bool.inspect} not a boolean" + end + end + def set_hook(var, my_proc, req_arity = 2) #:nodoc: case my_proc when Proc diff --git a/lib/unicorn/const.rb b/lib/unicorn/const.rb index dc75914..375f72f 100644 --- a/lib/unicorn/const.rb +++ b/lib/unicorn/const.rb @@ -7,8 +7,8 @@ # improve things much compared to constants. module Unicorn::Const - # The current version of Unicorn, currently 2.0.1 - UNICORN_VERSION = "2.0.1" + # The current version of Unicorn, currently 3.0.0pre1 + UNICORN_VERSION = "3.0.0pre1" # default TCP listen host address (0.0.0.0, all interfaces) DEFAULT_HOST = "0.0.0.0" diff --git a/lib/unicorn/http_request.rb b/lib/unicorn/http_request.rb index 2dcd839..1e3ac26 100644 --- a/lib/unicorn/http_request.rb +++ b/lib/unicorn/http_request.rb @@ -25,7 +25,15 @@ class Unicorn::HttpParser # A frozen format for this is about 15% faster REMOTE_ADDR = 'REMOTE_ADDR'.freeze RACK_INPUT = 'rack.input'.freeze - TeeInput = Unicorn::TeeInput + @@input_class = Unicorn::TeeInput + + def self.input_class + @@input_class + end + + def self.input_class=(klass) + @@input_class = klass + end # :startdoc: # Does the majority of the IO processing. It has been written in @@ -63,7 +71,8 @@ class Unicorn::HttpParser buf << socket.kgio_read!(16384) end while parse.nil? end - e[RACK_INPUT] = 0 == content_length ? NULL_IO : TeeInput.new(socket, self) + e[RACK_INPUT] = 0 == content_length ? + NULL_IO : @@input_class.new(socket, self) e.merge!(DEFAULTS) end end diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb index 74b2b24..0bb4359 100644 --- a/lib/unicorn/http_server.rb +++ b/lib/unicorn/http_server.rb @@ -355,6 +355,15 @@ class Unicorn::HttpServer kill_each_worker(:KILL) end + def rewindable_input + Unicorn::HttpRequest.input_class.method_defined?(:rewind) + end + + def rewindable_input=(bool) + Unicorn::HttpRequest.input_class = bool ? + Unicorn::TeeInput : Unicorn::StreamInput + end + private # wait for a signal hander to wake us up and then consume the pipe diff --git a/lib/unicorn/preread_input.rb b/lib/unicorn/preread_input.rb index ec83cb2..7a315b7 100644 --- a/lib/unicorn/preread_input.rb +++ b/lib/unicorn/preread_input.rb @@ -20,7 +20,7 @@ class PrereadInput def call(env) buf = "" input = env["rack.input"] - if buf = input.read(16384) + if input.respond_to?(:rewind) true while input.read(16384, buf) input.rewind end diff --git a/lib/unicorn/stream_input.rb b/lib/unicorn/stream_input.rb new file mode 100644 index 0000000..ef8997e --- /dev/null +++ b/lib/unicorn/stream_input.rb @@ -0,0 +1,156 @@ +# -*- encoding: binary -*- + +# When processing uploads, Unicorn may expose a StreamInput object under +# "rack.input" of the Rack (2.x) environment. +class Unicorn::StreamInput + # The I/O chunk size (in +bytes+) for I/O operations where + # the size cannot be user-specified when a method is called. + # The default is 16 kilobytes. + @@io_chunk_size = Unicorn::Const::CHUNK_SIZE + + # Initializes a new StreamInput object. You normally do not have to call + # this unless you are writing an HTTP server. + def initialize(socket, request) + @chunked = request.content_length.nil? + @socket = socket + @parser = request + @buf = request.buf + @rbuf = '' + @bytes_read = 0 + filter_body(@rbuf, @buf) unless @buf.empty? + end + + # :call-seq: + # ios.read([length [, buffer ]]) => string, buffer, or nil + # + # Reads at most length bytes from the I/O stream, or to the end of + # file if length is omitted or is nil. length must be a non-negative + # integer or nil. If the optional buffer argument is present, it + # must reference a String, which will receive the data. + # + # At end of file, it returns nil or '' depend on length. + # ios.read() and ios.read(nil) returns ''. + # ios.read(length [, buffer]) returns nil. + # + # If the Content-Length of the HTTP request is known (as is the common + # case for POST requests), then ios.read(length [, buffer]) will block + # until the specified length is read (or it is the last chunk). + # Otherwise, for uncommon "Transfer-Encoding: chunked" requests, + # ios.read(length [, buffer]) will return immediately if there is + # any data and only block when nothing is available (providing + # IO#readpartial semantics). + def read(*args) + length = args.shift + rv = args.shift || '' + if length.nil? + read_all(rv) + else + if length <= @rbuf.size + rv.replace(@rbuf.slice(0, length)) + @rbuf.replace(@rbuf.slice(length, @rbuf.size) || '') + else + rv.replace(@rbuf) + length -= @rbuf.size + @rbuf.replace('') + until length == 0 || eof? || (rv.size > 0 && @chunked) + @socket.kgio_read(length, @buf) or eof! + filter_body(@rbuf, @buf) + rv << @rbuf + length -= @rbuf.size + @rbuf.replace('') + end + end + rv = nil if rv.empty? && length != 0 + end + rv + end + + # :call-seq: + # ios.gets => string or nil + # + # Reads the next ``line'' from the I/O stream; lines are separated + # by the global record separator ($/, typically "\n"). A global + # record separator of nil reads the entire unread contents of ios. + # Returns nil if called at the end of file. + # This takes zero arguments for strict Rack::Lint compatibility, + # unlike IO#gets. + def gets + sep = $/ + if sep.nil? + read_all(rv = '') + return rv.empty? ? nil : rv + end + re = /\A(.*?#{Regexp.escape(sep)})/ + + begin + @rbuf.gsub!(re, '') and return $1 + if eof? + if @rbuf.empty? + return nil + else + rv = @rbuf.dup + @rbuf.replace('') + return rv + end + end + @socket.kgio_read(@@io_chunk_size, @buf) or eof! + filter_body(once = '', @buf) + @rbuf << once + end while true + end + + # :call-seq: + # ios.each { |line| block } => ios + # + # Executes the block for every ``line'' in *ios*, where lines are + # separated by the global record separator ($/, typically "\n"). + def each(&block) + while line = gets + yield line + end + + self # Rack does not specify what the return value is here + end + +private + + def eof? + if @parser.body_eof? + until @parser.parse + once = @socket.kgio_read(@@io_chunk_size) or eof! + @buf << once + end + @socket = nil + true + else + false + end + end + + def filter_body(dst, src) + rv = @parser.filter_body(dst, src) + @bytes_read += dst.size + rv + end + + def read_all(dst) + dst.replace(@rbuf) + @socket or return + until eof? + @socket.kgio_read(@@io_chunk_size, @buf) or eof! + filter_body(@rbuf, @buf) + dst << @rbuf + end + ensure + @rbuf.replace('') + end + + def eof! + # in case client only did a premature shutdown(SHUT_WR) + # we do support clients that shutdown(SHUT_WR) after the + # _entire_ request has been sent, and those will not have + # raised EOFError on us. + @socket.close if @socket + raise Unicorn::ClientShutdown, "bytes_read=#{@bytes_read}", [] + end +end diff --git a/lib/unicorn/tee_input.rb b/lib/unicorn/tee_input.rb index a3e01d2..ee3effd 100644 --- a/lib/unicorn/tee_input.rb +++ b/lib/unicorn/tee_input.rb @@ -11,34 +11,18 @@ # # When processing uploads, Unicorn exposes a TeeInput object under # "rack.input" of the Rack environment. -class Unicorn::TeeInput - attr_accessor :tmp, :socket, :parser, :env, :buf, :len, :buf2 - +class Unicorn::TeeInput < Unicorn::StreamInput # The maximum size (in +bytes+) to buffer in memory before # resorting to a temporary file. Default is 112 kilobytes. @@client_body_buffer_size = Unicorn::Const::MAX_BODY - # The I/O chunk size (in +bytes+) for I/O operations where - # the size cannot be user-specified when a method is called. - # The default is 16 kilobytes. - @@io_chunk_size = Unicorn::Const::CHUNK_SIZE - # Initializes a new TeeInput object. You normally do not have to call # this unless you are writing an HTTP server. def initialize(socket, request) - @socket = socket - @parser = request - @buf = request.buf - @env = request.env @len = request.content_length + super @tmp = @len && @len < @@client_body_buffer_size ? StringIO.new("") : Unicorn::TmpIO.new - @buf2 = "" - if @buf.size > 0 - @parser.filter_body(@buf2, @buf) and finalize_input - @tmp.write(@buf2) - @tmp.rewind - end end # :call-seq: @@ -59,15 +43,10 @@ class Unicorn::TeeInput # specified +length+ in a loop until it returns +nil+. def size @len and return @len - - if socket - pos = @tmp.pos - while tee(@@io_chunk_size, @buf2) - end - @tmp.seek(pos) - end - - @len = @tmp.size + pos = @bytes_read + consume! + @tmp.pos = pos + @len = @bytes_read end # :call-seq: @@ -90,24 +69,7 @@ class Unicorn::TeeInput # any data and only block when nothing is available (providing # IO#readpartial semantics). def read(*args) - @socket or return @tmp.read(*args) - - length = args.shift - if nil == length - rv = @tmp.read || "" - while tee(@@io_chunk_size, @buf2) - rv << @buf2 - end - rv - else - rv = args.shift || "" - diff = @tmp.size - @tmp.pos - if 0 == diff - ensure_length(tee(length, rv), length) - else - ensure_length(@tmp.read(diff > length ? length : diff, rv), length) - end - end + @socket ? tee(super) : @tmp.read(*args) end # :call-seq: @@ -120,43 +82,7 @@ class Unicorn::TeeInput # This takes zero arguments for strict Rack::Lint compatibility, # unlike IO#gets. def gets - @socket or return @tmp.gets - sep = $/ or return read - - orig_size = @tmp.size - if @tmp.pos == orig_size - tee(@@io_chunk_size, @buf2) or return nil - @tmp.seek(orig_size) - end - - sep_size = Rack::Utils.bytesize(sep) - line = @tmp.gets # cannot be nil here since size > pos - sep == line[-sep_size, sep_size] and return line - - # unlikely, if we got here, then @tmp is at EOF - begin - orig_size = @tmp.pos - tee(@@io_chunk_size, @buf2) or break - @tmp.seek(orig_size) - line << @tmp.gets - sep == line[-sep_size, sep_size] and return line - # @tmp is at EOF again here, retry the loop - end while true - - line - end - - # :call-seq: - # ios.each { |line| block } => ios - # - # Executes the block for every ``line'' in *ios*, where lines are - # separated by the global record separator ($/, typically "\n"). - def each(&block) - while line = gets - yield line - end - - self # Rack does not specify what the return value is here + @socket ? tee(super) : @tmp.gets end # :call-seq: @@ -166,59 +92,24 @@ class Unicorn::TeeInput # the offset (zero) of the +ios+ pointer. Subsequent reads will # start from the beginning of the previously-buffered input. def rewind + return 0 if @bytes_read == 0 + consume! if @socket @tmp.rewind # Rack does not specify what the return value is here end private - # tees off a +length+ chunk of data from the input into the IO - # backing store as well as returning it. +dst+ must be specified. - # returns nil if reading from the input returns nil - def tee(length, dst) - unless @parser.body_eof? - r = @socket.kgio_read(length, @buf) or eof! - unless @parser.filter_body(dst, @buf) - @tmp.write(dst) - @tmp.seek(0, IO::SEEK_END) # workaround FreeBSD/OSX + MRI 1.8.x bug - return dst - end - end - finalize_input + # consumes the stream of the socket + def consume! + junk = "" + nil while read(@@io_chunk_size, junk) end - def finalize_input - while @parser.trailers(@env, @buf).nil? - r = @socket.kgio_read(@@io_chunk_size) or eof! - @buf << r + def tee(buffer) + if buffer && (n = buffer.size) > 0 + @tmp.write(buffer) + @tmp.seek(0, IO::SEEK_END) # workaround FreeBSD/OSX + MRI 1.8.x bug end - @socket = nil - end - - # tee()s into +dst+ until it is of +length+ bytes (or until - # we've reached the Content-Length of the request body). - # Returns +dst+ (the exact object, not a duplicate) - # To continue supporting applications that need near-real-time - # streaming input bodies, this is a no-op for - # "Transfer-Encoding: chunked" requests. - def ensure_length(dst, length) - # len is nil for chunked bodies, so we can't ensure length for those - # since they could be streaming bidirectionally and we don't want to - # block the caller in that case. - return dst if dst.nil? || @len.nil? - - while dst.size < length && tee(length - dst.size, @buf2) - dst << @buf2 - end - - dst - end - - def eof! - # in case client only did a premature shutdown(SHUT_WR) - # we do support clients that shutdown(SHUT_WR) after the - # _entire_ request has been sent, and those will not have - # raised EOFError on us. - @socket.close if @socket - raise Unicorn::ClientShutdown, "bytes_read=#{@tmp.size}", [] + buffer end end diff --git a/t/t0013-rewindable-input-false.sh b/t/t0013-rewindable-input-false.sh new file mode 100755 index 0000000..0e89631 --- /dev/null +++ b/t/t0013-rewindable-input-false.sh @@ -0,0 +1,24 @@ +#!/bin/sh +. ./test-lib.sh +t_plan 4 "rewindable_input toggled to false" + +t_begin "setup and start" && { + unicorn_setup + echo rewindable_input false >> $unicorn_config + unicorn -D -c $unicorn_config t0013.ru + unicorn_wait_start +} + +t_begin "ensure worker is started" && { + test xOK = x$(curl -T t0013.ru -H Expect: -vsSf http://$listen/) +} + +t_begin "killing succeeds" && { + kill $unicorn_pid +} + +t_begin "check stderr" && { + check_stderr +} + +t_done diff --git a/t/t0013.ru b/t/t0013.ru new file mode 100644 index 0000000..48a3a34 --- /dev/null +++ b/t/t0013.ru @@ -0,0 +1,12 @@ +#\ -E none +use Rack::ContentLength +use Rack::ContentType, 'text/plain' +app = lambda do |env| + case env['rack.input'] + when Unicorn::StreamInput + [ 200, {}, %w(OK) ] + else + [ 500, {}, %w(NO) ] + end +end +run app diff --git a/t/t0014-rewindable-input-true.sh b/t/t0014-rewindable-input-true.sh new file mode 100755 index 0000000..dd48bc6 --- /dev/null +++ b/t/t0014-rewindable-input-true.sh @@ -0,0 +1,24 @@ +#!/bin/sh +. ./test-lib.sh +t_plan 4 "rewindable_input toggled to true" + +t_begin "setup and start" && { + unicorn_setup + echo rewindable_input true >> $unicorn_config + unicorn -D -c $unicorn_config t0014.ru + unicorn_wait_start +} + +t_begin "ensure worker is started" && { + test xOK = x$(curl -T t0014.ru -sSf http://$listen/) +} + +t_begin "killing succeeds" && { + kill $unicorn_pid +} + +t_begin "check stderr" && { + check_stderr +} + +t_done diff --git a/t/t0014.ru b/t/t0014.ru new file mode 100644 index 0000000..b0bd2b7 --- /dev/null +++ b/t/t0014.ru @@ -0,0 +1,12 @@ +#\ -E none +use Rack::ContentLength +use Rack::ContentType, 'text/plain' +app = lambda do |env| + case env['rack.input'] + when Unicorn::TeeInput + [ 200, {}, %w(OK) ] + else + [ 500, {}, %w(NO) ] + end +end +run app diff --git a/test/unit/test_http_parser.rb b/test/unit/test_http_parser.rb index 222c227..31cb2cb 100644 --- a/test/unit/test_http_parser.rb +++ b/test/unit/test_http_parser.rb @@ -76,12 +76,22 @@ class HttpParserTest < Test::Unit::TestCase assert parser.keepalive? end - def test_connection_keep_alive_ka_bad_method + def test_connection_keep_alive_no_body parser = HttpParser.new req = {} tmp = "POST / HTTP/1.1\r\nConnection: keep-alive\r\n\r\n" assert_equal req.object_id, parser.headers(req, tmp).object_id - assert ! parser.keepalive? + assert parser.keepalive? + end + + def test_connection_keep_alive_no_body_empty + parser = HttpParser.new + req = {} + tmp = "POST / HTTP/1.1\r\n" \ + "Content-Length: 0\r\n" \ + "Connection: keep-alive\r\n\r\n" + assert_equal req.object_id, parser.headers(req, tmp).object_id + assert parser.keepalive? end def test_connection_keep_alive_ka_bad_version @@ -461,7 +471,7 @@ class HttpParserTest < Test::Unit::TestCase assert_equal 'page=1', req['QUERY_STRING'] assert_equal "", s assert_equal m, req['REQUEST_METHOD'] - assert ! parser.keepalive? # TODO: read HTTP/1.2 when it's final + assert parser.keepalive? # TODO: read HTTP/1.2 when it's final } end diff --git a/test/unit/test_http_parser_ng.rb b/test/unit/test_http_parser_ng.rb index 65b843e..ce6c6e6 100644 --- a/test/unit/test_http_parser_ng.rb +++ b/test/unit/test_http_parser_ng.rb @@ -11,6 +11,19 @@ class HttpParserNgTest < Test::Unit::TestCase @parser = HttpParser.new end + def test_default_keepalive_is_off + assert ! @parser.keepalive? + assert ! @parser.next? + assert_nothing_raised do + @parser.buf << "GET / HTTP/1.1\r\nHost: example.com\r\n\r\n" + @parser.parse + end + assert @parser.keepalive? + @parser.reset + assert ! @parser.keepalive? + assert ! @parser.next? + end + def test_identity_byte_headers req = {} str = "PUT / HTTP/1.1\r\n" @@ -27,6 +40,12 @@ class HttpParserNgTest < Test::Unit::TestCase assert ! @parser.keepalive? assert @parser.headers? assert_equal 123, @parser.content_length + dst = "" + buf = '.' * 123 + @parser.filter_body(dst, buf) + assert_equal '.' * 123, dst + assert_equal "", buf + assert @parser.keepalive? end def test_identity_step_headers @@ -41,6 +60,12 @@ class HttpParserNgTest < Test::Unit::TestCase assert_equal 0, str.size assert ! @parser.keepalive? assert @parser.headers? + dst = "" + buf = '.' * 123 + @parser.filter_body(dst, buf) + assert_equal '.' * 123, dst + assert_equal "", buf + assert @parser.keepalive? end def test_identity_oneshot_header @@ -50,6 +75,12 @@ class HttpParserNgTest < Test::Unit::TestCase assert_equal '123', req['CONTENT_LENGTH'] assert_equal 0, str.size assert ! @parser.keepalive? + assert @parser.headers? + dst = "" + buf = '.' * 123 + @parser.filter_body(dst, buf) + assert_equal '.' * 123, dst + assert_equal "", buf end def test_identity_oneshot_header_with_body @@ -67,7 +98,7 @@ class HttpParserNgTest < Test::Unit::TestCase assert_equal 0, str.size assert_equal tmp, body assert_equal "", @parser.filter_body(tmp, str) - assert ! @parser.keepalive? + assert @parser.keepalive? end def test_identity_oneshot_header_with_body_partial @@ -85,7 +116,7 @@ class HttpParserNgTest < Test::Unit::TestCase assert_nil rv assert_equal "", str assert_equal str.object_id, @parser.filter_body(tmp, str).object_id - assert ! @parser.keepalive? + assert @parser.keepalive? end def test_identity_oneshot_header_with_body_slop @@ -99,7 +130,7 @@ class HttpParserNgTest < Test::Unit::TestCase assert_equal "G", @parser.filter_body(tmp, str) assert_equal 1, tmp.size assert_equal "a", tmp - assert ! @parser.keepalive? + assert @parser.keepalive? end def test_chunked @@ -122,6 +153,10 @@ class HttpParserNgTest < Test::Unit::TestCase assert_equal rv.object_id, @parser.filter_body(tmp, rv).object_id assert_equal "PUT", rv assert ! @parser.keepalive? + rv << "TY: FOO\r\n\r\n" + assert_equal req, @parser.trailers(req, rv) + assert_equal "FOO", req["HTTP_PUTTY"] + assert @parser.keepalive? end def test_two_chunks @@ -177,7 +212,7 @@ class HttpParserNgTest < Test::Unit::TestCase assert_equal req, @parser.trailers(req, moo = "\r\n") assert_equal "", moo assert @parser.body_eof? - assert ! @parser.keepalive? + assert @parser.keepalive? end def test_two_chunks_oneshot @@ -237,7 +272,7 @@ class HttpParserNgTest < Test::Unit::TestCase assert_nil @parser.trailers(req, str << "\r") assert_equal req, @parser.trailers(req, str << "\nGET / ") assert_equal "GET / ", str - assert ! @parser.keepalive? + assert @parser.keepalive? end def test_trailers_slowly @@ -297,14 +332,12 @@ class HttpParserNgTest < Test::Unit::TestCase assert_equal req, @parser.headers(req, str) assert_nil @parser.content_length assert_raise(HttpParserError) { @parser.filter_body('', str) } - assert ! @parser.keepalive? end def test_overflow_content_length n = HttpParser::LENGTH_MAX + 1 str = "PUT / HTTP/1.1\r\nContent-Length: #{n}\r\n\r\n" assert_raise(HttpParserError) { @parser.headers({}, str) } - assert ! @parser.keepalive? end def test_bad_chunk @@ -315,13 +348,11 @@ class HttpParserNgTest < Test::Unit::TestCase assert_equal req, @parser.headers(req, str) assert_nil @parser.content_length assert_raise(HttpParserError) { @parser.filter_body('', str) } - assert ! @parser.keepalive? end def test_bad_content_length str = "PUT / HTTP/1.1\r\nContent-Length: 7ff\r\n\r\n" assert_raise(HttpParserError) { @parser.headers({}, str) } - assert ! @parser.keepalive? end def test_bad_trailers @@ -338,7 +369,6 @@ class HttpParserNgTest < Test::Unit::TestCase assert_equal '', str str << "Transfer-Encoding: identity\r\n\r\n" assert_raise(HttpParserError) { @parser.trailers(req, str) } - assert ! @parser.keepalive? end def test_repeat_headers @@ -492,8 +522,9 @@ class HttpParserNgTest < Test::Unit::TestCase env1 = @parser.parse.dup assert_equal expect, env1 assert_equal str, @parser.buf - assert @parser.keepalive? - @parser.reset + assert ! @parser.env.empty? + assert @parser.next? + assert @parser.env.empty? env2 = @parser.parse.dup assert_equal expect, env2 assert_equal "", @parser.buf diff --git a/test/unit/test_stream_input.rb b/test/unit/test_stream_input.rb new file mode 100644 index 0000000..adf4571 --- /dev/null +++ b/test/unit/test_stream_input.rb @@ -0,0 +1,160 @@ +# -*- encoding: binary -*- + +require 'test/unit' +require 'digest/sha1' +require 'unicorn' + +class TestStreamInput < Test::Unit::TestCase + def setup + @rs = $/ + @env = {} + @rd, @wr = Kgio::UNIXSocket.pair + @rd.sync = @wr.sync = true + @start_pid = $$ + end + + def teardown + return if $$ != @start_pid + $/ = @rs + @rd.close rescue nil + @wr.close rescue nil + Process.waitall + end + + def test_read_small + r = init_request('hello') + si = Unicorn::StreamInput.new(@rd, r) + assert_equal 'hello', si.read + assert_equal '', si.read + assert_nil si.read(5) + assert_nil si.gets + end + + def test_gets_oneliner + r = init_request('hello') + si = Unicorn::StreamInput.new(@rd, r) + assert_equal 'hello', si.gets + assert_nil si.gets + end + + def test_gets_multiline + r = init_request("a\nb\n\n") + si = Unicorn::StreamInput.new(@rd, r) + assert_equal "a\n", si.gets + assert_equal "b\n", si.gets + assert_equal "\n", si.gets + assert_nil si.gets + end + + def test_gets_empty_rs + $/ = nil + r = init_request("a\nb\n\n") + si = Unicorn::StreamInput.new(@rd, r) + assert_equal "a\nb\n\n", si.gets + assert_nil si.gets + end + + def test_read_with_equal_len + r = init_request("abcde") + si = Unicorn::StreamInput.new(@rd, r) + assert_equal "abcde", si.read(5) + assert_nil si.read(5) + end + + def test_big_body_multi + r = init_request('.', Unicorn::Const::MAX_BODY + 1) + si = Unicorn::StreamInput.new(@rd, r) + assert_equal Unicorn::Const::MAX_BODY, @parser.content_length + assert ! @parser.body_eof? + nr = Unicorn::Const::MAX_BODY / 4 + pid = fork { + @rd.close + nr.times { @wr.write('....') } + @wr.close + } + @wr.close + assert_equal '.', si.read(1) + nr.times { |x| + assert_equal '....', si.read(4), "nr=#{x}" + } + assert_nil si.read(1) + status = nil + assert_nothing_raised { pid, status = Process.waitpid2(pid) } + assert status.success? + end + + def test_gets_long + r = init_request("hello", 5 + (4096 * 4 * 3) + "#$/foo#$/".size) + si = Unicorn::StreamInput.new(@rd, r) + status = line = nil + pid = fork { + @rd.close + 3.times { @wr.write("ffff" * 4096) } + @wr.write "#$/foo#$/" + @wr.close + } + @wr.close + assert_nothing_raised { line = si.gets } + assert_equal(4096 * 4 * 3 + 5 + $/.size, line.size) + assert_equal("hello" << ("ffff" * 4096 * 3) << "#$/", line) + assert_nothing_raised { line = si.gets } + assert_equal "foo#$/", line + assert_nil si.gets + assert_nothing_raised { pid, status = Process.waitpid2(pid) } + assert status.success? + end + + def test_read_with_buffer + r = init_request('hello') + si = Unicorn::StreamInput.new(@rd, r) + buf = '' + rv = si.read(4, buf) + assert_equal 'hell', rv + assert_equal 'hell', buf + assert_equal rv.object_id, buf.object_id + assert_equal 'o', si.read + assert_equal nil, si.read(5, buf) + end + + def test_read_with_buffer_clobbers + r = init_request('hello') + si = Unicorn::StreamInput.new(@rd, r) + buf = 'foo' + assert_equal 'hello', si.read(nil, buf) + assert_equal 'hello', buf + assert_equal '', si.read(nil, buf) + assert_equal '', buf + buf = 'asdf' + assert_nil si.read(5, buf) + assert_equal '', buf + end + + def test_read_zero + r = init_request('hello') + si = Unicorn::StreamInput.new(@rd, r) + assert_equal '', si.read(0) + buf = 'asdf' + rv = si.read(0, buf) + assert_equal rv.object_id, buf.object_id + assert_equal '', buf + assert_equal 'hello', si.read + assert_nil si.read(5) + assert_equal '', si.read(0) + buf = 'hello' + rv = si.read(0, buf) + assert_equal rv.object_id, buf.object_id + assert_equal '', rv + end + + def init_request(body, size = nil) + @parser = Unicorn::HttpParser.new + 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}" + assert_equal @env, @parser.headers(@env, @buf) + assert_equal body, @buf + @parser + end +end diff --git a/test/unit/test_tee_input.rb b/test/unit/test_tee_input.rb index a10ca34..e69c8f1 100644 --- a/test/unit/test_tee_input.rb +++ b/test/unit/test_tee_input.rb @@ -4,6 +4,10 @@ require 'test/unit' require 'digest/sha1' require 'unicorn' +class TeeInput < Unicorn::TeeInput + attr_accessor :tmp, :len +end + class TestTeeInput < Test::Unit::TestCase def setup @@ -28,7 +32,7 @@ class TestTeeInput < Test::Unit::TestCase def test_gets_long r = init_request("hello", 5 + (4096 * 4 * 3) + "#$/foo#$/".size) - ti = Unicorn::TeeInput.new(@rd, r) + ti = TeeInput.new(@rd, r) status = line = nil pid = fork { @rd.close @@ -49,7 +53,7 @@ class TestTeeInput < Test::Unit::TestCase def test_gets_short r = init_request("hello", 5 + "#$/foo".size) - ti = Unicorn::TeeInput.new(@rd, r) + ti = TeeInput.new(@rd, r) status = line = nil pid = fork { @rd.close @@ -68,7 +72,7 @@ class TestTeeInput < Test::Unit::TestCase def test_small_body r = init_request('hello') - ti = Unicorn::TeeInput.new(@rd, r) + ti = TeeInput.new(@rd, r) assert_equal 0, @parser.content_length assert @parser.body_eof? assert_equal StringIO, ti.tmp.class @@ -77,11 +81,12 @@ class TestTeeInput < Test::Unit::TestCase assert_equal 'hello', ti.read assert_equal '', ti.read assert_nil ti.read(4096) + assert_equal 5, ti.size end def test_read_with_buffer r = init_request('hello') - ti = Unicorn::TeeInput.new(@rd, r) + ti = TeeInput.new(@rd, r) buf = '' rv = ti.read(4, buf) assert_equal 'hell', rv @@ -96,7 +101,7 @@ class TestTeeInput < Test::Unit::TestCase def test_big_body r = init_request('.' * Unicorn::Const::MAX_BODY << 'a') - ti = Unicorn::TeeInput.new(@rd, r) + ti = TeeInput.new(@rd, r) assert_equal 0, @parser.content_length assert @parser.body_eof? assert_kind_of File, ti.tmp @@ -108,7 +113,7 @@ class TestTeeInput < Test::Unit::TestCase a, b = 300, 3 r = init_request('.' * b, 300) assert_equal 300, @parser.content_length - ti = Unicorn::TeeInput.new(@rd, r) + ti = TeeInput.new(@rd, r) pid = fork { @wr.write('.' * 197) sleep 1 # still a *potential* race here that would make the test moot... @@ -122,12 +127,11 @@ class TestTeeInput < Test::Unit::TestCase def test_big_body_multi r = init_request('.', Unicorn::Const::MAX_BODY + 1) - ti = Unicorn::TeeInput.new(@rd, r) + ti = TeeInput.new(@rd, r) assert_equal Unicorn::Const::MAX_BODY, @parser.content_length assert ! @parser.body_eof? assert_kind_of File, ti.tmp assert_equal 0, ti.tmp.pos - assert_equal 1, ti.tmp.size assert_equal Unicorn::Const::MAX_BODY + 1, ti.size nr = Unicorn::Const::MAX_BODY / 4 pid = fork { @@ -138,8 +142,8 @@ class TestTeeInput < Test::Unit::TestCase @wr.close assert_equal '.', ti.read(1) assert_equal Unicorn::Const::MAX_BODY + 1, ti.size - nr.times { - assert_equal '....', ti.read(4) + nr.times { |x| + assert_equal '....', ti.read(4), "nr=#{x}" assert_equal Unicorn::Const::MAX_BODY + 1, ti.size } assert_nil ti.read(1) @@ -163,7 +167,7 @@ class TestTeeInput < Test::Unit::TestCase @wr.write("0\r\n\r\n") } @wr.close - ti = Unicorn::TeeInput.new(@rd, @parser) + ti = TeeInput.new(@rd, @parser) assert_nil @parser.content_length assert_nil ti.len assert ! @parser.body_eof? @@ -201,7 +205,7 @@ class TestTeeInput < Test::Unit::TestCase end @wr.write("0\r\n\r\n") } - ti = Unicorn::TeeInput.new(@rd, @parser) + ti = TeeInput.new(@rd, @parser) assert_nil @parser.content_length assert_nil ti.len assert ! @parser.body_eof? @@ -230,7 +234,7 @@ class TestTeeInput < Test::Unit::TestCase @wr.write("Hello: World\r\n\r\n") } @wr.close - ti = Unicorn::TeeInput.new(@rd, @parser) + ti = TeeInput.new(@rd, @parser) assert_nil @parser.content_length assert_nil ti.len assert ! @parser.body_eof? @@ -241,6 +245,28 @@ class TestTeeInput < Test::Unit::TestCase assert status.success? end + def test_chunked_and_size_slow + @parser = Unicorn::HttpParser.new + @buf = "POST / HTTP/1.1\r\n" \ + "Host: localhost\r\n" \ + "Trailer: Hello\r\n" \ + "Transfer-Encoding: chunked\r\n" \ + "\r\n" + assert_equal @env, @parser.headers(@env, @buf) + assert_equal "", @buf + + @wr.write("9\r\nabcde") + ti = TeeInput.new(@rd, @parser) + assert_nil @parser.content_length + assert_equal "abcde", ti.read(9) + assert ! @parser.body_eof? + @wr.write("fghi\r\n0\r\nHello: World\r\n\r\n") + assert_equal 9, ti.size + assert_equal "fghi", ti.read(9) + assert_equal nil, ti.read(9) + assert_equal "World", @env['HTTP_HELLO'] + end + private def init_request(body, size = nil) |