use popen3 in Resolution.exec to catch stderr

So far messages to stderr haven't been catched by
Facter::Util::Resolution.exec and were insted printed out to
stderr. This will cause facter and even puppet to print to stderr
themself, which is not very nice when running puppetd by cron,
as you might get every run a mail if a command outputs to stderr.

We are now wrapping the command execution with Open3.popen3 to
catch stderr and passing them to the new introduced Facter.warn
method.

We are also catching multiline outputs chomping newlines and
returning an array if there have been more than one line. Otherwise
we return an array containing the different lines.

This prevents in general cases as described in #2766 and should
handle command execution in a bit saner way.

2 files changed, 61 insertions, 6 deletions

diff --git a/lib/facter/util/resolution.rb b/lib/facter/util/resolution.rb
index b9e28e8..f6afce6 100644
--- a/lib/facter/util/resolution.rb
+++ b/lib/facter/util/resolution.rb
@@ -7,6 +7,7 @@ require 'facter/util/confine'
 
 require 'timeout'
 require 'rbconfig'
+require 'open3'
 
 class Facter::Util::Resolution
     attr_accessor :interpreter, :code, :name, :timeout
@@ -42,17 +43,27 @@ class Facter::Util::Resolution
         end
 
         out = nil
+        err = nil
         begin
-            out = %x{#{code}}.chomp
+            Open3.popen3(code) do |stdin,stdout,stderr|
+              out = self.parse_output(stdout.readlines)
+              err = self.parse_output(stderr.readlines)
+            end
         rescue => detail
             $stderr.puts detail
             return nil
         end
-        if out == ""
-            return nil
-        else
-            return out
-        end
+        Facter.warn(err)
+
+        return nil if out == ""
+        out
+    end
+
+    def self.parse_output(output)
+        return nil unless output and output.size > 0
+        result = output.collect{|line| line.chomp }
+        return result.first unless result.size > 1
+        result
     end
 
     # Add a new confine to the resolution mechanism.
diff --git a/spec/unit/util/resolution.rb b/spec/unit/util/resolution.rb
index d4bb781..27cb150 100755
--- a/spec/unit/util/resolution.rb
+++ b/spec/unit/util/resolution.rb
@@ -227,10 +227,54 @@ describe Facter::Util::Resolution do
         Facter::Util::Resolution.should respond_to(:exec)
     end
 
+    it "should have a class method to parse output" do
+        Facter::Util::Resolution.should respond_to(:parse_output)
+    end
+
     # It's not possible, AFAICT, to mock %x{}, so I can't really test this bit.
     describe "when executing code" do
         it "should fail if any interpreter other than /bin/sh is requested" do
             lambda { Facter::Util::Resolution.exec("/something", "/bin/perl") }.should raise_error(ArgumentError)
         end
+
+        it "should produce stderr content as a warning" do
+            stdout = stdin = stub('fh', :readlines => ["aaa\n"])
+            stderr = stub('stderr', :readlines => %w{my content})
+            Open3.expects(:popen3).with("/bin/true").yields(stdin, stdout, stderr)
+
+            Facter.expects(:warn).with(['my','content'])
+            Facter::Util::Resolution.exec("/bin/true").should == 'aaa'
+        end
+
+        it "should produce nil as a warning if nothing is printed to stderr" do
+            stdout = stdin = stub('fh', :readlines => ["aaa\n"])
+            stderr = stub('stderr', :readlines => [])
+            Open3.expects(:popen3).with("/bin/true").yields(stdin, stdout, stderr)
+
+            Facter.expects(:warn).with(nil)
+            Facter::Util::Resolution.exec("/bin/true").should == 'aaa'
+        end
+    end
+
+    describe "when parsing output" do
+        it "should return nil on nil" do
+            Facter::Util::Resolution.parse_output(nil).should be_nil
+        end
+
+        it "should return nil on empty string" do
+            Facter::Util::Resolution.parse_output('').should be_nil
+        end
+
+        it "should return nil on an empty array" do
+            Facter::Util::Resolution.parse_output([]).should be_nil
+        end
+
+        it "should return a string on a 1 size array" do
+            Facter::Util::Resolution.parse_output(["aaa\n"]).should == "aaa"
+        end
+
+        it "should return an array with chomped new lines on an array" do
+            result = Facter::Util::Resolution.parse_output(["aaa\n","bbb\n","ccc\n"]).should == [ "aaa", "bbb", "ccc" ]
+        end
     end
 end