Auto merge of #6036 - nobu:feature/sh-by-popen, r=indirect

Call IO.popen instead of backticks

IO.popen with the command in an array doesn't need command line
quotes, and is safer.

Thanks so much for the contribution!
To make reviewing this PR a bit easier, please fill out answers to the following questions.

### What was the end-user problem that led to this PR?

Back-ticks with command line quoting is dangerous.

### What was your diagnosis of the problem?

Back-ticks is not for complex purpose.

### What is your fix for the problem, implemented in this PR?

Use `IO.popen` and redirection option instead.

### Why did you choose this fix out of the possible options?

`IO.popen` and options of `spawn` have been introduced for such purpose.

1 files changed, 49 insertions, 23 deletions

diff --git a/lib/bundler/gem_helper.rb b/lib/bundler/gem_helper.rb
index 1d7fc508d5..3458a2adda 100644
--- a/lib/bundler/gem_helper.rb
+++ b/lib/bundler/gem_helper.rb
@@ -2,6 +2,7 @@
 
 require "bundler/vendored_thor" unless defined?(Thor)
 require "bundler"
+require "shellwords"
 
 module Bundler
   class GemHelper
@@ -74,7 +75,7 @@ module Bundler
 
     def build_gem
       file_name = nil
-      sh("gem build -V '#{spec_path}'") do
+      sh(%W[gem build -V #{spec_path}]) do
         file_name = File.basename(built_gem_path)
         SharedHelpers.filesystem_access(File.join(base, "pkg")) {|p| FileUtils.mkdir_p(p) }
         FileUtils.mv(built_gem_path, "pkg")
@@ -85,17 +86,21 @@ module Bundler
 
     def install_gem(built_gem_path = nil, local = false)
       built_gem_path ||= build_gem
-      out, _ = sh_with_code("gem install '#{built_gem_path}'#{" --local" if local}")
-      raise "Couldn't install gem, run `gem install #{built_gem_path}' for more detailed output" unless out[/Successfully installed/]
+      cmd = %W[gem install #{built_gem_path}]
+      cmd << "--local" if local
+      out, status = sh_with_status(cmd)
+      unless status.success? && out[/Successfully installed/]
+        raise "Couldn't install gem, run `gem install #{built_gem_path}' for more detailed output"
+      end
       Bundler.ui.confirm "#{name} (#{version}) installed."
     end
 
   protected
 
     def rubygem_push(path)
-      gem_command = "gem push '#{path}'"
-      gem_command += " --key #{gem_key}" if gem_key
-      gem_command += " --host #{allowed_push_host}" if allowed_push_host
+      gem_command = %W[gem push #{path}]
+      gem_command << "--key" << gem_key if gem_key
+      gem_command << "--host" << allowed_push_host if allowed_push_host
       unless allowed_push_host || Bundler.user_home.join(".gem/credentials").file?
         raise "Your rubygems.org credentials aren't set. Run `gem push` to set them."
       end
@@ -127,12 +132,14 @@ module Bundler
 
     def perform_git_push(options = "")
       cmd = "git push #{options}"
-      out, code = sh_with_code(cmd)
-      raise "Couldn't git push. `#{cmd}' failed with the following output:\n\n#{out}\n" unless code == 0
+      out, status = sh_with_status(cmd)
+      return if status.success?
+      cmd = cmd.shelljoin if cmd.respond_to?(:shelljoin)
+      raise "Couldn't git push. `#{cmd}' failed with the following output:\n\n#{out}\n"
     end
 
     def already_tagged?
-      return false unless sh("git tag").split(/\n/).include?(version_tag)
+      return false unless sh(%w[git tag]).split(/\n/).include?(version_tag)
       Bundler.ui.confirm "Tag #{version_tag} has already been created."
       true
     end
@@ -142,20 +149,20 @@ module Bundler
     end
 
     def clean?
-      sh_with_code("git diff --exit-code")[1] == 0
+      sh_with_status(%w[git diff --exit-code])[1].success?
     end
 
     def committed?
-      sh_with_code("git diff-index --quiet --cached HEAD")[1] == 0
+      sh_with_status(%w[git diff-index --quiet --cached HEAD])[1].success?
     end
 
     def tag_version
-      sh "git tag -m \"Version #{version}\" #{version_tag}"
+      sh %W[git tag -m Version\ #{version} #{version_tag}]
       Bundler.ui.confirm "Tagged #{version_tag}."
       yield if block_given?
     rescue
       Bundler.ui.error "Untagging #{version_tag} due to error."
-      sh_with_code "git tag -d #{version_tag}"
+      sh_with_status %W[git tag -d #{version_tag}]
       raise
     end
 
@@ -172,22 +179,41 @@ module Bundler
     end
 
     def sh(cmd, &block)
-      out, code = sh_with_code(cmd, &block)
-      unless code.zero?
+      out, status = sh_with_status(cmd, &block)
+      unless status.success?
+        cmd = cmd.shelljoin if cmd.respond_to?(:shelljoin)
         raise(out.empty? ? "Running `#{cmd}` failed. Run this command directly for more detailed output." : out)
       end
       out
     end
 
     def sh_with_code(cmd, &block)
-      cmd += " 2>&1"
-      outbuf = String.new
-      Bundler.ui.debug(cmd)
-      SharedHelpers.chdir(base) do
-        outbuf = `#{cmd}`
-        status = $?.exitstatus
-        block.call(outbuf) if status.zero? && block
-        [outbuf, status]
+      outbuf, status = sh_with_code(cmd, &block)
+      [outbuf, (status && status.exitstatus) || -1]
+    end
+
+    if RUBY_VERSION >= "1.9"
+      def sh_with_status(cmd, &block)
+        Bundler.ui.debug(cmd)
+        SharedHelpers.chdir(base) do
+          outbuf = IO.popen(cmd, :err => [:child, :out], &:read)
+          status = $?
+          block.call(outbuf) if status.success? && block
+          [outbuf, status]
+        end
+      end
+    else
+      def sh_with_status(cmd, &block)
+        cmd = cmd.shelljoin if cmd.respond_to?(:shelljoin)
+        cmd += " 2>&1"
+        outbuf = String.new
+        Bundler.ui.debug(cmd)
+        SharedHelpers.chdir(base) do
+          outbuf = `#{cmd}`
+          status = $?
+          block.call(outbuf) if status.success? && block
+          [outbuf, status]
+        end
       end
     end