diff options
| -rw-r--r-- | actionpack/CHANGELOG | 2 | ||||
| -rwxr-xr-x | actionpack/lib/action_controller/cgi_ext/cgi_methods.rb | 161 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/cgi_process.rb | 2 | ||||
| -rwxr-xr-x | actionpack/test/controller/cgi_test.rb | 14 |
4 files changed, 106 insertions, 73 deletions
diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index a103a9fede..96bafacdcb 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -2,8 +2,6 @@ * Deprecation: test deprecated instance vars in partials. [Jeremy Kemper] -* Changed the POST parameter processing to use the new QueryStringParser and make the result a indifferent hash [DHH] - * Add UrlWriter to allow writing urls from Mailers and scripts. [Nicholas Seckar] * Clean up and run the Active Record integration tests by default. #5854 [kevin.clark@gmail.com, 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 26adbca13c..e58c743345 100755 --- a/actionpack/lib/action_controller/cgi_ext/cgi_methods.rb +++ b/actionpack/lib/action_controller/cgi_ext/cgi_methods.rb @@ -5,17 +5,13 @@ require 'strscan' # Static methods for parsing the query and request parameters that can be used in # a CGI extension class or testing in isolation. class CGIMethods #:nodoc: - class << self - # Returns a hash with the pairs from the query string. The implicit hash construction that is done in - # parse_request_params is not done here. + # DEPRECATED: Use parse_form_encoded_parameters def parse_query_parameters(query_string) - QueryStringScanner.new(query_string).parse + parse_form_encoded_parameters(query_string) end - # Returns the request (POST/GET) parameters in a parsed form where pairs such as "customer[address][street]" / - # "Somewhere cool!" are translated into a full hash hierarchy, like - # { "customer" => { "address" => { "street" => "Somewhere cool!" } } } + # DEPRECATED: Use parse_form_encoded_parameters def parse_request_parameters(params) parsed_params = {} @@ -33,6 +29,11 @@ class CGIMethods #:nodoc: parsed_params end + + # TODO: Docs + def parse_form_encoded_parameters(form_encoded_string) + FormEncodedStringScanner.decode(form_encoded_string) + end def parse_formatted_request_parameters(mime_type, raw_post_data) case strategy = ActionController::Base.param_parsers[mime_type] @@ -118,19 +119,24 @@ class CGIMethods #:nodoc: end end - class QueryStringScanner < StringScanner + class FormEncodedStringScanner < StringScanner attr_reader :top, :parent, :result - def initialize(string) - super(string) + def self.decode(form_encoded_string) + new(form_encoded_string).parse + end + + def initialize(form_encoded_string) + super(unescape_keys(form_encoded_string)) end KEY_REGEXP = %r{([^\[\]=&]+)} BRACKETED_KEY_REGEXP = %r{\[([^\[\]=&]+)\]} - # Parse the query string + def parse @result = {} + until eos? # Parse each & delimited chunk @parent, @top = nil, result @@ -150,79 +156,94 @@ class CGIMethods #:nodoc: 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 + bind(key, value) end - scan %r/\&+/ # Ignore multiple adjacent &'s - + + scan(%r/\&+/) # Ignore multiple adjacent &'s end return result 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 - # 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. - # = follows the key. (A value comes next) - # & 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 - nil - elsif check(/\[[^\]]/) # a[b] indicates that a is a hash - container key, Hash - nil - else # Presumably an = sign is next. - key + # 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. + # = follows the key. (A value comes next) + # & 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 + nil + elsif check(/\[[^\]]/) # a[b] indicates that a is a hash + container key, Hash + nil + else # Presumably an = sign is next. + key + end end - end - # Add a container to the stack. - # - 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 - end + # Add a container to the stack. + # + 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 + end - # Push a value onto the 'stack', which is actually only the top 2 items. - def push(value) - @parent, @top = @top, value - end + # Push a value onto the 'stack', which is actually only the top 2 items. + def push(value) + @parent, @top = @top, value + end - # Bind a key (which may be nil for items in an array) to the provided value. - def bind(key, value) - if top.is_a? Array - if key - if top[-1].is_a?(Hash) && ! top[-1].key?(key) - top[-1][key] = value + # Bind a key (which may be nil for items in an array) to the provided value. + def bind(key, value) + if top.is_a? Array + if key + if top[-1].is_a?(Hash) && ! top[-1].key?(key) + top[-1][key] = value + else + top << {key => value}.with_indifferent_access + push top.last + end else - top << {key => value}.with_indifferent_access - push top.last + top << value end + elsif top.is_a? Hash + key = CGI.unescape(key) + if top.key?(key) && parent.is_a?(Array) + parent << (@top = {}) + end + return top[key] ||= value else - top << value - end - elsif top.is_a? Hash - key = CGI.unescape(key) - if top.key?(key) && parent.is_a?(Array) - parent << (@top = {}) + # Do nothing? end - return top[key] ||= value - else - # Do nothing? + return value end - return value end - end end
\ No newline at end of file diff --git a/actionpack/lib/action_controller/cgi_process.rb b/actionpack/lib/action_controller/cgi_process.rb index d409c66a8a..e983912158 100644 --- a/actionpack/lib/action_controller/cgi_process.rb +++ b/actionpack/lib/action_controller/cgi_process.rb @@ -68,7 +68,7 @@ module ActionController #:nodoc: if ActionController::Base.param_parsers.has_key?(content_type) CGIMethods.parse_formatted_request_parameters(content_type, @env['RAW_POST_DATA']) else - CGIMethods.parse_query_parameters(@env['RAW_POST_DATA'] || "") + CGIMethods.parse_request_parameters(@cgi.params) end end diff --git a/actionpack/test/controller/cgi_test.rb b/actionpack/test/controller/cgi_test.rb index d235379e3d..256fd7e58f 100755 --- a/actionpack/test/controller/cgi_test.rb +++ b/actionpack/test/controller/cgi_test.rb @@ -17,6 +17,11 @@ class CGITest < Test::Unit::TestCase @query_string_without_equal = "action" @query_string_with_many_ampersands = "&action=create_customer&&&full_name=David%20Heinemeier%20Hansson" + @query_string_with_escaped_brackets = "subscriber%5Bfirst_name%5D=Jan5&subscriber%5B\ +last_name%5D=Waterman&subscriber%5Bemail%5D=drakn%40domain.tld&subscriber%5Bpassword\ +%5D=893ea&subscriber%5Bstreet%5D=&subscriber%5Bzip%5D=&subscriber%5Bcity%5D\ +=&commit=Update+info" + end def test_query_string @@ -29,6 +34,15 @@ class CGITest < Test::Unit::TestCase def test_deep_query_string expected = {'x' => {'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 |