(#20681) fix behaviour of delete_undef_values

The issue #20681 describe the error of delete() function
removing the elements from the origin array/hash/string.

This issue affected other delete functions. Because
ruby delete and delete_if functions make destructive
changes to the origin array/hash.

The delete_undef_values removed elements from the
origin array/hash and this is not the desired behaviour.

To solve this, we should dup or clone the array/hash
before using the delete or delete_if ruby functions.

We should also check if args[0] is not nil before using
dup, since dup on nil raises exception.

This fix the problem and add unit tests, so we could
enforce this behaviour and prevent regressions.

2 files changed, 18 insertions, 6 deletions

diff --git a/lib/puppet/parser/functions/delete_undef_values.rb b/lib/puppet/parser/functions/delete_undef_values.rb
index 4eecab2..532639e 100644
--- a/lib/puppet/parser/functions/delete_undef_values.rb
+++ b/lib/puppet/parser/functions/delete_undef_values.rb
@@ -18,16 +18,16 @@ Would return: ['A','',false]
     raise(Puppet::ParseError,
           "delete_undef_values(): Wrong number of arguments given " +
           "(#{args.size})") if args.size < 1
-
-    result = args[0]
+    
+    unless args[0].is_a? Array or args[0].is_a? Hash 
+      raise(Puppet::ParseError,
+            "delete_undef_values(): expected an array or hash, got #{args[0]} type  #{args[0].class} ")
+    end
+    result = args[0].dup 
     if result.is_a?(Hash)
       result.delete_if {|key, val| val.equal? :undef}
     elsif result.is_a?(Array)
       result.delete :undef
-    else
-      raise(Puppet::ParseError, 
-            "delete_undef_values(): Wrong argument type #{args[0].class} " +
-            "for first argument")
     end
     result
   end
diff --git a/spec/unit/puppet/parser/functions/delete_undef_values_spec.rb b/spec/unit/puppet/parser/functions/delete_undef_values_spec.rb
index 0536641..404aeda 100644
--- a/spec/unit/puppet/parser/functions/delete_undef_values_spec.rb
+++ b/spec/unit/puppet/parser/functions/delete_undef_values_spec.rb
@@ -26,4 +26,16 @@ describe "the delete_undef_values function" do
     result = scope.function_delete_undef_values([{'a'=>'A','b'=>:undef,'c'=>'C','d'=>'undef'}])
     result.should(eq({'a'=>'A','c'=>'C','d'=>'undef'}))
   end
+
+  it "should not change origin array passed as argument" do 
+    origin_array = ['a',:undef,'c','undef']
+    result = scope.function_delete_undef_values([origin_array])
+    origin_array.should(eq(['a',:undef,'c','undef']))
+  end 
+
+  it "should not change origin hash passed as argument" do 
+    origin_hash = { 'a' => 1, 'b' => :undef, 'c' => 'undef' } 
+    result = scope.function_delete_undef_values([origin_hash])
+    origin_hash.should(eq({ 'a' => 1, 'b' => :undef, 'c' => 'undef' }))
+  end 
 end