Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Switch to using FormEncodedPairParser for parsing request parameters.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4866 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
commit d34a346d9d928e8dbc18b415ce0343ed724dfb26 1 parent 9ece976
@seckar seckar authored
View
2  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]
View
167 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 @@ def original_filename
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 @@ def skip_term
# & 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 @@ def post_key_check(key)
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 @@ def bind(key, value)
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
View
4 actionpack/lib/action_controller/cgi_process.rb
@@ -145,7 +145,7 @@ def stale_session_check!
begin
Module.const_missing($1)
rescue LoadError, NameError => const_error
- raise ActionController::SessionRestoreError, <<end_msg
+ raise ActionController::SessionRestoreError, <<-end_msg
Session contains objects whose class definition isn\'t available.
Remember to require the classes for all objects kept in the session.
(Original exception: #{const_error.message} [#{const_error.class}])
@@ -207,4 +207,4 @@ def convert_content_type!(headers)
end
end
end
-end
+end
View
35 actionpack/test/controller/cgi_test.rb
@@ -16,12 +16,7 @@ def setup
@query_string_with_many_equal = "action=create_customer&full_name=abc=def=ghi"
@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"
-
+ "&action=create_customer&&&full_name=David%20Heinemeier%20Hansson"
end
def test_query_string
@@ -34,15 +29,6 @@ def test_query_string
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
@@ -61,6 +47,17 @@ def test_deep_query_string_with_array_of_hashes_with_one_pair
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 @@ def test_parse_params_with_non_alphanumeric_name
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
Please sign in to comment.
Something went wrong with that request. Please try again.