diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 21dc614d9..e4bc0f299 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -1,487 +1,487 @@
 # the class that actually walks our resource/property tree, collects the changes,
 # and performs them
 
 require 'puppet'
 require 'puppet/util/tagging'
 require 'puppet/application'
 require 'digest/sha1'
 
 class Puppet::Transaction
   require 'puppet/transaction/event'
   require 'puppet/transaction/event_manager'
   require 'puppet/transaction/resource_harness'
   require 'puppet/resource/status'
 
   attr_accessor :component, :catalog, :ignoreschedules, :for_network_device
   attr_accessor :configurator
 
   # The report, once generated.
   attr_reader :report
 
   # Routes and stores any events and subscriptions.
   attr_reader :event_manager
 
   # Handles most of the actual interacting with resources
   attr_reader :resource_harness
 
   include Puppet::Util
   include Puppet::Util::Tagging
 
   # Wraps application run state check to flag need to interrupt processing
   def stop_processing?
     Puppet::Application.stop_requested?
   end
 
   # Add some additional times for reporting
   def add_times(hash)
     hash.each do |name, num|
       report.add_times(name, num)
     end
   end
 
   # Are there any failed resources in this transaction?
   def any_failed?
     report.resource_statuses.values.detect { |status| status.failed? }
   end
 
   # Apply all changes for a resource
   def apply(resource, ancestor = nil)
     status = resource_harness.evaluate(resource)
     add_resource_status(status)
     event_manager.queue_events(ancestor || resource, status.events) unless status.failed?
   rescue => detail
     resource.err "Could not evaluate: #{detail}"
   end
 
   # Find all of the changed resources.
   def changed?
     report.resource_statuses.values.find_all { |status| status.changed }.collect { |status| catalog.resource(status.resource) }
   end
 
   # Find all of the applied resources (including failed attempts).
   def applied_resources
     report.resource_statuses.values.collect { |status| catalog.resource(status.resource) }
   end
 
   # Copy an important relationships from the parent to the newly-generated
   # child resource.
   def add_conditional_directed_dependency(parent, child, label=nil)
     relationship_graph.add_vertex(child)
     edge = parent.depthfirst? ? [child, parent] : [parent, child]
     if relationship_graph.edge?(*edge.reverse)
       parent.debug "Skipping automatic relationship to #{child}"
     else
       relationship_graph.add_edge(edge[0],edge[1],label)
     end
   end
 
   # Evaluate a single resource.
   def eval_resource(resource, ancestor = nil)
     if skip?(resource)
       resource_status(resource).skipped = true
     else
       resource_status(resource).scheduled = true
       apply(resource, ancestor)
     end
 
     # Check to see if there are any events queued for this resource
     event_manager.process_events(resource)
   end
 
   # This method does all the actual work of running a transaction.  It
   # collects all of the changes, executes them, and responds to any
   # necessary events.
   def evaluate
     add_dynamically_generated_resources
 
     Puppet.info "Applying configuration version '#{catalog.version}'" if catalog.version
 
     relationship_graph.traverse do |resource|
       if resource.is_a?(Puppet::Type::Component)
         Puppet.warning "Somehow left a component in the relationship graph"
       else
         seconds = thinmark { eval_resource(resource) }
         resource.info "Evaluated in %0.2f seconds" % seconds if Puppet[:evaltrace] and @catalog.host_config?
       end
     end
 
     Puppet.debug "Finishing transaction #{object_id}"
   end
 
   def events
     event_manager.events
   end
 
   def failed?(resource)
     s = resource_status(resource) and s.failed?
   end
 
   # Does this resource have any failed dependencies?
   def failed_dependencies?(resource)
     # First make sure there are no failed dependencies.  To do this,
     # we check for failures in any of the vertexes above us.  It's not
     # enough to check the immediate dependencies, which is why we use
     # a tree from the reversed graph.
     found_failed = false
 
 
     # When we introduced the :whit into the graph, to reduce the combinatorial
     # explosion of edges, we also ended up reporting failures for containers
     # like class and stage.  This is undesirable; while just skipping the
     # output isn't perfect, it is RC-safe. --daniel 2011-06-07
     suppress_report = (resource.class == Puppet::Type.type(:whit))
 
     relationship_graph.dependencies(resource).each do |dep|
       next unless failed?(dep)
       found_failed = true
 
       # See above. --daniel 2011-06-06
       unless suppress_report then
         resource.notice "Dependency #{dep} has failures: #{resource_status(dep).failed}"
       end
     end
 
     found_failed
   end
 
   def eval_generate(resource)
     return false unless resource.respond_to?(:eval_generate)
     raise Puppet::DevError,"Depthfirst resources are not supported by eval_generate" if resource.depthfirst?
     begin
       made = resource.eval_generate.uniq
       return false if made.empty?
