Fix that Hash#to_xml and Array#to_xml shouldn't modify their options hashes [#672 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>

4 files changed, 16 insertions, 0 deletions

diff --git a/activesupport/lib/active_support/core_ext/array/conversions.rb b/activesupport/lib/active_support/core_ext/array/conversions.rb
index 11846f265c..c53cf3f530 100644
--- a/activesupport/lib/active_support/core_ext/array/conversions.rb
+++ b/activesupport/lib/active_support/core_ext/array/conversions.rb
@@ -159,6 +159,7 @@ class Array
     raise "Not all elements respond to to_xml" unless all? { |e| e.respond_to? :to_xml }
     require 'builder' unless defined?(Builder)
 
+    options = options.dup
     options[:root]     ||= all? { |e| e.is_a?(first.class) && first.class.to_s != "Hash" } ? ActiveSupport::Inflector.pluralize(ActiveSupport::Inflector.underscore(first.class.name)) : "records"
     options[:children] ||= options[:root].singularize
     options[:indent]   ||= 2
diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb
index 2a34874d08..bd9419e1a2 100644
--- a/activesupport/lib/active_support/core_ext/hash/conversions.rb
+++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb
@@ -86,6 +86,7 @@ class Hash
   def to_xml(options = {})
     require 'builder' unless defined?(Builder)
 
+    options = options.dup
     options[:indent] ||= 2
     options.reverse_merge!({ :builder => Builder::XmlMarkup.new(:indent => options[:indent]),
                              :root => "hash" })
diff --git a/activesupport/test/core_ext/array_ext_test.rb b/activesupport/test/core_ext/array_ext_test.rb
index 24d33896ce..8198b9bd2c 100644
--- a/activesupport/test/core_ext/array_ext_test.rb
+++ b/activesupport/test/core_ext/array_ext_test.rb
@@ -302,6 +302,13 @@ class ArrayToXmlTests < Test::Unit::TestCase
     xml = [].to_xml
     assert_match(/type="array"\/>/, xml)
   end
+
+  def test_to_xml_dups_options
+    options = {:skip_instruct => true}
+    [].to_xml(options)
+    # :builder, etc, shouldn't be added to options
+    assert_equal({:skip_instruct => true}, options)
+  end
 end
 
 class ArrayExtractOptionsTests < Test::Unit::TestCase
diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb
index ece5466abb..8b0f3fc33c 100644
--- a/activesupport/test/core_ext/hash_ext_test.rb
+++ b/activesupport/test/core_ext/hash_ext_test.rb
@@ -880,6 +880,13 @@ class HashToXmlTest < Test::Unit::TestCase
     assert_equal 30,    alert_at.min
     assert_equal 45,    alert_at.sec
   end
+
+  def test_to_xml_dups_options
+    options = {:skip_instruct => true}
+    {}.to_xml(options)
+    # :builder, etc, shouldn't be added to options
+    assert_equal({:skip_instruct => true}, options)
+  end
 end
 
 class QueryTest < Test::Unit::TestCase