From 98535fc12cc381d562d916c4947a763511a828a7 Mon Sep 17 00:00:00 2001 From: blackst0ne Date: Tue, 8 Aug 2017 12:10:55 +1100 Subject: Add checks for branch existence before changing HEAD --- app/models/project.rb | 19 ++++++++++++------- app/services/projects/update_service.rb | 2 +- ...a-branch-that-doesn-t-exist-as-the-default-one.yml | 4 ++++ spec/models/project_spec.rb | 5 +++++ spec/services/projects/update_service_spec.rb | 7 +++++++ 5 files changed, 29 insertions(+), 8 deletions(-) create mode 100644 changelogs/unreleased/36010-api-v4-allows-setting-a-branch-that-doesn-t-exist-as-the-default-one.yml diff --git a/app/models/project.rb b/app/models/project.rb index e7baba2ef08..1575f38b2ea 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1046,13 +1046,18 @@ class Project < ActiveRecord::Base end def change_head(branch) - repository.before_change_head - repository.rugged.references.create('HEAD', - "refs/heads/#{branch}", - force: true) - repository.copy_gitattributes(branch) - repository.after_change_head - reload_default_branch + if repository.branch_exists?(branch) + repository.before_change_head + repository.rugged.references.create('HEAD', + "refs/heads/#{branch}", + force: true) + repository.copy_gitattributes(branch) + repository.after_change_head + reload_default_branch + else + errors.add(:base, "Could not change HEAD: branch '#{branch}' does not exist") + false + end end def forked_from?(project) diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index d81035e4eba..cf69007bc3b 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -10,7 +10,7 @@ module Projects end if changing_default_branch? - project.change_head(params[:default_branch]) + return error("Could not set the default branch") unless project.change_head(params[:default_branch]) end if project.update_attributes(params.except(:default_branch)) diff --git a/changelogs/unreleased/36010-api-v4-allows-setting-a-branch-that-doesn-t-exist-as-the-default-one.yml b/changelogs/unreleased/36010-api-v4-allows-setting-a-branch-that-doesn-t-exist-as-the-default-one.yml new file mode 100644 index 00000000000..04791e09b84 --- /dev/null +++ b/changelogs/unreleased/36010-api-v4-allows-setting-a-branch-that-doesn-t-exist-as-the-default-one.yml @@ -0,0 +1,4 @@ +--- +title: Add checks for branch existence before changing HEAD +merge_request: 13359 +author: Vitaliy @blackst0ne Klachkov diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 8f951605954..581fea65838 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1832,6 +1832,11 @@ describe Project do describe '#change_head' do let(:project) { create(:project, :repository) } + it 'returns error if branch does not exist' do + expect(project.change_head('unexisted-branch')).to be false + expect(project.errors.size).to eq(1) + end + it 'calls the before_change_head and after_change_head methods' do expect(project.repository).to receive(:before_change_head) expect(project.repository).to receive(:after_change_head) diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index d945e0aa1f0..1b282e82187 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -101,6 +101,13 @@ describe Projects::UpdateService, '#execute' do expect(Project.find(project.id).default_branch).to eq 'feature' end + + it 'does not change a default branch' do + # The branch 'unexisted-branch' does not exist. + update_project(project, admin, default_branch: 'unexisted-branch') + + expect(Project.find(project.id).default_branch).to eq 'master' + end end context 'when updating a project that contains container images' do -- cgit v1.2.1