From 71ab41a0996320acdc844148f6c142bd3237b1d2 Mon Sep 17 00:00:00 2001 From: Marcel Eeken Date: Mon, 12 Apr 2021 12:37:45 +0200 Subject: [PATCH] Deep duplicate in `deep_merge` so no references remain to the original hash in the result When deep merging a Hash, first a duplicate is created. The duplicate only does a shallow copy, so deeper level structures are not duplicated but remain references. This can lead to unexpected modifications of the original hash when a deeply nested hash is merged with another hash, and the newly returned hash gets modified. ``` x = { a: { b: "foo" } } y = { d: { e: "bar" } } z = x.deep_merge(y) # z => { a: { b: "foo" }, d: { e: "bar" } } z[:a][:b] = "baz" # z => { a: { b: "baz" }, d: { e: "bar" } } # x => { a: { b: "baz" } } ``` --- activesupport/CHANGELOG.md | 4 ++++ .../lib/active_support/core_ext/hash/deep_merge.rb | 2 +- activesupport/test/core_ext/hash_ext_test.rb | 10 ++++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 24d7cb7f59..bf7eca86be 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,7 @@ +* Fix issue in `Hash#deep_merge` where it did not properly duplicate a nested `Hash` + + *Marcel Eeken* + * Add `expires_at` argument to `ActiveSupport::Cache` `write` and `fetch` to set a cache entry TTL as an absolute time. ```ruby diff --git a/activesupport/lib/active_support/core_ext/hash/deep_merge.rb b/activesupport/lib/active_support/core_ext/hash/deep_merge.rb index 9bc50b7bc6..020eb78f03 100644 --- a/activesupport/lib/active_support/core_ext/hash/deep_merge.rb +++ b/activesupport/lib/active_support/core_ext/hash/deep_merge.rb @@ -16,7 +16,7 @@ class Hash # h1.deep_merge(h2) { |key, this_val, other_val| this_val + other_val } # # => { a: 100, b: 450, c: { c1: 300 } } def deep_merge(other_hash, &block) - dup.deep_merge!(other_hash, &block) + deep_dup.deep_merge!(other_hash, &block) end # Same as +deep_merge+, but modifies +self+. diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index 1e5c984b9d..ee2ada0b8a 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -300,6 +300,16 @@ class HashExtTest < ActiveSupport::TestCase assert_equal expected, hash_1 end + def test_deep_merge_with_nested_hash_returning_full_new_hash + hash_1 = { a: { b: "foo" } } + hash_2 = { d: "bar" } + + new_hash = hash_1.deep_merge(hash_2) + new_hash[:a][:b] = "baz" + + assert_equal("foo", hash_1[:a][:b]) + end + def test_reverse_merge defaults = { d: 0, a: "x", b: "y", c: 10 }.freeze options = { a: 1, b: 2 }