Merge pull request #3471 from chef/jdm/3066

Fix copying ntfs dacl and sacl when they are nil

2 files changed, 76 insertions, 6 deletions

diff --git a/lib/chef/file_content_management/deploy/mv_windows.rb b/lib/chef/file_content_management/deploy/mv_windows.rb
index 7504123012..0d16da9717 100644
--- a/lib/chef/file_content_management/deploy/mv_windows.rb
+++ b/lib/chef/file_content_management/deploy/mv_windows.rb
@@ -63,12 +63,22 @@ class Chef
             raise Chef::Exceptions::WindowsNotAdmin, "can not get the security information for '#{dst}' due to missing Administrator privileges."
           end
 
-          if dst_sd.dacl_present?
-            apply_dacl = ACL.create(dst_sd.dacl.select { |ace| !ace.inherited? })
+          dacl_present = dst_sd.dacl_present?
+          if dacl_present
+            if dst_sd.dacl.nil?
+              apply_dacl = nil
+            else
+              apply_dacl = ACL.create(dst_sd.dacl.select { |ace| !ace.inherited? })
+            end
           end
 
-          if dst_sd.sacl_present?
-            apply_sacl = ACL.create(dst_sd.sacl.select { |ace| !ace.inherited? })
+          sacl_present = dst_sd.sacl_present?
+          if sacl_present
+            if dst_sd.sacl.nil?
+              apply_sacl = nil
+            else
+              apply_sacl = ACL.create(dst_sd.sacl.select { |ace| !ace.inherited? })
+            end
           end
 
           #
@@ -84,8 +94,8 @@ class Chef
           dst_so = Security::SecurableObject.new(dst)
           dst_so.group = dst_sd.group
           dst_so.owner = dst_sd.owner
-          dst_so.set_dacl(apply_dacl, dst_sd.dacl_inherits?) if dst_sd.dacl_present?
-          dst_so.set_sacl(apply_sacl, dst_sd.sacl_inherits?) if dst_sd.sacl_present?
+          dst_so.set_dacl(apply_dacl, dst_sd.dacl_inherits?) if dacl_present
+          dst_so.set_sacl(apply_sacl, dst_sd.sacl_inherits?) if sacl_present
 
         end
       end
diff --git a/spec/unit/file_content_management/deploy/mv_windows_spec.rb b/spec/unit/file_content_management/deploy/mv_windows_spec.rb
index c52001cd26..2d1981befc 100644
--- a/spec/unit/file_content_management/deploy/mv_windows_spec.rb
+++ b/spec/unit/file_content_management/deploy/mv_windows_spec.rb
@@ -115,6 +115,66 @@ describe Chef::FileContentManagement::Deploy::MvWindows do
 
       end
 
+      context "and the target file has null dacl and sacl" do
+
+        before do
+          allow(target_file_security_descriptor).to receive(:dacl_present?).and_return(true)
+          allow(target_file_security_descriptor).to receive(:dacl).and_return(nil)
+          allow(target_file_security_descriptor).to receive(:dacl_inherits?).and_return(false)
+
+          allow(target_file_security_descriptor).to receive(:sacl_present?).and_return(true)
+          allow(target_file_security_descriptor).to receive(:sacl).and_return(nil)
+          allow(target_file_security_descriptor).to receive(:sacl_inherits?).and_return(false)
+
+          expect(updated_target_security_object).to receive(:set_dacl).with(nil, false)
+          expect(updated_target_security_object).to receive(:set_sacl).with(nil, false)
+        end
+
+
+        it "fixes up permissions and moves the file into place" do
+          content_deployer.deploy(staging_file_path, target_file_path)
+        end
+
+      end
+
+      context "and the target has an empty dacl and sacl" do
+        let(:original_target_file_dacl) { [] }
+        let(:original_target_file_sacl) { [] }
+
+        let(:empty_dacl) { double("Windows ACL with no dacl ACEs") }
+        let(:empty_sacl) { double("Windows ACL with no sacl ACEs") }
+
+        before do
+          allow(target_file_security_descriptor).to receive(:dacl_present?).and_return(true)
+          allow(target_file_security_descriptor).to receive(:dacl_inherits?).and_return(false)
+
+          allow(target_file_security_descriptor).to receive(:dacl).and_return(original_target_file_dacl)
+          expect(Chef::ReservedNames::Win32::Security::ACL).
+            to receive(:create).
+            with([]).
+            and_return(empty_dacl)
+
+
+          allow(target_file_security_descriptor).to receive(:sacl_present?).and_return(true)
+          allow(target_file_security_descriptor).to receive(:sacl_inherits?).and_return(false)
+
+          allow(target_file_security_descriptor).to receive(:sacl).and_return(original_target_file_sacl)
+          expect(Chef::ReservedNames::Win32::Security::ACL).
+            to receive(:create).
+            with([]).
+            and_return(empty_sacl)
+
+
+          expect(updated_target_security_object).to receive(:set_dacl).with(empty_dacl, false)
+          expect(updated_target_security_object).to receive(:set_sacl).with(empty_sacl, false)
+        end
+
+
+        it "fixes up permissions and moves the file into place" do
+          content_deployer.deploy(staging_file_path, target_file_path)
+        end
+      end
+
       context "and the target has a dacl and sacl" do
         let(:inherited_dacl_ace) { double("Windows dacl ace (inherited)", :inherited? => true) }
         let(:not_inherited_dacl_ace) { double("Windows dacl ace (not inherited)", :inherited? => false) }