Merge pull request #9328 from sikachu/ps-remove-xml-parser

Remove XML Parser from ActionDispatch

4 files changed, 26 insertions, 339 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 9b8b175450..b9bff4958f 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,13 +1,18 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Remove support for parsing XML parameters from request. If you still want to parse XML
+    parameters, please install `actionpack-xml_parser' gem.
+
+    *Prem Sichanugrist*
+
 *   Fix `time_zone_options_for_select` to call `dup` on the returned TimeZone array.
-    
+
     Previously if you supplied :priority_zones options to `time_zone_options_for_select`
     the memoized ActiveSupport::TimeZone.all array would be mutated.  Calling
     `dup` prevents mutation of the main TimeZones array.
-    
+
     *Brian McManus*
-    
+
 *   Remove support for parsing YAML parameters from request.
 
     *Aaron Patterson*
diff --git a/actionpack/lib/action_dispatch/middleware/params_parser.rb b/actionpack/lib/action_dispatch/middleware/params_parser.rb
index 0898ad82dd..0fa1e9b859 100644
--- a/actionpack/lib/action_dispatch/middleware/params_parser.rb
+++ b/actionpack/lib/action_dispatch/middleware/params_parser.rb
@@ -13,10 +13,7 @@ module ActionDispatch
       end
     end
 
-    DEFAULT_PARSERS = {
-      Mime::XML => :xml_simple,
-      Mime::JSON => :json
-    }
+    DEFAULT_PARSERS = { Mime::JSON => :json }
 
     def initialize(app, parsers = {})
       @app, @parsers = app, DEFAULT_PARSERS.merge(parsers)
@@ -36,19 +33,13 @@ module ActionDispatch
 
         return false if request.content_length.zero?
 
-        mime_type = content_type_from_legacy_post_data_format_header(env) ||
-          request.content_mime_type
-
-        strategy = @parsers[mime_type]
+        strategy = @parsers[request.content_mime_type]
 
         return false unless strategy
 
         case strategy
         when Proc
           strategy.call(request.raw_post)
-        when :xml_simple, :xml_node
-          data = request.deep_munge(Hash.from_xml(request.body.read) || {})
-          data.with_indifferent_access
         when :json
           data = ActiveSupport::JSON.decode(request.body)
           data = {:_json => data} unless data.is_a?(Hash)
@@ -56,23 +47,12 @@ module ActionDispatch
         else
           false
         end
-      rescue Exception => e # YAML, XML or Ruby code block errors
+      rescue Exception => e # JSON or Ruby code block errors
         logger(env).debug "Error occurred while parsing request parameters.\nContents:\n\n#{request.raw_post}"
 
         raise ParseError.new(e.message, e)
       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' then return Mime::YAML
-          when 'xml'  then return Mime::XML
-          end
-        end
-
-        nil
-      end
-
       def logger(env)
         env['action_dispatch.logger'] || ActiveSupport::Logger.new($stderr)
       end
diff --git a/actionpack/test/controller/webservice_test.rb b/actionpack/test/controller/webservice_test.rb
index 19d5652d81..b2dfd96606 100644
--- a/actionpack/test/controller/webservice_test.rb
+++ b/actionpack/test/controller/webservice_test.rb
@@ -32,169 +32,53 @@ class WebServiceTest < ActionDispatch::IntegrationTest
     end
   end
 
-  def test_post_xml
+  def test_post_json
     with_test_route_set do
-      post "/", '<entry attributed="true"><summary>content...</summary></entry>',
-        {'CONTENT_TYPE' => 'application/xml'}
+      post "/", '{"entry":{"summary":"content..."}}', 'CONTENT_TYPE' => 'application/json'
 
       assert_equal 'entry', @controller.response.body
       assert @controller.params.has_key?(:entry)
       assert_equal 'content...', @controller.params["entry"]['summary']
-      assert_equal 'true', @controller.params["entry"]['attributed']
     end
   end
 
-  def test_put_xml
+  def test_put_json
     with_test_route_set do
-      put "/", '<entry attributed="true"><summary>content...</summary></entry>',
-        {'CONTENT_TYPE' => 'application/xml'}
+      put "/", '{"entry":{"summary":"content..."}}', 'CONTENT_TYPE' => 'application/json'
 
       assert_equal 'entry', @controller.response.body
       assert @controller.params.has_key?(:entry)
       assert_equal 'content...', @controller.params["entry"]['summary']
-      assert_equal 'true', @controller.params["entry"]['attributed']
     end
   end
 
-  def test_put_xml_using_a_type_node
+  def test_register_and_use_json_simple
     with_test_route_set do
-      put "/", '<type attributed="true"><summary>content...</summary></type>',
-        {'CONTENT_TYPE' => 'application/xml'}
-
-      assert_equal 'type', @controller.response.body
-      assert @controller.params.has_key?(:type)
-      assert_equal 'content...', @controller.params["type"]['summary']
-      assert_equal 'true', @controller.params["type"]['attributed']
-    end
-  end
-
-  def test_put_xml_using_a_type_node_and_attribute
-    with_test_route_set do
-      put "/", '<type attributed="true"><summary type="boolean">false</summary></type>',
-        {'CONTENT_TYPE' => 'application/xml'}
-
-      assert_equal 'type', @controller.response.body
-      assert @controller.params.has_key?(:type)
-      assert_equal false, @controller.params["type"]['summary']
-      assert_equal 'true', @controller.params["type"]['attributed']
-    end
-  end
-
-  def test_post_xml_using_a_type_node
-    with_test_route_set do
-      post "/", '<font attributed="true"><type>arial</type></font>',
-        {'CONTENT_TYPE' => 'application/xml'}
-
-      assert_equal 'font', @controller.response.body
-      assert @controller.params.has_key?(:font)
-      assert_equal 'arial', @controller.params['font']['type']
-      assert_equal 'true', @controller.params["font"]['attributed']
-    end
-  end
-
-  def test_post_xml_using_a_root_node_named_type
-    with_test_route_set do
-      post "/", '<type type="integer">33</type>',
-        {'CONTENT_TYPE' => 'application/xml'}
-
-      assert @controller.params.has_key?(:type)
-      assert_equal 33, @controller.params['type']
-    end
-  end
-
-  def test_post_xml_using_an_attributted_node_named_type
-    with_test_route_set do
-      with_params_parsers Mime::XML => Proc.new { |data| Hash.from_xml(data)['request'].with_indifferent_access } do
-        post "/", '<request><type type="string">Arial,12</type><z>3</z></request>',
-          {'CONTENT_TYPE' => 'application/xml'}
-
-        assert_equal 'type, z', @controller.response.body
-        assert @controller.params.has_key?(:type)
-        assert_equal 'Arial,12', @controller.params['type'], @controller.params.inspect
-        assert_equal '3', @controller.params['z'], @controller.params.inspect
-      end
-    end
-  end
-
-  def test_post_xml_using_a_disallowed_type_attribute
-    $stderr = StringIO.new
-    with_test_route_set do
-      post '/', '<foo type="symbol">value</foo>', 'CONTENT_TYPE' => 'application/xml'
-      assert_response 500
-
-      post '/', '<foo type="yaml">value</foo>', 'CONTENT_TYPE' => 'application/xml'
-      assert_response 500
-    end
-  ensure
-    $stderr = STDERR
-  end
-
-  def test_register_and_use_xml_simple
-    with_test_route_set do
-      with_params_parsers Mime::XML => Proc.new { |data| Hash.from_xml(data)['request'].with_indifferent_access } do
-        post "/", '<request><summary>content...</summary><title>SimpleXml</title></request>',
-          {'CONTENT_TYPE' => 'application/xml'}
+      with_params_parsers Mime::JSON => Proc.new { |data| JSON.parse(data)['request'].with_indifferent_access } do
+        post "/", '{"request":{"summary":"content...","title":"JSON"}}',
+          'CONTENT_TYPE' => 'application/json'
 
         assert_equal 'summary, title', @controller.response.body
         assert @controller.params.has_key?(:summary)
         assert @controller.params.has_key?(:title)
         assert_equal 'content...', @controller.params["summary"]
-        assert_equal 'SimpleXml', @controller.params["title"]
+        assert_equal 'JSON', @controller.params["title"]
       end
     end
   end
 
-  def test_use_xml_ximple_with_empty_request
+  def test_use_json_with_empty_request
     with_test_route_set do
-      assert_nothing_raised { post "/", "", {'CONTENT_TYPE' => 'application/xml'} }
+      assert_nothing_raised { post "/", "", 'CONTENT_TYPE' => 'application/json' }
       assert_equal '', @controller.response.body
     end
   end
 
-  def test_dasherized_keys_as_xml
-    with_test_route_set do
-      post "/?full=1", "<first-key>\n<sub-key>...</sub-key>\n</first-key>",
-        {'CONTENT_TYPE' => 'application/xml'}
-      assert_equal 'action, controller, first_key(sub_key), full', @controller.response.body
-      assert_equal "...", @controller.params[:first_key][:sub_key]
-    end
-  end
-
-  def test_typecast_as_xml
-    with_test_route_set do
-      xml = <<-XML
-        <data>
-          <a type="integer">15</a>
-          <b type="boolean">false</b>
-          <c type="boolean">true</c>
-          <d type="date">2005-03-17</d>
-          <e type="datetime">2005-03-17T21:41:07Z</e>
-          <f>unparsed</f>
-          <g type="integer">1</g>
-          <g>hello</g>
-          <g type="date">1974-07-25</g>
-        </data>
-      XML
-      post "/", xml, {'CONTENT_TYPE' => 'application/xml'}
-
-      params = @controller.params
-      assert_equal 15, params[:data][:a]
-      assert_equal false, params[:data][:b]
-      assert_equal true, params[:data][:c]
-      assert_equal Date.new(2005,3,17), params[:data][:d]
-      assert_equal Time.utc(2005,3,17,21,41,7), params[:data][:e]
-      assert_equal "unparsed", params[:data][:f]
-      assert_equal [1, "hello", Date.new(1974,7,25)], params[:data][:g]
-    end
-  end
-
-  def test_entities_unescaped_as_xml_simple
+  def test_dasherized_keys_as_json
     with_test_route_set do
-      xml = <<-XML
-        <data>&lt;foo &quot;bar&apos;s&quot; &amp; friends&gt;</data>
-      XML
-      post "/", xml, {'CONTENT_TYPE' => 'application/xml'}
-      assert_equal %(<foo "bar's" & friends>), @controller.params[:data]
+      post "/?full=1", '{"first-key":{"sub-key":"..."}}', 'CONTENT_TYPE' => 'application/json'
+      assert_equal 'action, controller, first-key(sub-key), full', @controller.response.body
+      assert_equal "...", @controller.params['first-key']['sub-key']
     end
   end
 
diff --git a/actionpack/test/dispatch/request/xml_params_parsing_test.rb b/actionpack/test/dispatch/request/xml_params_parsing_test.rb
deleted file mode 100644
index f13b64a3c7..0000000000
--- a/actionpack/test/dispatch/request/xml_params_parsing_test.rb
+++ /dev/null
@@ -1,182 +0,0 @@
-require 'abstract_unit'
-
-class XmlParamsParsingTest < ActionDispatch::IntegrationTest
-  class TestController < ActionController::Base
-    class << self
-      attr_accessor :last_request_parameters
-    end
-
-    def parse
-      self.class.last_request_parameters = request.request_parameters
-      head :ok
-    end
-  end
-
-  def teardown
-    TestController.last_request_parameters = nil
-  end
-
-  test "parses a strict rack.input" do
-    class Linted
-      undef call if method_defined?(:call)
-      def call(env)
-        bar = env['action_dispatch.request.request_parameters']['foo']
-        result = "<ok>#{bar}</ok>"
-        [200, {"Content-Type" => "application/xml", "Content-Length" => result.length.to_s}, [result]]
-      end
-    end
-    req = Rack::MockRequest.new(ActionDispatch::ParamsParser.new(Linted.new))
-    resp = req.post('/', "CONTENT_TYPE" => "application/xml", :input => "<foo>bar</foo>", :lint => true)
-    assert_equal "<ok>bar</ok>", resp.body
-  end
-
-  def assert_parses(expected, xml)
-    with_test_routing do
-      post "/parse", xml, default_headers
-      assert_response :ok
-      assert_equal(expected, TestController.last_request_parameters)
-    end
-  end
-
-  test "nils are stripped from collections" do
-    assert_parses(
-      {"hash" => { "person" => nil} },
-      "<hash><person type=\"array\"><person nil=\"true\"/></person></hash>")
-    assert_parses(
-      {"hash" => { "person" => ['foo']} },
-      "<hash><person type=\"array\"><person>foo</person><person nil=\"true\"/></person>\n</hash>")
-  end
-
-  test "parses hash params" do
-    with_test_routing do
-      xml = "<person><name>David</name></person>"
-      post "/parse", xml, default_headers
-      assert_response :ok
-      assert_equal({"person" => {"name" => "David"}}, TestController.last_request_parameters)
-    end
-  end
-
-  test "parses single file" do
-    with_test_routing do
-      xml = "<person><name>David</name><avatar type='file' name='me.jpg' content_type='image/jpg'>#{::Base64.encode64('ABC')}</avatar></person>"
-      post "/parse", xml, default_headers
-      assert_response :ok
-
-      person = TestController.last_request_parameters
-      assert_equal "image/jpg", person['person']['avatar'].content_type
-      assert_equal "me.jpg", person['person']['avatar'].original_filename
-      assert_equal "ABC", person['person']['avatar'].read
-    end
-  end
-
-  test "logs error if parsing unsuccessful" do
-    with_test_routing do
-      output = StringIO.new
-      xml = "<person><name>David</name><avatar type='file' name='me.jpg' content_type='image/jpg'>#{::Base64.encode64('ABC')}</avatar></pineapple>"
-      post "/parse", xml, default_headers.merge('action_dispatch.show_exceptions' => true, 'action_dispatch.logger' => ActiveSupport::Logger.new(output))
-      assert_response :error
-      output.rewind && err = output.read
-      assert err =~ /Error occurred while parsing request parameters/
-    end
-  end
-
-  test "occurring a parse error if parsing unsuccessful" do
-    with_test_routing do
-      begin
-        $stderr = StringIO.new # suppress the log
-        xml = "<person><name>David</name></pineapple>"
-        exception = assert_raise(ActionDispatch::ParamsParser::ParseError) { post "/parse", xml, default_headers.merge('action_dispatch.show_exceptions' => false) }
-        assert_equal REXML::ParseException, exception.original_exception.class
-        assert_equal exception.original_exception.message, exception.message
-      ensure
-        $stderr = STDERR
-      end
-    end
-  end
-
-  test "parses multiple files" do
-    xml = <<-end_body
-      <person>
-        <name>David</name>
-        <avatars>
-          <avatar type='file' name='me.jpg' content_type='image/jpg'>#{::Base64.encode64('ABC')}</avatar>
-          <avatar type='file' name='you.gif' content_type='image/gif'>#{::Base64.encode64('DEF')}</avatar>
-        </avatars>
-      </person>
-    end_body
-
-    with_test_routing do
-      post "/parse", xml, default_headers
-      assert_response :ok
-    end
-
-    person = TestController.last_request_parameters
-
-    assert_equal "image/jpg", person['person']['avatars']['avatar'].first.content_type
-    assert_equal "me.jpg", person['person']['avatars']['avatar'].first.original_filename
-    assert_equal "ABC", person['person']['avatars']['avatar'].first.read
-
-    assert_equal "image/gif", person['person']['avatars']['avatar'].last.content_type
-    assert_equal "you.gif", person['person']['avatars']['avatar'].last.original_filename
-    assert_equal "DEF", person['person']['avatars']['avatar'].last.read
-  end
-
-  private
-    def with_test_routing
-      with_routing do |set|
-        set.draw do
-          post ':action', :to => ::XmlParamsParsingTest::TestController
-        end
-        yield
-      end
-    end
-
-    def default_headers
-      {'CONTENT_TYPE' => 'application/xml'}
-    end
-end
-
-class LegacyXmlParamsParsingTest < XmlParamsParsingTest
-  private
-    def default_headers
-      {'HTTP_X_POST_DATA_FORMAT' => 'xml'}
-    end
-end
-
-class RootLessXmlParamsParsingTest < ActionDispatch::IntegrationTest
-  class TestController < ActionController::Base
-    wrap_parameters :person, :format => :xml
-
-    class << self
-      attr_accessor :last_request_parameters
-    end
-
-    def parse
-      self.class.last_request_parameters = request.request_parameters
-      head :ok
-    end
-  end
-
-  def teardown
-    TestController.last_request_parameters = nil
-  end
-
-  test "parses hash params" do
-    with_test_routing do
-      xml = "<name>David</name>"
-      post "/parse", xml, {'CONTENT_TYPE' => 'application/xml'}
-      assert_response :ok
-      assert_equal({"name" => "David", "person" => {"name" => "David"}}, TestController.last_request_parameters)
-    end
-  end
-
-  private
-    def with_test_routing
-      with_routing do |set|
-        set.draw do
-          post ':action', :to => ::RootLessXmlParamsParsingTest::TestController
-        end
-        yield
-      end
-    end
-end