Greetings!
Please review the pull request #167: Tickets/2.7.x/9996 exec resources with multiline commands opened by (jhelwig)
Some more information about the pull request:
- Opened: Wed Oct 12 20:24:02 UTC 2011
- Based on: puppetlabs:2.7.x (5e3962c5b149c1eeaf62db9090b665b45ad8a128)
- Requested merge: jhelwig:tickets/2.7.x/9996-exec-resources-with-multiline-commands (652196fee6792f628fa4eaff0daabe975127a243)
Description:
Originally we were relying on the behavior that Array.new would call
when #to_a is called on a string, it does not always return[original_string]. Because string.to_a is effectively equivalent tostring.each_line.to_a (at least in Ruby 1.8.7) we were breaking
commands with embedded newlines.
Manually wrapping the passed in command in an array, and calling#flatten is much safer since it will not "helpfully" split up the
command string for us.
Thanks!
The Pull Request Bot
Diff follows:
diff --git a/acceptance/tests/resource/exec/accept_multi-line_commands.rb b/acceptance/tests/resource/exec/accept_multi-line_commands.rb
new file mode 100644
index 0000000..1e9aaf1
--- /dev/null
+++ b/acceptance/tests/resource/exec/accept_multi-line_commands.rb
@@ -0,0 +1,24 @@
+test_name "Be able to execute multi-line commands (#9996)"
+
+temp_file_name = "/tmp/9996-multi-line-commands.#{$$}"
+test_manifest = <<HERE
+exec { "test exec":
+ command => "/bin/echo '#Test' > #{temp_file_name};
+ /bin/echo 'bob' >> #{temp_file_name};"
+}
+HERE
+
+expected_results = <<HERE
+#Test
+bob
+HERE
+
+on agents, "rm -f #{temp_file_name}"
+
+apply_manifest_on agents, test_manifest
+
+on(agents, "cat #{temp_file_name}").each do |result|
+ assert_equal(expected_results, "#{result.stdout}", "Unexpected result for host '#{result.host}'")
+end
+
+on agents, "rm -f #{temp_file_name}"
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index 51fd4d7..b0a942b 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -300,7 +300,12 @@ module Util
def execute_posix(command, arguments, stdin, stdout, stderr)
child_pid = Kernel.fork do
- command = Array(command)
+ # We can't just call Array(command), and rely on it returning
+ # things like ['foo'], when passed ['foo'], because
+ # Array(command) will call command.to_a internally, which when
+ # given a string can end up doing Very Bad Things(TM), such as
+ # turning "/tmp/foo;\r\n /bin/echo" into ["/tmp/foo;\r\n", " /bin/echo"]
+ command = [command].flatten
Process.setsid
begin
$stdin.reopen(stdin)
diff --git a/lib/puppet/util/suidmanager.rb b/lib/puppet/util/suidmanager.rb
index 56f841a..d733883 100644
--- a/lib/puppet/util/suidmanager.rb
+++ b/lib/puppet/util/suidmanager.rb
@@ -150,4 +150,3 @@ module Puppet::Util::SUIDManager
end
module_function :system
end
-
diff --git a/spec/unit/util_spec.rb b/spec/unit/util_spec.rb
index 8ac8017..cff2788 100755
--- a/spec/unit/util_spec.rb
+++ b/spec/unit/util_spec.rb
@@ -220,6 +220,12 @@ describe Puppet::Util do
Puppet::Util.execute_posix(['test command', 'with', 'arguments'], {:uid => 50, :gid => 55}, @stdin, @stdout, @stderr)
end
+ it "should properly execute string commands with embedded newlines" do
+ Kernel.expects(:exec).with("/bin/echo 'foo' ; \n /bin/echo 'bar' ;")
+
+ Puppet::Util.execute_posix("/bin/echo 'foo' ; \n /bin/echo 'bar' ;", {:uid => 50, :gid => 55}, @stdin, @stdout, @stderr)
+ end
+
it "should return the pid of the child process" do
Puppet::Util.execute_posix('test command', {}, @stdin, @stdout, @stderr).should == pid
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.
