Merge pull request #3875 from chef/mcquin/monkeypatch-win32-registry-delete

Monkeypatch Win32::Registry methods delete_key, delete_value

5 files changed, 77 insertions, 40 deletions

diff --git a/lib/chef/monkey_patches/win32/registry.rb b/lib/chef/monkey_patches/win32/registry.rb
new file mode 100644
index 0000000000..5ebe67c45d
--- /dev/null
+++ b/lib/chef/monkey_patches/win32/registry.rb
@@ -0,0 +1,47 @@
+#
+# Copyright:: Copyright 2015 Chef Software, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'chef/win32/api/registry'
+require 'chef/win32/unicode'
+require 'win32/registry'
+
+module Win32
+  class Registry
+    module API
+      
+      extend Chef::ReservedNames::Win32::API::Registry
+
+      module_function
+
+      if RUBY_VERSION =~ /^2\.1/
+        # ::Win32::Registry#delete_value is broken in Ruby 2.1 (up to Ruby 2.1.6).
+        # This should be resolved in a later release (see note #9 in link below).
+        # https://bugs.ruby-lang.org/issues/10820
+        def DeleteValue(hkey, name)
+          check RegDeleteValueW(hkey, name.to_wstring)
+        end
+      end
+
+      # ::Win32::Registry#delete_key uses RegDeleteKeyW. We need to use
+      # RegDeleteKeyExW to properly support WOW64 systems.
+      def DeleteKey(hkey, name)
+        check RegDeleteKeyExW(hkey, name.to_wstring, 0, 0)
+      end
+      
+    end
+  end
+end
\ No newline at end of file
diff --git a/lib/chef/win32/api/registry.rb b/lib/chef/win32/api/registry.rb
index 45b91d7d32..cbbf6b66bb 100644
--- a/lib/chef/win32/api/registry.rb
+++ b/lib/chef/win32/api/registry.rb
@@ -39,6 +39,12 @@ class Chef
         safe_attach_function :RegDeleteKeyExW, [ :HKEY, :LPCTSTR, :LONG, :DWORD ], :LONG
         safe_attach_function :RegDeleteKeyExA, [ :HKEY, :LPCTSTR, :LONG, :DWORD ], :LONG
 
+        # LONG WINAPI RegDeleteValue(
+        #   _In_     HKEY    hKey,
+        #   _In_opt_ LPCTSTR lpValueName
+        # );
+        safe_attach_function :RegDeleteValueW, [ :HKEY, :LPCTSTR ], :LONG
+
       end
     end
   end
diff --git a/lib/chef/win32/registry.rb b/lib/chef/win32/registry.rb
index b25ce7937e..12ad08a965 100644
--- a/lib/chef/win32/registry.rb
+++ b/lib/chef/win32/registry.rb
@@ -21,6 +21,7 @@ require 'chef/win32/api'
 require 'chef/mixin/wide_string'
 
 if RUBY_PLATFORM =~ /mswin|mingw32|windows/
+  require 'chef/monkey_patches/win32/registry'
   require 'chef/win32/api/registry'
   require 'win32/registry'
   require 'win32/api'
@@ -126,37 +127,23 @@ class Chef
           Chef::Log.debug("Registry key #{key_path}, does not exist, not deleting")
           return true
         end
-        #key_path is in the form "HKLM\Software\Opscode" for example, extracting
-        #hive = HKLM,
-        #hive_namespace = ::Win32::Registry::HKEY_LOCAL_MACHINE
-        hive = key_path.split("\\").shift
-        hive_namespace, key_including_parent = get_hive_and_key(key_path)
-        if has_subkeys?(key_path)
-          if recursive == true
-            subkeys = get_subkeys(key_path)
-            subkeys.each do |key|
-              keypath_to_check = hive+"\\"+key_including_parent+"\\"+key
-              Chef::Log.debug("Deleting registry key #{key_path} recursively")
-              delete_key(keypath_to_check, true)
-            end
-            delete_key_ex(hive_namespace, key_including_parent)
-          else
-            raise Chef::Exceptions::Win32RegNoRecursive, "Registry key #{key_path} has subkeys, and recursive not specified"
-          end
-        else
-          delete_key_ex(hive_namespace, key_including_parent)
-          return true
+        if has_subkeys?(key_path) && !recursive
+          raise Chef::Exceptions::Win32RegNoRecursive, "Registry key #{key_path} has subkeys, and recursive not specified"
+        end
+        hive, key_including_parent = get_hive_and_key(key_path)
+        # key_including_parent: Software\\Root\\Branch\\Fruit
+        # key => Fruit
+        # key_parent => Software\\Root\\Branch
+        key_parts = key_including_parent.split("\\")
+        key = key_parts.pop
+        key_parent = key_parts.join("\\")
+        hive.open(key_parent, ::Win32::Registry::KEY_WRITE | registry_system_architecture) do |reg|
+          reg.delete_key(key, recursive)
         end
