Make sure that windows line endings are protected during template rendering.

11 files changed, 194 insertions, 77 deletions

diff --git a/lib/chef/deprecation/mixin/template.rb b/lib/chef/deprecation/mixin/template.rb
new file mode 100644
index 0000000000..36d18ad90d
--- /dev/null
+++ b/lib/chef/deprecation/mixin/template.rb
@@ -0,0 +1,49 @@
+#
+# Author:: Serdar Sutay (<serdar@opscode.com>)
+# Copyright:: Copyright (c) 2013 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.
+#
+
+require 'tempfile'
+require 'erubis'
+
+class Chef
+  module Deprecation
+    module Mixin
+      # == Deprecation::Provider::Mixin::Template
+      # This module contains the deprecated functions of
+      # Chef::Mixin::Template. These functions are refactored to different
+      # components. They are frozen and will be removed in Chef 12.
+      #
+
+      module Template
+        def render_template(template, context)
+          begin
+            eruby = Erubis::Eruby.new(template)
+            output = eruby.evaluate(context)
+          rescue Object => e
+            raise TemplateError.new(e, template, context)
+          end
+          Tempfile.open("chef-rendered-template") do |tempfile|
+            tempfile.print(output)
+            tempfile.close
+            yield tempfile
+          end
+        end
+      end
+    end
+  end
+end
+
diff --git a/lib/chef/deprecation/provider/template.rb b/lib/chef/deprecation/provider/template.rb
index 0f93c637f6..3d8071f00b 100644
--- a/lib/chef/deprecation/provider/template.rb
+++ b/lib/chef/deprecation/provider/template.rb
@@ -16,7 +16,7 @@
 # limitations under the License.
 #
 
-require 'chef/mixin/template'
+require 'chef/deprecation/mixin/template'
 
 class Chef
   module Deprecation
@@ -29,7 +29,7 @@ class Chef
       #
       module Template
 
-        include Chef::Mixin::Template
+        include Chef::Deprecation::Mixin::Template
 
         def template_finder
           @template_finder ||= begin
diff --git a/lib/chef/mixin/template.rb b/lib/chef/mixin/template.rb
index 9892447ca4..d6cf564f40 100644
--- a/lib/chef/mixin/template.rb
+++ b/lib/chef/mixin/template.rb
@@ -93,14 +93,47 @@ class Chef
           partial_context._extend_modules(@_extension_modules)
 
           template_location = @template_finder.find(partial_name, options)
-          eruby = Erubis::Eruby.new(IO.read(template_location))
-          eruby.evaluate(partial_context)
+          _render_template(IO.binread(template_location), partial_context)
+        end
+
+        def render_template(template_location)
+          _render_template(IO.binread(template_location), self)
+        end
+
+        def render_template_from_string(template)
+          _render_template(template, self)
         end
 
         ###
         # INTERNAL PUBLIC API
         ###
 
+        def _render_template(template, context)
+          # CHEF-2991
+          # Erubis always emits unix line endings during template
+          # rendering. This results in automatic conversion of windows
+          # line endings to linux line endings if the original template
+          # contains windows line endings. In order to fix this we
+          # determine the line ending style of the template before
+          # rendering and convert the line endings of the output if needed
+          # If template contains any windows line endings we emit
+          # the template result with windows line endings.
+          windows_line_endings = template.include? "\r\n"
+
+          begin
+            eruby = Erubis::Eruby.new(template)
+            output = eruby.evaluate(context)
+          rescue Object => e
+            raise TemplateError.new(e, template, context)
+          end
+
+          if windows_line_endings
+            output = output.gsub("\n","\r\n")
+          end
+
+          output
+        end
+
         def _extend_modules(module_names)
           module_names.each do |mod|
             [:node, :render].each do |core_method|
@@ -129,23 +162,6 @@ class Chef
         end
       end
 
-
-      # Render a template with Erubis.  Takes a template as a string, and a
-      # context hash.
-      def render_template(template, context)
-        begin
-          eruby = Erubis::Eruby.new(template)
-          output = eruby.evaluate(context)
-        rescue Object => e
-          raise TemplateError.new(e, template, context)
-        end
-        Tempfile.open("chef-rendered-template") do |tempfile|
-          tempfile.print(output)
-          tempfile.close
-          yield tempfile
-        end
-      end
-
       class TemplateError < RuntimeError
         attr_reader :original_exception, :context
         SOURCE_CONTEXT_WINDOW = 2
diff --git a/lib/chef/provider/template/content.rb b/lib/chef/provider/template/content.rb
index 9de9ad9ea2..f6cd470357 100644
--- a/lib/chef/provider/template/content.rb
+++ b/lib/chef/provider/template/content.rb
@@ -40,9 +40,12 @@ class Chef
           context[:node] = @run_context.node
           context[:template_finder] = template_finder
           context._extend_modules(@new_resource.helper_modules)
