Please review pull request #700: Bug/2.7.x/14123 don't explode when path has bad content opened by (daniel-pittman)
Description:
As reported in http://bugs.debian.org/669650, when the PATH contained an
expansion for a user that doesn't exist, Puppet handled it the same way Ruby
does - to raise an exception, and explode.
Leaving aside the sanity of literal tilde in the PATH, we shouldn't explode in
that case - we should just skip that entry and carry on to the next element in
the path instead.
Signed-off-by: Daniel Pittman [email protected]
- Opened: Mon Apr 23 20:14:51 UTC 2012
- Based on: puppetlabs:2.7.x (8310d33462382d1ff51f5bfdc48f49503be489dd)
- Requested merge: daniel-pittman:bug/2.7.x/14123-don't-explode-when-path-has-bad-content (36d959a3186162b64c031e39cced196fb36f43b8)
Diff follows:
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index 148b618..8e2e474 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -170,16 +170,21 @@ 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))
- 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
+ begin
+ dest = File.expand_path(File.join(dir, bin))
+ 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
+ return dest if FileTest.file? dest and FileTest.executable? dest
+ rescue ArgumentError => e
+ raise unless e.to_s =~ /doesn't exist/
+ # ...otherwise, we just skip the non-existent entry, and do nothing.
end
- return dest if FileTest.file? dest and FileTest.executable? dest
end
end
nil
diff --git a/spec/unit/util_spec.rb b/spec/unit/util_spec.rb
index 7e6a633..2742302 100755
--- a/spec/unit/util_spec.rb
+++ b/spec/unit/util_spec.rb
@@ -571,6 +571,17 @@ def stub_process_wait(exitstatus)
Puppet::Util.which(base).should be_nil
end
+ it "should ignore ~user directories if the user doesn't exist" do
+ # Windows treats *any* user as a "user that doesn't exist", which means
+ # that this will work correctly across all our platforms, and should
+ # behave consistently. If they ever implement it correctly (eg: to do
+ # the lookup for real) it should just work transparently.
+ baduser = 'if_this_user_exists_I_will_eat_my_hat'
+ Puppet::Util::Execution.withenv("PATH" => "~#{baduser}:#{base}") do
+ Puppet::Util.which('foo').should == path
+ end
+ end
+
describe "on POSIX systems" do
before :each do
Puppet.features.stubs(:posix?).returns true
-- 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.