+        Chef::Log.debug("Registry key #{key_path} deleted")
         true
       end
 
-      #Using the 'RegDeleteKeyEx' Windows API that correctly supports WOW64 systems (Win2003)
-      #instead of the 'RegDeleteKey'
-      def delete_key_ex(hive, key)
-        hive_num = hive.hkey - (1 << 32)
-        RegDeleteKeyExW(hive_num, wstring(key), ::Win32::Registry::KEY_WRITE | registry_system_architecture, 0) == 0
-      end
-
       def key_exists?(key_path)
         hive, key = get_hive_and_key(key_path)
         begin
diff --git a/spec/functional/win32/registry_helper_spec.rb b/spec/functional/win32/registry_helper_spec.rb
index 9ef6fd006f..ba0ae12495 100644
--- a/spec/functional/win32/registry_helper_spec.rb
+++ b/spec/functional/win32/registry_helper_spec.rb
@@ -556,11 +556,11 @@ describe 'Chef::Win32::Registry', :windows_only do
       end
 
       after(:all) do
-        ::Win32::Registry::HKEY_LOCAL_MACHINE.open("Software\\Root", ::Win32::Registry::KEY_ALL_ACCESS | 0x0100) do |reg|
-          reg.delete_key("Trunk", true)
+        ::Win32::Registry::HKEY_LOCAL_MACHINE.open("Software", ::Win32::Registry::KEY_ALL_ACCESS | 0x0100) do |reg|
+          reg.delete_key("Root", true)
         end
-        ::Win32::Registry::HKEY_LOCAL_MACHINE.open("Software\\Root", ::Win32::Registry::KEY_ALL_ACCESS | 0x0200) do |reg|
-          reg.delete_key("Trunk", true)
+        ::Win32::Registry::HKEY_LOCAL_MACHINE.open("Software", ::Win32::Registry::KEY_ALL_ACCESS | 0x0200) do |reg|
+          reg.delete_key("Root", true)
         end
       end
 
diff --git a/spec/unit/registry_helper_spec.rb b/spec/unit/registry_helper_spec.rb
index b2d0b7b125..a30608add7 100644
--- a/spec/unit/registry_helper_spec.rb
+++ b/spec/unit/registry_helper_spec.rb
@@ -176,28 +176,25 @@ describe Chef::Provider::RegistryKey do
   describe "delete_key", :windows_only do
     it "deletes key if it has subkeys and recursive is set to true" do
       expect(@registry).to receive(:key_exists?).with(key_path).and_return(true)
-      expect(@registry).to receive(:get_hive_and_key).with(key_path).and_return([@hive_mock, key])
       expect(@registry).to receive(:has_subkeys?).with(key_path).and_return(true)
-      expect(@registry).to receive(:get_subkeys).with(key_path).and_return([sub_key])
-      expect(@registry).to receive(:key_exists?).with(key_path+"\\"+sub_key).and_return(true)
-      expect(@registry).to receive(:get_hive_and_key).with(key_path+"\\"+sub_key).and_return([@hive_mock, key+"\\"+sub_key])
-      expect(@registry).to receive(:has_subkeys?).with(key_path+"\\"+sub_key).and_return(false)
-      expect(@registry).to receive(:delete_key_ex).twice
+      expect(@registry).to receive(:get_hive_and_key).with(key_path).and_return([@hive_mock, key])
+      expect(@hive_mock).to receive(:open).with(key_parent, ::Win32::Registry::KEY_WRITE | @registry.registry_system_architecture).and_yield(@reg_mock)
+      expect(@reg_mock).to receive(:delete_key).with(key_to_delete, true).and_return(true)
       @registry.delete_key(key_path, true)
     end
 
     it "raises an exception if it has subkeys but recursive is set to false" do
       expect(@registry).to receive(:key_exists?).with(key_path).and_return(true)
-      expect(@registry).to receive(:get_hive_and_key).with(key_path).and_return([@hive_mock, key])
       expect(@registry).to receive(:has_subkeys?).with(key_path).and_return(true)
       expect{@registry.delete_key(key_path, false)}.to raise_error(Chef::Exceptions::Win32RegNoRecursive)
     end
 
     it "deletes key if the key exists and has no subkeys" do
       expect(@registry).to receive(:key_exists?).with(key_path).and_return(true)
-      expect(@registry).to receive(:get_hive_and_key).with(key_path).and_return([@hive_mock, key])
       expect(@registry).to receive(:has_subkeys?).with(key_path).and_return(false)
-      expect(@registry).to receive(:delete_key_ex)
+      expect(@registry).to receive(:get_hive_and_key).with(key_path).and_return([@hive_mock, key])
+      expect(@hive_mock).to receive(:open).with(key_parent, ::Win32::Registry::KEY_WRITE | @registry.registry_system_architecture).and_yield(@reg_mock)
+      expect(@reg_mock).to receive(:delete_key).with(key_to_delete, true).and_return(true)
       @registry.delete_key(key_path, true)
     end
   end