Pipe RPUSH command to Redis via stdin to avoid overrunning command-lineuse-stdin

Closes gitlab-org/gitlab-ce#17329

3 files changed, 27 insertions, 13 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 63b04cc..aeb1991 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,5 @@
 v3.0.0
+  - Pipe RPUSH command to Redis via stdin to avoid overrunning command-line
   - Remove rm-tag command (Robert Schilling)
   - Remove create-branch and rm-branch commands (Robert Schilling)
   - Update PostReceive worker so it logs a unique JID in Sidekiq
diff --git a/lib/gitlab_post_receive.rb b/lib/gitlab_post_receive.rb
index 0fff479..20e46c5 100644
--- a/lib/gitlab_post_receive.rb
+++ b/lib/gitlab_post_receive.rb
@@ -2,6 +2,7 @@ require_relative 'gitlab_init'
 require_relative 'gitlab_net'
 require 'json'
 require 'base64'
+require 'open3'
 require 'securerandom'
 
 class GitlabPostReceive
@@ -74,11 +75,20 @@ class GitlabPostReceive
 
     queue = "#{config.redis_namespace}:queue:post_receive"
     msg = JSON.dump({ 'class' => 'PostReceive', 'args' => [@repo_path, @actor, changes], 'jid' => @jid  })
-    if system(*config.redis_command, 'rpush', queue, msg,
-              err: '/dev/null', out: '/dev/null')
-      return true
-    else
-      puts "GitLab: An unexpected error occurred (redis-cli returned #{$?.exitstatus})."
+
+    begin
+      result = Open3.popen2(*config.redis_command, '--pipe') do |stdin, stdout, wait_thr|
+        stdin.write("RPUSH '#{queue}' '#{msg}'")
+        stdin.close
+        wait_thr.value
+      end
+
+      return true if result == 0
+
+      puts "GitLab: An unexpected error occurred (redis-cli returned #{result.to_i})."
+      return false
+    rescue => e
+      puts "GitLab: An unexpected error occurred running redis-cli"
       return false
     end
   end
diff --git a/spec/gitlab_post_receive_spec.rb b/spec/gitlab_post_receive_spec.rb
index 9d7696e..86d0526 100644
--- a/spec/gitlab_post_receive_spec.rb
+++ b/spec/gitlab_post_receive_spec.rb
@@ -1,3 +1,4 @@
+# coding: utf-8
 require 'spec_helper'
 require 'gitlab_post_receive'
 
@@ -21,7 +22,7 @@ describe GitlabPostReceive do
 
     before do
       GitlabConfig.any_instance.stub(redis_command: %w(env -i redis-cli))
-      allow(gitlab_post_receive).to receive(:system).and_return(true)
+      allow(Open3).to receive(:popen2).and_return(0)
     end
 
     it "prints the broadcast message" do
@@ -47,13 +48,15 @@ describe GitlabPostReceive do
     end
 
     it "pushes a Sidekiq job onto the queue" do
-      expect(gitlab_post_receive).to receive(:system).with(
+      stdin = double('stdin')
+
+      expect(stdin).to receive(:write).with(
+        %Q/RPUSH 'resque:gitlab:queue:post_receive' '{"class":"PostReceive","args":["#{repo_path}","#{actor}",#{base64_changes.inspect}],"jid":"#{gitlab_post_receive.jid}"}'/)
+      expect(stdin).to receive(:close)
+      expect(Open3).to receive(:popen2).with(
         *[
-          *%w(env -i redis-cli rpush resque:gitlab:queue:post_receive), 
-          %Q/{"class":"PostReceive","args":["#{repo_path}","#{actor}",#{base64_changes.inspect}],"jid":"#{gitlab_post_receive.jid}"}/,
-          { err: "/dev/null", out: "/dev/null" }
-        ]
-      ).and_return(true)
+          *%w(env -i redis-cli --pipe)
+        ]).and_yield(stdin, double('stdout'), double('wait_thr', value: 0)).and_return(0)
 
       gitlab_post_receive.exec
     end
@@ -72,7 +75,7 @@ describe GitlabPostReceive do
     context "when the redis command fails" do
 
       before do
-        allow(gitlab_post_receive).to receive(:system).and_return(false)
+        allow(Open3).to receive(:popen2).and_return(1)
         allow($?).to receive(:exitstatus).and_return(nil)
       end