Please review pull request #473: (#12336) Util::'which' may fail if user's path contains a tilde opened by (cprice-puppet)

Description:

In certain circumstances (user's HOME environment variable is not set, or has
been unset during ruby execution, plus PATH contains a literal ~ character), Util::w
hich would raise an ArgumentError. Handle this error by logging a warning (one time only) and ignoring this element of the PATH.

  • Opened: Wed Feb 08 02:44:33 UTC 2012
  • Based on: puppetlabs:master (83f61eb7652c0d19788a03ee6c6f23a59cbfb13c)
  • Requested merge: cprice-puppet:bug/master/12336-PATH-contains-tilde (1c0360e7cf95bbaa76440fa835e581727018dc3e)

Diff follows:

diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index d25e4d3..737a928 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -188,12 +188,25 @@ def which(bin)
       return bin if FileTest.file? bin and FileTest.executable? bin
     else
       ENV['PATH'].split(File::PATH_SEPARATOR).each do |dir|
-        dest = File.expand_path(File.join(dir, bin))
+        begin
+          dest = File.expand_path(File.join(dir, bin))
+        rescue ArgumentError => e
+          # if the user's PATH contains a literal tilde (~) character and HOME is not set, we may get
+          # an ArgumentError here.  Let's check to see if that is the case; if not, re-raise whatever error
+          # was thrown.
+          raise e unless ((dir =~ /~/) && ((ENV['HOME'].nil? || ENV['HOME'] == "")))
+
+          # if we get here they have a tilde in their PATH.  We'll issue a single warning about this and then
+          # ignore this path element and carry on with our lives.
+          Puppet::Util::Warnings.warnonce("PATH contains a ~ character, and HOME is not set; ignoring PATH element '#{dir}'.")
+          next
+        end
         if Puppet.features.microsoft_windows? && File.extname(dest).empty?
           exts = ENV['PATHEXT']
           exts = exts ? exts.split(File::PATH_SEPARATOR) : %w[.COM .EXE .BAT .CMD]
           exts.each do |ext|
             destext = File.expand_path(dest + ext)
+
             return destext if FileTest.file? destext and FileTest.executable? destext
           end
         end
diff --git a/spec/unit/util_spec.rb b/spec/unit/util_spec.rb
index 3369c49..3561f01 100755
--- a/spec/unit/util_spec.rb
+++ b/spec/unit/util_spec.rb
@@ -216,6 +216,13 @@
       Puppet::Util.which('doesnotexist').should be_nil
     end
 
+    it "should warn if the user's HOME is not set but their PATH contains a ~" do
+      Puppet::Util.withenv({:HOME => nil, :PATH => "~/bin:/usr/bin:/bin"}) do
+        Puppet::Util::Warnings.expects(:warnonce).once
+        Puppet::Util.which('foo').should_not be_nil
+      end
+    end
+
     it "should reject directories" do
       Puppet::Util.which(base).should be_nil
     end

    

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to