diff --git a/lib/rack/constants.rb b/lib/rack/constants.rb index 5b0c181e5..13365935b 100644 --- a/lib/rack/constants.rb +++ b/lib/rack/constants.rb @@ -54,13 +54,11 @@ module Rack RACK_RESPONSE_FINISHED = 'rack.response_finished' RACK_REQUEST_FORM_INPUT = 'rack.request.form_input' RACK_REQUEST_FORM_HASH = 'rack.request.form_hash' - RACK_REQUEST_FORM_PAIRS = 'rack.request.form_pairs' RACK_REQUEST_FORM_VARS = 'rack.request.form_vars' RACK_REQUEST_FORM_ERROR = 'rack.request.form_error' RACK_REQUEST_COOKIE_HASH = 'rack.request.cookie_hash' RACK_REQUEST_COOKIE_STRING = 'rack.request.cookie_string' RACK_REQUEST_QUERY_HASH = 'rack.request.query_hash' - RACK_REQUEST_QUERY_PAIRS = 'rack.request.query_pairs' RACK_REQUEST_QUERY_STRING = 'rack.request.query_string' RACK_METHODOVERRIDE_ORIGINAL_METHOD = 'rack.methodoverride.original_method' end diff --git a/lib/rack/multipart.rb b/lib/rack/multipart.rb index 4b02fb3e8..165b4db3a 100644 --- a/lib/rack/multipart.rb +++ b/lib/rack/multipart.rb @@ -19,31 +19,6 @@ class MissingInputError < StandardError include BadRequest end - # Accumulator for multipart form data, conforming to the QueryParser API. - # In future, the Parser could return the pair list directly, but that would - # change its API. - class ParamList # :nodoc: - def self.make_params - new - end - - def self.normalize_params(params, key, value) - params << [key, value] - end - - def initialize - @pairs = [] - end - - def <<(pair) - @pairs << pair - end - - def to_params_hash - @pairs - end - end - class << self def parse_multipart(env, params = Rack::Utils.default_query_parser) unless io = env[RACK_INPUT] diff --git a/lib/rack/query_parser.rb b/lib/rack/query_parser.rb index 525900f7b..28cbce18f 100644 --- a/lib/rack/query_parser.rb +++ b/lib/rack/query_parser.rb @@ -38,42 +38,19 @@ def initialize(params_class, param_depth_limit) @param_depth_limit = param_depth_limit end - # Originally stolen from Mongrel, now with some modifications: + # Stolen from Mongrel, with some small modifications: # Parses a query string by breaking it up at the '&'. You can also use this # to parse cookies by changing the characters used in the second parameter # (which defaults to '&'). - # - # Returns an array of 2-element arrays, where the first element is the - # key and the second element is the value. - def split_query(qs, separator = nil, &unescaper) - pairs = [] - - if qs && !qs.empty? - unescaper ||= method(:unescape) - - qs.split(separator ? (COMMON_SEP[separator] || /[#{separator}] */n) : DEFAULT_SEP).each do |p| - next if p.empty? - pair = p.split('=', 2).map!(&unescaper) - pair << nil if pair.length == 1 - pairs << pair - end - end - - pairs - rescue ArgumentError => e - raise InvalidParameterError, e.message, e.backtrace - end - - # Parses a query string by breaking it up at the '&'. You can also use this - # to parse cookies by changing the characters used in the second parameter - # (which defaults to '&'). - # - # Returns a hash where each value is a string (when a key only appears once) - # or an array of strings (when a key appears more than once). def parse_query(qs, separator = nil, &unescaper) + unescaper ||= method(:unescape) + params = make_params - split_query(qs, separator, &unescaper).each do |k, v| + (qs || '').split(separator ? (COMMON_SEP[separator] || /[#{separator}] */n) : DEFAULT_SEP).each do |p| + next if p.empty? + k, v = p.split('=', 2).map!(&unescaper) + if cur = params[k] if cur.class == Array params[k] << v @@ -85,7 +62,7 @@ def parse_query(qs, separator = nil, &unescaper) end end - params.to_h + return params.to_h end # parse_nested_query expands a query string into structural types. Supported @@ -96,11 +73,17 @@ def parse_query(qs, separator = nil, &unescaper) def parse_nested_query(qs, separator = nil) params = make_params - split_query(qs, separator).each do |k, v| - _normalize_params(params, k, v, 0) + unless qs.nil? || qs.empty? + (qs || '').split(separator ? (COMMON_SEP[separator] || /[#{separator}] */n) : DEFAULT_SEP).each do |p| + k, v = p.split('=', 2).map! { |s| unescape(s) } + + _normalize_params(params, k, v, 0) + end end - params.to_h + return params.to_h + rescue ArgumentError => e + raise InvalidParameterError, e.message, e.backtrace end # normalize_params recursively expands parameters into structural types. If @@ -112,14 +95,6 @@ def normalize_params(params, name, v, _depth=nil) _normalize_params(params, name, v, 0) end - # This value is used by default when a parameter is missing (nil). This - # usually happens when a parameter is specified without an `=value` part. - # The default value is an empty string, but this can be overridden by - # subclasses. - def missing_value - String.new - end - private def _normalize_params(params, name, v, depth) raise ParamsTooDeepError if depth >= param_depth_limit @@ -154,8 +129,6 @@ def missing_value return if k.empty? - v ||= missing_value - if after == '' if k == '[]' && depth != 0 return [v] diff --git a/lib/rack/request.rb b/lib/rack/request.rb index fbdead8ab..a3eb9926a 100644 --- a/lib/rack/request.rb +++ b/lib/rack/request.rb @@ -480,115 +480,14 @@ def parseable_data? PARSEABLE_DATA_MEDIA_TYPES.include?(media_type) end - # Given a current input value, and a validity key, check if the cache - # is valid, and if so, return the cached value. If not, yield the - # current value to the block, and set the cache to the result. - # - # This method does not use cache_key, so it is shared between all - # instance of Rack::Request and it's sub-classes. - private def cache_for(key, validity_key, current_value) - # Get the current value of the validity key and compare it with the input value: - if get_header(validity_key).equal?(current_value) - # If the values are the same, then the cache is valid, so return the cached value. - if has_header?(key) - value = get_header(key) - # If the cached value is an exception, then re-raise it. - if value.is_a?(Exception) - raise value.class, value.message, cause: value.cause - else - # Otherwise, return the cached value. - return value - end - end - end - - # If the cache is not valid, then yield the current value to the block: - value = yield(current_value) - - # Set the validity key to the current value so that we can detect changes: - set_header(validity_key, current_value) - - # Set the cache to the result of the block, and return the result: - set_header(key, value) - rescue => error - # If an exception is raised, then set the cache to the exception, and re-raise it: - set_header(validity_key, current_value) - set_header(key, error) - raise - end - - # This cache key is used by cached values generated by class_cache_for, - # specfically GET and POST. This is to ensure that the cache is not - # shared between instances of different classes which have different - # behaviour. This includes sub-classes that override query_parser or - # expand_params. - def cache_key - query_parser - end - - # Given a current input value, and a validity key, check if the cache - # is valid, and if so, return the cached value. If not, yield the - # current value to the block, and set the cache to the result. - # - # This method uses cache_key to ensure that the cache is not shared - # between instances of different classes which have different - # behaviour of the cached operations. - private def class_cache_for(key, validity_key, current_value) - # The cache is organised in the env as: - # env[key][cache_key] = value - # and is valid as long as env[validity_key].equal?(current_value) - - cache_key = self.cache_key - - # Get the current value of the validity key and compare it with the input value: - if get_header(validity_key).equal?(current_value) - # Lookup the cache for the current cache key: - if cache = get_header(key) - if cache.key?(cache_key) - # If the cache is valid, then return the cached value. - value = cache[cache_key] - if value.is_a?(Exception) - # If the cached value is an exception, then re-raise it. - raise value.class, value.message, cause: value.cause - else - # Otherwise, return the cached value. - return value - end - end - end - end - - # If the cache was not defined for this cache key, then create a new cache: - unless cache - cache = Hash.new.compare_by_identity - set_header(key, cache) - end - - begin - # Yield the current value to the block to generate an updated value: - value = yield(current_value) - - # Only set this after generating the value, so that if an error or other cache depending on the same key, it will be invalidated correctly: - set_header(validity_key, current_value) - return cache[cache_key] = value - rescue => error - set_header(validity_key, current_value) - cache[cache_key] = error - raise - end - end - # Returns the data received in the query string. def GET - class_cache_for(RACK_REQUEST_QUERY_HASH, RACK_REQUEST_QUERY_STRING, query_string) do - expand_params(query_param_list) - end - end - - def query_param_list - cache_for(RACK_REQUEST_QUERY_PAIRS, RACK_REQUEST_QUERY_STRING, query_string) do - set_header(RACK_REQUEST_QUERY_HASH, nil) - split_query(query_string, '&') + if get_header(RACK_REQUEST_QUERY_STRING) == query_string + get_header(RACK_REQUEST_QUERY_HASH) + else + query_hash = parse_query(query_string, '&') + set_header(RACK_REQUEST_QUERY_STRING, query_string) + set_header(RACK_REQUEST_QUERY_HASH, query_hash) end end @@ -597,33 +496,46 @@ def query_param_list # This method support both application/x-www-form-urlencoded and # multipart/form-data. def POST - class_cache_for(RACK_REQUEST_FORM_HASH, RACK_REQUEST_FORM_INPUT, get_header(RACK_INPUT)) do - expand_params(body_param_list) + if error = get_header(RACK_REQUEST_FORM_ERROR) + raise error.class, error.message, cause: error.cause end - end - def body_param_list - cache_for(RACK_REQUEST_FORM_PAIRS, RACK_REQUEST_FORM_INPUT, get_header(RACK_INPUT)) do |rack_input| + begin + rack_input = get_header(RACK_INPUT) + + # If the form hash was already memoized: + if form_hash = get_header(RACK_REQUEST_FORM_HASH) + # And it was memoized from the same input: + if get_header(RACK_REQUEST_FORM_INPUT).equal?(rack_input) + return form_hash + end + end + + # Otherwise, figure out how to parse the input: if rack_input.nil? - form_pairs = [] + set_header RACK_REQUEST_FORM_INPUT, nil + set_header(RACK_REQUEST_FORM_HASH, {}) elsif form_data? || parseable_data? - unless form_pairs = Rack::Multipart.extract_multipart(self, Rack::Multipart::ParamList) - form_vars = rack_input.read + unless set_header(RACK_REQUEST_FORM_HASH, parse_multipart) + form_vars = get_header(RACK_INPUT).read # Fix for Safari Ajax postings that always append \0 # form_vars.sub!(/\0\z/, '') # performance replacement: form_vars.slice!(-1) if form_vars.end_with?("\0") - # Removing this line breaks Rail test "test_filters_rack_request_form_vars"! - set_header(RACK_REQUEST_FORM_VARS, form_vars) - - form_pairs = split_query(form_vars, '&') + set_header RACK_REQUEST_FORM_VARS, form_vars + set_header RACK_REQUEST_FORM_HASH, parse_query(form_vars, '&') end + + set_header RACK_REQUEST_FORM_INPUT, get_header(RACK_INPUT) + get_header RACK_REQUEST_FORM_HASH else - form_pairs = [] + set_header RACK_REQUEST_FORM_INPUT, get_header(RACK_INPUT) + set_header(RACK_REQUEST_FORM_HASH, {}) end - - form_pairs + rescue => error + set_header(RACK_REQUEST_FORM_ERROR, error) + raise end end @@ -760,28 +672,6 @@ def parse_multipart Rack::Multipart.extract_multipart(self, query_parser) end - def split_query(query, d = '&') - query_parser = query_parser() - unless query_parser.respond_to?(:split_query) - query_parser = Utils.default_query_parser - unless query_parser.respond_to?(:split_query) - query_parser = QueryParser.make_default(0) - end - end - - query_parser.split_query(query, d) - end - - def expand_params(pairs, query_parser = query_parser()) - params = query_parser.make_params - - pairs.each do |key, value| - query_parser.normalize_params(params, key, value) - end - - params.to_params_hash - end - def split_header(value) value ? value.strip.split(/[,\s]+/) : [] end diff --git a/test/spec_query_parser.rb b/test/spec_query_parser.rb index 5f4708186..dbb8b14ed 100644 --- a/test/spec_query_parser.rb +++ b/test/spec_query_parser.rb @@ -7,25 +7,9 @@ end describe Rack::QueryParser do - def query_parser - @query_parser ||= Rack::QueryParser.new(Rack::QueryParser::Params, 8) - end - - it "has a default value" do - assert_equal "", query_parser.missing_value - end + query_parser ||= Rack::QueryParser.make_default(8) it "can normalize values with missing values" do - query_parser.parse_nested_query("a=a").must_equal({"a" => "a"}) - query_parser.parse_nested_query("a=").must_equal({"a" => ""}) - query_parser.parse_nested_query("a").must_equal({"a" => ""}) - end - - it "can override default missing value" do - def query_parser.missing_value - nil - end - query_parser.parse_nested_query("a=a").must_equal({"a" => "a"}) query_parser.parse_nested_query("a=").must_equal({"a" => ""}) query_parser.parse_nested_query("a").must_equal({"a" => nil}) diff --git a/test/spec_request.rb b/test/spec_request.rb index e525621a7..9966762ea 100644 --- a/test/spec_request.rb +++ b/test/spec_request.rb @@ -574,10 +574,9 @@ def self.req(headers) it "parse the query string" do request = make_request(Rack::MockRequest.env_for("/?foo=bar&quux=bla¬hing&empty=")) request.query_string.must_equal "foo=bar&quux=bla¬hing&empty=" - request.GET.must_equal "foo" => "bar", "quux" => "bla", "nothing" => "", "empty" => "" + request.GET.must_equal "foo" => "bar", "quux" => "bla", "nothing" => nil, "empty" => "" request.POST.must_be :empty? - request.params.must_equal "foo" => "bar", "quux" => "bla", "nothing" => "", "empty" => "" - request.query_param_list.must_equal [["foo", "bar"], ["quux", "bla"], ["nothing", nil], ["empty", ""]] + request.params.must_equal "foo" => "bar", "quux" => "bla", "nothing" => nil, "empty" => "" end it "handles invalid unicode in query string value" do @@ -1554,19 +1553,16 @@ def initialize(*) rack_input.write(input) rack_input.rewind - form_hash_cache = {} + form_hash = {} req = make_request Rack::MockRequest.env_for( "/", - "rack.request.form_hash" => form_hash_cache, + "rack.request.form_hash" => form_hash, "rack.request.form_input" => rack_input, :input => rack_input ) - form_hash = {'foo' => 'bar'}.freeze - form_hash_cache[req.cache_key] = form_hash - - req.POST.must_equal form_hash + req.POST.must_be_same_as form_hash end it "conform to the Rack spec" do @@ -1964,53 +1960,4 @@ def make_request(env) DelegateRequest.new super(env) end end - - class UpperRequest < Rack::Request - def expand_params(parameters) - parameters.map do |(key, value)| - [key.upcase, value] - end.to_h - end - - # If this is not specified, the behaviour becomes order dependent. - def cache_key - :my_request - end - end - - it "correctly expands parameters" do - env = {"QUERY_STRING" => "foo=bar"} - - request = Rack::Request.new(env) - request.query_param_list.must_equal [["foo", "bar"]] - request.GET.must_equal "foo" => "bar" - - upper_request = UpperRequest.new(env) - upper_request.query_param_list.must_equal [["foo", "bar"]] - upper_request.GET.must_equal "FOO" => "bar" - - env['QUERY_STRING'] = "foo=bar&bar=baz" - - request.GET.must_equal "foo" => "bar", "bar" => "baz" - upper_request.GET.must_equal "FOO" => "bar", "BAR" => "baz" - end - - class BrokenRequest < Rack::Request - def expand_params(parameters) - raise "boom" - end - end - - it "raises an error if expand_params raises an error" do - env = {"QUERY_STRING" => "foo=bar"} - - request = Rack::Request.new(env) - request.GET.must_equal "foo" => "bar" - - broken_request = BrokenRequest.new(env) - lambda { broken_request.GET }.must_raise RuntimeError - - # Subsequnt calls also raise an error: - lambda { broken_request.GET }.must_raise RuntimeError - end end diff --git a/test/spec_utils.rb b/test/spec_utils.rb index b771eafae..64e070ed8 100644 --- a/test/spec_utils.rb +++ b/test/spec_utils.rb @@ -141,7 +141,7 @@ def assert_nested_query(exp, act) it "parse nested query strings correctly" do Rack::Utils.parse_nested_query("foo"). - must_equal "foo" => "" + must_equal "foo" => nil Rack::Utils.parse_nested_query("foo="). must_equal "foo" => "" Rack::Utils.parse_nested_query("foo=bar"). @@ -158,7 +158,7 @@ def assert_nested_query(exp, act) Rack::Utils.parse_nested_query("&foo=1&&bar=2"). must_equal "foo" => "1", "bar" => "2" Rack::Utils.parse_nested_query("foo&bar="). - must_equal "foo" => "", "bar" => "" + must_equal "foo" => nil, "bar" => "" Rack::Utils.parse_nested_query("foo=bar&baz="). must_equal "foo" => "bar", "baz" => "" Rack::Utils.parse_nested_query("my+weird+field=q1%212%22%27w%245%267%2Fz8%29%3F"). @@ -168,19 +168,19 @@ def assert_nested_query(exp, act) must_equal "pid=1234" => "1023", "a" => "b" Rack::Utils.parse_nested_query("foo[]"). - must_equal "foo" => [""] + must_equal "foo" => [nil] Rack::Utils.parse_nested_query("foo[]="). must_equal "foo" => [""] Rack::Utils.parse_nested_query("foo[]=bar"). must_equal "foo" => ["bar"] Rack::Utils.parse_nested_query("foo[]=bar&foo"). - must_equal "foo" => "" + must_equal "foo" => nil Rack::Utils.parse_nested_query("foo[]=bar&foo["). - must_equal "foo" => ["bar"], "foo[" => "" + must_equal "foo" => ["bar"], "foo[" => nil Rack::Utils.parse_nested_query("foo[]=bar&foo[=baz"). must_equal "foo" => ["bar"], "foo[" => "baz" Rack::Utils.parse_nested_query("foo[]=bar&foo[]"). - must_equal "foo" => ["bar", ""] + must_equal "foo" => ["bar", nil] Rack::Utils.parse_nested_query("foo[]=bar&foo[]="). must_equal "foo" => ["bar", ""] @@ -192,7 +192,7 @@ def assert_nested_query(exp, act) must_equal "foo" => ["bar"], "baz" => ["1", "2", "3"] Rack::Utils.parse_nested_query("x[y][z]"). - must_equal "x" => { "y" => { "z" => "" } } + must_equal "x" => { "y" => { "z" => nil } } Rack::Utils.parse_nested_query("x[y][z]=1"). must_equal "x" => { "y" => { "z" => "1" } } Rack::Utils.parse_nested_query("x[y][z][]=1").