Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
View | Details | Raw Unified | Return to bug 494768 | Differences between
and this patch

Collapse All | Expand All

(-)a/lib/puppet/type/file.rb (-23 / +12 lines)
Lines 736-771 Puppet::Type.newtype(:file) do Link Here
736
  def write(property)
736
  def write(property)
737
    remove_existing(:file)
737
    remove_existing(:file)
738
738
739
    use_temporary_file = write_temporary_file?
739
    assumed_default_mode = 0644
740
    if use_temporary_file
741
      path = "#{self[:path]}.puppettmp_#{rand(10000)}"
742
      path = "#{self[:path]}.puppettmp_#{rand(10000)}" while ::File.exists?(path) or ::File.symlink?(path)
743
    else
744
      path = self[:path]
745
    end
746
740
747
    mode = self.should(:mode) # might be nil
741
    mode = self.should(:mode) # might be nil
748
    umask = mode ? 000 : 022
742
    mode_int = mode ? symbolic_mode_to_int(mode, assumed_default_mode) : nil
749
    mode_int = mode ? symbolic_mode_to_int(mode, 0644) : nil
743
750
744
    if write_temporary_file?
751
    content_checksum = Puppet::Util.withumask(umask) { ::File.open(path, 'wb', mode_int ) { |f| write_content(f) } }
745
      Puppet::Util.replace_file(self[:path], mode_int) do |file|
752
746
        file.binmode
753
    # And put our new file in place
747
        content_checksum = write_content(file)
754
    if use_temporary_file # This is only not true when our file is empty.
748
        file.flush
755
      begin
749
        fail_if_checksum_is_wrong(file.path, content_checksum) if validate_checksum?
756
        fail_if_checksum_is_wrong(path, content_checksum) if validate_checksum?
757
        ::File.rename(path, self[:path])
758
      rescue => detail
759
        fail "Could not rename temporary file #{path} to #{self[:path]}: #{detail}"
760
      ensure
761
        # Make sure the created file gets removed
762
        ::File.unlink(path) if FileTest.exists?(path)
763
      end
750
      end
751
    else
752
      umask = mode ? 000 : 022
753
      Puppet::Util.withumask(umask) { ::File.open(self[:path], 'wb', mode_int ) { |f| write_content(f) } }
764
    end
754
    end
765
755
766
    # make sure all of the modes are actually correct
756
    # make sure all of the modes are actually correct
767
    property_fix
757
    property_fix
768
769
  end
758
  end
770
759
771
  private
760
  private
(-)a/lib/puppet/util.rb (-41 / +22 lines)
Lines 554-569 module Util Link Here
554
    # and specifically handle the platform, which has all sorts of magic.
554
    # and specifically handle the platform, which has all sorts of magic.
555
    # So, unlike Unix, we don't pre-prep security; we use the default "quite
555
    # So, unlike Unix, we don't pre-prep security; we use the default "quite
556
    # secure" tempfile permissions instead.  Magic happens later.
556
    # secure" tempfile permissions instead.  Magic happens later.
557
    unless Puppet.features.microsoft_windows?
557
    if !Puppet.features.microsoft_windows?
558
      # Grab the current file mode, and fall back to the defaults.
558
      # Grab the current file mode, and fall back to the defaults.
559
      stat = file.lstat rescue OpenStruct.new(:mode => default_mode,
559
      if file.exist?
560
                                              :uid  => Process.euid,
560
        stat = file.lstat
561
                                              :gid  => Process.egid)
561
        tempfile.chown(stat.uid, stat.gid)
562
        effective_mode = stat.mode
563
      else
564
        effective_mode = default_mode
565
      end
562
566
563
      # We only care about the bottom four slots, which make the real mode,
567
      if effective_mode
564
      # and not the rest of the platform stat call fluff and stuff.
568
        # We only care about the bottom four slots, which make the real mode,
565
      tempfile.chmod(stat.mode & 07777)
569
        # and not the rest of the platform stat call fluff and stuff.
566
      tempfile.chown(stat.uid, stat.gid)
570
        tempfile.chmod(effective_mode & 07777)
571
      end
567
    end
572
    end
568
573
569
    # OK, now allow the caller to write the content of the file.
574
    # OK, now allow the caller to write the content of the file.
Lines 586-626 module Util Link Here
586
    tempfile.close
591
    tempfile.close
587
592
588
    if Puppet.features.microsoft_windows?
593
    if Puppet.features.microsoft_windows?
589
      # This will appropriately clone the file, but only if the file we are
594
      # Windows ReplaceFile needs a file to exist, so touch handles this
590
      # replacing exists.  Which is kind of annoying; thanks Microsoft.
595
      if !file.exist?
591
      #
596
        FileUtils.touch(file.to_s)
592
      # So, to avoid getting into an infinite loop we will retry once if the
597
        if default_mode
593
      # file doesn't exist, but only the once...
598
          Puppet::Util::Windows::Security.set_mode(default_mode, file.to_s)
594
      have_retried = false
