From 52f447afff4f88e6357641e430e63bb599a9089e Mon Sep 17 00:00:00 2001 From: Rob Reynolds Date: Tue, 17 Feb 2015 09:35:46 -0600 Subject: [PATCH] (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 | 13 ++++++++ lib/puppet/file_system/file_impl.rb | 8 +++++ lib/puppet/file_system/memory_impl.rb | 4 +++ lib/puppet/parser/functions/file.rb | 4 ++- lib/puppet/parser/templatewrapper.rb | 3 +- spec/unit/file_system_spec.rb | 34 +++++++++++++++++++++ spec/unit/parser/functions/file_spec.rb | 10 ++++-- spec/unit/parser/functions/template_spec.rb | 2 +- spec/unit/parser/templatewrapper_spec.rb | 2 +- 9 files changed, 74 insertions(+), 6 deletions(-) diff --git a/lib/puppet/file_system.rb b/lib/puppet/file_system.rb index f4314546796..a0950780b3b 100644 --- a/lib/puppet/file_system.rb +++ b/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 diff --git a/lib/puppet/file_system/file_impl.rb b/lib/puppet/file_system/file_impl.rb index d4cd605b75b..6a86ba5f68d 100644 --- a/lib/puppet/file_system/file_impl.rb +++ b/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 diff --git a/lib/puppet/file_system/memory_impl.rb b/lib/puppet/file_system/memory_impl.rb index 6fa35782398..12c63cd79b6 100644 --- a/lib/puppet/file_system/memory_impl.rb +++ b/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? diff --git a/lib/puppet/parser/functions/file.rb b/lib/puppet/parser/functions/file.rb index cde496ab4de..e8ae32abc3b 100644 --- a/lib/puppet/parser/functions/file.rb +++ b/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 diff --git a/lib/puppet/parser/templatewrapper.rb b/lib/puppet/parser/templatewrapper.rb index 00576c86106..110ca193653 100644 --- a/lib/puppet/parser/templatewrapper.rb +++ b/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 diff --git a/spec/unit/file_system_spec.rb b/spec/unit/file_system_spec.rb index cd6be8e1fa7..dee51474de1 100644 --- a/spec/unit/file_system_spec.rb +++ b/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 diff --git a/spec/unit/parser/functions/file_spec.rb b/spec/unit/parser/functions/file_spec.rb index 7406b33091e..95b204f9cdb 100755 --- a/spec/unit/parser/functions/file_spec.rb +++ b/spec/unit/parser/functions/file_spec.rb @@ -15,7 +15,7 @@ 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 @@ -23,7 +23,13 @@ def with_file_content(content) 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 diff --git a/spec/unit/parser/functions/template_spec.rb b/spec/unit/parser/functions/template_spec.rb index c48e061c706..d47506c286e 100755 --- a/spec/unit/parser/functions/template_spec.rb +++ b/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 diff --git a/spec/unit/parser/templatewrapper_spec.rb b/spec/unit/parser/templatewrapper_spec.rb index 83985423beb..b503b56aa6e 100755 --- a/spec/unit/parser/templatewrapper_spec.rb +++ b/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