Prevent duplicate keys in cookbook version JSON

Reported in CHEF-4571. The recipes component of cookbook version
metadata (cbv.metadata.recipes) serialized to JSON with duplicate keys
if a metadata.rb was present that provided a recipe line matching the
cookbook name (providing a description for the default recipe).

The root cause is the behavior of the JSON serialized when called on a
hash containing matching string and symbol keys. For example:

    chef > {"key1" => "sam", :key1 => "wat"}.to_json
     => "{\"key1\":\"sam\",\"key1\":\"wat\"}"

So the specific fix is to prevent inserting a symbol key for the
recipe name.

4 files changed, 28 insertions, 4 deletions

diff --git a/lib/chef/cookbook/metadata.rb b/lib/chef/cookbook/metadata.rb
index 18368bd99f..b9b32c8224 100644
--- a/lib/chef/cookbook/metadata.rb
+++ b/lib/chef/cookbook/metadata.rb
@@ -110,8 +110,8 @@ class Chef
         @version = Version.new "0.0.0"
         if cookbook
           @recipes = cookbook.fully_qualified_recipe_names.inject({}) do |r, e|
-            e = self.name if e =~ /::default$/
-            r[e] = ""
+            e = self.name.to_s if e =~ /::default$/
+            r[e] ||= ""
             self.provides e
             r
           end
diff --git a/spec/data/cookbooks/openldap/metadata.rb b/spec/data/cookbooks/openldap/metadata.rb
new file mode 100644
index 0000000000..ab0dface9d
--- /dev/null
+++ b/spec/data/cookbooks/openldap/metadata.rb
@@ -0,0 +1,8 @@
+name              "openldap"
+maintainer        "Opscode, Inc."
+maintainer_email  "cookbooks@opscode.com"
+license           "Apache 2.0"
+description       "Installs and configures all aspects of openldap using Debian style symlinks with helper definitions"
+long_description  "The long description for the openldap cookbook from metadata.rb"
+version           "8.9.10"
+recipe            "openldap", "Main Open LDAP configuration"
diff --git a/spec/unit/cookbook/syntax_check_spec.rb b/spec/unit/cookbook/syntax_check_spec.rb
index 4b0f77c5d5..85d6950a45 100644
--- a/spec/unit/cookbook/syntax_check_spec.rb
+++ b/spec/unit/cookbook/syntax_check_spec.rb
@@ -32,7 +32,7 @@ describe Chef::Cookbook::SyntaxCheck do
     @attr_files = %w{default.rb smokey.rb}.map { |f| File.join(cookbook_path, 'attributes', f) }
     @defn_files = %w{client.rb server.rb}.map { |f| File.join(cookbook_path, 'definitions', f)}
     @recipes = %w{default.rb gigantor.rb one.rb}.map { |f| File.join(cookbook_path, 'recipes', f) }
-    @ruby_files = @attr_files + @defn_files + @recipes
+    @ruby_files = @attr_files + @defn_files + @recipes + [File.join(cookbook_path, "metadata.rb")]
     basenames = %w{ helpers_via_partial_test.erb
                     helper_test.erb
                     openldap_stuff.conf.erb
diff --git a/spec/unit/cookbook_loader_spec.rb b/spec/unit/cookbook_loader_spec.rb
index 59079a82de..4c21c124e0 100644
--- a/spec/unit/cookbook_loader_spec.rb
+++ b/spec/unit/cookbook_loader_spec.rb
@@ -141,7 +141,7 @@ describe Chef::CookbookLoader do
       end
 
       it "should load the metadata for the cookbook" do
-        @cookbook_loader.metadata[:openldap].name.should == :openldap
+        @cookbook_loader.metadata[:openldap].name.to_s.should == "openldap"
         @cookbook_loader.metadata[:openldap].should be_a_kind_of(Chef::Cookbook::Metadata)
       end
 
@@ -173,6 +173,22 @@ describe Chef::CookbookLoader do
       seen.should have_key("openldap")
     end
 
+    it "should not duplicate keys when serialized to JSON" do
+      # Chef JSON serialization will generate duplicate keys if given
+      # a Hash containing matching string and symbol keys. See CHEF-4571.
+      aa = @cookbook_loader["openldap"]
+      aa.to_hash["metadata"].recipes.keys.should_not include(:openldap)
+      aa.to_hash["metadata"].recipes.keys.should include("openldap")
+      expected_desc = "Main Open LDAP configuration"
+      aa.to_hash["metadata"].recipes["openldap"].should == expected_desc
+      raw = aa.to_hash["metadata"].recipes.to_json
+      search_str = "\"openldap\":\""
+      key_idx = raw.index(search_str)
+      key_idx.should be > 0
+      dup_idx = raw[(key_idx + 1)..-1].index(search_str)
+      dup_idx.should be_nil
+    end
+
     it "should not load the cookbook again when accessed" do
       @cookbook_loader.should_not_receive('load_cookbook')
       @cookbook_loader["openldap"]