diff --git a/CHANGELOG b/CHANGELOG index 4d3a87f556f..bd66a92933d 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -73,6 +73,7 @@ v 7.9.0 (unreleased) - Block and unblock user if he/she was blocked/unblocked in Active Directory - Raise recommended number of unicorn workers from 2 to 3 - Use same layout and interactivity for project members as group members. + - 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