Prevent gitlab-shell character encoding issues by receiving its changes as raw data.

5 files changed, 30 insertions, 10 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 7bc561b6e06..6efa5f87439 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -68,6 +68,7 @@ v 7.9.0 (unreleased)
   - Execute hooks and services when branch or tag is created or deleted through web interface.
   - Block and unblock user if he/she was blocked/unblocked in Active Directory
   - Raise recommended number of unicorn workers from 2 to 3
+  - Prevent gitlab-shell character encoding issues by receiving its changes as raw data.
 
 v 7.8.4
   - Fix issue_tracker_id substitution in custom issue trackers
diff --git a/Gemfile b/Gemfile
index 44f024a4b8e..b7b7b0a1c28 100644
--- a/Gemfile
+++ b/Gemfile
@@ -177,6 +177,9 @@ gem 'ace-rails-ap'
 # Keyboard shortcuts
 gem 'mousetrap-rails'
 
+# Detect and convert string character encoding
+gem 'charlock_holmes'
+
 # Shutting down requests that take too long
 gem "slowpoke"
 
diff --git a/Gemfile.lock b/Gemfile.lock
index b331c29f7ef..d3209ede86e 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -684,6 +684,7 @@ DEPENDENCIES
   cal-heatmap-rails (~> 0.0.1)
   capybara (~> 2.2.1)
   carrierwave
+  charlock_holmes
   coffee-rails
   colored
   coveralls
diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb
index ecc6c8e53a3..0c3ee6ba4ff 100644
--- a/app/workers/post_receive.rb
+++ b/app/workers/post_receive.rb
@@ -21,7 +21,9 @@ class PostReceive
       return false
     end
 
-    changes = changes.lines if changes.kind_of?(String)
+    changes = Base64.decode64(changes) unless changes.include?(" ")
+    changes = utf8_encode_changes(changes)
+    changes = changes.lines
 
     changes.each do |change|
       oldrev, newrev, ref = change.strip.split(' ')
@@ -41,6 +43,19 @@ class PostReceive
     end
   end
 
+  def utf8_encode_changes(changes)
+    changes = changes.dup
+    
+    changes.force_encoding("UTF-8")
+    return changes if changes.valid_encoding?
+
+    # Convert non-UTF-8 branch/tag names to UTF-8 so they can be dumped as JSON.
+    detection = CharlockHolmes::EncodingDetector.detect(changes)
+    return changes unless detection && detection[:encoding]
+
+    CharlockHolmes::Converter.convert(changes, detection[:encoding], 'UTF-8')
+  end
+
   def log(message)
     Gitlab::GitLogger.error("POST-RECEIVE: #{message}")
   end
diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb
index 8eabc46112b..df1a2b84a53 100644
--- a/spec/workers/post_receive_spec.rb
+++ b/spec/workers/post_receive_spec.rb
@@ -1,6 +1,10 @@
 require 'spec_helper'
 
 describe PostReceive do
+  let(:changes) { "123456 789012 refs/heads/tést\n654321 210987 refs/tags/tag" }
+  let(:wrongly_encoded_changes) { changes.encode("ISO-8859-1").force_encoding("UTF-8") }
+  let(:base64_changes) { Base64.encode64(wrongly_encoded_changes) }
+  
   context "as a resque worker" do
     it "reponds to #perform" do
       expect(PostReceive.new).to respond_to(:perform)
@@ -14,7 +18,7 @@ describe PostReceive do
 
     it "fetches the correct project" do
       expect(Project).to receive(:find_with_namespace).with(project.path_with_namespace).and_return(project)
-      PostReceive.new.perform(pwd(project), key_id, changes)
+      PostReceive.new.perform(pwd(project), key_id, base64_changes)
     end
 
     it "does not run if the author is not in the project" do
@@ -22,24 +26,20 @@ describe PostReceive do
 
       expect(project).not_to receive(:execute_hooks)
 
-      expect(PostReceive.new.perform(pwd(project), key_id, changes)).to be_falsey
+      expect(PostReceive.new.perform(pwd(project), key_id, base64_changes)).to be_falsey
     end
 
     it "asks the project to trigger all hooks" do
       Project.stub(find_with_namespace: project)
-      expect(project).to receive(:execute_hooks)
-      expect(project).to receive(:execute_services)
+      expect(project).to receive(:execute_hooks).twice
+      expect(project).to receive(:execute_services).twice
       expect(project).to receive(:update_merge_requests)
 
-      PostReceive.new.perform(pwd(project), key_id, changes)
+      PostReceive.new.perform(pwd(project), key_id, base64_changes)
     end
   end
 
   def pwd(project)
     File.join(Gitlab.config.gitlab_shell.repos_path, project.path_with_namespace)
   end
-
-  def changes
-    'd14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master'
-  end
 end