-      made = Hash[made.map(&:name).zip(made)]
+      made = made.inject({}) {|a,v| a.merge(v.name => v) }
     rescue => detail
       puts detail.backtrace if Puppet[:trace]
       resource.err "Failed to generate additional resources using 'eval_generate: #{detail}"
       return false
     end
     made.values.each do |res|
       begin
         res.tag(*resource.tags)
         @catalog.add_resource(res)
         res.finish
       rescue Puppet::Resource::Catalog::DuplicateResourceError
         res.info "Duplicate generated resource; skipping"
       end
     end
     sentinel = Puppet::Type.type(:whit).new(:name => "completed_#{resource.title}", :catalog => resource.catalog)
 
     # The completed whit is now the thing that represents the resource is done
     relationship_graph.adjacent(resource,:direction => :out,:type => :edges).each { |e|
       # But children run as part of the resource, not after it
       next if made[e.target.name]
 
       add_conditional_directed_dependency(sentinel, e.target, e.label)
       relationship_graph.remove_edge! e
     }
 
     default_label = Puppet::Resource::Catalog::Default_label
     made.values.each do |res|
       # Depend on the nearest ancestor we generated, falling back to the
       # resource if we have none
       parent_name = res.ancestors.find { |a| made[a] and made[a] != res }
       parent = made[parent_name] || resource
 
       add_conditional_directed_dependency(parent, res)
 
       # This resource isn't 'completed' until each child has run
       add_conditional_directed_dependency(res, sentinel, default_label)
     end
 
     # This edge allows the resource's events to propagate, though it isn't
     # strictly necessary for ordering purposes
     add_conditional_directed_dependency(resource, sentinel, default_label)
     true
   end
 
   # A general method for recursively generating new resources from a
   # resource.
   def generate_additional_resources(resource)
     return unless resource.respond_to?(:generate)
     begin
       made = resource.generate
     rescue => detail
       puts detail.backtrace if Puppet[:trace]
       resource.err "Failed to generate additional resources using 'generate': #{detail}"
     end
     return unless made
     made = [made] unless made.is_a?(Array)
     made.uniq.each do |res|
       begin
         res.tag(*resource.tags)
         @catalog.add_resource(res)
         res.finish
         add_conditional_directed_dependency(resource, res)
         generate_additional_resources(res)
       rescue Puppet::Resource::Catalog::DuplicateResourceError
         res.info "Duplicate generated resource; skipping"
       end
     end
   end
 
   def add_dynamically_generated_resources
     @catalog.vertices.each { |resource| generate_additional_resources(resource) }
   end
 
   # Should we ignore tags?
   def ignore_tags?
     ! (@catalog.host_config? or Puppet[:name] == "puppet")
   end
 
   # this should only be called by a Puppet::Type::Component resource now
   # and it should only receive an array
   def initialize(catalog, report = nil)
     @catalog = catalog
 
     @report = report || Puppet::Transaction::Report.new("apply", catalog.version)
 
     @event_manager = Puppet::Transaction::EventManager.new(self)
 
     @resource_harness = Puppet::Transaction::ResourceHarness.new(self)
 
     @prefetched_providers = Hash.new { |h,k| h[k] = {} }
   end
 
   def resources_by_provider(type_name, provider_name)
     unless @resources_by_provider
       @resources_by_provider = Hash.new { |h, k| h[k] = Hash.new { |h, k| h[k] = {} } }
 
       @catalog.vertices.each do |resource|
         if resource.class.attrclass(:provider)
           prov = resource.provider && resource.provider.class.name
           @resources_by_provider[resource.type][prov][resource.name] = resource
         end
       end
     end
 
     @resources_by_provider[type_name][provider_name] || {}
   end
 
   def prefetch_if_necessary(resource)
     provider_class = resource.provider.class
     return unless provider_class.respond_to?(:prefetch) and !prefetched_providers[resource.type][provider_class.name]
 
     resources = resources_by_provider(resource.type, provider_class.name)
 
     if provider_class == resource.class.defaultprovider
       providerless_resources = resources_by_provider(resource.type, nil)
       providerless_resources.values.each {|res| res.provider = provider_class.name}
       resources.merge! providerless_resources
     end
 
     prefetch(provider_class, resources)
   end
 
   attr_reader :prefetched_providers
 
   # Prefetch any providers that support it, yo.  We don't support prefetching
   # types, just providers.
   def prefetch(provider_class, resources)
     type_name = provider_class.resource_type.name
     return if @prefetched_providers[type_name][provider_class.name]
     Puppet.debug "Prefetching #{provider_class.name} resources for #{type_name}"
     begin
       provider_class.prefetch(resources)
     rescue => detail
       puts detail.backtrace if Puppet[:trace]
       Puppet.err "Could not prefetch #{type_name} provider '#{provider_class.name}': #{detail}"
     end
     @prefetched_providers[type_name][provider_class.name] = true
   end
 
   # We want to monitor changes in the relationship graph of our
   # catalog but this is complicated by the fact that the catalog
   # both is_a graph and has_a graph, by the fact that changes to
   # the structure of the object can have adverse serialization
   # effects, by threading issues, by order-of-initialization issues,
   # etc.
   #
   # Since the proper lifetime/scope of the monitoring is a transaction
   # and the transaction is already commiting a mild law-of-demeter
   # transgression, we cut the Gordian knot here by simply wrapping the
   # transaction's view of the resource graph to capture and maintain
   # the information we need.  Nothing outside the transaction needs
   # this information, and nothing outside the transaction can see it
   # except via the Transaction#relationship_graph
 
   class Relationship_graph_wrapper
     require 'puppet/rb_tree_map'
     attr_reader :real_graph,:transaction,:ready,:generated,:done,:blockers,:unguessable_deterministic_key
     def initialize(real_graph,transaction)
       @real_graph = real_graph
       @transaction = transaction
       @ready = Puppet::RbTreeMap.new
       @generated = {}
       @done = {}
       @blockers = {}
       @unguessable_deterministic_key = Hash.new { |h,k| h[k] = Digest::SHA1.hexdigest("NaCl, MgSO4 (salts) and then #{k.ref}") }
       @providerless_types = []
       vertices.each do |v|
         blockers[v] = direct_dependencies_of(v).length
         enqueue(v) if blockers[v] == 0
       end
     end
     def method_missing(*args,&block)
       real_graph.send(*args,&block)
     end
     def add_vertex(v)
       real_graph.add_vertex(v)
     end
     def add_edge(f,t,label=nil)
       key = unguessable_deterministic_key[t]
 
       ready.delete(key)
 
       real_graph.add_edge(f,t,label)
     end
     # Decrement the blocker count for the resource by 1. If the number of
     # blockers is unknown, count them and THEN decrement by 1.
     def unblock(resource)
       blockers[resource] ||= direct_dependencies_of(resource).select { |r2| !done[r2] }.length
       if blockers[resource] > 0
         blockers[resource] -= 1
       else
         resource.warning "appears to have a negative number of dependencies"
       end
       blockers[resource] <= 0
     end
     def enqueue(*resources)
       resources.each do |resource|
         key = unguessable_deterministic_key[resource]
         ready[key] = resource
       end
     end
     def finish(resource)
       direct_dependents_of(resource).each do |v|
         enqueue(v) if unblock(v)
       end
       done[resource] = true
     end
     def next_resource
       ready.delete_min
     end
     def traverse(&block)
       real_graph.report_cycles_in_graph
 
       deferred_resources = []
 
       while (resource = next_resource) && !transaction.stop_processing?
         if resource.suitable?
           made_progress = true
 
           transaction.prefetch_if_necessary(resource)
 
           # If we generated resources, we don't know what they are now
           # blocking, so we opt to recompute it, rather than try to track every
           # change that would affect the number.
           blockers.clear if transaction.eval_generate(resource)
 
           yield resource
 
           finish(resource)
         else
           deferred_resources << resource
         end
 
         if ready.empty? and deferred_resources.any?
           if made_progress
             enqueue(*deferred_resources)
           else
             fail_unsuitable_resources(deferred_resources)
           end
 
           made_progress = false
           deferred_resources = []
         end
       end
 
       # Just once per type. No need to punish the user.
       @providerless_types.uniq.each do |type|
         Puppet.err "Could not find a suitable provider for #{type}"
       end
     end
 
     def fail_unsuitable_resources(resources)
       resources.each do |resource|
         # We don't automatically assign unsuitable providers, so if there
         # is one, it must have been selected by the user.
         if resource.provider
           resource.err "Provider #{resource.provider.class.name} is not functional on this host"
         else
           @providerless_types << resource.type
         end
 
         transaction.resource_status(resource).failed = true
 
         finish(resource)
       end
     end
   end
 
   def relationship_graph
     @relationship_graph ||= Relationship_graph_wrapper.new(catalog.relationship_graph,self)
   end
 
   def add_resource_status(status)
     report.add_resource_status status
   end
 
   def resource_status(resource)
     report.resource_statuses[resource.to_s] || add_resource_status(Puppet::Resource::Status.new(resource))
   end
 
   # Is the resource currently scheduled?
   def scheduled?(resource)
     self.ignoreschedules or resource_harness.scheduled?(resource_status(resource), resource)
   end
 
   # Should this resource be skipped?
   def skip?(resource)
     if missing_tags?(resource)
       resource.debug "Not tagged with #{tags.join(", ")}"
     elsif ! scheduled?(resource)
       resource.debug "Not scheduled"
     elsif failed_dependencies?(resource)
       # When we introduced the :whit into the graph, to reduce the combinatorial
       # explosion of edges, we also ended up reporting failures for containers
       # like class and stage.  This is undesirable; while just skipping the
       # output isn't perfect, it is RC-safe. --daniel 2011-06-07
       unless resource.class == Puppet::Type.type(:whit) then
         resource.warning "Skipping because of failed dependencies"
       end
     elsif resource.virtual?
       resource.debug "Skipping because virtual"
     elsif resource.appliable_to_device? ^ for_network_device
       resource.debug "Skipping #{resource.appliable_to_device? ? 'device' : 'host'} resources because running on a #{for_network_device ? 'device' : 'host'}"
     else
       return false
     end
     true
   end
 
   # The tags we should be checking.
   def tags
     self.tags = Puppet[:tags] unless defined?(@tags)
 
     super
   end
 
   def handle_qualified_tags( qualified )
     # The default behavior of Puppet::Util::Tagging is
     # to split qualified tags into parts. That would cause
     # qualified tags to match too broadly here.
     return
   end
 
   # Is this resource tagged appropriately?
   def missing_tags?(resource)
     return false if ignore_tags?
     return false if tags.empty?
 
     not resource.tagged?(*tags)
   end
 end
 
 require 'puppet/transaction/report'
 
