(PUP-927) Read preserving line endings

Previously, puppet's built in `file` and `template` functions would
read file contents using `File.read`. This would not cause problems
on *nix but could cause EOL conversion from `\r\n` to `\n` on Windows.

Add `Puppet::FileSystem.read_preserve_line_endings` for cases where line
endings are important and use `:mode => 'rb'` checking first against utf-8
encoding, falling back to default_external when not valid, and falling back
to `File.read` when all else fails.

lib/puppet/file_system.rb

@@ -126,6 +126,19 @@ def self.read(path)
     @impl.read(assert_path(path))
   end
 
+  # Read a file keeping the original line endings intact. This
+  # attempts to open files using binary mode using some encoding
+  # overrides and falling back to IO.read when none of the
+  # encodings are valid.
+  #
+  # @return [String] The contents of the file
+  #
+  # @api public
+  #
+  def self.read_preserve_line_endings(path)
+    @impl.read_preserve_line_endings(assert_path(path))
+  end
+
   # @return [String] The binary contents of the file
   #
   # @api public

lib/puppet/file_system/file_impl.rb

@@ -75,6 +75,14 @@ def read(path)
     path.read
   end
 
+  def read_preserve_line_endings(path)
+    contents = path.read( :mode => 'rb', :encoding => Encoding::UTF_8)
+    contents = path.read( :mode => 'rb', :encoding => Encoding::default_external) unless contents.valid_encoding?
+    contents = path.read unless contents.valid_encoding?
+
+    contents
+  end
+
   def binread(path)
     raise NotImplementedError
   end

lib/puppet/file_system/memory_impl.rb

@@ -44,6 +44,10 @@ def read(path)
     handle.read
   end
 
+  def read_preserve_line_endings(path)
+    read(path)
+  end
+
   def open(path, *args, &block)
     handle = assert_path(path).handle
     if block_given?

lib/puppet/parser/functions/file.rb

@@ -1,3 +1,5 @@
+require 'puppet/file_system'
+
 Puppet::Parser::Functions::newfunction(
   :file, :arity => -2, :type => :rvalue,
   :doc => "Loads a file from a module and returns its contents as a string.
@@ -24,7 +26,7 @@
     end
 
     if path
-      File.read(path)
+      Puppet::FileSystem.read_preserve_line_endings(path)
     else
       raise Puppet::ParseError, "Could not find any files from #{vals.join(", ")}"
     end

lib/puppet/parser/templatewrapper.rb

@@ -1,5 +1,6 @@
 require 'puppet/parser/files'
 require 'erb'
+require 'puppet/file_system'
 
 # A simple wrapper for templates, so they don't have full access to
 # the scope objects.
@@ -70,7 +71,7 @@ def result(string = nil)
     if string
       template_source = "inline template"
     else
-      string = File.read(@__file__)
+      string = Puppet::FileSystem.read_preserve_line_endings(@__file__)
       template_source = @__file__
     end
 

spec/unit/file_system_spec.rb

@@ -5,6 +5,14 @@
 describe "Puppet::FileSystem" do
   include PuppetSpec::Files
 
+  def with_file_content(content)
+    path = tmpfile('file-system')
+    file = File.new(path, 'wb')
+    file.sync = true
+    file.print content
+    yield path
+  end
+
   context "#exclusive_open" do
     it "opens ands allows updating of an existing file" do
       file = file_containing("file_to_update", "the contents")
@@ -96,6 +104,32 @@ def increment_counter_in_multiple_processes(file, num_procs, options)
     end
   end
 
+  context "read_preserve_line_endings" do
+    it "should read a file with line feed" do
+      with_file_content("file content \n") do |file|
+        expect(Puppet::FileSystem.read_preserve_line_endings(file)).to eq("file content \n")
+      end
+    end
+
+    it "should read a file with carriage return line feed" do
+      with_file_content("file content \r\n") do |file|
+        expect(Puppet::FileSystem.read_preserve_line_endings(file)).to eq("file content \r\n")
+      end
+    end
+
+    it "should read a mixed file using only the first line newline when lf" do
+      with_file_content("file content \nsecond line \r\n") do |file|
+        expect(Puppet::FileSystem.read_preserve_line_endings(file)).to eq("file content \nsecond line \r\n")
+      end
+    end
+
+    it "should read a mixed file using only the first line newline when crlf" do
+      with_file_content("file content \r\nsecond line \n") do |file|
+        expect(Puppet::FileSystem.read_preserve_line_endings(file)).to eq("file content \r\nsecond line \n")
+      end
+    end
+  end
+
   describe "symlink",
     :if => ! Puppet.features.manages_symlinks? &&
     Puppet.features.microsoft_windows? do

spec/unit/parser/functions/file_spec.rb

@@ -15,15 +15,21 @@
 
   def with_file_content(content)
     path = tmpfile('file-function')
-    file = File.new(path, 'w')
+    file = File.new(path, 'wb')
     file.sync = true
     file.print content
     yield path
   end
 
   it "should read a file" do
     with_file_content('file content') do |name|
-      expect(scope.function_file([name])).to eq("file content")
+      expect(scope.function_file([name])).to eq('file content')
+    end
+  end
+
+  it "should read a file keeping line endings intact" do
+    with_file_content("file content\r\n") do |name|
+      expect(scope.function_file([name])).to eq("file content\r\n")
     end
   end
 

spec/unit/parser/functions/template_spec.rb

@@ -82,7 +82,7 @@
   end
 
   def eval_template(content)
-    File.stubs(:read).with("template").returns(content)
+    Puppet::FileSystem.stubs(:read_preserve_line_endings).with("template").returns(content)
     Puppet::Parser::Files.stubs(:find_template).returns("template")
     scope.function_template(['template'])
   end

spec/unit/parser/templatewrapper_spec.rb

@@ -94,7 +94,7 @@ def given_a_template_file(name, contents)
     Puppet::Parser::Files.stubs(:find_template).
       with(name, anything()).
       returns(full_name)
-    File.stubs(:read).with(full_name).returns(contents)
+    Puppet::FileSystem.stubs(:read_preserve_line_endings).with(full_name).returns(contents)
 
     full_name
   end