From d34a346d9d928e8dbc18b415ce0343ed724dfb26 Mon Sep 17 00:00:00 2001 From: Nicholas Seckar Date: Wed, 30 Aug 2006 05:50:02 +0000 Subject: Switch to using FormEncodedPairParser for parsing request parameters. git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4866 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- actionpack/CHANGELOG | 2 + .../lib/action_controller/cgi_ext/cgi_methods.rb | 167 +++++++-------------- actionpack/lib/action_controller/cgi_process.rb | 4 +- actionpack/test/controller/cgi_test.rb | 35 ++--- 4 files changed, 76 insertions(+), 132 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 917be84b3b..3f81822176 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Switch to using FormEncodedPairParser for parsing request parameters. [Nicholas Seckar, DHH] + * respond_to .html now always renders #{action_name}.rhtml so that registered custom template handlers do not override it in priority. Custom mime types require a block and throw proper error now. [Tobias Luetke] * Deprecation: test deprecated instance vars in partials. [Jeremy Kemper] diff --git a/actionpack/lib/action_controller/cgi_ext/cgi_methods.rb b/actionpack/lib/action_controller/cgi_ext/cgi_methods.rb index e58c743345..bb7be80560 100755 --- a/actionpack/lib/action_controller/cgi_ext/cgi_methods.rb +++ b/actionpack/lib/action_controller/cgi_ext/cgi_methods.rb @@ -8,31 +8,39 @@ class CGIMethods #:nodoc: class << self # DEPRECATED: Use parse_form_encoded_parameters def parse_query_parameters(query_string) - parse_form_encoded_parameters(query_string) + pairs = query_string.split('&').collect do |chunk| + next if chunk.empty? + key, value = chunk.split('=', 2) + value = (value.nil? || value.empty?) ? nil : CGI.unescape(value) + [ key, value ] + end.compact + + FormEncodedPairParser.new(pairs).result end # DEPRECATED: Use parse_form_encoded_parameters def parse_request_parameters(params) - parsed_params = {} - - for key, value in params - next unless key - value = [value] if key =~ /.*\[\]$/ - unless key.include?('[') - # much faster to test for the most common case first (GET) - # and avoid the call to build_deep_hash - parsed_params[key] = get_typed_value(value[0]) - else - build_deep_hash(get_typed_value(value[0]), parsed_params, get_levels(key)) + parser = FormEncodedPairParser.new + + finished = false + until finished + finished = true + for key, value in params + next unless key + if !key.include?('[') + # much faster to test for the most common case first (GET) + # and avoid the call to build_deep_hash + parser.result[key] = get_typed_value(value[0]) + elsif value.is_a?(Array) + parser.parse(key, get_typed_value(value.shift)) + finished = false unless value.empty? + else + raise TypeError, "Expected array, found #{value.inspect}" + end end end - parsed_params - end - - # TODO: Docs - def parse_form_encoded_parameters(form_encoded_string) - FormEncodedStringScanner.decode(form_encoded_string) + parser.result end def parse_formatted_request_parameters(mime_type, raw_post_data) @@ -93,99 +101,40 @@ class CGIMethods #:nodoc: value.to_s end end - - PARAMS_HASH_RE = /^([^\[]+)(\[.*\])?(.)?.*$/ - def get_levels(key) - all, main, bracketed, trailing = PARAMS_HASH_RE.match(key).to_a - if main.nil? - [] - elsif trailing - [key] - elsif bracketed - [main] + bracketed.slice(1...-1).split('][') - else - [main] - end - end - - def build_deep_hash(value, hash, levels) - if levels.length == 0 - value - elsif hash.nil? - { levels.first => build_deep_hash(value, nil, levels[1..-1]) } - else - hash.update({ levels.first => build_deep_hash(value, hash[levels.first], levels[1..-1]) }) - end - end end - class FormEncodedStringScanner < StringScanner + class FormEncodedPairParser < StringScanner attr_reader :top, :parent, :result - def self.decode(form_encoded_string) - new(form_encoded_string).parse - end - - def initialize(form_encoded_string) - super(unescape_keys(form_encoded_string)) + def initialize(pairs = []) + super('') + @result = {} + pairs.each { |key, value| parse(key, value) } end - + KEY_REGEXP = %r{([^\[\]=&]+)} BRACKETED_KEY_REGEXP = %r{\[([^\[\]=&]+)\]} - - def parse - @result = {} - - until eos? - # Parse each & delimited chunk - @parent, @top = nil, result - - # First scan the bare key - key = scan(KEY_REGEXP) or (skip_term and next) + # Parse the query string + def parse(key, value) + self.string = key + @top, @parent = result, nil + + # First scan the bare key + key = scan(KEY_REGEXP) or return + key = post_key_check(key) + + # Then scan as many nestings as present + until eos? + r = scan(BRACKETED_KEY_REGEXP) or return + key = self[1] key = post_key_check(key) - - # Then scan as many nestings as present - until check(/\=/) || eos? - r = scan(BRACKETED_KEY_REGEXP) or (skip_term and break) - key = self[1] - key = post_key_check(key) - end - - # Scan the value if we see an = - if scan %r{=} - value = scan(/[^\&]+/) # scan_until doesn't handle \Z - value = CGI.unescape(value) if value # May be nil when eos? - bind(key, value) - end - - scan(%r/\&+/) # Ignore multiple adjacent &'s end - - return result + + bind(key, value) end private - # Turn keys like person%5Bname%5D into person[name], so they can be processed as hashes - def unescape_keys(query_string) - query_string.split('&').collect do |fragment| - key, value = fragment.split('=', 2) - - if key - key = key.gsub(/%5D/, ']').gsub(/%5B/, '[') - [ key, value ].join("=") - else - fragment - end - end.join('&') - end - - # Skip over the current term by scanning past the next &, or to - # then end of the string if there is no next & - def skip_term - scan_until(%r/\&+/) || scan(/.+/) - end - # After we see a key, we must look ahead to determine our next action. Cases: # # [] follows the key. Then the value must be an array. @@ -193,16 +142,13 @@ class CGIMethods #:nodoc: # & or the end of string follows the key. Then the key is a flag. # otherwise, a hash follows the key. def post_key_check(key) - if eos? || check(/\&/) # a& or a\Z indicates a is a flag. - bind key, nil # Curiously enough, the flag's value is nil - nil - elsif scan(/\[\]/) # a[b][] indicates that b is an array - container key, Array + if scan(/\[\]/) # a[b][] indicates that b is an array + container(key, Array) nil elsif check(/\[[^\]]/) # a[b] indicates that a is a hash - container key, Hash + container(key, Hash) nil - else # Presumably an = sign is next. + else # End of key? We do nothing. key end end @@ -212,8 +158,8 @@ class CGIMethods #:nodoc: def container(key, klass) raise TypeError if top.is_a?(Hash) && top.key?(key) && ! top[key].is_a?(klass) value = bind(key, klass.new) - raise TypeError unless value.is_a? klass - push value + raise TypeError unless value.is_a?(klass) + push(value) end # Push a value onto the 'stack', which is actually only the top 2 items. @@ -236,13 +182,12 @@ class CGIMethods #:nodoc: end elsif top.is_a? Hash key = CGI.unescape(key) - if top.key?(key) && parent.is_a?(Array) - parent << (@top = {}) - end + parent << (@top = {}) if top.key?(key) && parent.is_a?(Array) return top[key] ||= value else - # Do nothing? + raise ArgumentError, "Don't know what to do: top is #{top.inspect}" end + return value end end diff --git a/actionpack/lib/action_controller/cgi_process.rb b/actionpack/lib/action_controller/cgi_process.rb index e983912158..3170bc1ce7 100644 --- a/actionpack/lib/action_controller/cgi_process.rb +++ b/actionpack/lib/action_controller/cgi_process.rb @@ -145,7 +145,7 @@ module ActionController #:nodoc: begin Module.const_missing($1) rescue LoadError, NameError => const_error - raise ActionController::SessionRestoreError, < {'y' => {'z' => '10'}}} assert_equal(expected, CGIMethods.parse_query_parameters('x[y][z]=10')) - expected = {"commit"=>"Update info", - "subscriber" => { - "city" => nil, "zip"=>nil, - "last_name" => "Waterman", "street" => nil, - "password" =>"893ea", "first_name" => "Jan5" , - "email" => "drakn@domain.tld" - } - } - assert_equal(expected, CGIMethods.parse_query_parameters(@query_string_with_escaped_brackets)) end def test_deep_query_string_with_array @@ -61,6 +47,17 @@ last_name%5D=Waterman&subscriber%5Bemail%5D=drakn%40domain.tld&subscriber%5Bpass assert_equal("10", CGIMethods.parse_query_parameters('x[y][][z]=10&x[y][][z]=20').with_indifferent_access[:x][:y].first[:z]) end + def test_request_hash_parsing + query = { + "note[viewers][viewer][][type]" => ["User", "Group"], + "note[viewers][viewer][][id]" => ["1", "2"] + } + + expected = { "note" => { "viewers"=>{"viewer"=>[{ "id"=>"1", "type"=>"User"}, {"type"=>"Group", "id"=>"2"} ]} } } + + assert_equal(expected, CGIMethods.parse_request_parameters(query)) + end + def test_deep_query_string_with_array_of_hashes_with_multiple_pairs assert_equal( {'x' => {'y' => [{'z' => '10', 'w' => 'a'}, {'z' => '20', 'w' => 'b'}]}}, @@ -239,25 +236,25 @@ last_name%5D=Waterman&subscriber%5Bemail%5D=drakn%40domain.tld&subscriber%5Bpass def test_parse_params_with_single_brackets_in_middle input = { "a/b[c]d" => %w(e) } - expected = { "a/b[c]d" => "e" } + expected = { "a/b" => {} } assert_equal expected, CGIMethods.parse_request_parameters(input) end def test_parse_params_with_separated_brackets input = { "a/b@[c]d[e]" => %w(f) } - expected = { "a/b@" => { "c]d[e" => "f" }} + expected = { "a/b@" => { }} assert_equal expected, CGIMethods.parse_request_parameters(input) end def test_parse_params_with_separated_brackets_and_array input = { "a/b@[c]d[e][]" => %w(f) } - expected = { "a/b@" => { "c]d[e" => ["f"] }} + expected = { "a/b@" => { }} assert_equal expected , CGIMethods.parse_request_parameters(input) end def test_parse_params_with_unmatched_brackets_and_array input = { "a/b@[c][d[e][]" => %w(f) } - expected = { "a/b@" => { "c" => { "d[e" => ["f"] }}} + expected = { "a/b@" => { "c" => { }}} assert_equal expected, CGIMethods.parse_request_parameters(input) end -- cgit v1.2.3