diff --git a/spec/integration/util/windows/security_spec.rb b/spec/integration/util/windows/security_spec.rb
index 57e1ea8d3..3990c2908 100755
--- a/spec/integration/util/windows/security_spec.rb
+++ b/spec/integration/util/windows/security_spec.rb
@@ -1,631 +1,631 @@
 #!/usr/bin/env ruby
 require 'spec_helper'
 
 require 'puppet/util/adsi'
 
 if Puppet.features.microsoft_windows?
   class WindowsSecurityTester
     require 'puppet/util/windows/security'
     include Puppet::Util::Windows::Security
   end
 end
 
 describe "Puppet::Util::Windows::Security", :if => Puppet.features.microsoft_windows? do
   include PuppetSpec::Files
 
   before :all do
     @sids = {
       :current_user => Puppet::Util::ADSI.sid_for_account(Sys::Admin.get_login),
       :admin => Puppet::Util::ADSI.sid_for_account("Administrator"),
       :guest => Puppet::Util::ADSI.sid_for_account("Guest"),
       :users => Win32::Security::SID::BuiltinUsers,
       :power_users => Win32::Security::SID::PowerUsers,
     }
   end
 
   let (:sids) { @sids }
   let (:winsec) { WindowsSecurityTester.new }
 
   shared_examples_for "only child owner" do
     it "should allow child owner" do
       check_child_owner
     end
 
     it "should deny parent owner" do
       lambda { check_parent_owner }.should raise_error(Errno::EACCES)
     end
 
     it "should deny group" do
       lambda { check_group }.should raise_error(Errno::EACCES)
     end
 
     it "should deny other" do
       lambda { check_other }.should raise_error(Errno::EACCES)
     end
   end
 
   shared_examples_for "a securable object" do
     describe "on a volume that doesn't support ACLs" do
       [:owner, :group, :mode].each do |p|
         it "should return nil #{p}" do
           winsec.stubs(:supports_acl?).returns false
 
           winsec.send("get_#{p}", path).should be_nil
         end
       end
     end
 
     describe "on a volume that supports ACLs" do
       describe "for a normal user" do
         before :each do
           Puppet.features.stubs(:root?).returns(false)
         end
 
         after :each do
           winsec.set_mode(WindowsSecurityTester::S_IRWXU, parent)
           winsec.set_mode(WindowsSecurityTester::S_IRWXU, path) if File.exists?(path)
         end
 
         describe "#supports_acl?" do
           %w[c:/ c:\\ c:/windows/system32 \\\\localhost\\C$ \\\\127.0.0.1\\C$\\foo].each do |path|
             it "should accept #{path}" do
               winsec.should be_supports_acl(path)
             end
           end
 
           it "should raise an exception if it cannot get volume information" do
             expect {
               winsec.supports_acl?('foobar')
             }.to raise_error(Puppet::Error, /Failed to get volume information/)
           end
         end
 
         describe "#owner=" do
           it "should allow setting to the current user" do
             winsec.set_owner(sids[:current_user], path)
           end
 
           it "should raise an exception when setting to a different user" do
             lambda { winsec.set_owner(sids[:guest], path) }.should raise_error(Puppet::Error, /This security ID may not be assigned as the owner of this object./)
           end
         end
 
         describe "#owner" do
           it "it should not be empty" do
             winsec.get_owner(path).should_not be_empty
           end
 
           it "should raise an exception if an invalid path is provided" do
             lambda { winsec.get_owner("c:\\doesnotexist.txt") }.should raise_error(Puppet::Error, /The system cannot find the file specified./)
           end
         end
 
         describe "#group=" do
           it "should allow setting to a group the current owner is a member of" do
             winsec.set_group(sids[:users], path)
           end
 
           # Unlike unix, if the user has permission to WRITE_OWNER, which the file owner has by default,
           # then they can set the primary group to a group that the user does not belong to.
           it "should allow setting to a group the current owner is not a member of" do
             winsec.set_group(sids[:power_users], path)
           end
         end
 
         describe "#group" do
           it "should not be empty" do
             winsec.get_group(path).should_not be_empty
           end
 
           it "should raise an exception if an invalid path is provided" do
             lambda { winsec.get_group("c:\\doesnotexist.txt") }.should raise_error(Puppet::Error, /The system cannot find the file specified./)
           end
         end
 
         describe "#mode=" do
