Merge branch 'zj-gitaly-encoding-issue' into 'master'

Encode revision for gitattributes ref

Closes gitaly#1032 and #43278

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

3 files changed, 73 insertions, 52 deletions

diff --git a/changelogs/unreleased/zj-gitaly-encoding-issue.yml b/changelogs/unreleased/zj-gitaly-encoding-issue.yml
new file mode 100644
index 00000000000..073d8f38e4b
--- /dev/null
+++ b/changelogs/unreleased/zj-gitaly-encoding-issue.yml
@@ -0,0 +1,5 @@
+---
+title: Encode branch name as binary before creating a RPC request to copy attributes
+merge_request: 17291
+author:
+type: fixed
diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb
index 60706b4f0d8..35c6cfe5817 100644
--- a/lib/gitlab/gitaly_client/repository_service.rb
+++ b/lib/gitlab/gitaly_client/repository_service.rb
@@ -41,7 +41,7 @@ module Gitlab
       end
 
       def apply_gitattributes(revision)
-        request = Gitaly::ApplyGitattributesRequest.new(repository: @gitaly_repo, revision: revision)
+        request = Gitaly::ApplyGitattributesRequest.new(repository: @gitaly_repo, revision: encode_binary(revision))
         GitalyClient.call(@storage, :repository_service, :apply_gitattributes, request)
       end
 
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 9feaaec3f3b..420672ccf12 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -1406,79 +1406,95 @@ describe Gitlab::Git::Repository, seed_helper: true do
   end
 
   describe "#copy_gitattributes" do
-    let(:attributes_path) { File.join(SEED_STORAGE_PATH, TEST_REPO_PATH, 'info/attributes') }
+    shared_examples 'applying git attributes' do
+      let(:attributes_path) { File.join(SEED_STORAGE_PATH, TEST_REPO_PATH, 'info/attributes') }
 
-    it "raises an error with invalid ref" do
-      expect { repository.copy_gitattributes("invalid") }.to raise_error(Gitlab::Git::Repository::InvalidRef)
-    end
-
-    context "with no .gitattrbutes" do
-      before do
-        repository.copy_gitattributes("master")
+      after do
+        FileUtils.rm_rf(attributes_path) if Dir.exist?(attributes_path)
       end
 
-      it "does not have an info/attributes" do
-        expect(File.exist?(attributes_path)).to be_falsey
+      it "raises an error with invalid ref" do
+        expect { repository.copy_gitattributes("invalid") }.to raise_error(Gitlab::Git::Repository::InvalidRef)
       end
 
-      after do
-        FileUtils.rm_rf(attributes_path)
-      end
-    end
+      context 'when forcing encoding issues' do
+        let(:branch_name) { "ʕ•ᴥ•ʔ" }
 
-    context "with .gitattrbutes" do
-      before do
-        repository.copy_gitattributes("gitattributes")
-      end
+        before do
+          repository.create_branch(branch_name, "master")
+        end
 
-      it "has an info/attributes" do
-        expect(File.exist?(attributes_path)).to be_truthy
-      end
+        after do
+          repository.rm_branch(branch_name, user: build(:admin))
+        end
 
-      it "has the same content in info/attributes as .gitattributes" do
-        contents = File.open(attributes_path, "rb") { |f| f.read }
-        expect(contents).to eq("*.md binary\n")
-      end
+        it "doesn't raise with a valid unicode ref" do
+          expect { repository.copy_gitattributes(branch_name) }.not_to raise_error
 
-      after do
-        FileUtils.rm_rf(attributes_path)
+          repository
+        end
       end
-    end
 
-    context "with updated .gitattrbutes" do
-      before do
-        repository.copy_gitattributes("gitattributes")
-        repository.copy_gitattributes("gitattributes-updated")
-      end
+      context "with no .gitattrbutes" do
+        before do
+          repository.copy_gitattributes("master")
+        end
 
-      it "has an info/attributes" do
-        expect(File.exist?(attributes_path)).to be_truthy
+        it "does not have an info/attributes" do
+          expect(File.exist?(attributes_path)).to be_falsey
+        end
       end
 
-      it "has the updated content in info/attributes" do
-        contents = File.read(attributes_path)
-        expect(contents).to eq("*.txt binary\n")
-      end
+      context "with .gitattrbutes" do
+        before do
+          repository.copy_gitattributes("gitattributes")
+        end
 
-      after do
-        FileUtils.rm_rf(attributes_path)
-      end
-    end
+        it "has an info/attributes" do
+          expect(File.exist?(attributes_path)).to be_truthy
+        end
 
-    context "with no .gitattrbutes in HEAD but with previous info/attributes" do
-      before do
-        repository.copy_gitattributes("gitattributes")
-        repository.copy_gitattributes("master")
+        it "has the same content in info/attributes as .gitattributes" do
+          contents = File.open(attributes_path, "rb") { |f| f.read }
+          expect(contents).to eq("*.md binary\n")
+        end
       end
 
-      it "does not have an info/attributes" do
-        expect(File.exist?(attributes_path)).to be_falsey
+      context "with updated .gitattrbutes" do
+        before do
+          repository.copy_gitattributes("gitattributes")
+          repository.copy_gitattributes("gitattributes-updated")
+        end
+
+        it "has an info/attributes" do
+          expect(File.exist?(attributes_path)).to be_truthy
+        end
+
+        it "has the updated content in info/attributes" do
+          contents = File.read(attributes_path)
+          expect(contents).to eq("*.txt binary\n")
+        end
       end
 
-      after do
-        FileUtils.rm_rf(attributes_path)
+      context "with no .gitattrbutes in HEAD but with previous info/attributes" do
+        before do
+          repository.copy_gitattributes("gitattributes")
+          repository.copy_gitattributes("master")
+        end
+
+        it "does not have an info/attributes" do
+          expect(File.exist?(attributes_path)).to be_falsey
+        end
       end
     end
+
+    context 'when gitaly is enabled' do
+      it_behaves_like 'applying git attributes'
+    end
+
+    context 'when gitaly is disabled', :disable_gitaly do
+      it_behaves_like 'applying git attributes'
+    end
   end
 
   describe '#ref_exists?' do