-          file = nil
-          render_template(IO.read(template_location), context) { |t| file = t }
-          file
+          output = context.render_template(template_location)
+
+          tempfile = Tempfile.open("chef-rendered-template")
+          tempfile.write(output)
+          tempfile.close
+          tempfile
         end
 
         def template_finder
diff --git a/spec/data/cookbooks/openldap/templates/default/all_windows_line_endings.erb b/spec/data/cookbooks/openldap/templates/default/all_windows_line_endings.erb
new file mode 100644
index 0000000000..727db12387
--- /dev/null
+++ b/spec/data/cookbooks/openldap/templates/default/all_windows_line_endings.erb
@@ -0,0 +1,4 @@
+Template rendering libraries

+should support

+different line endings

+

diff --git a/spec/data/cookbooks/openldap/templates/default/no_windows_line_endings.erb b/spec/data/cookbooks/openldap/templates/default/no_windows_line_endings.erb
new file mode 100644
index 0000000000..e83c03b01a
--- /dev/null
+++ b/spec/data/cookbooks/openldap/templates/default/no_windows_line_endings.erb
@@ -0,0 +1,4 @@
+Template rendering libraries
+should support
+different line endings
+
diff --git a/spec/data/cookbooks/openldap/templates/default/some_windows_line_endings.erb b/spec/data/cookbooks/openldap/templates/default/some_windows_line_endings.erb
new file mode 100644
index 0000000000..fa25999b6a
--- /dev/null
+++ b/spec/data/cookbooks/openldap/templates/default/some_windows_line_endings.erb
@@ -0,0 +1,4 @@
+Template rendering libraries
+should support

+different line endings
+

diff --git a/spec/functional/resource/template_spec.rb b/spec/functional/resource/template_spec.rb
index 6d42eeceac..f73f239b41 100644
--- a/spec/functional/resource/template_spec.rb
+++ b/spec/functional/resource/template_spec.rb
@@ -183,4 +183,59 @@ describe Chef::Resource::Template do
 
     end
   end
+
+  describe "when template source contains windows style line endings" do
+
+    include_context "diff disabled"
+
+    let (:expected_content) {
+      "Template rendering libraries\r\nshould support\r\ndifferent line endings\r\n\r\n"
+    }
+
+    context "for all lines" do
+      let!(:resource) do
+        r = create_resource
+        r.source "all_windows_line_endings.erb"
+        r
+      end
+
+      it "output should contain windows line endings" do
+        resource.run_action(:create)
+        IO.read(path).each_line do |line|
+          line.should end_with("\r\n")
+        end
+      end
+    end
+
+    context "for some lines" do
+      let!(:resource) do
+        r = create_resource
+        r.source "some_windows_line_endings.erb"
+        r
+      end
+
+      it "output should contain windows line endings" do
+        resource.run_action(:create)
+        IO.read(path).each_line do |line|
+          line.should end_with("\r\n")
+        end
+      end
+    end
+
+    context "for no lines" do
+      let!(:resource) do
+        r = create_resource
+        r.source "no_windows_line_endings.erb"
+        r
+      end
+
+      it "output should not contain windows line endings" do
+        resource.run_action(:create)
+        IO.read(path).each_line do |line|
+          line.should_not end_with("\r\n")
+        end
+      end
+    end
+  end
+
 end
diff --git a/spec/unit/cookbook/syntax_check_spec.rb b/spec/unit/cookbook/syntax_check_spec.rb
index 7007d06920..f84c87a8fb 100644
--- a/spec/unit/cookbook/syntax_check_spec.rb
+++ b/spec/unit/cookbook/syntax_check_spec.rb
@@ -37,7 +37,10 @@ describe Chef::Cookbook::SyntaxCheck do
                     helper_test.erb
                     openldap_stuff.conf.erb
                     openldap_variable_stuff.conf.erb
-                    test.erb }
+                    test.erb
+                    some_windows_line_endings.erb
+                    all_windows_line_endings.erb
+                    no_windows_line_endings.erb }
     @template_files = basenames.map { |f| File.join(cookbook_path, 'templates', 'default', f)}
   end
 
diff --git a/spec/unit/formatters/error_inspectors/resource_failure_inspector_spec.rb b/spec/unit/formatters/error_inspectors/resource_failure_inspector_spec.rb
index fa96845aca..b6d48d0255 100644
--- a/spec/unit/formatters/error_inspectors/resource_failure_inspector_spec.rb
+++ b/spec/unit/formatters/error_inspectors/resource_failure_inspector_spec.rb
@@ -83,14 +83,15 @@ describe Chef::Formatters::ErrorInspectors::ResourceFailureInspector do
         @description = Chef::Formatters::ErrorDescription.new("Error Converging Resource:")
         @template_class = Class.new { include Chef::Mixin::Template }
         @template = @template_class.new
