Merge branch 'git-user-encoding' into 'master'

Fix encoding bugs in Gitlab::Git::User

Closes #40330

See merge request gitlab-org/gitlab-ce!15539

5 files changed, 32 insertions, 8 deletions

diff --git a/lib/gitlab/encoding_helper.rb b/lib/gitlab/encoding_helper.rb
index 99dfee3dd9b..582028493e9 100644
--- a/lib/gitlab/encoding_helper.rb
+++ b/lib/gitlab/encoding_helper.rb
@@ -17,6 +17,10 @@ module Gitlab
       return nil unless message.respond_to?(:force_encoding)
       return message if message.encoding == Encoding::UTF_8 && message.valid_encoding?
 
+      if message.respond_to?(:frozen?) && message.frozen?
+        message = message.dup
+      end
+
       message.force_encoding("UTF-8")
       return message if message.valid_encoding?
 
diff --git a/lib/gitlab/git/user.rb b/lib/gitlab/git/user.rb
index e6b61417de1..e573cd0e143 100644
--- a/lib/gitlab/git/user.rb
+++ b/lib/gitlab/git/user.rb
@@ -8,7 +8,12 @@ module Gitlab
       end
 
       def self.from_gitaly(gitaly_user)
-        new(gitaly_user.gl_username, gitaly_user.name, gitaly_user.email, gitaly_user.gl_id)
+        new(
+          gitaly_user.gl_username,
+          Gitlab::EncodingHelper.encode!(gitaly_user.name),
+          Gitlab::EncodingHelper.encode!(gitaly_user.email),
+          gitaly_user.gl_id
+        )
       end
 
       def initialize(username, name, email, gl_id)
@@ -23,7 +28,7 @@ module Gitlab
       end
 
       def to_gitaly
-        Gitaly::User.new(gl_username: username, gl_id: gl_id, name: name, email: email)
+        Gitaly::User.new(gl_username: username, gl_id: gl_id, name: name.b, email: email.b)
       end
     end
   end
diff --git a/spec/lib/gitlab/encoding_helper_spec.rb b/spec/lib/gitlab/encoding_helper_spec.rb
index 9151c66afb3..f6e5c55240f 100644
--- a/spec/lib/gitlab/encoding_helper_spec.rb
+++ b/spec/lib/gitlab/encoding_helper_spec.rb
@@ -9,6 +9,7 @@ describe Gitlab::EncodingHelper do
       ["nil", nil, nil],
       ["empty string", "".encode("ASCII-8BIT"), "".encode("UTF-8")],
       ["invalid utf-8 encoded string", "my bad string\xE5".force_encoding("UTF-8"), "my bad string"],
+      ["frozen non-ascii string", "é".force_encoding("ASCII-8BIT").freeze, "é".encode("UTF-8")],
       [
         'leaves ascii only string as is',
         'ascii only string',
diff --git a/spec/lib/gitlab/git/user_spec.rb b/spec/lib/gitlab/git/user_spec.rb
index eb8db819045..99d850e1df9 100644
--- a/spec/lib/gitlab/git/user_spec.rb
+++ b/spec/lib/gitlab/git/user_spec.rb
@@ -1,9 +1,9 @@
 require 'spec_helper'
 
 describe Gitlab::Git::User do
-  let(:username) { 'janedo' }
-  let(:name) { 'Jane Doe' }
-  let(:email) { 'janedoe@example.com' }
+  let(:username) { 'janedoe' }
+  let(:name) { 'Jane Doé' }
+  let(:email) { 'janedoé@example.com' }
   let(:gl_id) { 'user-123' }
   let(:user) do
     described_class.new(username, name, email, gl_id)
@@ -13,7 +13,7 @@ describe Gitlab::Git::User do
 
   describe '.from_gitaly' do
     let(:gitaly_user) do
-      Gitaly::User.new(gl_username: username, name: name, email: email, gl_id: gl_id)
+      Gitaly::User.new(gl_username: username, name: name.b, email: email.b, gl_id: gl_id)
     end
 
     subject { described_class.from_gitaly(gitaly_user) }
@@ -48,8 +48,13 @@ describe Gitlab::Git::User do
     it 'creates a Gitaly::User with the correct data' do
       expect(subject).to be_a(Gitaly::User)
       expect(subject.gl_username).to eq(username)
-      expect(subject.name).to eq(name)
-      expect(subject.email).to eq(email)
+
+      expect(subject.name).to eq(name.b)
+      expect(subject.name).to be_a_binary_string
+
+      expect(subject.email).to eq(email.b)
+      expect(subject.email).to be_a_binary_string
+
       expect(subject.gl_id).to eq(gl_id)
     end
   end
diff --git a/spec/support/matchers/be_a_binary_string.rb b/spec/support/matchers/be_a_binary_string.rb
new file mode 100644
index 00000000000..f041ae76167
--- /dev/null
+++ b/spec/support/matchers/be_a_binary_string.rb
@@ -0,0 +1,9 @@
+RSpec::Matchers.define :be_a_binary_string do |_|
+  match do |actual|
+    actual.is_a?(String) && actual.encoding == Encoding.find('ASCII-8BIT')
+  end
+
+  description do
+    "be a String with binary encoding"
+  end
+end