From b37436b080ff3733736e70d1343d5f328ed840e4 Mon Sep 17 00:00:00 2001 From: Lamont Granquist Date: Wed, 15 Mar 2017 12:05:43 -0700 Subject: Chef-13 freeze merged node attribute Signed-off-by: Lamont Granquist --- RELEASE_NOTES.md | 7 +++++++ lib/chef/node/immutable_collections.rb | 10 ++++++++-- lib/chef/node/mixin/immutablize_array.rb | 2 +- lib/chef/node/mixin/immutablize_hash.rb | 2 +- spec/unit/node/attribute_spec.rb | 12 ++++++++++++ 5 files changed, 29 insertions(+), 4 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 676b658068..0219369b7b 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -82,3 +82,10 @@ n["foo"] << "buzz" before this would have mutated the original string in-place so that `node["foo"]` and `node.default["foo"]` would have changed to "fizzbuzz" while now they remain "fizz" and only the mutable `n["foo"]` copy is changed to "fizzbuzz". +### Freezing immutable merged attributes + +Since Chef 11 merged node attributes have been intended to be immutable but the merged strings have not been frozen. In Chef 13, in the +process of merging the node attributes strings and other simple objects are dup'd and frozen. In order to get a mutable copy, you can +now correctly use the `node.dup` or `node.to_hash` methods, or you should mutate the object correctly through its precedence level like +`node.default["some_string"] << "appending_this"`. + diff --git a/lib/chef/node/immutable_collections.rb b/lib/chef/node/immutable_collections.rb index 13a8aefe97..be9285a755 100644 --- a/lib/chef/node/immutable_collections.rb +++ b/lib/chef/node/immutable_collections.rb @@ -22,8 +22,14 @@ require "chef/node/mixin/immutablize_hash" class Chef class Node - module Immutablize + # For elements like Fixnums, true, nil... + def safe_dup(e) + e.dup + rescue TypeError + e + end + def immutablize(value) case value when Hash @@ -31,7 +37,7 @@ class Chef when Array ImmutableArray.new(value, __root__, __node__, __precedence__) else - value + safe_dup(value).freeze end end end diff --git a/lib/chef/node/mixin/immutablize_array.rb b/lib/chef/node/mixin/immutablize_array.rb index cfa7266b9a..bd330cf8a9 100644 --- a/lib/chef/node/mixin/immutablize_array.rb +++ b/lib/chef/node/mixin/immutablize_array.rb @@ -1,5 +1,5 @@ #-- -# Copyright:: Copyright 2016, Chef Software, Inc. +# Copyright:: Copyright 2016-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/lib/chef/node/mixin/immutablize_hash.rb b/lib/chef/node/mixin/immutablize_hash.rb index f09e6944fc..f6b22ed7d7 100644 --- a/lib/chef/node/mixin/immutablize_hash.rb +++ b/lib/chef/node/mixin/immutablize_hash.rb @@ -1,5 +1,5 @@ #-- -# Copyright:: Copyright 2016, Chef Software, Inc. +# Copyright:: Copyright 2016-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/spec/unit/node/attribute_spec.rb b/spec/unit/node/attribute_spec.rb index 0869b83e43..3dd0d0f650 100644 --- a/spec/unit/node/attribute_spec.rb +++ b/spec/unit/node/attribute_spec.rb @@ -1244,4 +1244,16 @@ describe Chef::Node::Attribute do @attributes.default["foo"]["bar"]["baz"] = "quux" end end + + describe "frozen immutable strings" do + it "strings in hashes should be frozen" do + @attributes.default["foo"]["bar"]["baz"] = "fizz" + expect { @attributes["foo"]["bar"]["baz"] << "buzz" }.to raise_error(RuntimeError, "can't modify frozen String") + end + + it "strings in arrays should be frozen" do + @attributes.default["foo"]["bar"] = [ "fizz" ] + expect { @attributes["foo"]["bar"][0] << "buzz" }.to raise_error(RuntimeError, "can't modify frozen String") + end + end end -- cgit v1.2.1