diff options
| -rw-r--r-- | .travis.yml | 7 | ||||
| -rw-r--r-- | README.rdoc | 1 | ||||
| -rw-r--r-- | SPEC | 78 | ||||
| -rw-r--r-- | lib/rack.rb | 1 | ||||
| -rw-r--r-- | lib/rack/builder.rb | 2 | ||||
| -rw-r--r-- | lib/rack/chunked.rb | 13 | ||||
| -rw-r--r-- | lib/rack/conditionalget.rb | 5 | ||||
| -rw-r--r-- | lib/rack/lint.rb | 94 | ||||
| -rw-r--r-- | lib/rack/lobster.rb | 11 | ||||
| -rw-r--r-- | lib/rack/mock.rb | 9 | ||||
| -rw-r--r-- | lib/rack/multipart/parser.rb | 7 | ||||
| -rw-r--r-- | lib/rack/request.rb | 8 | ||||
| -rw-r--r-- | lib/rack/response.rb | 1 | ||||
| -rw-r--r-- | lib/rack/server.rb | 46 | ||||
| -rw-r--r-- | lib/rack/showexceptions.rb | 31 | ||||
| -rw-r--r-- | lib/rack/tempfile_reaper.rb | 22 | ||||
| -rw-r--r-- | lib/rack/utils.rb | 36 | ||||
| -rw-r--r-- | test/spec_chunked.rb | 16 | ||||
| -rw-r--r-- | test/spec_lobster.rb | 2 | ||||
| -rw-r--r-- | test/spec_mock.rb | 8 | ||||
| -rw-r--r-- | test/spec_request.rb | 57 | ||||
| -rw-r--r-- | test/spec_response.rb | 5 | ||||
| -rw-r--r-- | test/spec_server.rb | 20 | ||||
| -rw-r--r-- | test/spec_showexceptions.rb | 65 | ||||
| -rw-r--r-- | test/spec_tempfile_reaper.rb | 63 | ||||
| -rw-r--r-- | test/spec_utils.rb | 27 |
26 files changed, 485 insertions, 150 deletions
diff --git a/.travis.yml b/.travis.yml index 588bf0ea..aa2eca43 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,14 +9,11 @@ rvm: - 1.9.2 - 1.9.3 - 2.0.0 - - 2.1.0 + - 2.1 - ruby-head - - rbx + - rbx-2 - jruby - ree -matrix: - allow_failures: - - rvm: rbx notifications: email: false irc: "irc.freenode.org#rack" diff --git a/README.rdoc b/README.rdoc index b6f383dc..d5bd6d2e 100644 --- a/README.rdoc +++ b/README.rdoc @@ -33,6 +33,7 @@ These web servers include Rack handlers in their distributions: * Unicorn * unixrack * uWSGI +* yahns * Zbatery Any valid Rack app will run the same on all these handlers, without @@ -40,7 +40,17 @@ below. <tt>QUERY_STRING</tt>:: The portion of the request URL that follows the <tt>?</tt>, if any. May be empty, but is always required! -<tt>SERVER_NAME</tt>, <tt>SERVER_PORT</tt>:: When combined with <tt>SCRIPT_NAME</tt> and <tt>PATH_INFO</tt>, these variables can be used to complete the URL. Note, however, that <tt>HTTP_HOST</tt>, if present, should be used in preference to <tt>SERVER_NAME</tt> for reconstructing the request URL. <tt>SERVER_NAME</tt> and <tt>SERVER_PORT</tt> can never be empty strings, and so are always required. +<tt>SERVER_NAME</tt>, <tt>SERVER_PORT</tt>:: + When combined with <tt>SCRIPT_NAME</tt> and + <tt>PATH_INFO</tt>, these variables can be + used to complete the URL. Note, however, + that <tt>HTTP_HOST</tt>, if present, + should be used in preference to + <tt>SERVER_NAME</tt> for reconstructing + the request URL. + <tt>SERVER_NAME</tt> and <tt>SERVER_PORT</tt> + can never be empty strings, and so + are always required. <tt>HTTP_</tt> Variables:: Variables corresponding to the client-supplied HTTP request headers (i.e., variables whose @@ -49,24 +59,47 @@ below. variables should correspond with the presence or absence of the appropriate HTTP header in the - request. See <a href="https://tools.ietf.org/html/rfc3875#section-4.1.18"> - RFC3875 section 4.1.18</a> for specific behavior. + request. See + <a href="https://tools.ietf.org/html/rfc3875#section-4.1.18"> + RFC3875 section 4.1.18</a> for + specific behavior. In addition to this, the Rack environment must include these Rack-specific variables: -<tt>rack.version</tt>:: The Array representing this version of Rack. See Rack::VERSION, that corresponds to the version of this SPEC. -<tt>rack.url_scheme</tt>:: +http+ or +https+, depending on the request URL. +<tt>rack.version</tt>:: The Array representing this version of Rack + See Rack::VERSION, that corresponds to + the version of this SPEC. +<tt>rack.url_scheme</tt>:: +http+ or +https+, depending on the + request URL. <tt>rack.input</tt>:: See below, the input stream. <tt>rack.errors</tt>:: See below, the error stream. -<tt>rack.multithread</tt>:: true if the application object may be simultaneously invoked by another thread in the same process, false otherwise. -<tt>rack.multiprocess</tt>:: true if an equivalent application object may be simultaneously invoked by another process, false otherwise. -<tt>rack.run_once</tt>:: true if the server expects (but does not guarantee!) that the application will only be invoked this one time during the life of its containing process. Normally, this will only be true for a server based on CGI (or something similar). -<tt>rack.hijack?</tt>:: present and true if the server supports connection hijacking. See below, hijacking. -<tt>rack.hijack</tt>:: an object responding to #call that must be called at least once before using rack.hijack_io. It is recommended #call return rack.hijack_io as well as setting it in env if necessary. -<tt>rack.hijack_io</tt>:: if rack.hijack? is true, and rack.hijack has received #call, this will contain an object resembling an IO. See hijacking. +<tt>rack.multithread</tt>:: true if the application object may be + simultaneously invoked by another thread + in the same process, false otherwise. +<tt>rack.multiprocess</tt>:: true if an equivalent application object + may be simultaneously invoked by another + process, false otherwise. +<tt>rack.run_once</tt>:: true if the server expects + (but does not guarantee!) that the + application will only be invoked this one + time during the life of its containing + process. Normally, this will only be true + for a server based on CGI + (or something similar). +<tt>rack.hijack?</tt>:: present and true if the server supports + connection hijacking. See below, hijacking. +<tt>rack.hijack</tt>:: an object responding to #call that must be + called at least once before using + rack.hijack_io. + It is recommended #call return rack.hijack_io + as well as setting it in env if necessary. +<tt>rack.hijack_io</tt>:: if rack.hijack? is true, and rack.hijack + has received #call, this will contain + an object resembling an IO. See hijacking. Additional environment specifications have approved to standardized middleware APIs. None of these are required to be implemented by the server. -<tt>rack.session</tt>:: A hash like interface for storing request session data. +<tt>rack.session</tt>:: A hash like interface for storing + request session data. The store must implement: store(key, value) (aliased as []=); fetch(key, default = nil) (aliased as []); @@ -110,15 +143,18 @@ must be opened in binary mode, for Ruby 1.9 compatibility. The input stream must respond to +gets+, +each+, +read+ and +rewind+. * +gets+ must be called without arguments and return a string, or +nil+ on EOF. -* +read+ behaves like IO#read. Its signature is <tt>read([length, [buffer]])</tt>. - If given, +length+ must be a non-negative Integer (>= 0) or +nil+, and +buffer+ must - be a String and may not be nil. If +length+ is given and not nil, then this method - reads at most +length+ bytes from the input stream. If +length+ is not given or nil, - then this method reads all data until EOF. - When EOF is reached, this method returns nil if +length+ is given and not nil, or "" - if +length+ is not given or is nil. - If +buffer+ is given, then the read data will be placed into +buffer+ instead of a - newly created String object. +* +read+ behaves like IO#read. + Its signature is <tt>read([length, [buffer]])</tt>. + If given, +length+ must be a non-negative Integer (>= 0) or +nil+, + and +buffer+ must be a String and may not be nil. + If +length+ is given and not nil, then this method reads at most + +length+ bytes from the input stream. + If +length+ is not given or nil, then this method reads + all data until EOF. + When EOF is reached, this method returns nil if +length+ is given + and not nil, or "" if +length+ is not given or is nil. + If +buffer+ is given, then the read data will be placed + into +buffer+ instead of a newly created String object. * +each+ must be called without arguments and only yield Strings. * +rewind+ must be called without arguments. It rewinds the input stream back to the beginning. It must not raise Errno::ESPIPE: diff --git a/lib/rack.rb b/lib/rack.rb index 57119df3..341514c5 100644 --- a/lib/rack.rb +++ b/lib/rack.rb @@ -53,6 +53,7 @@ module Rack autoload :ShowExceptions, "rack/showexceptions" autoload :ShowStatus, "rack/showstatus" autoload :Static, "rack/static" + autoload :TempfileReaper, "rack/tempfile_reaper" autoload :URLMap, "rack/urlmap" autoload :Utils, "rack/utils" autoload :Multipart, "rack/multipart" diff --git a/lib/rack/builder.rb b/lib/rack/builder.rb index a4f6c575..bda3be27 100644 --- a/lib/rack/builder.rb +++ b/lib/rack/builder.rb @@ -157,7 +157,7 @@ module Rack def generate_map(default_app, mapping) mapped = default_app ? {'/' => default_app} : {} - mapping.each { |r,b| mapped[r] = self.class.new(default_app, &b) } + mapping.each { |r,b| mapped[r] = self.class.new(default_app, &b).to_app } URLMap.new(mapped) end end diff --git a/lib/rack/chunked.rb b/lib/rack/chunked.rb index a400756a..ea221fa9 100644 --- a/lib/rack/chunked.rb +++ b/lib/rack/chunked.rb @@ -39,11 +39,22 @@ module Rack @app = app end + # pre-HTTP/1.0 (informally "HTTP/0.9") HTTP requests did not have + # a version (nor response headers) + def chunkable_version?(ver) + case ver + when "HTTP/1.0", nil, "HTTP/0.9" + false + else + true + end + end + def call(env) status, headers, body = @app.call(env) headers = HeaderHash.new(headers) - if env['HTTP_VERSION'] == 'HTTP/1.0' || + if ! chunkable_version?(env['HTTP_VERSION']) || STATUS_WITH_NO_ENTITY_BODY.include?(status) || headers['Content-Length'] || headers['Transfer-Encoding'] diff --git a/lib/rack/conditionalget.rb b/lib/rack/conditionalget.rb index ed87c54e..88573166 100644 --- a/lib/rack/conditionalget.rb +++ b/lib/rack/conditionalget.rb @@ -28,7 +28,10 @@ module Rack status = 304 headers.delete('Content-Type') headers.delete('Content-Length') - body = [] + original_body = body + body = Rack::BodyProxy.new([]) do + original_body.close if original_body.respond_to?(:close) + end end [status, headers, body] else diff --git a/lib/rack/lint.rb b/lib/rack/lint.rb index 3978b70a..667c34a6 100644 --- a/lib/rack/lint.rb +++ b/lib/rack/lint.rb @@ -102,7 +102,17 @@ module Rack ## follows the <tt>?</tt>, if any. May be ## empty, but is always required! - ## <tt>SERVER_NAME</tt>, <tt>SERVER_PORT</tt>:: When combined with <tt>SCRIPT_NAME</tt> and <tt>PATH_INFO</tt>, these variables can be used to complete the URL. Note, however, that <tt>HTTP_HOST</tt>, if present, should be used in preference to <tt>SERVER_NAME</tt> for reconstructing the request URL. <tt>SERVER_NAME</tt> and <tt>SERVER_PORT</tt> can never be empty strings, and so are always required. + ## <tt>SERVER_NAME</tt>, <tt>SERVER_PORT</tt>:: + ## When combined with <tt>SCRIPT_NAME</tt> and + ## <tt>PATH_INFO</tt>, these variables can be + ## used to complete the URL. Note, however, + ## that <tt>HTTP_HOST</tt>, if present, + ## should be used in preference to + ## <tt>SERVER_NAME</tt> for reconstructing + ## the request URL. + ## <tt>SERVER_NAME</tt> and <tt>SERVER_PORT</tt> + ## can never be empty strings, and so + ## are always required. ## <tt>HTTP_</tt> Variables:: Variables corresponding to the ## client-supplied HTTP request @@ -112,29 +122,60 @@ module Rack ## variables should correspond with ## the presence or absence of the ## appropriate HTTP header in the - ## request. See <a href="https://tools.ietf.org/html/rfc3875#section-4.1.18"> - ## RFC3875 section 4.1.18</a> for specific behavior. + ## request. See + ## <a href="https://tools.ietf.org/html/rfc3875#section-4.1.18"> + ## RFC3875 section 4.1.18</a> for + ## specific behavior. ## In addition to this, the Rack environment must include these ## Rack-specific variables: - ## <tt>rack.version</tt>:: The Array representing this version of Rack. See Rack::VERSION, that corresponds to the version of this SPEC. - ## <tt>rack.url_scheme</tt>:: +http+ or +https+, depending on the request URL. + ## <tt>rack.version</tt>:: The Array representing this version of Rack + ## See Rack::VERSION, that corresponds to + ## the version of this SPEC. + + ## <tt>rack.url_scheme</tt>:: +http+ or +https+, depending on the + ## request URL. + ## <tt>rack.input</tt>:: See below, the input stream. + ## <tt>rack.errors</tt>:: See below, the error stream. - ## <tt>rack.multithread</tt>:: true if the application object may be simultaneously invoked by another thread in the same process, false otherwise. - ## <tt>rack.multiprocess</tt>:: true if an equivalent application object may be simultaneously invoked by another process, false otherwise. - ## <tt>rack.run_once</tt>:: true if the server expects (but does not guarantee!) that the application will only be invoked this one time during the life of its containing process. Normally, this will only be true for a server based on CGI (or something similar). - ## <tt>rack.hijack?</tt>:: present and true if the server supports connection hijacking. See below, hijacking. - ## <tt>rack.hijack</tt>:: an object responding to #call that must be called at least once before using rack.hijack_io. It is recommended #call return rack.hijack_io as well as setting it in env if necessary. - ## <tt>rack.hijack_io</tt>:: if rack.hijack? is true, and rack.hijack has received #call, this will contain an object resembling an IO. See hijacking. - ## + + ## <tt>rack.multithread</tt>:: true if the application object may be + ## simultaneously invoked by another thread + ## in the same process, false otherwise. + + ## <tt>rack.multiprocess</tt>:: true if an equivalent application object + ## may be simultaneously invoked by another + ## process, false otherwise. + + ## <tt>rack.run_once</tt>:: true if the server expects + ## (but does not guarantee!) that the + ## application will only be invoked this one + ## time during the life of its containing + ## process. Normally, this will only be true + ## for a server based on CGI + ## (or something similar). + + ## <tt>rack.hijack?</tt>:: present and true if the server supports + ## connection hijacking. See below, hijacking. + + ## <tt>rack.hijack</tt>:: an object responding to #call that must be + ## called at least once before using + ## rack.hijack_io. + ## It is recommended #call return rack.hijack_io + ## as well as setting it in env if necessary. + + ## <tt>rack.hijack_io</tt>:: if rack.hijack? is true, and rack.hijack + ## has received #call, this will contain + ## an object resembling an IO. See hijacking. ## Additional environment specifications have approved to ## standardized middleware APIs. None of these are required to ## be implemented by the server. - ## <tt>rack.session</tt>:: A hash like interface for storing request session data. + ## <tt>rack.session</tt>:: A hash like interface for storing + ## request session data. ## The store must implement: if session = env['rack.session'] ## store(key, value) (aliased as []=); @@ -218,7 +259,6 @@ module Rack } } - ## ## There are the following restrictions: ## * <tt>rack.version</tt> must be an array of Integers. @@ -311,15 +351,23 @@ module Rack v end - ## * +read+ behaves like IO#read. Its signature is <tt>read([length, [buffer]])</tt>. - ## If given, +length+ must be a non-negative Integer (>= 0) or +nil+, and +buffer+ must - ## be a String and may not be nil. If +length+ is given and not nil, then this method - ## reads at most +length+ bytes from the input stream. If +length+ is not given or nil, - ## then this method reads all data until EOF. - ## When EOF is reached, this method returns nil if +length+ is given and not nil, or "" - ## if +length+ is not given or is nil. - ## If +buffer+ is given, then the read data will be placed into +buffer+ instead of a - ## newly created String object. + ## * +read+ behaves like IO#read. + ## Its signature is <tt>read([length, [buffer]])</tt>. + ## + ## If given, +length+ must be a non-negative Integer (>= 0) or +nil+, + ## and +buffer+ must be a String and may not be nil. + ## + ## If +length+ is given and not nil, then this method reads at most + ## +length+ bytes from the input stream. + ## + ## If +length+ is not given or nil, then this method reads + ## all data until EOF. + ## + ## When EOF is reached, this method returns nil if +length+ is given + ## and not nil, or "" if +length+ is not given or is nil. + ## + ## If +buffer+ is given, then the read data will be placed + ## into +buffer+ instead of a newly created String object. def read(*args) assert("rack.input#read called with too many arguments") { args.size <= 2 diff --git a/lib/rack/lobster.rb b/lib/rack/lobster.rb index d1a7f7bc..195bd945 100644 --- a/lib/rack/lobster.rb +++ b/lib/rack/lobster.rb @@ -32,9 +32,14 @@ module Rack def call(env) req = Request.new(env) if req.GET["flip"] == "left" - lobster = LobsterString.split("\n"). - map { |line| line.ljust(42).reverse }. - join("\n") + lobster = LobsterString.split("\n").map do |line| + line.ljust(42).reverse. + gsub('\\', 'TEMP'). + gsub('/', '\\'). + gsub('TEMP', '/'). + gsub('{','}'). + gsub('(',')') + end.join("\n") href = "?flip=right" elsif req.GET["flip"] == "crash" raise "Lobster crashed" diff --git a/lib/rack/mock.rb b/lib/rack/mock.rb index 3ba314e4..3c02c1fe 100644 --- a/lib/rack/mock.rb +++ b/lib/rack/mock.rb @@ -77,9 +77,16 @@ module Rack body.close if body.respond_to?(:close) end + # For historical reasons, we're pinning to RFC 2396. It's easier for users + # and we get support from ruby 1.8 to 2.2 using this method. + def self.parse_uri_rfc2396(uri) + @parser ||= defined?(URI::RFC2396_Parser) ? URI::RFC2396_Parser.new : URI + @parser.parse(uri) + end + # Return the Rack environment used for a request to +uri+. def self.env_for(uri="", opts={}) - uri = URI(uri) + uri = parse_uri_rfc2396(uri) uri.path = "/#{uri.path}" unless uri.path[0] == ?/ env = DEFAULT_ENV.dup diff --git a/lib/rack/multipart/parser.rb b/lib/rack/multipart/parser.rb index fa47fd16..00e59ba7 100644 --- a/lib/rack/multipart/parser.rb +++ b/lib/rack/multipart/parser.rb @@ -16,10 +16,10 @@ module Rack content_length = env['CONTENT_LENGTH'] content_length = content_length.to_i if content_length - new($1, io, content_length) + new($1, io, content_length, env) end - def initialize(boundary, io, content_length) + def initialize(boundary, io, content_length, env) @buf = "" if @buf.respond_to? :force_encoding @@ -31,6 +31,7 @@ module Rack @io = io @content_length = content_length @boundary_size = Utils.bytesize(@boundary) + EOL.size + @env = env if @content_length @content_length -= @boundary_size @@ -112,7 +113,7 @@ module Rack filename = get_filename(head) if filename - body = Tempfile.new("RackMultipart") + (@env['rack.tempfiles'] ||= []) << body = Tempfile.new("RackMultipart") body.binmode if body.respond_to?(:binmode) end diff --git a/lib/rack/request.rb b/lib/rack/request.rb index 07627ddb..5a446acd 100644 --- a/lib/rack/request.rb +++ b/lib/rack/request.rb @@ -353,12 +353,6 @@ module Rack forwarded_ips = split_ip_addresses(@env['HTTP_X_FORWARDED_FOR']) - if client_ip = @env['HTTP_CLIENT_IP'] - # If forwarded_ips doesn't include the client_ip, it might be an - # ip spoofing attempt, so we ignore HTTP_CLIENT_IP - return client_ip if forwarded_ips.include?(client_ip) - end - return reject_trusted_ip_addresses(forwarded_ips).last || @env["REMOTE_ADDR"] end @@ -372,7 +366,7 @@ module Rack end def parse_query(qs) - Utils.parse_nested_query(qs) + Utils.parse_nested_query(qs, '&') end def parse_multipart(env) diff --git a/lib/rack/response.rb b/lib/rack/response.rb index bd39da3b..12536710 100644 --- a/lib/rack/response.rb +++ b/lib/rack/response.rb @@ -129,6 +129,7 @@ module Rack def forbidden?; status == 403; end def not_found?; status == 404; end def method_not_allowed?; status == 405; end + def i_m_a_teapot?; status == 418; end def unprocessable?; status == 422; end def redirect?; [301, 302, 303, 307].include? status; end diff --git a/lib/rack/server.rb b/lib/rack/server.rb index 37483afb..d2f0b954 100644 --- a/lib/rack/server.rb +++ b/lib/rack/server.rb @@ -1,7 +1,10 @@ require 'optparse' + module Rack + class Server + class Options def parse!(args) options = {} @@ -202,29 +205,44 @@ module Rack @app ||= options[:builder] ? build_app_from_string : build_app_and_options_from_config end - def self.logging_middleware - lambda { |server| - server.server.name =~ /CGI/ ? nil : [Rack::CommonLogger, $stderr] - } - end + class << self + def logging_middleware + lambda { |server| + server.server.name =~ /CGI/ ? nil : [Rack::CommonLogger, $stderr] + } + end - def self.middleware - @middleware ||= begin + def default_middleware_by_environment m = Hash.new {|h,k| h[k] = []} - m["deployment"].concat [ + m["deployment"] = [ [Rack::ContentLength], [Rack::Chunked], - logging_middleware + logging_middleware, + [Rack::TempfileReaper] ] - m["development"].concat m["deployment"] + [[Rack::ShowExceptions], [Rack::Lint]] + m["development"] = [ + [Rack::ContentLength], + [Rack::Chunked], + logging_middleware, + [Rack::ShowExceptions], + [Rack::Lint], + [Rack::TempfileReaper] + ] + m end + + # Aliased for backwards-compatibility + alias :middleware :default_middleware_by_environment end - def middleware - self.class.middleware + def default_middleware_by_environment + self.class.default_middleware_by_environment end + # Aliased for backwards-compatibility + alias :middleware :default_middleware_by_environment + def start &blk if options[:warn] $-w = true @@ -304,7 +322,8 @@ module Rack end def build_app(app) - middleware[options[:environment]].reverse_each do |middleware| + middlewares = default_middleware_by_environment[options[:environment]] + middlewares.reverse_each do |middleware| middleware = middleware.call(self) if middleware.respond_to?(:call) next unless middleware klass, *args = middleware @@ -364,4 +383,5 @@ module Rack end end + end diff --git a/lib/rack/showexceptions.rb b/lib/rack/showexceptions.rb index c91ca07c..731aea49 100644 --- a/lib/rack/showexceptions.rb +++ b/lib/rack/showexceptions.rb @@ -28,23 +28,32 @@ module Rack env["rack.errors"].puts(exception_string) env["rack.errors"].flush - if prefers_plain_text?(env) - content_type = "text/plain" - body = [exception_string] - else + if accepts_html?(env) content_type = "text/html" body = pretty(env, e) + else + content_type = "text/plain" + body = exception_string end - [500, - {"Content-Type" => content_type, - "Content-Length" => Rack::Utils.bytesize(body.join).to_s}, - body] + [ + 500, + { + "Content-Type" => content_type, + "Content-Length" => Rack::Utils.bytesize(body).to_s, + }, + [body], + ] + end + + def prefers_plaintext?(env) + !accepts_html(env) end - def prefers_plain_text?(env) - env["HTTP_X_REQUESTED_WITH"] == "XMLHttpRequest" && (!env["HTTP_ACCEPT"] || !env["HTTP_ACCEPT"].include?("text/html")) + def accepts_html?(env) + Rack::Utils.best_q_match(env["HTTP_ACCEPT"], %w[text/html]) end + private :accepts_html? def dump_exception(exception) string = "#{exception.class}: #{exception.message}\n" @@ -85,7 +94,7 @@ module Rack end }.compact - [@template.result(binding)] + @template.result(binding) end def h(obj) # :nodoc: diff --git a/lib/rack/tempfile_reaper.rb b/lib/rack/tempfile_reaper.rb new file mode 100644 index 00000000..1500b06a --- /dev/null +++ b/lib/rack/tempfile_reaper.rb @@ -0,0 +1,22 @@ +require 'rack/body_proxy' + +module Rack + + # Middleware tracks and cleans Tempfiles created throughout a request (i.e. Rack::Multipart) + # Ideas/strategy based on posts by Eric Wong and Charles Oliver Nutter + # https://groups.google.com/forum/#!searchin/rack-devel/temp/rack-devel/brK8eh-MByw/sw61oJJCGRMJ + class TempfileReaper + def initialize(app) + @app = app + end + + def call(env) + env['rack.tempfiles'] ||= [] + status, headers, body = @app.call(env) + body_proxy = BodyProxy.new(body) do + env['rack.tempfiles'].each { |f| f.close! } unless env['rack.tempfiles'].nil? + end + [status, headers, body_proxy] + end + end +end diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb index 6c2bf907..27ac956f 100644 --- a/lib/rack/utils.rb +++ b/lib/rack/utils.rb @@ -22,6 +22,10 @@ module Rack # applications adopted from all kinds of Ruby libraries. module Utils + # ParameterTypeError is the error that is raised when incoming structural + # parameters (parsed by parse_nested_query) contain conflicting types. + class ParameterTypeError < TypeError; end + # URI escapes. (CGI style space to +) def escape(s) URI.encode_www_form_component(s) @@ -87,6 +91,11 @@ module Rack end module_function :parse_query + # parse_nested_query expands a query string into structural types. Supported + # types are Arrays, Hashes and basic value types. It is possible to supply + # query strings with parameters of conflicting types, in this case a + # ParameterTypeError is raised. Users are encouraged to return a 400 in this + # case. def parse_nested_query(qs, d = nil) params = KeySpaceConstrainedParams.new @@ -100,6 +109,9 @@ module Rack end module_function :parse_nested_query + # normalize_params recursively expands parameters into structural types. If + # the structural types represented by two different parameter names are in + # conflict, a ParameterTypeError is raised. def normalize_params(params, name, v = nil) name =~ %r(\A[\[\]]*([^\[\]]+)\]*) k = $1 || '' @@ -113,12 +125,12 @@ module Rack params[name] = v elsif after == "[]" params[k] ||= [] - raise TypeError, "expected Array (got #{params[k].class.name}) for param `#{k}'" unless params[k].is_a?(Array) + raise ParameterTypeError, "expected Array (got #{params[k].class.name}) for param `#{k}'" unless params[k].is_a?(Array) params[k] << v elsif after =~ %r(^\[\]\[([^\[\]]+)\]$) || after =~ %r(^\[\](.+)$) child_key = $1 params[k] ||= [] - raise TypeError, "expected Array (got #{params[k].class.name}) for param `#{k}'" unless params[k].is_a?(Array) + raise ParameterTypeError, "expected Array (got #{params[k].class.name}) for param `#{k}'" unless params[k].is_a?(Array) if params_hash_type?(params[k].last) && !params[k].last.key?(child_key) normalize_params(params[k].last, child_key, v) else @@ -126,7 +138,7 @@ module Rack end else params[k] ||= params.class.new - raise TypeError, "expected Hash (got #{params[k].class.name}) for param `#{k}'" unless params_hash_type?(params[k]) + raise ParameterTypeError, "expected Hash (got #{params[k].class.name}) for param `#{k}'" unless params_hash_type?(params[k]) params[k] = normalize_params(params[k], after, v) end @@ -184,13 +196,14 @@ module Rack def best_q_match(q_value_header, available_mimes) values = q_values(q_value_header) - values.map do |req_mime, quality| - match = available_mimes.first { |am| Rack::Mime.match?(am, req_mime) } + matches = values.map do |req_mime, quality| + match = available_mimes.find { |am| Rack::Mime.match?(am, req_mime) } next unless match [match, quality] end.compact.sort_by do |match, quality| (match.split('/', 2).count('*') * -10) + quality - end.last.first + end.last + matches && matches.first end module_function :best_q_match @@ -532,7 +545,11 @@ module Rack hash.keys.each do |key| value = hash[key] if value.kind_of?(self.class) - hash[key] = value.to_params_hash + if value.object_id == self.object_id + hash[key] = hash + else + hash[key] = value.to_params_hash + end elsif value.kind_of?(Array) value.map! {|x| x.kind_of?(self.class) ? x.to_params_hash : x} end @@ -587,6 +604,7 @@ module Rack 415 => 'Unsupported Media Type', 416 => 'Requested Range Not Satisfiable', 417 => 'Expectation Failed', + 418 => 'I\'m a teapot', 422 => 'Unprocessable Entity', 423 => 'Locked', 424 => 'Failed Dependency', @@ -611,7 +629,7 @@ module Rack STATUS_WITH_NO_ENTITY_BODY = Set.new((100..199).to_a << 204 << 205 << 304) SYMBOL_TO_STATUS_CODE = Hash[*HTTP_STATUS_CODES.map { |code, message| - [message.downcase.gsub(/\s|-/, '_').to_sym, code] + [message.downcase.gsub(/\s|-|'/, '_').to_sym, code] }.flatten] def status_code(status) @@ -637,7 +655,7 @@ module Rack part == '..' ? clean.pop : clean << part end - clean.unshift '/' if parts.first.empty? + clean.unshift '/' if parts.empty? || parts.first.empty? ::File.join(*clean) end diff --git a/test/spec_chunked.rb b/test/spec_chunked.rb index 12f21581..0a6d9ff1 100644 --- a/test/spec_chunked.rb +++ b/test/spec_chunked.rb @@ -64,6 +64,22 @@ describe Rack::Chunked do body.join.should.equal 'Hello World!' end + should 'not modify response when client is ancient, pre-HTTP/1.0' do + app = lambda { |env| [200, {"Content-Type" => "text/plain"}, ['Hello', ' ', 'World!']] } + check = lambda do + status, headers, body = chunked(app).call(@env.dup) + status.should.equal 200 + headers.should.not.include 'Transfer-Encoding' + body.join.should.equal 'Hello World!' + end + + @env.delete('HTTP_VERSION') # unicorn will do this on pre-HTTP/1.0 requests + check.call + + @env['HTTP_VERSION'] = 'HTTP/0.9' # not sure if this happens in practice + check.call + end + should 'not modify response when Transfer-Encoding header already present' do app = lambda { |env| [200, {"Content-Type" => "text/plain", 'Transfer-Encoding' => 'identity'}, ['Hello', ' ', 'World!']] diff --git a/test/spec_lobster.rb b/test/spec_lobster.rb index 56a54795..c6ec2b06 100644 --- a/test/spec_lobster.rb +++ b/test/spec_lobster.rb @@ -47,7 +47,7 @@ describe Rack::Lobster do should "be flippable" do res = lobster.get("/?flip=left") res.should.be.ok - res.body.should.include "(,,,(,,(,(" + res.body.should.include "),,,),,),)" end should "provide crashing for testing purposes" do diff --git a/test/spec_mock.rb b/test/spec_mock.rb index f49b1961..3ebd7776 100644 --- a/test/spec_mock.rb +++ b/test/spec_mock.rb @@ -30,6 +30,14 @@ describe Rack::MockRequest do env.should.include "rack.version" end + should "return an environment with a path" do + env = Rack::MockRequest.env_for("http://www.example.com/parse?location[]=1&location[]=2&age_group[]=2") + env["QUERY_STRING"].should.equal "location[]=1&location[]=2&age_group[]=2" + env["PATH_INFO"].should.equal "/parse" + env.should.be.kind_of Hash + env.should.include "rack.version" + end + should "provide sensible defaults" do res = Rack::MockRequest.new(app).request diff --git a/test/spec_request.rb b/test/spec_request.rb index bd67ce63..e5ec254e 100644 --- a/test/spec_request.rb +++ b/test/spec_request.rb @@ -130,6 +130,14 @@ describe Rack::Request do req.params.should.equal "foo" => "bar", "quux" => "bla" end + should "not truncate query strings containing semi-colons #543" do + req = Rack::Request.new(Rack::MockRequest.env_for("/?foo=bar&quux=b;la")) + req.query_string.should.equal "foo=bar&quux=b;la" + req.GET.should.equal "foo" => "bar", "quux" => "b;la" + req.POST.should.be.empty + req.params.should.equal "foo" => "bar", "quux" => "b;la" + end + should "limit the keys from the GET query string" do env = Rack::MockRequest.env_for("/?foo=bar") @@ -740,6 +748,31 @@ EOF req.POST["mean"][:tempfile].read.should.equal "--AaB03xha" end + should "record tempfiles from multipart form data in env[rack.tempfiles]" do + input = <<EOF +--AaB03x\r +content-disposition: form-data; name="fileupload"; filename="foo.jpg"\r +Content-Type: image/jpeg\r +Content-Transfer-Encoding: base64\r +\r +/9j/4AAQSkZJRgABAQAAAQABAAD//gA+Q1JFQVRPUjogZ2QtanBlZyB2MS4wICh1c2luZyBJSkcg\r +--AaB03x\r +content-disposition: form-data; name="fileupload"; filename="bar.jpg"\r +Content-Type: image/jpeg\r +Content-Transfer-Encoding: base64\r +\r +/9j/4AAQSkZJRgABAQAAAQABAAD//gA+Q1JFQVRPUjogZ2QtanBlZyB2MS4wICh1c2luZyBJSkcg\r +--AaB03x--\r +EOF + env = Rack::MockRequest.env_for("/", + "CONTENT_TYPE" => "multipart/form-data, boundary=AaB03x", + "CONTENT_LENGTH" => input.size, + :input => input) + req = Rack::Request.new(env) + req.params + env['rack.tempfiles'].size.should.equal(2) + end + should "detect invalid multipart form data" do input = <<EOF --AaB03x\r @@ -1037,12 +1070,6 @@ EOF 'HTTP_CLIENT_IP' => '1.1.1.1' res.body.should.equal '1.1.1.1' - # Spoofing attempt - res = mock.get '/', - 'HTTP_X_FORWARDED_FOR' => '1.1.1.1', - 'HTTP_CLIENT_IP' => '2.2.2.2' - res.body.should.equal '1.1.1.1' - res = mock.get '/', 'HTTP_X_FORWARDED_FOR' => '8.8.8.8, 9.9.9.9' res.body.should.equal '9.9.9.9' @@ -1061,6 +1088,24 @@ EOF res.body.should.equal '3.4.5.6' end + should "not allow IP spoofing via Client-IP and X-Forwarded-For headers" do + mock = Rack::MockRequest.new(Rack::Lint.new(ip_app)) + + # IP Spoofing attempt: + # Client sends X-Forwarded-For: 6.6.6.6 + # Client-IP: 6.6.6.6 + # Load balancer adds X-Forwarded-For: 2.2.2.3, 192.168.0.7 + # App receives: X-Forwarded-For: 6.6.6.6 + # X-Forwarded-For: 2.2.2.3, 192.168.0.7 + # Client-IP: 6.6.6.6 + # Rack env: HTTP_X_FORWARDED_FOR: '6.6.6.6, 2.2.2.3, 192.168.0.7' + # HTTP_CLIENT_IP: '6.6.6.6' + res = mock.get '/', + 'HTTP_X_FORWARDED_FOR' => '6.6.6.6, 2.2.2.3, 192.168.0.7', + 'HTTP_CLIENT_IP' => '6.6.6.6' + res.body.should.equal '2.2.2.3' + end + should "regard local addresses as proxies" do req = Rack::Request.new(Rack::MockRequest.env_for("/")) req.trusted_proxy?('127.0.0.1').should.equal 0 diff --git a/test/spec_response.rb b/test/spec_response.rb index 031488bb..6b13c0c9 100644 --- a/test/spec_response.rb +++ b/test/spec_response.rb @@ -251,6 +251,11 @@ describe Rack::Response do res.should.be.client_error res.should.be.method_not_allowed + res.status = 418 + res.should.not.be.successful + res.should.be.client_error + res.should.be.i_m_a_teapot + res.status = 422 res.should.not.be.successful res.should.be.client_error diff --git a/test/spec_server.rb b/test/spec_server.rb index 44d4bcbb..01b4f562 100644 --- a/test/spec_server.rb +++ b/test/spec_server.rb @@ -30,14 +30,24 @@ describe Rack::Server do should "not include Rack::Lint in deployment or none environments" do server = Rack::Server.new(:app => 'foo') - server.middleware['deployment'].flatten.should.not.include(Rack::Lint) - server.middleware['none'].flatten.should.not.include(Rack::Lint) + server.default_middleware_by_environment['deployment'].flatten.should.not.include(Rack::Lint) + server.default_middleware_by_environment['none'].flatten.should.not.include(Rack::Lint) end should "not include Rack::ShowExceptions in deployment or none environments" do server = Rack::Server.new(:app => 'foo') - server.middleware['deployment'].flatten.should.not.include(Rack::ShowExceptions) - server.middleware['none'].flatten.should.not.include(Rack::ShowExceptions) + server.default_middleware_by_environment['deployment'].flatten.should.not.include(Rack::ShowExceptions) + server.default_middleware_by_environment['none'].flatten.should.not.include(Rack::ShowExceptions) + end + + should "always return an empty array for unknown environments" do + server = Rack::Server.new(:app => 'foo') + server.default_middleware_by_environment['production'].should.equal [] + end + + should "include Rack::TempfileReaper in deployment environment" do + server = Rack::Server.new(:app => 'foo') + server.middleware['deployment'].flatten.should.include(Rack::TempfileReaper) end should "support CGI" do @@ -53,7 +63,7 @@ describe Rack::Server do should "not force any middleware under the none configuration" do server = Rack::Server.new(:app => 'foo') - server.middleware['none'].should.be.empty + server.default_middleware_by_environment['none'].should.be.empty end should "use a full path to the pidfile" do diff --git a/test/spec_showexceptions.rb b/test/spec_showexceptions.rb index bdd5ce5b..7d50c59f 100644 --- a/test/spec_showexceptions.rb +++ b/test/spec_showexceptions.rb @@ -16,7 +16,7 @@ describe Rack::ShowExceptions do )) lambda{ - res = req.get("/") + res = req.get("/", "HTTP_ACCEPT" => "text/html") }.should.not.raise res.should.be.a.server_error @@ -26,7 +26,7 @@ describe Rack::ShowExceptions do res.should =~ /ShowExceptions/ end - it "responds with plain text on AJAX requests accepting anything but HTML" do + it "responds with HTML only to requests accepting HTML" do res = nil req = Rack::MockRequest.new( @@ -34,39 +34,32 @@ describe Rack::ShowExceptions do lambda{|env| raise RuntimeError, "It was never supposed to work" } )) - lambda{ - res = req.get("/", "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest") - }.should.not.raise - - res.should.be.a.server_error - res.status.should.equal 500 - - res.content_type.should.equal "text/plain" - - res.body.should.include "RuntimeError: It was never supposed to work\n" - res.body.should.include __FILE__ - end - - it "responds with HTML on AJAX requests accepting HTML" do - res = nil - - req = Rack::MockRequest.new( - show_exceptions( - lambda{|env| raise RuntimeError, "It was never supposed to work" } - )) - - lambda{ - res = req.get("/", "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", "HTTP_ACCEPT" => "text/html") - }.should.not.raise - - res.should.be.a.server_error - res.status.should.equal 500 - - res.content_type.should.equal "text/html" - - res.body.should.include "RuntimeError" - res.body.should.include "It was never supposed to work" - res.body.should.include Rack::Utils.escape_html(__FILE__) + [ + # Serve text/html when the client accepts text/html + ["text/html", ["/", {"HTTP_ACCEPT" => "text/html"}]], + ["text/html", ["/", {"HTTP_ACCEPT" => "*/*"}]], + # Serve text/plain when the client does not accept text/html + ["text/plain", ["/"]], + ["text/plain", ["/", {"HTTP_ACCEPT" => "application/json"}]] + ].each do |exmime, rargs| + lambda{ + res = req.get(*rargs) + }.should.not.raise + + res.should.be.a.server_error + res.status.should.equal 500 + + res.content_type.should.equal exmime + + res.body.should.include "RuntimeError" + res.body.should.include "It was never supposed to work" + + if exmime == "text/html" + res.body.should.include '</html>' + else + res.body.should.not.include '</html>' + end + end end it "handles exceptions without a backtrace" do @@ -79,7 +72,7 @@ describe Rack::ShowExceptions do ) lambda{ - res = req.get("/") + res = req.get("/", "HTTP_ACCEPT" => "text/html") }.should.not.raise res.should.be.a.server_error diff --git a/test/spec_tempfile_reaper.rb b/test/spec_tempfile_reaper.rb new file mode 100644 index 00000000..ac39d878 --- /dev/null +++ b/test/spec_tempfile_reaper.rb @@ -0,0 +1,63 @@ +require 'rack/tempfile_reaper' +require 'rack/lint' +require 'rack/mock' + +describe Rack::TempfileReaper do + class MockTempfile + attr_reader :closed + + def initialize + @closed = false + end + + def close! + @closed = true + end + end + + before do + @env = Rack::MockRequest.env_for + end + + def call(app) + Rack::Lint.new(Rack::TempfileReaper.new(app)).call(@env) + end + + should 'do nothing (i.e. not bomb out) without env[rack.tempfiles]' do + app = lambda { |_| [200, {}, ['Hello, World!']] } + response = call(app) + response[2].close + response[0].should.equal(200) + end + + should 'close env[rack.tempfiles] when body is closed' do + tempfile1, tempfile2 = MockTempfile.new, MockTempfile.new + @env['rack.tempfiles'] = [ tempfile1, tempfile2 ] + app = lambda { |_| [200, {}, ['Hello, World!']] } + call(app)[2].close + tempfile1.closed.should.equal true + tempfile2.closed.should.equal true + end + + should 'initialize env[rack.tempfiles] when not already present' do + tempfile = MockTempfile.new + app = lambda do |env| + env['rack.tempfiles'] << tempfile + [200, {}, ['Hello, World!']] + end + call(app)[2].close + tempfile.closed.should.equal true + end + + should 'append env[rack.tempfiles] when already present' do + tempfile1, tempfile2 = MockTempfile.new, MockTempfile.new + @env['rack.tempfiles'] = [ tempfile1 ] + app = lambda do |env| + env['rack.tempfiles'] << tempfile2 + [200, {}, ['Hello, World!']] + end + call(app)[2].close + tempfile1.closed.should.equal true + tempfile2.closed.should.equal true + end +end diff --git a/test/spec_utils.rb b/test/spec_utils.rb index c3867965..6391f951 100644 --- a/test/spec_utils.rb +++ b/test/spec_utils.rb @@ -123,6 +123,17 @@ describe Rack::Utils do Rack::Utils.parse_query(",foo=bar;,", ";,").should.equal "foo" => "bar" end + should "not create infinite loops with cycle structures" do + ex = { "foo" => nil } + ex["foo"] = ex + + params = Rack::Utils::KeySpaceConstrainedParams.new + params['foo'] = params + lambda { + params.to_params_hash.to_s.should.equal ex.to_s + }.should.not.raise + end + should "parse nested query strings correctly" do Rack::Utils.parse_nested_query("foo"). should.equal "foo" => nil @@ -202,15 +213,15 @@ describe Rack::Utils do should.equal "x" => {"y" => [{"z" => "1", "w" => "a"}, {"z" => "2", "w" => "3"}]} lambda { Rack::Utils.parse_nested_query("x[y]=1&x[y]z=2") }. - should.raise(TypeError). + should.raise(Rack::Utils::ParameterTypeError). message.should.equal "expected Hash (got String) for param `y'" lambda { Rack::Utils.parse_nested_query("x[y]=1&x[]=1") }. - should.raise(TypeError). + should.raise(Rack::Utils::ParameterTypeError). message.should.match(/expected Array \(got [^)]*\) for param `x'/) lambda { Rack::Utils.parse_nested_query("x[y]=1&x[y][][w]=2") }. - should.raise(TypeError). + should.raise(Rack::Utils::ParameterTypeError). message.should.equal "expected Array (got String) for param `y'" end @@ -300,9 +311,15 @@ describe Rack::Utils do # Higher quality matches are preferred Rack::Utils.best_q_match("text/*;q=0.5,text/plain;q=1.0", %w[text/plain text/html]).should.equal "text/plain" + # Respect requested content type + Rack::Utils.best_q_match("application/json", %w[application/vnd.lotus-1-2-3 application/json]).should.equal "application/json" + # All else equal, the available mimes are preferred in order Rack::Utils.best_q_match("text/*", %w[text/html text/plain]).should.equal "text/html" Rack::Utils.best_q_match("text/plain,text/html", %w[text/html text/plain]).should.equal "text/html" + + # When there are no matches, return nil: + Rack::Utils.best_q_match("application/json", %w[text/html text/plain]).should.equal nil end should "escape html entities [&><'\"/]" do @@ -403,6 +420,10 @@ describe Rack::Utils do should "not clean directory traversal with encoded periods" do Rack::Utils.clean_path_info("/%2E%2E/README").should.equal "/%2E%2E/README" end + + should "clean slash only paths" do + Rack::Utils.clean_path_info("/").should.equal "/" + end end describe Rack::Utils, "byte_range" do |