FormEncodedStringParser needs a tad more work before it can handle POST data (like file handling), so were backing out for a bit

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4833 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

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