Merge branch 'pp'

* pp:
  remove outdated comment
  all parameter parsing is done through the request object now.
  let the request object handle parsing XML posts
  remove setting request parameters for JSON requests
  remove the request parameter from `parse_formatted_parameters`
  do not instantiate a param parser middleware
  push the parameter parsers on to the class
  stop eagerly parsing parameters
  only wrap the strategy with exception handling
  pull `normalize_encode_params` up
  remove the `default` parameter from the parser method
  move parameter parsing to the request object

actionpack/lib/action_controller/test_case.rb

@@ -33,6 +33,9 @@ module ActionController
 
       self.session = session
       self.session_options = TestSession::DEFAULT_OPTIONS
+      @custom_param_parsers = {
+        Mime::XML => lambda { |raw_post| Hash.from_xml(raw_post)['hash'] }
+      }
     end
 
     def query_string=(string)
@@ -74,26 +77,18 @@ module ActionController
             set_header k, 'application/x-www-form-urlencoded'
           end
 
-          # FIXME: setting `request_parametes` is normally handled by the
-          # params parser middleware, and we should remove this roundtripping
-          # when we switch to caling `call` on the controller
-
           case content_mime_type.to_sym
           when nil
             raise "Unknown Content-Type: #{content_type}"
           when :json
             data = ActiveSupport::JSON.encode(non_path_parameters)
-            params = ActiveSupport::JSON.decode(data).with_indifferent_access
-            self.request_parameters = params
           when :xml
             data = non_path_parameters.to_xml
-            params = Hash.from_xml(data)['hash']
-            self.request_parameters = params
           when :url_encoded_form
             data = non_path_parameters.to_query
           else
+            @custom_param_parsers[content_mime_type] = ->(_) { non_path_parameters }
             data = non_path_parameters.to_query
-            self.request_parameters = non_path_parameters
           end
         end
 
@@ -136,6 +131,12 @@ module ActionController
         "multipart/form-data; boundary=#{Rack::Test::MULTIPART_BOUNDARY}"
       end
     end.new
+
+    private
+
+    def params_parsers
+      super.merge @custom_param_parsers
+    end
   end
 
   class LiveTestResponse < Live::Response

actionpack/lib/action_dispatch/http/parameters.rb

@@ -3,6 +3,20 @@ module ActionDispatch
     module Parameters
       PARAMETERS_KEY = 'action_dispatch.request.path_parameters'
 
+      DEFAULT_PARSERS = {
+        Mime::JSON => lambda { |raw_post|
+          data = ActiveSupport::JSON.decode(raw_post)
+          data.is_a?(Hash) ? data : {:_json => data}
+        }
+      }
+
+      def self.included(klass)
+        class << klass
+          attr_accessor :parameter_parsers
+        end
+
+        klass.parameter_parsers = DEFAULT_PARSERS
+      end
       # Returns both GET and POST \parameters in a single hash.
       def parameters
         params = get_header("action_dispatch.request.parameters")
@@ -31,6 +45,27 @@ module ActionDispatch
       def path_parameters
         get_header(PARAMETERS_KEY) || {}
       end
+
+      private
+
+      def parse_formatted_parameters(parsers)
+        return yield if content_length.zero?
+
+        strategy = parsers.fetch(content_mime_type) { return yield }
+
+        begin
+          strategy.call(raw_post)
+        rescue => e # JSON or Ruby code block errors
+          my_logger = logger || ActiveSupport::Logger.new($stderr)
+          my_logger.debug "Error occurred while parsing request parameters.\nContents:\n\n#{raw_post}"
+
+          raise ParamsParser::ParseError.new(e.message, e)
+        end
+      end
+
+      def params_parsers
+        ActionDispatch::Request.parameter_parsers
+      end
     end
   end
 end

actionpack/lib/action_dispatch/http/request.rb

@@ -348,8 +348,14 @@ module ActionDispatch
     # Override Rack's POST method to support indifferent access
     def POST
       fetch_header("action_dispatch.request.request_parameters") do
-        self.request_parameters = Request::Utils.normalize_encode_params(super || {})
+        pr = parse_formatted_parameters(params_parsers) do |params|
+          super || {}
+        end
+        self.request_parameters = Request::Utils.normalize_encode_params(pr)
       end
+    rescue ParamsParser::ParseError # one of the parse strategies blew up
+      self.request_parameters = Request::Utils.normalize_encode_params(super || {})
+      raise
     rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e
       raise ActionController::BadRequest.new(:request, e)
     end

actionpack/lib/action_dispatch/middleware/params_parser.rb

@@ -18,48 +18,13 @@ module ActionDispatch
       end
     end
 
-    DEFAULT_PARSERS = {
-      Mime::JSON => lambda { |raw_post|
-        data = ActiveSupport::JSON.decode(raw_post)
-        data = {:_json => data} unless data.is_a?(Hash)
-        Request::Utils.normalize_encode_params(data)
-      }
-    }
-
     # Create a new +ParamsParser+ middleware instance.
     #
     # The +parsers+ argument can take Hash of parsers where key is identifying
     # content mime type, and value is a lambda that is going to process data.
-    def initialize(app, parsers = {})
+    def self.new(app, parsers = {})
-      @app, @parsers = app, DEFAULT_PARSERS.merge(parsers)
+      ActionDispatch::Request.parameter_parsers = ActionDispatch::Request::DEFAULT_PARSERS.merge(parsers)
+      app
     end
-
-    def call(env)
-      request = Request.new(env)
-
-      parse_formatted_parameters(request, @parsers) do |params|
-        request.request_parameters = params
-      end
-
-      @app.call(env)
-    end
-
-    private
-      def parse_formatted_parameters(request, parsers)
-        return if request.content_length.zero?
-
-        strategy = parsers.fetch(request.content_mime_type) { return nil }
-
-        yield strategy.call(request.raw_post)
-
-      rescue => e # JSON or Ruby code block errors
-        logger(request).debug "Error occurred while parsing request parameters.\nContents:\n\n#{request.raw_post}"
-
-        raise ParseError.new(e.message, e)
-      end
-
-      def logger(request)
-        request.logger || ActiveSupport::Logger.new($stderr)
-      end
   end
 end

actionpack/test/controller/webservice_test.rb

@@ -97,24 +97,28 @@ class WebServiceTest < ActionDispatch::IntegrationTest
   end
 
   def test_parsing_json_doesnot_rescue_exception
-    with_test_route_set do
+    req = Class.new(ActionDispatch::Request) do
-      with_params_parsers Mime::JSON => Proc.new { |data| raise Interrupt } do
+      def params_parsers
-        assert_raises(Interrupt) do
+        { Mime::JSON => Proc.new { |data| raise Interrupt } }
-          post "/",
-            params: '{"title":"JSON"}}',
-            headers: { 'CONTENT_TYPE' => 'application/json' }
-        end
       end
+
+      def content_length; get_header('rack.input').length; end
+    end.new({ 'rack.input' => StringIO.new('{"title":"JSON"}}'), 'CONTENT_TYPE' => 'application/json' })
+
+    assert_raises(Interrupt) do
+      req.request_parameters
     end
   end
 
   private
     def with_params_parsers(parsers = {})
       old_session = @integration_session
-      @app = ActionDispatch::ParamsParser.new(app.routes, parsers)
+      original_parsers = ActionDispatch::Request.parameter_parsers
+      ActionDispatch::Request.parameter_parsers = original_parsers.merge parsers
       reset!
       yield
     ensure
+      ActionDispatch::Request.parameter_parsers = original_parsers
       @integration_session = old_session
     end