-          (0000..0700).step(0100).each do |mode|
+          (0000..0700).step(0100) do |mode|
             it "should enforce mode #{mode.to_s(8)}" do
               winsec.set_mode(mode, path)
 
               check_access(mode, path)
             end
           end
 
           it "should round-trip all 128 modes that do not require deny ACEs" do
             0.upto(1).each do |s|
               0.upto(7).each do |u|
                 0.upto(u).each do |g|
                   0.upto(g).each do |o|
                     # if user is superset of group, and group superset of other, then
                     # no deny ace is required, and mode can be converted to win32
                     # access mask, and back to mode without loss of information
                     # (provided the owner and group are not the same)
                     next if ((u & g) != g) or ((g & o) != o)
 
                     mode = (s << 9 | u << 6 | g << 3 | o << 0)
                     winsec.set_mode(mode, path)
                     winsec.get_mode(path).to_s(8).should == mode.to_s(8)
                   end
                 end
               end
             end
           end
 
           describe "for modes that require deny aces" do
             it "should map everyone to group and owner" do
               winsec.set_mode(0426, path)
               winsec.get_mode(path).to_s(8).should == "666"
             end
 
             it "should combine user and group modes when owner and group sids are equal" do
               winsec.set_group(winsec.get_owner(path), path)
 
               winsec.set_mode(0410, path)
               winsec.get_mode(path).to_s(8).should == "550"
             end
           end
 
           describe "for read-only objects" do
             before :each do
               winsec.add_attributes(path, WindowsSecurityTester::FILE_ATTRIBUTE_READONLY)
               (winsec.get_attributes(path) & WindowsSecurityTester::FILE_ATTRIBUTE_READONLY).should be_nonzero
             end
 
             it "should make them writable if any sid has write permission" do
               winsec.set_mode(WindowsSecurityTester::S_IWUSR, path)
               (winsec.get_attributes(path) & WindowsSecurityTester::FILE_ATTRIBUTE_READONLY).should == 0
             end
 
             it "should leave them read-only if no sid has write permission" do
               winsec.set_mode(WindowsSecurityTester::S_IRUSR | WindowsSecurityTester::S_IXGRP, path)
               (winsec.get_attributes(path) & WindowsSecurityTester::FILE_ATTRIBUTE_READONLY).should be_nonzero
             end
           end
 
           it "should raise an exception if an invalid path is provided" do
             lambda { winsec.set_mode(sids[:guest], "c:\\doesnotexist.txt") }.should raise_error(Puppet::Error, /The system cannot find the file specified./)
           end
         end
 
         describe "#mode" do
           it "should report when extra aces are encounted" do
             winsec.set_acl(path, true) do |acl|
               (544..547).each do |rid|
                 winsec.add_access_allowed_ace(acl, WindowsSecurityTester::STANDARD_RIGHTS_ALL, "S-1-5-32-#{rid}")
               end
             end
             mode = winsec.get_mode(path)
             (mode & WindowsSecurityTester::S_IEXTRA).should_not == 0
           end
 
           it "should warn if a deny ace is encountered" do
             winsec.set_acl(path) do |acl|
               winsec.add_access_denied_ace(acl, WindowsSecurityTester::FILE_GENERIC_WRITE, sids[:guest])
               winsec.add_access_allowed_ace(acl, WindowsSecurityTester::STANDARD_RIGHTS_ALL | WindowsSecurityTester::SPECIFIC_RIGHTS_ALL, sids[:current_user])
             end
 
             Puppet.expects(:warning).with("Unsupported access control entry type: 0x1")
 
             winsec.get_mode(path)
           end
 
           it "should skip inherit-only ace" do
             winsec.set_acl(path) do |acl|
               winsec.add_access_allowed_ace(acl, WindowsSecurityTester::STANDARD_RIGHTS_ALL | WindowsSecurityTester::SPECIFIC_RIGHTS_ALL, sids[:current_user])
               winsec.add_access_allowed_ace(acl, WindowsSecurityTester::FILE_GENERIC_READ, Win32::Security::SID::Everyone, WindowsSecurityTester::INHERIT_ONLY_ACE | WindowsSecurityTester::OBJECT_INHERIT_ACE)
             end
 
             (winsec.get_mode(path) & WindowsSecurityTester::S_IRWXO).should == 0
           end
 
           it "should raise an exception if an invalid path is provided" do
             lambda { winsec.get_mode("c:\\doesnotexist.txt") }.should raise_error(Puppet::Error, /The system cannot find the file specified./)
           end
         end
 
         describe "inherited access control entries" do
           it "should be absent when the access control list is protected" do
             winsec.set_mode(WindowsSecurityTester::S_IRWXU, path)
             (winsec.get_mode(path) & WindowsSecurityTester::S_IEXTRA).should == 0
           end
 
           it "should be present when the access control list is unprotected" do
             # add a bunch of aces to the parent with permission to add children
             allow = WindowsSecurityTester::STANDARD_RIGHTS_ALL | WindowsSecurityTester::SPECIFIC_RIGHTS_ALL
             inherit = WindowsSecurityTester::OBJECT_INHERIT_ACE | WindowsSecurityTester::CONTAINER_INHERIT_ACE
 
             winsec.set_acl(parent, true) do |acl|
               winsec.add_access_allowed_ace(acl, allow, "S-1-1-0", inherit) # everyone
 
               (544..547).each do |rid|
                 winsec.add_access_allowed_ace(acl, WindowsSecurityTester::STANDARD_RIGHTS_ALL, "S-1-5-32-#{rid}", inherit)
               end
             end
 
             # unprotect child, it should inherit from parent
             winsec.set_mode(WindowsSecurityTester::S_IRWXU, path, false)
             (winsec.get_mode(path) & WindowsSecurityTester::S_IEXTRA).should == WindowsSecurityTester::S_IEXTRA
           end
         end
       end
 
       describe "for an administrator", :if => Puppet.features.root? do
         before :each do
           winsec.set_mode(WindowsSecurityTester::S_IRWXU | WindowsSecurityTester::S_IRWXG, path)
           winsec.set_group(sids[:guest], path)
           winsec.set_owner(sids[:guest], path)
           lambda { File.open(path, 'r') }.should raise_error(Errno::EACCES)
         end
 
         after :each do
           if File.exists?(path)
             winsec.set_owner(sids[:current_user], path)
             winsec.set_mode(WindowsSecurityTester::S_IRWXU, path)
           end
         end
 
         describe "#owner=" do
           it "should accept a user sid" do
             winsec.set_owner(sids[:admin], path)
             winsec.get_owner(path).should == sids[:admin]
           end
 
           it "should accept a group sid" do
             winsec.set_owner(sids[:power_users], path)
             winsec.get_owner(path).should == sids[:power_users]
           end
 
           it "should raise an exception if an invalid sid is provided" do
             lambda { winsec.set_owner("foobar", path) }.should raise_error(Puppet::Error, /Failed to convert string SID/)
           end
 
           it "should raise an exception if an invalid path is provided" do
             lambda { winsec.set_owner(sids[:guest], "c:\\doesnotexist.txt") }.should raise_error(Puppet::Error, /The system cannot find the file specified./)
           end
         end
 
         describe "#group=" do
           it "should accept a group sid" do
             winsec.set_group(sids[:power_users], path)
             winsec.get_group(path).should == sids[:power_users]
           end
 
           it "should accept a user sid" do
             winsec.set_group(sids[:admin], path)
             winsec.get_group(path).should == sids[:admin]
           end
 
           it "should allow owner and group to be the same sid" do
             winsec.set_mode(0610, path)
             winsec.set_owner(sids[:power_users], path)
             winsec.set_group(sids[:power_users], path)
 
             winsec.get_owner(path).should == sids[:power_users]
             winsec.get_group(path).should == sids[:power_users]
             # note group execute permission added to user ace, and then group rwx value
             # reflected to match
             winsec.get_mode(path).to_s(8).should == "770"
           end
 
           it "should raise an exception if an invalid sid is provided" do
             lambda { winsec.set_group("foobar", path) }.should raise_error(Puppet::Error, /Failed to convert string SID/)
           end
 
           it "should raise an exception if an invalid path is provided" do
             lambda { winsec.set_group(sids[:guest], "c:\\doesnotexist.txt") }.should raise_error(Puppet::Error, /The system cannot find the file specified./)
           end
         end
 
         describe "when the sid is NULL" do
           it "should retrieve an empty owner sid"
           it "should retrieve an empty group sid"
         end
 
         describe "when the sid refers to a deleted trustee" do
           it "should retrieve the user sid" do
             sid = nil
             user = Puppet::Util::ADSI::User.create("delete_me_user")
             user.commit
             begin
               sid = Sys::Admin::get_user(user.name).sid
               winsec.set_owner(sid, path)
               winsec.set_mode(WindowsSecurityTester::S_IRWXU, path)
             ensure
               Puppet::Util::ADSI::User.delete(user.name)
             end
 
             winsec.get_owner(path).should == sid
             winsec.get_mode(path).should == WindowsSecurityTester::S_IRWXU
           end
 
           it "should retrieve the group sid" do
             sid = nil
             group = Puppet::Util::ADSI::Group.create("delete_me_group")
             group.commit
             begin
               sid = Sys::Admin::get_group(group.name).sid
               winsec.set_group(sid, path)
               winsec.set_mode(WindowsSecurityTester::S_IRWXG, path)
             ensure
               Puppet::Util::ADSI::Group.delete(group.name)
             end
             winsec.get_group(path).should == sid
             winsec.get_mode(path).should == WindowsSecurityTester::S_IRWXG
           end
         end
 
         describe "#mode" do
           it "should deny all access when the DACL is empty" do
             winsec.set_acl(path, true) { |acl| }
 
             winsec.get_mode(path).should == 0
           end
 
           # REMIND: ruby crashes when trying to set a NULL DACL
           # it "should allow all when it is nil" do
           #   winsec.set_owner(sids[:current_user], path)
           #   winsec.open_file(path, WindowsSecurityTester::READ_CONTROL | WindowsSecurityTester::WRITE_DAC) do |handle|
           #     winsec.set_security_info(handle, WindowsSecurityTester::DACL_SECURITY_INFORMATION | WindowsSecurityTester::PROTECTED_DACL_SECURITY_INFORMATION, nil)
           #   end
           #   winsec.get_mode(path).to_s(8).should == "777"
           # end
         end
 
         describe "#string_to_sid_ptr" do
           it "should raise an error if an invalid SID is specified" do
             expect do
               winsec.string_to_sid_ptr('foobar')
             end.to raise_error(Puppet::Util::Windows::Error) { |error| error.code.should == 1337 }
           end
 
           it "should yield if a block is given" do
             yielded = nil
             winsec.string_to_sid_ptr('S-1-1-0') do |sid|
               yielded = sid
             end
             yielded.should_not be_nil
           end
 
           it "should allow no block to be specified" do
             winsec.string_to_sid_ptr('S-1-1-0').should be_true
           end
         end
 
         describe "when the parent directory" do
           before :each do
             winsec.set_owner(sids[:current_user], parent)
             winsec.set_owner(sids[:current_user], path)
             winsec.set_mode(0777, path, false)
           end
 
           def check_child_owner
             winsec.set_group(sids[:guest], parent)
             winsec.set_owner(sids[:guest], parent)
 
             check_delete(path)
           end
 
         def check_parent_owner
           winsec.set_group(sids[:guest], path)
           winsec.set_owner(sids[:guest], path)
 
           check_delete(path)
         end
 
         def check_group
           winsec.set_group(sids[:current_user], path)
           winsec.set_owner(sids[:guest], path)
 
           winsec.set_owner(sids[:guest], parent)
 
           check_delete(path)
         end
 
         def check_other
           winsec.set_group(sids[:guest], path)
           winsec.set_owner(sids[:guest], path)
 
           winsec.set_owner(sids[:guest], parent)
 
           check_delete(path)
         end
 
         describe "is writable and executable" do
           describe "and sticky bit is set" do
             before :each do
               winsec.set_mode(01777, parent)
             end
 
             it "should allow child owner" do
               check_child_owner
             end
 
             it "should allow parent owner" do
               check_parent_owner
             end
 
             it "should deny group" do
               lambda { check_group }.should raise_error(Errno::EACCES)
             end
 
             it "should deny other" do
               lambda { check_other }.should raise_error(Errno::EACCES)
             end
           end
 
           describe "and sticky bit is not set" do
             before :each do
               winsec.set_mode(0777, parent)
             end
 
             it "should allow child owner" do
               check_child_owner
             end
 
             it "should allow parent owner" do
               check_parent_owner
             end
 
             it "should allow group" do
               check_group
             end
 
             it "should allow other" do
               check_other
             end
           end
         end
 
         describe "is not writable" do
           before :each do
             winsec.set_mode(0555, parent)
           end
 
           it_behaves_like "only child owner"
         end
 
         describe "is not executable" do
           before :each do
             winsec.set_mode(0666, parent)
           end
 
           it_behaves_like "only child owner"
         end
       end
     end
   end
   end
 
   describe "file" do
     let (:parent) do
       tmpdir('win_sec_test_file')
     end
 
     let (:path) do
       path = File.join(parent, 'childfile')
       File.new(path, 'w').close
       path
     end
 
     it_behaves_like "a securable object" do
       def check_access(mode, path)
         if (mode & WindowsSecurityTester::S_IRUSR).nonzero?
           check_read(path)
         else
           lambda { check_read(path) }.should raise_error(Errno::EACCES)
         end
 
         if (mode & WindowsSecurityTester::S_IWUSR).nonzero?
           check_write(path)
         else
           lambda { check_write(path) }.should raise_error(Errno::EACCES)
         end
 
         if (mode & WindowsSecurityTester::S_IXUSR).nonzero?
           lambda { check_execute(path) }.should raise_error(Errno::ENOEXEC)
         else
           lambda { check_execute(path) }.should raise_error(Errno::EACCES)
         end
       end
 
       def check_read(path)
         File.open(path, 'r').close
       end
 
       def check_write(path)
         File.open(path, 'w').close
       end
 
       def check_execute(path)
         Kernel.exec(path)
       end
 
       def check_delete(path)
         File.delete(path)
       end
     end
 
     describe "locked files" do
       let (:explorer) { File.join(Dir::WINDOWS, "explorer.exe") }
 
       it "should get the owner" do
         winsec.get_owner(explorer).should match /^S-1-5-/
       end
 
       it "should get the group" do
         winsec.get_group(explorer).should match /^S-1-5-/
       end
 
       it "should get the mode" do
         winsec.get_mode(explorer).should == (WindowsSecurityTester::S_IRWXU | WindowsSecurityTester::S_IRWXG | WindowsSecurityTester::S_IEXTRA)
       end
     end
   end
 
   describe "directory" do
     let (:parent) do
       tmpdir('win_sec_test_dir')
     end
 
     let (:path) do
       path = File.join(parent, 'childdir')
       Dir.mkdir(path)
       path
     end
 
     it_behaves_like "a securable object" do
       def check_access(mode, path)
         if (mode & WindowsSecurityTester::S_IRUSR).nonzero?
           check_read(path)
         else
           lambda { check_read(path) }.should raise_error(Errno::EACCES)
         end
 
         if (mode & WindowsSecurityTester::S_IWUSR).nonzero?
           check_write(path)
         else
           lambda { check_write(path) }.should raise_error(Errno::EACCES)
         end
 
         if (mode & WindowsSecurityTester::S_IXUSR).nonzero?
           check_execute(path)
         else
           lambda { check_execute(path) }.should raise_error(Errno::EACCES)
         end
       end
 
       def check_read(path)
         Dir.entries(path)
       end
 
       def check_write(path)
         Dir.mkdir(File.join(path, "subdir"))
       end
 
       def check_execute(path)
         Dir.chdir(path) {}
       end
 
       def check_delete(path)
         Dir.rmdir(path)
       end
     end
 
     describe "inheritable aces" do
       it "should be applied to child objects" do
         mode640 = WindowsSecurityTester::S_IRUSR | WindowsSecurityTester::S_IWUSR | WindowsSecurityTester::S_IRGRP
         winsec.set_mode(mode640, path)
 
         newfile = File.join(path, "newfile.txt")
         File.new(newfile, "w").close
 
         newdir = File.join(path, "newdir")
         Dir.mkdir(newdir)
 
         [newfile, newdir].each do |p|
           winsec.get_mode(p).to_s(8).should == mode640.to_s(8)
         end
       end
     end
   end
 end
