(#6541) Fix content with checksum truncation bug

The patch for #6107 fd73874147a1aaa3a047f904a0bc1ae67780a2e4 introduced
a bug when content was an invalid checksum.  Rather than error the
checksum was invalid, it would overwrite the file with empty string,
essentially truncating it.

The problem with #6107 is that when I wrote it, I didn't realize that
the content parameter was munged to be nil when it was a checksum, and
then chunking method special cased nil content to mean you should check
the filebucket.  #6107 intended to fix the case where content REALLY WAS
nil, and handle that by returning an empty string.

This patch fixes it so that we check to see if we really passed in a
checksum when chunking, and only then going to the filebucket.

Surprisingly it is possible to have a content checksum should value set
from source, so we have to be careful not to assume the use of the
filebucket whenever there's a checksum.  The following manifest produces
this situation:

  file { "/tmp/mydir" :
    source  => '/tmp/sourcedir',
    recurse => true,
  }

I've said it before, and sure I'll say it again, but long term the file
provider really needs a refactor.  I'll write some acceptance tests for
file behavior right after committing this so that the refactoring will
be easier.

Reviewed-by: Daniel Pittman <daniel@puppetlabs.com>

1 files changed, 22 insertions, 1 deletions

diff --git a/spec/unit/type/file/content_spec.rb b/spec/unit/type/file/content_spec.rb
index 7d23399cf..bd2b2adaf 100755
--- a/spec/unit/type/file/content_spec.rb
+++ b/spec/unit/type/file/content_spec.rb
@@ -375,17 +375,38 @@ describe content do
         @content.each_chunk_from('i_am_a_string') { |chunk| chunk.should == 'i_am_a_string' }
       end
 
+      # The following manifest is a case where source and content.should are both set
+      # file { "/tmp/mydir" :
+      #   source  => '/tmp/sourcedir',
+      #   recurse => true,
+      # }
+      it "when content checksum comes from source" do
+        source_param = Puppet::Type.type(:file).attrclass(:source)
+        source = source_param.new(:resource => @resource)
+        @content.should = "{md5}123abcd"
+
+        @content.expects(:chunk_file_from_source).returns('from_source')
+        @content.each_chunk_from(source) { |chunk| chunk.should == 'from_source' }
+      end
+
       it "when no content, source, but ensure present" do
         @resource[:ensure] = :present
         @content.each_chunk_from(nil) { |chunk| chunk.should == '' }
       end
 
+      # you might do this if you were just auditing
       it "when no content, source, but ensure file" do
         @resource[:ensure] = :file
         @content.each_chunk_from(nil) { |chunk| chunk.should == '' }
       end
 
-      it "when no content or source" do
+      it "when source_or_content is nil and content not a checksum" do
+        @content.each_chunk_from(nil) { |chunk| chunk.should == '' }
+      end
+
+      # the content is munged so that if it's a checksum nil gets passed in
+      it "when content is a checksum it should try to read from filebucket" do
+        @content.should = "{md5}123abcd"
         @content.expects(:read_file_from_filebucket).once.returns('im_a_filebucket')
         @content.each_chunk_from(nil) { |chunk| chunk.should == 'im_a_filebucket' }
       end