CHEF-5322: Add utility for validating Windows paths

5 files changed, 240 insertions, 0 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8631d74cea..e1cbeb7ee2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -46,6 +46,7 @@
 * Fixed Ruby 2.0 Windows compatibility issues around ruby-wmi gem by replacing it with wmi-lite gem.
 * Set proxy environment variables if preset in config. (CHEF-4712)
 * Automatically enable verify_api_cert when running chef-client in local-mode. (Chef Issues 1464)
+* Add helper to warn for broken [windows] paths. (CHEF-5322)
 
 ## Release: 11.12.4 (04/30/2014)
 http://www.getchef.com/blog/2014/04/30/release-chef-client-11-12-4-ohai-7-0-4/
diff --git a/lib/chef/provider/package/windows.rb b/lib/chef/provider/package/windows.rb
index be1de0b969..25be5b822c 100644
--- a/lib/chef/provider/package/windows.rb
+++ b/lib/chef/provider/package/windows.rb
@@ -18,6 +18,7 @@
 
 require 'chef/resource/windows_package'
 require 'chef/provider/package'
+require 'chef/util/path_helper'
 
 class Chef
   class Provider
@@ -32,6 +33,8 @@ class Chef
 
         # load_current_resource is run in Chef::Provider#run_action when not in whyrun_mode?
         def load_current_resource
+          @new_resource.source(Chef::Util::PathHelper.validate_path(@new_resource.source))
+
           @current_resource = Chef::Resource::WindowsPackage.new(@new_resource.name)
           @current_resource.version(package_provider.installed_version)
           @new_resource.version(package_provider.package_version)
diff --git a/lib/chef/util/path_helper.rb b/lib/chef/util/path_helper.rb
new file mode 100644
index 0000000000..534a9087ae
--- /dev/null
+++ b/lib/chef/util/path_helper.rb
@@ -0,0 +1,94 @@
+#
+# Author:: Bryan McLellan <btm@loftninjas.org>
+# Copyright:: Copyright (c) 2014 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/platform'
+require 'chef/exceptions'
+
+class Chef
+  class Util
+    class PathHelper
+      # Maximum characters in a standard Windows path (260 including drive letter and NUL)
+      WIN_MAX_PATH = 259
+
+      def self.validate_path(path)
+        if Chef::Platform.windows?
+          unless printable?(path)
+            msg = "Path '#{path}' contains non-printable characters. Check that backslashes are escaped with another backslash (e.g. C:\\\\Windows) in double-quoted strings."
+            Chef::Log.error(msg)
+            raise Chef::Exceptions::ValidationFailed, msg
+          end
+            
+          if windows_max_length_exceeded?(path)
+            Chef::Log.debug("Path '#{path}' is longer than #{WIN_MAX_PATH}, prefixing with'\\\\?\\'")
+            path.insert(0, "\\\\?\\")
+          end
+        end
+
+        path
+      end
+
+      def self.windows_max_length_exceeded?(path)
+        # Check to see if paths without the \\?\ prefix are over the maximum allowed length for the Windows API
+        # http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.85%29.aspx
+        unless path =~ /^\\\\?\\/
+          if path.length > WIN_MAX_PATH
+            return true
+          end
+        end
+        
+        false
+      end
+
+      def self.printable?(string)
+        # returns true if string is free of non-printable characters (escape sequences)
+        # this returns false for whitespace escape sequences as well, e.g. \n\t
+        if string =~ /[^[:print:]]/
+          false
+        else
+          true
+        end
+      end
+
+      # Produces a comparable path.
+      def self.canonical_path(path, add_prefix=true)
+        # Rather than find an equivalent for File.absolute_path on 1.8.7, just bail out
+        raise NotImplementedError, "This feature is not supported on Ruby versions < 1.9" if RUBY_VERSION.to_f < 1.9
+
+        # First remove extra separators and resolve any relative paths
+        abs_path = File.absolute_path(path)
+
+        if Chef::Platform.windows?
+          # Add the \\?\ API prefix on Windows unless add_prefix is false
+          # Downcase on Windows where paths are still case-insensitive
+          abs_path.gsub!(::File::SEPARATOR, ::File::ALT_SEPARATOR)
+          if add_prefix && abs_path !~ /^\\\\?\\/
+            abs_path.insert(0, "\\\\?\\")
+          end
+
+          abs_path.downcase!
+        end
+
+        abs_path
+      end
+
+      def self.paths_eql?(path1, path2)
+        canonical_path(path1) == canonical_path(path2)
+      end
+    end
+  end
+end
diff --git a/spec/unit/provider/package/windows_spec.rb b/spec/unit/provider/package/windows_spec.rb
index e94404eea5..b4ababb243 100644
--- a/spec/unit/provider/package/windows_spec.rb
+++ b/spec/unit/provider/package/windows_spec.rb
@@ -27,6 +27,7 @@ describe Chef::Provider::Package::Windows, :windows_only do
 
   describe "load_current_resource" do
     before(:each) do
+      Chef::Util::PathHelper.stub(:validate_path)
       provider.stub(:package_provider).and_return(double('package_provider',
           :installed_version => "1.0", :package_version => "2.0"))
     end
@@ -46,6 +47,11 @@ describe Chef::Provider::Package::Windows, :windows_only do
       provider.load_current_resource
       expect(provider.new_resource.version).to eql("2.0")
     end
+
+    it "checks that the source path is valid" do
+      expect(Chef::Util::PathHelper).to receive(:validate_path)
+      provider.load_current_resource
+    end
   end
 
   describe "package_provider" do