diff --git a/spec/unit/file_bucket/dipper_spec.rb b/spec/unit/file_bucket/dipper_spec.rb
index 1e92b2b02..063aec8f9 100755
--- a/spec/unit/file_bucket/dipper_spec.rb
+++ b/spec/unit/file_bucket/dipper_spec.rb
@@ -1,170 +1,170 @@
 #!/usr/bin/env rspec
 require 'spec_helper'
 
 require 'pathname'
 
 require 'puppet/file_bucket/dipper'
 require 'puppet/indirector/file_bucket_file/rest'
 
 describe Puppet::FileBucket::Dipper do
   include PuppetSpec::Files
 
   def make_tmp_file(contents)
     file = tmpfile("file_bucket_file")
     File.open(file, 'wb') { |f| f.write(contents) }
     file
   end
 
   it "should fail in an informative way when there are failures checking for the file on the server" do
     @dipper = Puppet::FileBucket::Dipper.new(:Path => make_absolute("/my/bucket"))
 
     file = make_tmp_file('contents')
     Puppet::FileBucket::File.indirection.expects(:head).raises ArgumentError
 
     lambda { @dipper.backup(file) }.should raise_error(Puppet::Error)
   end
 
   it "should fail in an informative way when there are failures backing up to the server" do
     @dipper = Puppet::FileBucket::Dipper.new(:Path => make_absolute("/my/bucket"))
 
     file = make_tmp_file('contents')
     Puppet::FileBucket::File.indirection.expects(:head).returns false
     Puppet::FileBucket::File.indirection.expects(:save).raises ArgumentError
 
     lambda { @dipper.backup(file) }.should raise_error(Puppet::Error)
   end
 
   it "should backup files to a local bucket" do
     Puppet[:bucketdir] = "/non/existent/directory"
     file_bucket = tmpdir("bucket")
 
     @dipper = Puppet::FileBucket::Dipper.new(:Path => file_bucket)
 
     file = make_tmp_file("my\r\ncontents")
     checksum = "f0d7d4e480ad698ed56aeec8b6bd6dea"
     Digest::MD5.hexdigest("my\r\ncontents").should == checksum
 
     @dipper.backup(file).should == checksum
     File.exists?("#{file_bucket}/f/0/d/7/d/4/e/4/f0d7d4e480ad698ed56aeec8b6bd6dea/contents").should == true
   end
 
   it "should not backup a file that is already in the bucket" do
     @dipper = Puppet::FileBucket::Dipper.new(:Path => "/my/bucket")
 
     file = make_tmp_file("my\r\ncontents")
     checksum = Digest::MD5.hexdigest("my\r\ncontents")
 
     Puppet::FileBucket::File.indirection.expects(:head).returns true
     Puppet::FileBucket::File.indirection.expects(:save).never
     @dipper.backup(file).should == checksum
   end
 
   it "should retrieve files from a local bucket" do
     @dipper = Puppet::FileBucket::Dipper.new(:Path => "/my/bucket")
 
     checksum = Digest::MD5.hexdigest('my contents')
 
     request = nil
 
     Puppet::FileBucketFile::File.any_instance.expects(:find).with{ |r| request = r }.once.returns(Puppet::FileBucket::File.new('my contents'))
 
     @dipper.getfile(checksum).should == 'my contents'
 
     request.key.should == "md5/#{checksum}"
   end
 
   it "should backup files to a remote server" do
     @dipper = Puppet::FileBucket::Dipper.new(:Server => "puppetmaster", :Port => "31337")
 
     file = make_tmp_file("my\r\ncontents")
     checksum = Digest::MD5.hexdigest("my\r\ncontents")
 
     real_path = Pathname.new(file).realpath
 
     request1 = nil
     request2 = nil
 
     Puppet::FileBucketFile::Rest.any_instance.expects(:head).with { |r| request1 = r }.once.returns(nil)
     Puppet::FileBucketFile::Rest.any_instance.expects(:save).with { |r| request2 = r }.once
 
     @dipper.backup(file).should == checksum
     [request1, request2].each do |r|
       r.server.should == 'puppetmaster'
       r.port.should == 31337
       r.key.should == "md5/#{checksum}/#{real_path}"
     end
   end
 
   it "should retrieve files from a remote server" do
     @dipper = Puppet::FileBucket::Dipper.new(:Server => "puppetmaster", :Port => "31337")
 
     checksum = Digest::MD5.hexdigest('my contents')
 
     request = nil
 
     Puppet::FileBucketFile::Rest.any_instance.expects(:find).with { |r| request = r }.returns(Puppet::FileBucket::File.new('my contents'))
 
     @dipper.getfile(checksum).should == "my contents"
 
     request.server.should == 'puppetmaster'
     request.port.should == 31337
     request.key.should == "md5/#{checksum}"
   end
 
   describe "#restore" do
     shared_examples_for "a restorable file" do
       let(:contents) { "my\ncontents" }
       let(:md5) { Digest::MD5.hexdigest(contents) }
       let(:dest) { tmpfile('file_bucket_dest') }
 
       it "should restore the file" do
         request = nil
 
         klass.any_instance.expects(:find).with { |r| request = r }.returns(Puppet::FileBucket::File.new(contents))
 
         dipper.restore(dest, md5).should == md5
