Merge branch '37025-error-500-in-non-utf8-branch-names' into 'master'

Resolve "Error 500 in non-UTF8 branch names"

This could potentially "duplicate" branches on the UI, but it prevents
500 errors. The real issue here is the fact that a branch name with a
invalid UTF-8 characters.

Closes #37025

See merge request !14090

5 files changed, 30 insertions, 2 deletions

diff --git a/changelogs/unreleased/37025-error-500-in-non-utf8-branch-names.yml b/changelogs/unreleased/37025-error-500-in-non-utf8-branch-names.yml
new file mode 100644
index 00000000000..f3118cf0f2f
--- /dev/null
+++ b/changelogs/unreleased/37025-error-500-in-non-utf8-branch-names.yml
@@ -0,0 +1,4 @@
+---
+title: Fixed non-UTF-8 valid branch names from causing an error.
+merge_request: 14090
+type: fixed
diff --git a/lib/gitlab/git.rb b/lib/gitlab/git.rb
index 8c9acbc9fbe..b4b6326cfdd 100644
--- a/lib/gitlab/git.rb
+++ b/lib/gitlab/git.rb
@@ -11,7 +11,7 @@ module Gitlab
       include Gitlab::EncodingHelper
 
       def ref_name(ref)
-        encode! ref.sub(/\Arefs\/(tags|heads|remotes)\//, '')
+        encode_utf8(ref).sub(/\Arefs\/(tags|heads|remotes)\//, '')
       end
 
       def branch_name(ref)
diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb
index 745d051a5c1..5e0b57e9b2e 100644
--- a/spec/controllers/projects/branches_controller_spec.rb
+++ b/spec/controllers/projects/branches_controller_spec.rb
@@ -367,5 +367,20 @@ describe Projects::BranchesController do
         expect(parsed_response.first).to eq 'master'
       end
     end
+
+    context 'when branch contains an invalid UTF-8 sequence' do
+      before do
+        project.repository.create_branch("wrong-\xE5-utf8-sequence")
+      end
+
+      it 'return with a status 200' do
+        get :index,
+            namespace_id: project.namespace,
+            project_id: project,
+            format: :html
+
+        expect(response).to have_http_status(200)
+      end
+    end
   end
 end
diff --git a/spec/lib/gitlab/data_builder/push_spec.rb b/spec/lib/gitlab/data_builder/push_spec.rb
index cb430b47463..befdc18d1aa 100644
--- a/spec/lib/gitlab/data_builder/push_spec.rb
+++ b/spec/lib/gitlab/data_builder/push_spec.rb
@@ -47,7 +47,7 @@ describe Gitlab::DataBuilder::Push do
     include_examples 'deprecated repository hook data'
 
     it 'does not raise an error when given nil commits' do
-      expect { described_class.build(spy, spy, spy, spy, spy, nil) }
+      expect { described_class.build(spy, spy, spy, spy, 'refs/tags/v1.1.0', nil) }
         .not_to raise_error
     end
   end
diff --git a/spec/lib/gitlab/git_spec.rb b/spec/lib/gitlab/git_spec.rb
index 4702a978f19..494dfe0e595 100644
--- a/spec/lib/gitlab/git_spec.rb
+++ b/spec/lib/gitlab/git_spec.rb
@@ -1,3 +1,4 @@
+# coding: utf-8
 require 'spec_helper'
 
 describe Gitlab::Git do
@@ -29,4 +30,12 @@ describe Gitlab::Git do
       end
     end
   end
+
+  describe '.ref_name' do
+    it 'ensure ref is a valid UTF-8 string' do
+      utf8_invalid_ref = Gitlab::Git::BRANCH_REF_PREFIX + "an_invalid_ref_\xE5"
+
+      expect(described_class.ref_name(utf8_invalid_ref)).to eq("an_invalid_ref_å")
+    end
+  end
 end