WIP: binmode guessinglcg/windows-binmode-guessing

4 files changed, 120 insertions, 27 deletions

diff --git a/lib/chef/deprecation/provider/file.rb b/lib/chef/deprecation/provider/file.rb
index 0e9105247c..07f706000d 100644
--- a/lib/chef/deprecation/provider/file.rb
+++ b/lib/chef/deprecation/provider/file.rb
@@ -38,15 +38,6 @@ class Chef
           result
         end
 
-        def is_binary?(path)
-          ::File.open(path) do |file|
-
-            buff = file.read(Chef::Config[:diff_filesize_threshold])
-            buff = "" if buff.nil?
-            return buff !~ /^[\r[:print:]]*$/
-          end
-        end
-
         def diff_current(temp_path)
           suppress_resource_reporting = false
 
diff --git a/lib/chef/mixin/is_binary.rb b/lib/chef/mixin/is_binary.rb
new file mode 100644
index 0000000000..f6b10e8408
--- /dev/null
+++ b/lib/chef/mixin/is_binary.rb
@@ -0,0 +1,82 @@
+#
+# Author:: Lamont Granquist <lamont@opscode.com>
+# Copyright:: Copyright (c) 2011-2012 Opscode, 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.
+#
+
+class Chef
+  module Mixin
+    module IsBinary
+
+      # Helper to determine if a file is binary or not.  Does a best guess.
+      #
+      # This will handle files that are in the ruby default_external encoding correctly (most likely UTF-8),
+      # but if Chef is running UTF-8 and you feed this Shift_JIS or Latin-1 it will get it wrong.
+      #
+      # @param file [String] the pathname to the file
+      # @param length [Integer] the number of bytes to read (defaults to 16384, 0 or nil will slurp the whole file)
+      # @return [TrueClass, FalseClass] if the file is binary or not
+      #
+      def is_binary?(file, length = 16384)
+        # now we just check for the entire string being printable chars + whitespace or not
+        buf = read_buf(file, length)
+        begin
+          return buf !~ /\A[\s[:print:]]*\z/m
+        rescue ArgumentError => e
+          return true if e.message =~ /invalid byte sequence/
+          raise
+        end
+      end
+
+      def line_endings(file, length = 16384)
+        line_endings = []
+        buf = read_buf(file, length)
+        # remove any truncated line endings at the end
+        while buf[-1] =~ /[\r\n]/
+          buf.chop!
+        end
+        line_endings << :windows if buf =~ /\r\n/m
+        line_endings << :unix if buf =~ /\r(?!\n)/m
+        line_endings
+      end
+
+      private
+
+      def read_buf(file, length)
+        if length.nil? || length == 0
+          # if we slurp the whole thing to RAM then we get useful encoding on the string, so its simple
+          buf = IO.read(file)
+          buf = "" if buf.nil?
+        else
+          # binread gives us ASCII-8BIT and loses encoding obviously, but it truncates binary reads which we want
+          buf = IO.binread(file, length)
+          buf = "" if buf.nil?
+          # ruby 1.8 folks with non-ASCII characters get hosed here.  you must upgrade to get encoding (WONTFIX).
+          if Object.const_defined? :Encoding
+            buffer_length = buf.length
+            # then we just twiddle the encoding to the default external encoding (which will succeed but may not result in a valid string)
+            buf.force_encoding(Encoding.default_external)
+            # if we read the full buffer, then assume the last character was truncated and left up to 3 characters of garbage
+            if length == buffer_length
+              buf = buf[0..-3]
+            end
+          end
+        end
+        buf
+      end
+    end
+  end
+end
+
diff --git a/lib/chef/provider/file.rb b/lib/chef/provider/file.rb
index b2127d7c87..01e18876de 100644
--- a/lib/chef/provider/file.rb
+++ b/lib/chef/provider/file.rb
@@ -27,6 +27,7 @@ require 'chef/scan_access_control'
 require 'chef/mixin/checksum'
 require 'chef/mixin/shell_out'
 require 'chef/mixin/file_class'
+require 'chef/mixin/is_binary'
 require 'chef/util/backup'
 require 'chef/util/diff'
 require 'chef/deprecation/provider/file'
@@ -51,6 +52,7 @@ class Chef
       include Chef::Mixin::ShellOut
       include Chef::Util::Selinux
       include Chef::Mixin::FileClass