-        @context = {:chef => "cool"}
+        @context = Chef::Mixin::Template::TemplateContext.new({})
+        @context[:chef] = "cool"
 
         @resource = template("/tmp/foo.txt") do
           mode "0644"
         end
 
         @error = begin
-                   @template.render_template("foo\nbar\nbaz\n<%= this_is_not_defined %>\nquin\nqunx\ndunno", @context) {|r| r}
+                   @context.render_template_from_string("foo\nbar\nbaz\n<%= this_is_not_defined %>\nquin\nqunx\ndunno")
                  rescue  Chef::Mixin::Template::TemplateError => e
                    e
                  end
diff --git a/spec/unit/mixin/template_spec.rb b/spec/unit/mixin/template_spec.rb
index 630f207732..aecbb5186c 100644
--- a/spec/unit/mixin/template_spec.rb
+++ b/spec/unit/mixin/template_spec.rb
@@ -18,33 +18,23 @@
 
 require 'spec_helper'
 
-class TinyTemplateClass; include Chef::Mixin::Template; end
 require 'cgi'
 describe Chef::Mixin::Template, "render_template" do
 
   before :each do
-    @template = TinyTemplateClass.new
     @context = Chef::Mixin::Template::TemplateContext.new({})
   end
 
   it "should render the template evaluated in the given context" do
     @context[:foo] = "bar"
-    @template.render_template("<%= @foo %>", @context) do |tmp|
-      tmp.open.read.should == "bar"
-    end
+    output = @context.render_template_from_string("<%= @foo %>")
+    output.should == "bar"
   end
 
   it "should provide a node method to access @node" do
     @context[:node] = "tehShizzle"
-    @template.render_template("<%= node %>", @context) do |tmp|
-      tmp.open.read.should == "tehShizzle"
-    end
-  end
-
-  it "should yield the tempfile it renders the template to" do
-    @template.render_template("abcdef", {}) do |tempfile|
-      tempfile.should be_kind_of(Tempfile)
-    end
+    output = @context.render_template_from_string("<%= @node %>")
+    output.should == "tehShizzle"
   end
 
   describe "with a template resource" do
@@ -73,9 +63,8 @@ describe Chef::Mixin::Template, "render_template" do
     end
 
     it "should provide a render method" do
-      @content_provider.render_template("before {<%= render 'test.erb' %>} after", @template_context) do |tmp|
-        tmp.open.read.should == "before {We could be diving for pearls!\n} after"
-      end
+      output = @template_context.render_template_from_string("before {<%= render 'test.erb' %>} after")
+      output.should == "before {We could be diving for pearls!\n} after"
     end
 
     it "should render local files" do
@@ -84,9 +73,8 @@ describe Chef::Mixin::Template, "render_template" do
         tf.puts "test"
         tf.rewind
 
-        @content_provider.render_template("before {<%= render '#{tf.path}', :local => true %>} after", @template_context) do |tmp|
-          tmp.open.read.should == "before {test\n} after"
-        end
+        output = @template_context.render_template_from_string("before {<%= render '#{tf.path}', :local => true %>} after")
+        output.should == "before {test\n} after"
       ensure
         tf.close
       end
@@ -95,9 +83,8 @@ describe Chef::Mixin::Template, "render_template" do
     it "should render partials from a different cookbook" do
       @template_context[:template_finder] = Chef::Provider::TemplateFinder.new(@run_context, 'apache2', @node)
 
-      @content_provider.render_template("before {<%= render 'test.erb', :cookbook => 'openldap' %>} after", @template_context) do |tmp|
-        tmp.open.read.should == "before {We could be diving for pearls!\n} after"
-      end
+      output = @template_context.render_template_from_string("before {<%= render 'test.erb', :cookbook => 'openldap' %>} after")
+      output.should == "before {We could be diving for pearls!\n} after"
     end
 
     it "should render using the source argument if provided" do
@@ -106,9 +93,8 @@ describe Chef::Mixin::Template, "render_template" do
         tf.puts "test"
         tf.rewind
 
-        @content_provider.render_template("before {<%= render 'something', :local => true, :source => '#{tf.path}' %>} after", @template_context) do |tmp|
-          tmp.open.read.should == "before {test\n} after"
-        end
+        output = @template_context.render_template_from_string("before {<%= render 'something', :local => true, :source => '#{tf.path}' %>} after")
+        output.should == "before {test\n} after"
       ensure
         tf.close
       end
