Merge pull request #10784 from senny/10526_prevent_key_transformation

Prevent side effects in `Hash#with_indifferent_access`.

3 files changed, 25 insertions, 6 deletions

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 1982811500..ae3fd811b3 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,9 @@
+*   Prevent side effects to hashes inside arrays when
+    `Hash#with_indifferent_access` is called.
+    Fixes #10526
+
+    *Yves Senn*
+
 *   Raise an error when multiple `included` blocks are defined for a Concern.
     The old behavior would silently discard previously defined blocks, running
     only the last one.
diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb
index bdb8877f55..0a81a8393d 100644
--- a/activesupport/lib/active_support/hash_with_indifferent_access.rb
+++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb
@@ -91,7 +91,7 @@ module ActiveSupport
     #
     # This value can be later fetched using either +:key+ or +'key'+.
     def []=(key, value)
-      regular_writer(convert_key(key), convert_value(value))
+      regular_writer(convert_key(key), convert_value(value, for: :assignment))
     end
 
     alias_method :store, :[]=
@@ -231,7 +231,7 @@ module ActiveSupport
     def to_hash
       _new_hash= {}
       each do |key, value|
-        _new_hash[convert_key(key)] = convert_value(value, true)
+        _new_hash[convert_key(key)] = convert_value(value, for: :to_hash)
       end
       Hash.new(default).merge!(_new_hash)
     end
@@ -241,12 +241,18 @@ module ActiveSupport
         key.kind_of?(Symbol) ? key.to_s : key
       end
 
-      def convert_value(value, _convert_for_to_hash = false)
+      def convert_value(value, options = {})
         if value.is_a? Hash
-          _convert_for_to_hash ? value.to_hash : value.nested_under_indifferent_access
+          if options[:for] == :to_hash
+            value.to_hash
+          else
+            value.nested_under_indifferent_access
+          end
         elsif value.is_a?(Array)
-          value = value.dup if value.frozen?
-          value.map! { |e| convert_value(e, _convert_for_to_hash) }
+          unless options[:for] == :assignment
+            value = value.dup
+          end
+          value.map! { |e| convert_value(e, options) }
         else
           value
         end
diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb
index dfcc6cd12a..39bd0a2dd4 100644
--- a/activesupport/test/core_ext/hash_ext_test.rb
+++ b/activesupport/test/core_ext/hash_ext_test.rb
@@ -503,6 +503,13 @@ class HashExtTest < ActiveSupport::TestCase
     assert_equal [1], hash[:a]
   end
 
+  def test_with_indifferent_access_has_no_side_effects_on_existing_hash
+    hash = {content: [{:foo => :bar, 'bar' => 'baz'}]}
+    hash.with_indifferent_access
+
+    assert_equal [:foo, "bar"], hash[:content].first.keys
+  end
+
   def test_indifferent_hash_with_array_of_hashes
     hash = { "urls" => { "url" => [ { "address" => "1" }, { "address" => "2" } ] }}.with_indifferent_access
     assert_equal "1", hash[:urls][:url].first[:address]