diff --git a/changelogs/unreleased/zj-gitaly-encoding-issue.yml b/changelogs/unreleased/zj-gitaly-encoding-issue.yml new file mode 100644 index 0000000000000000000000000000000000000000..073d8f38e4bd8fff321e0ad10160a3d303c19480 --- /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 603457d0664683d0f4512020e2d968da2ea0d1af..d7e40471798cc8e5dea3dabd4a409b636dec3f02 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 13358995383e8ef5df066e2f6f2769352e874e51..4214f692f4cf19bb9f9d70fa1ebef94527a7b82c 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