Merge branch 'gitaly-encodings' into 'master'9-3-stable-with-gitaly-encodings

Fix gitaly ref encoding bugs

Closes #34156

See merge request !12522

4 files changed, 44 insertions, 11 deletions

diff --git a/lib/gitlab/git.rb b/lib/gitlab/git.rb
index 936606152e9..4175746be39 100644
--- a/lib/gitlab/git.rb
+++ b/lib/gitlab/git.rb
@@ -7,8 +7,10 @@ module Gitlab
     CommandError = Class.new(StandardError)
 
     class << self
+      include Gitlab::EncodingHelper
+
       def ref_name(ref)
-        ref.sub(/\Arefs\/(tags|heads)\//, '')
+        encode! ref.sub(/\Arefs\/(tags|heads)\//, '')
       end
 
       def branch_name(ref)
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 85695d0a4df..e3ec2c84c48 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -113,9 +113,7 @@ module Gitlab
       def local_branches(sort_by: nil)
         gitaly_migrate(:local_branches) do |is_enabled|
           if is_enabled
-            gitaly_ref_client.local_branches(sort_by: sort_by).map do |gitaly_branch|
-              Gitlab::Git::Branch.new(self, gitaly_branch.name, gitaly_branch)
-            end
+            gitaly_ref_client.local_branches(sort_by: sort_by)
           else
             branches(filter: :local, sort_by: sort_by)
           end
diff --git a/lib/gitlab/gitaly_client/ref.rb b/lib/gitlab/gitaly_client/ref.rb
index 227fe45642e..4ab7def12a8 100644
--- a/lib/gitlab/gitaly_client/ref.rb
+++ b/lib/gitlab/gitaly_client/ref.rb
@@ -1,10 +1,13 @@
 module Gitlab
   module GitalyClient
     class Ref
+      include Gitlab::EncodingHelper
+
       attr_accessor :stub
 
       # 'repository' is a Gitlab::Git::Repository
       def initialize(repository)
+        @repository = repository
         @gitaly_repo = repository.gitaly_repository
         @stub = GitalyClient.stub(:ref, repository.storage)
       end
@@ -18,12 +21,12 @@ module Gitlab
 
       def branch_names
         request = Gitaly::FindAllBranchNamesRequest.new(repository: @gitaly_repo)
-        consume_refs_response(stub.find_all_branch_names(request), prefix: 'refs/heads/')
+        consume_refs_response(stub.find_all_branch_names(request)) { |name| Gitlab::Git.branch_name(name) }
       end
 
       def tag_names
         request = Gitaly::FindAllTagNamesRequest.new(repository: @gitaly_repo)
-        consume_refs_response(stub.find_all_tag_names(request), prefix: 'refs/tags/')
+        consume_refs_response(stub.find_all_tag_names(request)) { |name| Gitlab::Git.tag_name(name) }
       end
 
       def find_ref_name(commit_id, ref_prefix)
@@ -52,10 +55,8 @@ module Gitlab
 
       private
 
-      def consume_refs_response(response, prefix:)
-        response.flat_map do |r|
-          r.names.map { |name| name.sub(/\A#{Regexp.escape(prefix)}/, '') }
-        end
+      def consume_refs_response(response)
+        response.flat_map { |message| message.names.map { |name| yield(name) } }
       end
 
       def sort_by_param(sort_by)
@@ -65,7 +66,15 @@ module Gitlab
       end
 
       def consume_branches_response(response)
-        response.flat_map { |r| r.branches }
+        response.flat_map do |message|
+          message.branches.map do |gitaly_branch|
+            Gitlab::Git::Branch.new(
+              @repository,
+              encode!(gitaly_branch.name.dup),
+              gitaly_branch.commit_id
+            )
+          end
+        end
       end
     end
   end
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index e1e4aa9fde9..daf5e5e2044 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -24,6 +24,10 @@ describe Gitlab::Git::Repository, seed_helper: true do
       end
     end
 
+    it 'returns UTF-8' do
+      expect(repository.root_ref.encoding).to eq(Encoding.find('UTF-8'))
+    end
+
     context 'with gitaly enabled' do
       before { stub_gitaly }
       after { Gitlab::GitalyClient.clear_stubs! }
@@ -116,6 +120,11 @@ describe Gitlab::Git::Repository, seed_helper: true do
     it 'has SeedRepo::Repo::BRANCHES.size elements' do
       expect(subject.size).to eq(SeedRepo::Repo::BRANCHES.size)
     end
+
+    it 'returns UTF-8' do
+      expect(subject.first.encoding).to eq(Encoding.find('UTF-8'))
+    end
+
     it { is_expected.to include("master") }
     it { is_expected.not_to include("branch-from-space") }
 
@@ -146,10 +155,15 @@ describe Gitlab::Git::Repository, seed_helper: true do
     subject { repository.tag_names }
 
     it { is_expected.to be_kind_of Array }
+
     it 'has SeedRepo::Repo::TAGS.size elements' do
       expect(subject.size).to eq(SeedRepo::Repo::TAGS.size)
     end
 
+    it 'returns UTF-8' do
+      expect(subject.first.encoding).to eq(Encoding.find('UTF-8'))
+    end
+
     describe '#last' do
       subject { super().last }
       it { is_expected.to eq("v1.2.1") }
@@ -1283,6 +1297,16 @@ describe Gitlab::Git::Repository, seed_helper: true do
       before { stub_gitaly }
       after { Gitlab::GitalyClient.clear_stubs! }
 
+      it 'returns a Branch with UTF-8 fields' do
+        branches = @repo.local_branches.to_a
+        expect(branches.size).to be > 0
+        utf_8 = Encoding.find('utf-8')
+        branches.each do |branch|
+          expect(branch.name.encoding).to eq(utf_8)
+          expect(branch.target.encoding).to eq(utf_8) unless branch.target.nil?
+        end
+      end
+
       it 'gets the branches from GitalyClient' do
         expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:local_branches).
           and_return([])