Build query string and POST params parser on top of Rack::Request. Also switch our multipart parser to use Racks. Moved XML, JSON, and YAML parsers into ActionController::ParamsParser middleware [#1661 state:resolved]

actionpack/lib/action_controller.rb

@@ -58,6 +58,7 @@ module ActionController
   autoload :Layout, 'action_controller/layout'
   autoload :MiddlewareStack, 'action_controller/middleware_stack'
   autoload :MimeResponds, 'action_controller/mime_responds'
+  autoload :ParamsParser, 'action_controller/params_parser'
   autoload :PolymorphicRoutes, 'action_controller/polymorphic_routes'
   autoload :RecordIdentifier, 'action_controller/record_identifier'
   autoload :Request, 'action_controller/request'
@@ -74,6 +75,7 @@ module ActionController
   autoload :TestCase, 'action_controller/test_case'
   autoload :TestProcess, 'action_controller/test_process'
   autoload :Translation, 'action_controller/translation'
+  autoload :UploadedFile, 'action_controller/uploaded_file'
   autoload :UploadedStringIO, 'action_controller/uploaded_file'
   autoload :UploadedTempfile, 'action_controller/uploaded_file'
   autoload :UrlEncodedPairParser, 'action_controller/url_encoded_pair_parser'

actionpack/lib/action_controller/base.rb

@@ -301,10 +301,7 @@ module ActionController #:nodoc:
     # A YAML parser is also available and can be turned on with:
     #
     #   ActionController::Base.param_parsers[Mime::YAML] = :yaml
-    @@param_parsers = { Mime::MULTIPART_FORM   => :multipart_form,
+    @@param_parsers = {}
-                        Mime::URL_ENCODED_FORM => :url_encoded_form,
-                        Mime::XML              => :xml_simple,
-                        Mime::JSON             => :json }
     cattr_accessor :param_parsers
 
     # Controls the default charset for all renders.

actionpack/lib/action_controller/middlewares.rb

@@ -19,4 +19,5 @@ use "ActiveRecord::QueryCache", :if => lambda { defined?(ActiveRecord) }
 end
 
 use ActionController::RewindableInput
+use ActionController::ParamsParser
 use Rack::MethodOverride

actionpack/lib/action_controller/params_parser.rb

@@ -0,0 +1,71 @@
+module ActionController
+  class ParamsParser
+    ActionController::Base.param_parsers[Mime::XML] = :xml_simple
+    ActionController::Base.param_parsers[Mime::JSON] = :json
+
+    def initialize(app)
+      @app = app
+    end
+
+    def call(env)
+      if params = parse_formatted_parameters(env)
+        env["action_controller.request.request_parameters"] = params
+      end
+
+      @app.call(env)
+    end
+
+    private
+      def parse_formatted_parameters(env)
+        request = Request.new(env)
+
+        return false if request.content_length.zero?
+
+        mime_type = content_type_from_legacy_post_data_format_header(env) || request.content_type
+        strategy = ActionController::Base.param_parsers[mime_type]
+
+        return false unless strategy
+
+        case strategy
+          when Proc
+            strategy.call(request.raw_post)
+          when :xml_simple, :xml_node
+            body = request.raw_post
+            body.blank? ? {} : Hash.from_xml(body).with_indifferent_access
+          when :yaml
+            YAML.load(request.raw_post)
+          when :json
+            body = request.raw_post
+            if body.blank?
+              {}
+            else
+              data = ActiveSupport::JSON.decode(body)
+              data = {:_json => data} unless data.is_a?(Hash)
+              data.with_indifferent_access
+            end
+          else
+            false
+        end
+      rescue Exception => e # YAML, XML or Ruby code block errors
+        raise
+          { "body" => request.raw_post,
+            "content_type" => request.content_type,
+            "content_length" => request.content_length,
+            "exception" => "#{e.message} (#{e.class})",
+            "backtrace" => e.backtrace }
+      end
+
+      def content_type_from_legacy_post_data_format_header(env)
+        if x_post_format = env['HTTP_X_POST_DATA_FORMAT']
+          case x_post_format.to_s.downcase
+            when 'yaml'
+              return Mime::YAML
+            when 'xml'
+              return Mime::XML
+          end
+        end
+
+        nil
+      end
+  end
+end

actionpack/lib/action_controller/rack_ext.rb

@@ -1,2 +1,3 @@
 require 'action_controller/rack_ext/lock'
 require 'action_controller/rack_ext/multipart'
+require 'action_controller/rack_ext/parse_query'

actionpack/lib/action_controller/rack_ext/parse_query.rb

@@ -0,0 +1,18 @@
+# Rack does not automatically cleanup Safari 2 AJAX POST body
+# This has not yet been commited to Rack, please +1 this ticket:
+# http://rack.lighthouseapp.com/projects/22435/tickets/19
+
+module Rack
+  module Utils
+    alias_method :parse_query_without_ajax_body_cleanup, :parse_query
+    module_function :parse_query_without_ajax_body_cleanup
+
+    def parse_query(qs, d = '&;')
+      qs = qs.dup
+      qs.chop! if qs[-1] == 0
+      qs.gsub!(/&_=$/, '')
+      parse_query_without_ajax_body_cleanup(qs, d)
+    end
+    module_function :parse_query
+  end
+end

actionpack/lib/action_controller/request.rb

@@ -9,11 +9,6 @@ module ActionController
   class Request < Rack::Request
     extend ActiveSupport::Memoizable
 
-    def initialize(env)
-      super
-      @parser = ActionController::RequestParser.new(env)
-    end
-
     %w[ AUTH_TYPE GATEWAY_INTERFACE
         PATH_TRANSLATED REMOTE_HOST
         REMOTE_IDENT REMOTE_USER REMOTE_ADDR
@@ -92,7 +87,11 @@ module ActionController
     # For backward compatibility, the post \format is extracted from the
     # X-Post-Data-Format HTTP header if present.
     def content_type
-      Mime::Type.lookup(@parser.content_type_without_parameters)
+      if @env['CONTENT_TYPE'] =~ /^([^,\;]*)/
+        Mime::Type.lookup($1.strip.downcase)
+      else
+        nil
+      end
     end
     memoize :content_type
 
@@ -380,7 +379,11 @@ EOM
     # Read the request \body. This is useful for web services that need to
     # work with raw requests directly.
     def raw_post
-      @parser.raw_post
+      unless @env.include? 'RAW_POST_DATA'
+        @env['RAW_POST_DATA'] = body.read(@env['CONTENT_LENGTH'].to_i)
+        body.rewind if body.respond_to?(:rewind)
+      end
+      @env['RAW_POST_DATA']
     end
 
     # Returns both GET and POST \parameters in a single hash.
@@ -409,19 +412,30 @@ EOM
       @env["rack.routing_args"] ||= {}
     end
 
+    # The request body is an IO input stream. If the RAW_POST_DATA environment
+    # variable is already set, wrap it in a StringIO.
     def body
-      @parser.body
+      if raw_post = @env['RAW_POST_DATA']
+        raw_post.force_encoding(Encoding::BINARY) if raw_post.respond_to?(:force_encoding)
+        StringIO.new(raw_post)
+      else
+        @env['rack.input']
+      end
+    end
+
+    def form_data?
+      FORM_DATA_MEDIA_TYPES.include?(content_type.to_s)
     end
 
     # Override Rack's GET method to support nested query strings
     def GET
-      @parser.query_parameters
+      @env["action_controller.request.query_parameters"] ||= UrlEncodedPairParser.parse_query_parameters(query_string)
     end
     alias_method :query_parameters, :GET
 
     # Override Rack's POST method to support nested query strings
     def POST
-      @parser.request_parameters
+      @env["action_controller.request.request_parameters"] ||= UrlEncodedPairParser.parse_hash_parameters(super)
     end
     alias_method :request_parameters, :POST
 

actionpack/lib/action_controller/request_parser.rb

@@ -1,315 +0,0 @@
-module ActionController
-  class RequestParser
-    def initialize(env)
-      @env = env
-      freeze
-    end
-
-    def request_parameters
-      @env["action_controller.request_parser.request_parameters"] ||= parse_formatted_request_parameters
-    end
-
-    def query_parameters
-      @env["action_controller.request_parser.query_parameters"] ||= self.class.parse_query_parameters(query_string)
-    end
-
-    # Returns the query string, accounting for server idiosyncrasies.
-    def query_string
-      @env['QUERY_STRING'].present? ? @env['QUERY_STRING'] : (@env['REQUEST_URI'].split('?', 2)[1] || '')
-    end
-
-    # The request body is an IO input stream. If the RAW_POST_DATA environment
-    # variable is already set, wrap it in a StringIO.
-    def body
-      if raw_post = @env['RAW_POST_DATA']
-        raw_post.force_encoding(Encoding::BINARY) if raw_post.respond_to?(:force_encoding)
-        StringIO.new(raw_post)
-      else
-        @env['rack.input']
-      end
-    end
-
-    # The raw content type string with its parameters stripped off.
-    def content_type_without_parameters
-      self.class.extract_content_type_without_parameters(content_type_with_parameters)
-    end
-
-    def raw_post
-      unless @env.include? 'RAW_POST_DATA'
-        @env['RAW_POST_DATA'] = body.read(content_length)
-        body.rewind if body.respond_to?(:rewind)
-      end
-      @env['RAW_POST_DATA']
-    end
-
-    private
-
-    def parse_formatted_request_parameters
-      return {} if content_length.zero?
-
-      content_type, boundary = self.class.extract_multipart_boundary(content_type_with_parameters)
-
-      # Don't parse params for unknown requests.
-      return {} if content_type.blank?
-
-      mime_type = Mime::Type.lookup(content_type)
-      strategy = ActionController::Base.param_parsers[mime_type]
-
-      # Only multipart form parsing expects a stream.
-      body = (strategy && strategy != :multipart_form) ? raw_post : self.body
-
-      case strategy
-        when Proc
-          strategy.call(body)
-        when :url_encoded_form
-          self.class.clean_up_ajax_request_body! body
-          self.class.parse_query_parameters(body)
-        when :multipart_form
-          self.class.parse_multipart_form_parameters(body, boundary, content_length, @env)
-        when :xml_simple, :xml_node
-          body.blank? ? {} : Hash.from_xml(body).with_indifferent_access
-        when :yaml
-          YAML.load(body)
-        when :json
-          if body.blank?
-            {}
-          else
-            data = ActiveSupport::JSON.decode(body)
-            data = {:_json => data} unless data.is_a?(Hash)
-            data.with_indifferent_access
-          end
-        else
-          {}
-      end
-    rescue Exception => e # YAML, XML or Ruby code block errors
-      raise
-      { "body" => body,
-        "content_type" => content_type_with_parameters,
-        "content_length" => content_length,
-        "exception" => "#{e.message} (#{e.class})",
-        "backtrace" => e.backtrace }
-    end
-
-    def content_length
-      @env['CONTENT_LENGTH'].to_i
-    end
-
-    # The raw content type string. Use when you need parameters such as
-    # charset or boundary which aren't included in the content_type MIME type.
-    # Overridden by the X-POST_DATA_FORMAT header for backward compatibility.
-    def content_type_with_parameters
-      content_type_from_legacy_post_data_format_header || @env['CONTENT_TYPE'].to_s
-    end
-
-    def content_type_from_legacy_post_data_format_header
-      if x_post_format = @env['HTTP_X_POST_DATA_FORMAT']
-        case x_post_format.to_s.downcase
-          when 'yaml';  'application/x-yaml'
-          when 'xml';   'application/xml'
-        end
-      end
-    end
-
-    class << self
-      def parse_query_parameters(query_string)
-        return {} if query_string.blank?
-
-        pairs = query_string.split('&').collect do |chunk|
-          next if chunk.empty?
-          key, value = chunk.split('=', 2)
-          next if key.empty?
-          value = value.nil? ? nil : CGI.unescape(value)
-          [ CGI.unescape(key), value ]
-        end.compact
-
-        UrlEncodedPairParser.new(pairs).result
-      end
-
-      def parse_request_parameters(params)
-        parser = UrlEncodedPairParser.new
-
-        params = params.dup
-        until params.empty?
-          for key, value in params
-            if key.blank?
-              params.delete key
-            elsif !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])
-              params.delete key
-            elsif value.is_a?(Array)
-              parser.parse(key, get_typed_value(value.shift))
-              params.delete key if value.empty?
-            else
-              raise TypeError, "Expected array, found #{value.inspect}"
-            end
-          end
-        end
-
-        parser.result
-      end
-
-      def parse_multipart_form_parameters(body, boundary, body_size, env)
-        parse_request_parameters(read_multipart(body, boundary, body_size, env))
-      end
-
-      def extract_multipart_boundary(content_type_with_parameters)
-        if content_type_with_parameters =~ MULTIPART_BOUNDARY
-          ['multipart/form-data', $1.dup]
-        else
-          extract_content_type_without_parameters(content_type_with_parameters)
-        end
-      end
-
-      def extract_content_type_without_parameters(content_type_with_parameters)
-        $1.strip.downcase if content_type_with_parameters =~ /^([^,\;]*)/
-      end
-
-      def clean_up_ajax_request_body!(body)
-        body.chop! if body[-1] == 0
-        body.gsub!(/&_=$/, '')
-      end
-
-
-      private
-        def get_typed_value(value)
-          case value
-            when String
-              value
-            when NilClass
-              ''
-            when Array
-              value.map { |v| get_typed_value(v) }
-            else
-              if value.respond_to? :original_filename
-                # Uploaded file
-                if value.original_filename
-                  value
-                # Multipart param
-                else
-                  result = value.read
-                  value.rewind
-                  result
-                end
-              # Unknown value, neither string nor multipart.
-              else
-                raise "Unknown form value: #{value.inspect}"
-              end
-          end
-        end
-
-        MULTIPART_BOUNDARY = %r|\Amultipart/form-data.*boundary=\"?([^\";,]+)\"?|n
-
-        EOL = "\015\012"
-
-        def read_multipart(body, boundary, body_size, env)
-          params = Hash.new([])
-          boundary = "--" + boundary
-          quoted_boundary = Regexp.quote(boundary)
-          buf = ""
-          bufsize = 10 * 1024
-          boundary_end=""
-
-          # start multipart/form-data
-          body.binmode if defined? body.binmode
-          case body
-          when File
-            body.set_encoding(Encoding::BINARY) if body.respond_to?(:set_encoding)
-          when StringIO
-            body.string.force_encoding(Encoding::BINARY) if body.string.respond_to?(:force_encoding)
-          end
-          boundary_size = boundary.size + EOL.size
-          body_size -= boundary_size
-          status = body.read(boundary_size)
-          if nil == status
-            raise EOFError, "no content body"
-          elsif boundary + EOL != status
-            raise EOFError, "bad content body"
-          end
-
-          loop do
-            head = nil
-            content =
-              if 10240 < body_size
-                UploadedTempfile.new("CGI")
-              else
-                UploadedStringIO.new
-              end
-            content.binmode if defined? content.binmode
-
-            until head and /#{quoted_boundary}(?:#{EOL}|--)/n.match(buf)
-
-              if (not head) and /#{EOL}#{EOL}/n.match(buf)
-                buf = buf.sub(/\A((?:.|\n)*?#{EOL})#{EOL}/n) do
-                  head = $1.dup
-                  ""
-                end
-                next
-              end
-
-              if head and ( (EOL + boundary + EOL).size < buf.size )
-                content.print buf[0 ... (buf.size - (EOL + boundary + EOL).size)]
-                buf[0 ... (buf.size - (EOL + boundary + EOL).size)] = ""
-              end
-
-              c = if bufsize < body_size
-                    body.read(bufsize)
-                  else
-                    body.read(body_size)
-                  end
-              if c.nil? || c.empty?
-                raise EOFError, "bad content body"
-              end
-              buf.concat(c)
-              body_size -= c.size
-            end
-
-            buf = buf.sub(/\A((?:.|\n)*?)(?:[\r\n]{1,2})?#{quoted_boundary}([\r\n]{1,2}|--)/n) do
-              content.print $1
-              if "--" == $2
-                body_size = -1
-              end
-              boundary_end = $2.dup
-              ""
-            end
-
-            content.rewind
-
-            head =~ /Content-Disposition:.* filename=(?:"((?:\\.|[^\"])*)"|([^;]*))/ni
-            if filename = $1 || $2
-              if /Mac/ni.match(env['HTTP_USER_AGENT']) and
-                  /Mozilla/ni.match(env['HTTP_USER_AGENT']) and
-                  (not /MSIE/ni.match(env['HTTP_USER_AGENT']))
-                filename = CGI.unescape(filename)
-              end
-              content.original_path = filename.dup
-            end
-
-            head =~ /Content-Type: ([^\r]*)/ni
-            content.content_type = $1.dup if $1
-
-            head =~ /Content-Disposition:.* name="?([^\";]*)"?/ni
-            name = $1.dup if $1
-
-            if params.has_key?(name)
-              params[name].push(content)
-            else
-              params[name] = [content]
-            end
-            break if body_size == -1
-          end
-          raise EOFError, "bad boundary end of body part" unless boundary_end=~/--/
-
-          begin
-            body.rewind if body.respond_to?(:rewind)
-          rescue Errno::ESPIPE
-            # Handles exceptions raised by input streams that cannot be rewound
-            # such as when using plain CGI under Apache
-          end
-
-          params
-        end
-    end # class << self
-  end
-end

actionpack/lib/action_controller/uploaded_file.rb

@@ -7,6 +7,13 @@ module ActionController
       end
     end
 
+    def self.extended(object)
+      object.class_eval do
+        attr_accessor :original_path, :content_type
+        alias_method :local_path, :path
+      end
+    end
+
     # Take the basename of the upload's original filename.
     # This handles the full Windows paths given by Internet Explorer
     # (and perhaps other broken user agents) without affecting

actionpack/lib/action_controller/url_encoded_pair_parser.rb

@@ -1,5 +1,66 @@
 module ActionController
   class UrlEncodedPairParser < StringScanner #:nodoc:
+    class << self
+      def parse_query_parameters(query_string)
+        return {} if query_string.blank?
+
+        pairs = query_string.split('&').collect do |chunk|
+          next if chunk.empty?
+          key, value = chunk.split('=', 2)
+          next if key.empty?
+          value = value.nil? ? nil : CGI.unescape(value)
+          [ CGI.unescape(key), value ]
+        end.compact
+
+        new(pairs).result
+      end
+
+      def parse_hash_parameters(params)
+        parser = new
+
+        params = params.dup
+        until params.empty?
+          for key, value in params
+            if key.blank?
+              params.delete(key)
+            elsif value.is_a?(Array)
+              parser.parse(key, get_typed_value(value.shift))
+              params.delete(key) if value.empty?
+            else
+              parser.parse(key, get_typed_value(value))
+              params.delete(key)
+            end
+          end
+        end
+
+        parser.result
+      end
+
+      private
+        def get_typed_value(value)
+          case value
+          when String
+            value
+          when NilClass
+            ''
+          when Array
+            value.map { |v| get_typed_value(v) }
+          when Hash
+            if value.has_key?(:tempfile)
+              upload = value[:tempfile]
+              upload.extend(UploadedFile)
+              upload.original_path = value[:filename]
+              upload.content_type = value[:type]
+              upload
+            else
+              nil
+            end
+          else
+            raise "Unknown form value: #{value.inspect}"
+          end
+        end
+    end
+
     attr_reader :top, :parent, :result
 
     def initialize(pairs = [])

actionpack/test/controller/rack_test.rb

@@ -57,7 +57,7 @@ class BaseRackTest < Test::Unit::TestCase
     @request.env['REQUEST_METHOD'] = 'POST'
     @request.env['CONTENT_LENGTH'] = data.length
     @request.env['CONTENT_TYPE'] = 'application/x-www-form-urlencoded; charset=utf-8'
-    @request.env['RAW_POST_DATA'] = data
+    @request.env['rack.input'] = StringIO.new(data)
   end
 end
 

actionpack/test/controller/request/multipart_params_parsing_test.rb

@@ -36,7 +36,7 @@ class MultipartParamsParsingTest < ActionController::IntegrationTest
     assert_equal 'bar', params['foo']
 
     file = params['file']
-    assert_kind_of StringIO, file
+    assert_kind_of Tempfile, file
     assert_equal 'file.txt', file.original_filename
     assert_equal "text/plain", file.content_type
     assert_equal 'contents', file.read
@@ -77,13 +77,13 @@ class MultipartParamsParsingTest < ActionController::IntegrationTest
     assert_equal 'bar', params['foo']
 
     file = params['file']
-    assert_kind_of StringIO, file
+    assert_kind_of Tempfile, file
     assert_equal 'file.csv', file.original_filename
     assert_nil file.content_type
     assert_equal 'contents', file.read
 
     file = params['flowers']
-    assert_kind_of StringIO, file
+    assert_kind_of Tempfile, file
     assert_equal 'flowers.jpg', file.original_filename
     assert_equal "image/jpeg", file.content_type
     assert_equal 19512, file.size