(#5566) Treat source only File checksums as syntax errors when used with content

Certain checksum types (ctime, mtime) only make sense when used with the
'source' File parameter, since there is no way to check them on raw
strings.

Given the limitations of the current checksumming implementations, it is
likely to introduce unexpected behavior when using the 'none' checksum
type and either one of the 'source', and 'content' File parameters.
Because of this, it is now a syntax error to use a checksum of 'none' with
either parameter.

Paired-with: Jesse Wolfe <jesse@puppetlabs.com>

3 files changed, 89 insertions, 1 deletions

diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 7c4280bee..c66a53c5e 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -271,6 +271,7 @@ Puppet::Type.newtype(:file) do
   end
 
   CREATORS = [:content, :source, :target]
+  SOURCE_ONLY_CHECKSUMS = [:none, :ctime, :mtime]
 
   validate do
     creator_count = 0
@@ -282,6 +283,12 @@ Puppet::Type.newtype(:file) do
 
     self.fail "You cannot specify a remote recursion without a source" if !self[:source] and self[:recurse] == :remote
 
+    self.fail "You cannot specify source when using checksum 'none'" if self[:checksum] == :none && !self[:source].nil?
+
+    SOURCE_ONLY_CHECKSUMS.each do |checksum_type|
+      self.fail "You cannot specify content when using checksum '#{checksum_type}'" if self[:checksum] == checksum_type && !self[:content].nil?
+    end
+
     self.warning "Possible error: recurselimit is set but not recurse, no recursion will happen" if !self[:recurse] and self[:recurselimit]
   end
 
diff --git a/lib/puppet/util/checksums.rb b/lib/puppet/util/checksums.rb
index 6fdf14ecf..e129301e6 100644
--- a/lib/puppet/util/checksums.rb
+++ b/lib/puppet/util/checksums.rb
@@ -68,7 +68,9 @@ module Puppet::Util::Checksums
     nil
   end
 
-  alias :ctime_stream :mtime_stream
+  def mtime(content)
+    ""
+  end
 
   # Calculate a checksum using Digest::SHA1.
   def sha1(content)
@@ -108,6 +110,12 @@ module Puppet::Util::Checksums
     File.stat(filename).send(:ctime)
   end
 
+  alias :ctime_stream :mtime_stream
+
+  def ctime(content)
+    ""
+  end
+
   # Return a "no checksum"
   def none_file(filename)
     ""
@@ -119,6 +127,10 @@ module Puppet::Util::Checksums
     ""
   end
 
+  def none(content)
+    ""
+  end
+
   private
 
   # Perform an incremental checksum on a file.
diff --git a/spec/unit/type/file_spec.rb b/spec/unit/type/file_spec.rb
index 944bd6bac..698dff52a 100755
--- a/spec/unit/type/file_spec.rb
+++ b/spec/unit/type/file_spec.rb
@@ -823,6 +823,75 @@ describe Puppet::Type.type(:file) do
     end
   end
 
+  describe "when specifying both source, and content properties" do
+    before do
+      @file[:source]  = '/one'
+      @file[:content] = 'file contents'
+    end
+
+    it "should raise an exception" do
+      lambda {@file.validate }.should raise_error(/You cannot specify more than one of/)
+    end
+  end
+
+  describe "when using source" do
+    before do
+      @file[:source]   = '/one'
+    end
+    Puppet::Type::File::ParameterChecksum.value_collection.values.reject {|v| v == :none}.each do |checksum_type|
+      describe "with checksum '#{checksum_type}'" do
+        before do
+          @file[:checksum] = checksum_type
+        end
+
+        it 'should validate' do
+
+          lambda { @file.validate }.should_not raise_error
+        end
+      end
+    end
+
+    describe "with checksum 'none'" do
+      before do
+        @file[:checksum] = :none
+      end
+
+      it 'should raise an exception when validating' do
+        lambda { @file.validate }.should raise_error(/You cannot specify source when using checksum 'none'/)
+      end
+    end
+  end
+
+  describe "when using content" do
+    SOURCE_ONLY_CHECKSUMS = [:none, :ctime, :mtime]
+
+    before do
+      @file[:content] = 'file contents'
+    end
+
+    (Puppet::Type::File::ParameterChecksum.value_collection.values - SOURCE_ONLY_CHECKSUMS).each do |checksum_type|
+      describe "with checksum '#{checksum_type}'" do
+        before do
+          @file[:checksum] = checksum_type
+        end
+
+        it 'should validate' do
+          lambda { @file.validate }.should_not raise_error
+        end
+      end
+    end
+
+    SOURCE_ONLY_CHECKSUMS.each do |checksum_type|
+      describe "with checksum '#{checksum_type}'" do
+        it 'should raise an exception when validating' do
+          @file[:checksum] = checksum_type
+
+          lambda { @file.validate }.should raise_error(/You cannot specify content when using checksum '#{checksum_type}'/)
+        end
+      end
+    end
+  end
+
   describe "when returning resources with :eval_generate" do
     before do
       @graph = stub 'graph', :add_edge => nil