+      include Chef::Mixin::IsBinary
 
       extend Chef::Deprecation::Warnings
       include Chef::Deprecation::Provider::File
@@ -118,6 +120,7 @@ class Chef
       def action_create
         do_unlink
         do_create_file
+        do_fix_line_endings
         do_contents_changes
         do_acl_changes
         do_selinux
@@ -323,6 +326,33 @@ class Chef
         end
       end
 
+      def bad_file_endings_on_windows?
+        # someone on a unix workstation uploaded a file to deploy to a windows server
+        Chef::Platform.windows? && line_endings(tempfile, 65536).include?(:unix)
+      end
+
+      def bad_file_endings_on_unix?
+        # someone on a windows workstation uploaded a file to deploy to a unix server
+        !Chef::Platform.windows? && line_endings(tempfile, 65536).include?(:windows)
+      end
+
+      def do_fix_line_endings
+        # a nil tempfile is okay, means the resource has no content or no new content
+        unless tempfile.nil? || is_binary?(tempfile)
+          if bad_file_endings_on_windows? || bad_file_endings_on_unix?
+            # roll our own line ending conversion, because not-binmode on unix does not do any conversion
+            fixed_tempfile = Tempfile.open("chef-file-fixed", "wb+")
+            File.readlines(tempfile.path).each do |line|
+              line.chomp!
+              fixed_tempfile.puts line
+            end
+            fixed_tempfile.close
+            # deploy our fixed tempfile instead of the one from the content provider
+            @tempfile = fixed_tempfile
+          end
+        end
+      end
+
       # do_contents_changes needs to know if do_create_file created a file or not
       def file_created?
         @file_created == true
@@ -396,7 +426,8 @@ class Chef
       end
 
       def tempfile
-        content.tempfile
+        # we can update the tempfile if we modify the line endings
+        @tempfile ||= content.tempfile
       end
 
       def short_cksum(checksum)
diff --git a/lib/chef/util/diff.rb b/lib/chef/util/diff.rb
index 7bce52d874..d243cf5f59 100644
--- a/lib/chef/util/diff.rb
+++ b/lib/chef/util/diff.rb
@@ -42,6 +42,7 @@
 
 require 'diff/lcs'
 require 'diff/lcs/hunk'
+require 'chef/mixin/is_binary'
 
 class Chef
   class Util
@@ -50,6 +51,8 @@ class Chef
       # @todo: move coercion to UTF-8 into to_json
       # @todo: replace shellout to diff -u with diff-lcs gem
 
+      include Chef::Mixin::IsBinary
+
       def for_output
         # formatted output to a terminal uses arrays of strings and returns error strings
         @diff.nil? ? [ @error ] : @diff
@@ -82,7 +85,7 @@ class Chef
           end
         end
       end
-      
+
       # produces a unified-output-format diff with 3 lines of context
       # ChefFS uses udiff() directly
       def udiff(old_file, new_file)
@@ -135,8 +138,8 @@ class Chef
         end
 
         # MacOSX(BSD?) diff will *sometimes* happily spit out nasty binary diffs
-        return "(current file is binary, diff output suppressed)" if is_binary?(old_file)
-        return "(new content is binary, diff output suppressed)" if is_binary?(new_file)
+        return "(current file is binary, diff output suppressed)" if is_binary?(old_file, nil)
+        return "(new content is binary, diff output suppressed)" if is_binary?(new_file, nil)
 
         begin
           Chef::Log.debug("running: diff -u #{old_file} #{new_file}")
@@ -161,20 +164,6 @@ class Chef
         end
       end
 
-      def is_binary?(path)
-        File.open(path) do |file|
-          # XXX: this slurps into RAM, but we should have already checked our diff has a reasonable size
-          buff = file.read
-          buff = "" if buff.nil?
-          begin
-            return buff !~ /\A[\s[:print:]]*\z/m
-          rescue ArgumentError => e
-            return true if e.message =~ /invalid byte sequence/
-            raise
-          end
-        end
-      end
-
       def encode_diff_for_json(diff_str)
         if Object.const_defined? :Encoding
           diff_str.encode!('UTF-8', :invalid => :replace, :undef => :replace, :replace => '?')