Commit 3b1d5a1d authored by Douwe Maan's avatar Douwe Maan

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

parent 60df262c
...@@ -68,6 +68,7 @@ v 7.9.0 (unreleased) ...@@ -68,6 +68,7 @@ v 7.9.0 (unreleased)
- Execute hooks and services when branch or tag is created or deleted through web interface. - 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 - Block and unblock user if he/she was blocked/unblocked in Active Directory
- Raise recommended number of unicorn workers from 2 to 3 - 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 v 7.8.4
- Fix issue_tracker_id substitution in custom issue trackers - Fix issue_tracker_id substitution in custom issue trackers
......
...@@ -177,6 +177,9 @@ gem 'ace-rails-ap' ...@@ -177,6 +177,9 @@ gem 'ace-rails-ap'
# Keyboard shortcuts # Keyboard shortcuts
gem 'mousetrap-rails' gem 'mousetrap-rails'
# Detect and convert string character encoding
gem 'charlock_holmes'
# Shutting down requests that take too long # Shutting down requests that take too long
gem "slowpoke" gem "slowpoke"
......
...@@ -684,6 +684,7 @@ DEPENDENCIES ...@@ -684,6 +684,7 @@ DEPENDENCIES
cal-heatmap-rails (~> 0.0.1) cal-heatmap-rails (~> 0.0.1)
capybara (~> 2.2.1) capybara (~> 2.2.1)
carrierwave carrierwave
charlock_holmes
coffee-rails coffee-rails
colored colored
coveralls coveralls
......
...@@ -21,7 +21,9 @@ class PostReceive ...@@ -21,7 +21,9 @@ class PostReceive
return false return false
end 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| changes.each do |change|
oldrev, newrev, ref = change.strip.split(' ') oldrev, newrev, ref = change.strip.split(' ')
...@@ -41,6 +43,19 @@ class PostReceive ...@@ -41,6 +43,19 @@ class PostReceive
end end
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) def log(message)
Gitlab::GitLogger.error("POST-RECEIVE: #{message}") Gitlab::GitLogger.error("POST-RECEIVE: #{message}")
end end
......
require 'spec_helper' require 'spec_helper'
describe PostReceive do 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 context "as a resque worker" do
it "reponds to #perform" do it "reponds to #perform" do
expect(PostReceive.new).to respond_to(:perform) expect(PostReceive.new).to respond_to(:perform)
...@@ -14,7 +18,7 @@ describe PostReceive do ...@@ -14,7 +18,7 @@ describe PostReceive do
it "fetches the correct project" do it "fetches the correct project" do
expect(Project).to receive(:find_with_namespace).with(project.path_with_namespace).and_return(project) 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 end
it "does not run if the author is not in the project" do it "does not run if the author is not in the project" do
...@@ -22,24 +26,20 @@ describe PostReceive do ...@@ -22,24 +26,20 @@ describe PostReceive do
expect(project).not_to receive(:execute_hooks) 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 end
it "asks the project to trigger all hooks" do it "asks the project to trigger all hooks" do
Project.stub(find_with_namespace: project) Project.stub(find_with_namespace: project)
expect(project).to receive(:execute_hooks) expect(project).to receive(:execute_hooks).twice
expect(project).to receive(:execute_services) expect(project).to receive(:execute_services).twice
expect(project).to receive(:update_merge_requests) 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
end end
def pwd(project) def pwd(project)
File.join(Gitlab.config.gitlab_shell.repos_path, project.path_with_namespace) File.join(Gitlab.config.gitlab_shell.repos_path, project.path_with_namespace)
end end
def changes
'd14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master'
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment