Fix nil deep_merginglcg/fix-nil-deep-merge

While this behavior was fixed for attributes a long, long time ago it
turns out we don't use this deep_merge mixin for constructing the
deep merge of attribute precedence levels any more.

This bug however surfaces because we still use deep_merge to merge
json attributes into normal attributes.  That means that it is not
possible to pass a nil / null value in through -j and have it override
a normal attribute which is pulled back in from the node object on
the chef-server.

So this fixes that behavior to be consistent with the behavior of
attributes that we've had for several years now.

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

2 files changed, 15 insertions, 12 deletions

diff --git a/lib/chef/mixin/deep_merge.rb b/lib/chef/mixin/deep_merge.rb
index 300ae1a31f..ad9c9e89dd 100644
--- a/lib/chef/mixin/deep_merge.rb
+++ b/lib/chef/mixin/deep_merge.rb
@@ -75,14 +75,7 @@ class Chef
       # @api private
       #
       def deep_merge!(source, dest)
-        # if dest doesn't exist, then simply copy source to it
-        if dest.nil?
-          dest = source; return dest
-        end
-
         case source
-        when nil
-          dest
         when Hash
           if dest.is_a?(Hash)
             source.each do |src_key, src_value|
@@ -147,11 +140,6 @@ class Chef
             end
           end
           merge_onto
-
-        # If merge_with is nil, don't replace merge_onto
-        elsif merge_with.nil?
-          merge_onto
-
         # In all other cases, replace merge_onto with merge_with
         else
           merge_with
diff --git a/spec/unit/mixin/deep_merge_spec.rb b/spec/unit/mixin/deep_merge_spec.rb
index 2122008616..c6681b3d16 100644
--- a/spec/unit/mixin/deep_merge_spec.rb
+++ b/spec/unit/mixin/deep_merge_spec.rb
@@ -236,6 +236,14 @@ describe Chef::Mixin::DeepMerge, "deep_merge!" do
     @dm.deep_merge!(hash_src, hash_dst)
     expect(hash_dst).to eq({ "item" => "orange" })
   end
+
+  it "should overwrite a string with a nil when merging nil values" do
+    hash_src = { "item" => nil }
+    hash_dst = { "item" => "orange" }
+    @dm.deep_merge!(hash_src, hash_dst)
+    expect(hash_dst).to eq({ "item" => nil })
+  end
+
 end # deep_merge!
 
 # Chef specific
@@ -338,5 +346,12 @@ describe Chef::Mixin::DeepMerge do
       merge_with_hash = { "top_level_a" => 2, "top_level_b" => true }
       @dm.hash_only_merge(merge_ee_hash, merge_with_hash)
     end
+
+    it "should overwrite a string with a nil when merging nil values" do
+      hash_src = { "item" => nil }
+      hash_dst = { "item" => "orange" }
+      merged_result = @dm.hash_only_merge(hash_dst, hash_src)
+      expect(merged_result).to eq({ "item" => nil })
+    end
   end
 end