diff --git a/spec/unit/util/path_helper_spec.rb b/spec/unit/util/path_helper_spec.rb
new file mode 100644
index 0000000000..36ba19fce6
--- /dev/null
+++ b/spec/unit/util/path_helper_spec.rb
@@ -0,0 +1,136 @@
+#
+# Author:: Bryan McLellan <btm@loftninjas.org>
+# Copyright:: Copyright (c) 2014 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/util/path_helper'
+require 'spec_helper'
+
+describe Chef::Util::PathHelper do
+  let(:path_helper) { Chef::Util::PathHelper }
+
+  describe "validate_path" do
+    context "on windows" do
+      before(:each) do
+        # pass by default
+        Chef::Platform.stub(:windows?).and_return(true)
+        path_helper.stub(:printable?).and_return(true)
+        path_helper.stub(:windows_max_length_exceeded?).and_return(false)
+      end
+
+      it "returns the path if the path passes the tests" do
+        expect(path_helper.validate_path("C:\\ThisIsRigged")).to eql("C:\\ThisIsRigged")
+      end
+
+      it "does not raise an error if everything looks great" do
+        expect { path_helper.validate_path("C:\\cool path\\dude.exe") }.not_to raise_error
+      end
+
+      it "raises an error if the path has invalid characters" do
+        path_helper.stub(:printable?).and_return(false)
+        expect { path_helper.validate_path("Newline!\n") }.to raise_error(Chef::Exceptions::ValidationFailed)
+      end
+
+      it "Adds the \\\\?\\ prefix if the path exceeds MAX_LENGTH and does not have it" do
+        long_path = "C:\\" + "a" * 250 + "\\" + "b" * 250
+        prefixed_long_path = "\\\\?\\" + long_path
+        path_helper.stub(:windows_max_length_exceeded?).and_return(true)
+        expect(path_helper.validate_path(long_path)).to eql(prefixed_long_path)
+      end
+    end
+  end
+
+  describe "windows_max_length_exceeded?" do
+    it "returns true if the path is too long (259 + NUL) for the API" do
+      expect(path_helper.windows_max_length_exceeded?("C:\\" + "a" * 250 + "\\" + "b" * 6)).to be_true
+    end
+
+    it "returns false if the path is not too long (259 + NUL) for the standard API" do
+      expect(path_helper.windows_max_length_exceeded?("C:\\" + "a" * 250 + "\\" + "b" * 5)).to be_false
+    end
+
+    it "returns false if the path is over 259 characters but uses the \\\\?\\ prefix" do
+      expect(path_helper.windows_max_length_exceeded?("\\\\?\\C:\\" + "a" * 250 + "\\" + "b" * 250)).to be_false
+    end
+  end
+
+  describe "printable?" do
+    it "returns true if the string contains no non-printable characters" do
+      expect(path_helper.printable?("C:\\Program Files (x86)\\Microsoft Office\\Files.lst")).to be_true
+    end
+
+    it "returns true when given 'abc' in unicode" do
+      expect(path_helper.printable?("\u0061\u0062\u0063")).to be_true
+    end
+
+    it "returns true when given japanese unicode" do
+      expect(path_helper.printable?("\uff86\uff87\uff88")).to be_true
+    end
+
+    it "returns false if the string contains a non-printable character" do
+      expect(path_helper.printable?("\my files\work\notes.txt")).to be_false
+    end
+
+    # This isn't necessarily a requirement, but here to be explicit about functionality.
+    it "returns false if the string contains a newline or tab" do
+      expect(path_helper.printable?("\tThere's no way,\n\t *no* way,\n\t that you came from my loins.\n")).to be_false
+    end
+  end
+
+  describe "canonical_path" do
+    context "on windows", :windows_only do
+      it "returns an absolute path with backslashes instead of slashes" do
+        expect(path_helper.canonical_path("\\\\?\\C:/windows/win.ini")).to eq("\\\\?\\c:\\windows\\win.ini")
+      end
+
+      it "adds the \\\\?\\ prefix if it is missing" do
+        expect(path_helper.canonical_path("C:/windows/win.ini")).to eq("\\\\?\\c:\\windows\\win.ini")
+      end
+
+      it "returns a lowercase path" do
+        expect(path_helper.canonical_path("\\\\?\\C:\\CASE\\INSENSITIVE")).to eq("\\\\?\\c:\\case\\insensitive")
+      end
+    end
+
+    context "not on windows", :unix_only  do
+      context "ruby is at least 1.9", :ruby_gte_19_only do
+        it "returns a canonical path" do
+          expect(path_helper.canonical_path("/etc//apache.d/sites-enabled/../sites-available/default")).to eq("/etc/apache.d/sites-available/default")
+        end
+      end
+
+      context "ruby is less than 1.9", :ruby_18_only do
+        it "returns a canonical path" do
+          expect { path_helper.canonical_path("/etc//apache.d/sites-enabled/../sites-available/default") }.to raise_error(NotImplementedError)
+        end
+      end
+    end
+  end
+
+  describe "paths_eql?" do
+    it "returns true if the paths are the same" do
+      path_helper.stub(:canonical_path).with("bandit").and_return("c:/bandit/bandit")
+      path_helper.stub(:canonical_path).with("../bandit/bandit").and_return("c:/bandit/bandit")
+      expect(path_helper.paths_eql?("bandit", "../bandit/bandit")).to be_true
+    end
+
+    it "returns false if the paths are different" do
+      path_helper.stub(:canonical_path).with("bandit").and_return("c:/Bo/Bandit")
+      path_helper.stub(:canonical_path).with("../bandit/bandit").and_return("c:/bandit/bandit")
+      expect(path_helper.paths_eql?("bandit", "../bandit/bandit")).to be_false
+     end
+  end
+end