-        Digest::MD5.file(dest).hexdigest.should == md5
+        Digest::MD5.hexdigest(Puppet::Util.binread(dest)).should == md5
 
         request.key.should == "md5/#{md5}"
         request.server.should == server
         request.port.should == port
       end
 
       it "should skip restoring if existing file has the same checksum" do
         crnl = "my\r\ncontents"
         File.open(dest, 'wb') {|f| f.print(crnl) }
 
         dipper.expects(:getfile).never
         dipper.restore(dest, Digest::MD5.hexdigest(crnl)).should be_nil
       end
 
       it "should overwrite existing file if it has different checksum" do
         klass.any_instance.expects(:find).returns(Puppet::FileBucket::File.new(contents))
 
         File.open(dest, 'wb') {|f| f.print('other contents') }
 
         dipper.restore(dest, md5).should == md5
       end
     end
 
     describe "when restoring from a remote server" do
       let(:klass) { Puppet::FileBucketFile::Rest }
       let(:server) { "puppetmaster" }
       let(:port) { 31337 }
 
       it_behaves_like "a restorable file" do
         let (:dipper) { Puppet::FileBucket::Dipper.new(:Server => server, :Port => port.to_s) }
       end
     end
 
     describe "when restoring from a local server" do
       let(:klass) { Puppet::FileBucketFile::File }
       let(:server) { nil }
       let(:port) { nil }
 
       it_behaves_like "a restorable file" do
         let (:dipper) { Puppet::FileBucket::Dipper.new(:Path => "/my/bucket") }
       end
     end
   end
 end