@@ -117,49 +103,42 @@ describe Chef::Mixin::Template, "render_template" do
     it "should pass the node to partials" do
       @node.normal[:slappiness] = "happiness"
 
-      @content_provider.render_template("before {<%= render 'openldap_stuff.conf.erb' %>} after", @template_context) do |tmp|
-        tmp.open.read.should == "before {slappiness is happiness} after"
-      end
+      output = @template_context.render_template_from_string("before {<%= render 'openldap_stuff.conf.erb' %>} after")
+      output.should == "before {slappiness is happiness} after"
     end
 
     it "should pass the original variables to partials" do
       @template_context[:secret] = 'candy'
 
-      @content_provider.render_template("before {<%= render 'openldap_variable_stuff.conf.erb' %>} after", @template_context) do |tmp|
-        tmp.open.read.should == "before {super secret is candy} after"
-      end
+      output = @template_context.render_template_from_string("before {<%= render 'openldap_variable_stuff.conf.erb' %>} after")
+      output == "before {super secret is candy} after"
     end
 
     it "should pass variables to partials" do
-      @content_provider.render_template("before {<%= render 'openldap_variable_stuff.conf.erb', :variables => {:secret => 'whatever' } %>} after", @template_context) do |tmp|
-        tmp.open.read.should == "before {super secret is whatever} after"
-      end
+      output = @template_context.render_template_from_string("before {<%= render 'openldap_variable_stuff.conf.erb', :variables => {:secret => 'whatever' } %>} after")
+      output.should == "before {super secret is whatever} after"
     end
 
     it "should pass variables to partials even if they are named the same" do
       @template_context[:secret] = 'one'
 
-      @content_provider.render_template("before {<%= render 'openldap_variable_stuff.conf.erb', :variables => {:secret => 'two' } %>} after <%= @secret %>", @template_context) do |tmp|
-        tmp.open.read.should == "before {super secret is two} after one"
-      end
+      output = @template_context.render_template_from_string("before {<%= render 'openldap_variable_stuff.conf.erb', :variables => {:secret => 'two' } %>} after <%= @secret %>")
+      output.should == "before {super secret is two} after one"
     end
 
     it "should pass nil for missing variables in partials" do
-      @content_provider.render_template("before {<%= render 'openldap_variable_stuff.conf.erb', :variables => {} %>} after", @template_context) do |tmp|
-        tmp.open.read.should == "before {super secret is } after"
-      end
+      output = @template_context.render_template_from_string("before {<%= render 'openldap_variable_stuff.conf.erb', :variables => {} %>} after")
+      output.should == "before {super secret is } after"
 
-      @content_provider.render_template("before {<%= render 'openldap_variable_stuff.conf.erb' %>} after", @template_context) do |tmp|
-        tmp.open.read.should == "before {super secret is } after"
-      end
+      output = @template_context.render_template_from_string("before {<%= render 'openldap_variable_stuff.conf.erb' %>} after")
+    output.should == "before {super secret is } after"
     end
 
     it "should render nested partials" do
       path = File.expand_path(File.join(CHEF_SPEC_DATA, "partial_one.erb"))
 
-      @content_provider.render_template("before {<%= render '#{path}', :local => true %>} after", @template_context) do |tmp|
-        tmp.open.read.should == "before {partial one We could be diving for pearls!\n calling home\n} after"
-      end
+      output = @template_context.render_template_from_string("before {<%= render '#{path}', :local => true %>} after")
+      output.should == "before {partial one We could be diving for pearls!\n calling home\n} after"
     end
 
     describe "when customizing the template context" do
@@ -196,8 +175,7 @@ describe Chef::Mixin::Template, "render_template" do
 
   describe "when an exception is raised in the template" do
     def do_raise
-      @context = {:chef => "cool"}
-      @template.render_template("foo\nbar\nbaz\n<%= this_is_not_defined %>\nquin\nqunx\ndunno", @context) {|r| r}
+      @context.render_template_from_string("foo\nbar\nbaz\n<%= this_is_not_defined %>\nquin\nqunx\ndunno")
     end
 
     it "should catch and re-raise the exception as a TemplateError" do
@@ -205,7 +183,7 @@ describe Chef::Mixin::Template, "render_template" do
     end
 
     it "should raise an error if an attempt is made to access node but it is nil" do
-      lambda {@template.render_template("<%= node %>",{}) {|r| r}}.should raise_error(Chef::Mixin::Template::TemplateError)
+      lambda {@context.render_template_from_string("<%= node %>") {|r| r}}.should raise_error(Chef::Mixin::Template::TemplateError)
     end
 
     describe "the raised TemplateError" do