595
596
      begin
597
        # Yes, the arguments are reversed compared to the rename in the rest
598
        # of the world.
599
        Puppet::Util::Windows::File.replace_file(file, tempfile.path)
600
      rescue Puppet::Util::Windows::Error => e
601
        # This might race, but there are enough possible cases that there
602
        # isn't a good, solid "better" way to do this, and the next call
603
        # should fail in the same way anyhow.
604
        raise if have_retried or File.exist?(file)
605
        have_retried = true
606
607
        # OK, so, we can't replace a file that doesn't exist, so let us put
608
        # one in place and set the permissions.  Then we can retry and the
609
        # magic makes this all work.
610
        #
611
        # This is the least-worst option for handling Windows, as far as we
612
        # can determine.
613
        File.open(file, 'a') do |fh|
614
          # this space deliberately left empty for auto-close behaviour,
615
          # append mode, and not actually changing any of the content.
616
        end
599
        end
617
618
        # Set the permissions to what we want.
619
        Puppet::Util::Windows::Security.set_mode(default_mode, file.to_s)
620
621
        # ...and finally retry the operation.
622
        retry
623
      end
600
      end
601
      # Yes, the arguments are reversed compared to the rename in the rest
602
      # of the world.
603
      Puppet::Util::Windows::File.replace_file(file, tempfile.path)
604
624
    else
605
    else
625
      File.rename(tempfile.path, file)
606
      File.rename(tempfile.path, file)
626
    end
607
    end
(-)a/spec/integration/type/file_spec.rb (-14 / +1 lines)
Lines 494-513 describe Puppet::Type.type(:file) do Link Here
494
      bucket.bucket.getfile(foomd5).should == "fooyay"
494
      bucket.bucket.getfile(foomd5).should == "fooyay"
495
      bucket.bucket.getfile(barmd5).should == "baryay"
495
      bucket.bucket.getfile(barmd5).should == "baryay"
496
    end
496
    end
497
498
    it "should propagate failures encountered when renaming the temporary file" do
499
      file = described_class.new :path => path, :content => "foo"
500
      file.stubs(:perform_backup).returns(true)
501
502
      catalog.add_resource file
503
504
      File.open(path, "w") { |f| f.print "bar" }
505
506
      File.expects(:rename).raises ArgumentError
507
508
      expect { file.write(:content) }.to raise_error(Puppet::Error, /Could not rename temporary file/)
509
      File.read(path).should == "bar"
510
    end
511
  end
497
  end
512
498
513
  describe "when recursing" do
499
  describe "when recursing" do
Lines 1034-1039 describe Puppet::Type.type(:file) do Link Here
1034
    describe "on Windows systems", :if => Puppet.features.microsoft_windows? do
1020
    describe "on Windows systems", :if => Puppet.features.microsoft_windows? do
1035
      it "should provide valid default values when ACLs are not supported" do
1021
      it "should provide valid default values when ACLs are not supported" do
1036
        Puppet::Util::Windows::Security.stubs(:supports_acl?).with(source).returns false
1022
        Puppet::Util::Windows::Security.stubs(:supports_acl?).with(source).returns false
1023
        Puppet::Util::Windows::Security.stubs(:supports_acl?).with(path).returns true
1037
1024
1038
        file = described_class.new(
1025
        file = described_class.new(
1039
          :path   => path,
1026
          :path   => path,
(-)a/spec/unit/type/file_spec.rb (-30 lines)
Lines 1121-1155 describe Puppet::Type.type(:file) do Link Here
1121
  end
1121
  end
1122
1122
1123
  describe "#write" do
1123
  describe "#write" do
1124
    it "should propagate failures encountered when renaming the temporary file" do
1125
      File.stubs(:open)
1126
      File.expects(:rename).raises ArgumentError
1127
1128
      file[:backup] = 'puppet'
1129
1130
      file.stubs(:validate_checksum?).returns(false)
1131
1132
      property = stub('content_property', :actual_content => "something", :length => "something".length)
1133
      file.stubs(:property).with(:content).returns(property)
1134
1135
      expect { file.write(:content) }.to raise_error(Puppet::Error)
1136
    end
1137
1138
    it "should delegate writing to the content property" do
1139
      filehandle = stub_everything 'fh'
1140
      File.stubs(:open).yields(filehandle)
1141
      File.stubs(:rename)
1142
      property = stub('content_property', :actual_content => "something", :length => "something".length)
1143
      file[:backup] = 'puppet'
1144
1145
      file.stubs(:validate_checksum?).returns(false)
1146
      file.stubs(:property).with(:content).returns(property)
1147
1148
      property.expects(:write).with(filehandle)
1149
1150
      file.write(:content)
1151
    end
1152
1153
    describe "when validating the checksum" do
1124
    describe "when validating the checksum" do
1154
      before { file.stubs(:validate_checksum?).returns(true) }
1125
      before { file.stubs(:validate_checksum?).returns(true) }
1155
1126
1156
- 

Return to bug 494768