Commit 47978e97 authored by Douglas Barbosa Alexandre's avatar Douglas Barbosa Alexandre
Browse files

Doesn't check if path exists on disk when renaming a hashed project

parent 5ec7c5a35e4e
...@@ -64,6 +64,7 @@ class Project < ActiveRecord::Base ...@@ -64,6 +64,7 @@ class Project < ActiveRecord::Base
# Storage specific hooks # Storage specific hooks
after_initialize :use_hashed_storage after_initialize :use_hashed_storage
after_create :check_repository_absence!
after_create :ensure_storage_path_exists after_create :ensure_storage_path_exists
after_save :ensure_storage_path_exists, if: :namespace_id_changed? after_save :ensure_storage_path_exists, if: :namespace_id_changed?
...@@ -228,7 +229,7 @@ class Project < ActiveRecord::Base ...@@ -228,7 +229,7 @@ class Project < ActiveRecord::Base
validates :import_url, importable_url: true, if: [:external_import?, :import_url_changed?] validates :import_url, importable_url: true, if: [:external_import?, :import_url_changed?]
validates :star_count, numericality: { greater_than_or_equal_to: 0 } validates :star_count, numericality: { greater_than_or_equal_to: 0 }
validate :check_limit, on: :create validate :check_limit, on: :create
validate :check_repository_path_availability, on: [:create, :update], if: ->(project) { !project.persisted? || project.renamed? } validate :check_repository_path_availability, on: :update, if: ->(project) { project.renamed? }
validate :avatar_type, validate :avatar_type,
if: ->(project) { project.avatar.present? && project.avatar_changed? } if: ->(project) { project.avatar.present? && project.avatar_changed? }
validates :avatar, file_size: { maximum: 200.kilobytes.to_i } validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
...@@ -1025,7 +1026,9 @@ def check_repository_path_availability ...@@ -1025,7 +1026,9 @@ def check_repository_path_availability
expires_full_path_cache # we need to clear cache to validate renames correctly expires_full_path_cache # we need to clear cache to validate renames correctly
if gitlab_shell.exists?(repository_storage_path, "#{disk_path}.git") # Check if repository with same path already exists on disk we can
# skip this for the hashed storage because the path does not change
if legacy_storage? && repository_with_same_path_already_exists?
errors.add(:base, 'There is already a repository with that name on disk') errors.add(:base, 'There is already a repository with that name on disk')
return false return false
end end
...@@ -1613,6 +1616,19 @@ def wiki_reference_count ...@@ -1613,6 +1616,19 @@ def wiki_reference_count
Gitlab::ReferenceCounter.new(gl_repository(is_wiki: true)).value Gitlab::ReferenceCounter.new(gl_repository(is_wiki: true)).value
end end
def check_repository_absence!
return if skip_disk_validation
if repository_storage_path.blank? || repository_with_same_path_already_exists?
errors.add(:base, 'There is already a repository with that name on disk')
throw :abort
end
end
def repository_with_same_path_already_exists?
gitlab_shell.exists?(repository_storage_path, "#{disk_path}.git")
end
# set last_activity_at to the same as created_at # set last_activity_at to the same as created_at
def set_last_activity_at def set_last_activity_at
update_column(:last_activity_at, self.created_at) update_column(:last_activity_at, self.created_at)
......
...@@ -149,6 +149,9 @@ ...@@ -149,6 +149,9 @@
end end
context 'when another repository already exists on disk' do context 'when another repository already exists on disk' do
let(:repository_storage) { 'default' }
let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] }
let(:opts) do let(:opts) do
{ {
name: 'Existing', name: 'Existing',
...@@ -156,31 +159,59 @@ ...@@ -156,31 +159,59 @@
} }
end end
let(:repository_storage) { 'default' } context 'with legacy storage' do
let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } before do
gitlab_shell.add_repository(repository_storage_path, "#{user.namespace.full_path}/existing")
end
before do after do
gitlab_shell.add_repository(repository_storage, "#{user.namespace.full_path}/existing") gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing")
end end
after do it 'does not allow to create a project when path matches existing repository on disk' do
gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") project = create_project(user, opts)
end
it 'does not allow to create project with same path' do expect(project).not_to be_persisted
project = create_project(user, opts) expect(project).to respond_to(:errors)
expect(project.errors.messages).to have_key(:base)
expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk')
end
expect(project).to respond_to(:errors) it 'does not allow to import project when path matches existing repository on disk' do
expect(project.errors.messages).to have_key(:base) project = create_project(user, opts.merge({ import_url: 'https://gitlab.com/gitlab-org/gitlab-test.git' }))
expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk')
expect(project).not_to be_persisted
expect(project).to respond_to(:errors)
expect(project.errors.messages).to have_key(:base)
expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk')
end
end end
it 'does not allow to import a project with the same path' do context 'with hashed storage' do
project = create_project(user, opts.merge({ import_url: 'https://gitlab.com/gitlab-org/gitlab-test.git' })) let(:hash) { '6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b' }
let(:hashed_path) { '@hashed/6b/86/6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b' }
before do
stub_application_setting(hashed_storage_enabled: true)
allow(Digest::SHA2).to receive(:hexdigest) { hash }
end
before do
gitlab_shell.add_repository(repository_storage_path, hashed_path)
end
after do
gitlab_shell.remove_repository(repository_storage_path, hashed_path)
end
it 'does not allow to create a project when path matches existing repository on disk' do
project = create_project(user, opts)
expect(project).to respond_to(:errors) expect(project).not_to be_persisted
expect(project.errors.messages).to have_key(:base) expect(project).to respond_to(:errors)
expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') expect(project.errors.messages).to have_key(:base)
expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk')
end
end end
end end
end end
......
...@@ -152,22 +152,40 @@ ...@@ -152,22 +152,40 @@
let(:repository_storage) { 'default' } let(:repository_storage) { 'default' }
let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] }
before do context 'with legacy storage' do
gitlab_shell.add_repository(repository_storage, "#{user.namespace.full_path}/existing") before do
end gitlab_shell.add_repository(repository_storage_path, "#{user.namespace.full_path}/existing")
end
after do after do
gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing")
end
it 'does not allow renaming when new path matches existing repository on disk' do
result = update_project(project, admin, path: 'existing')
expect(result).to include(status: :error)
expect(result[:message]).to match('There is already a repository with that name on disk')
expect(project).not_to be_valid
expect(project.errors.messages).to have_key(:base)
expect(project.errors.messages[:base]).to include('There is already a repository with that name on disk')
end
end end
it 'does not allow renaming when new path matches existing repository on disk' do context 'with hashed storage' do
result = update_project(project, admin, path: 'existing') let(:project) { create(:project, :repository, creator: user, namespace: user.namespace) }
expect(result).to include(status: :error) before do
expect(result[:message]).to match('There is already a repository with that name on disk') stub_application_setting(hashed_storage_enabled: true)
expect(project).not_to be_valid end
expect(project.errors.messages).to have_key(:base)
expect(project.errors.messages[:base]).to include('There is already a repository with that name on disk') it 'does not check if new path matches existing repository on disk' do
expect(project).not_to receive(:repository_with_same_path_already_exists?)
result = update_project(project, admin, path: 'existing')
expect(result).to include(status: :success)
end
end end
end end
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment