From b44ccb283f68c10e33ac6c51b695fd0afd70642a Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Tue, 14 Aug 2018 11:20:21 -0700 Subject: [PATCH] Resolve "Orphaned upload files are accessible via project exports" --- lib/gitlab/import_export/uploads_manager.rb | 19 ++++++----- .../import_export/uploads_manager_spec.rb | 32 +++++++++++++++++++ 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/lib/gitlab/import_export/uploads_manager.rb b/lib/gitlab/import_export/uploads_manager.rb index 07875ebb56a..e0d4235e65b 100644 --- a/lib/gitlab/import_export/uploads_manager.rb +++ b/lib/gitlab/import_export/uploads_manager.rb @@ -13,13 +13,11 @@ module Gitlab end def save - copy_files(@from, uploads_export_path) if File.directory?(@from) - if File.file?(@from) && @relative_export_path == 'avatar' copy_files(@from, File.join(uploads_export_path, @project.avatar.filename)) end - copy_from_object_storage + copy_project_uploads true rescue => e @@ -48,14 +46,19 @@ module Gitlab UploadService.new(@project, File.open(upload, 'r'), FileUploader, uploader_context).execute end - def copy_from_object_storage - return unless Gitlab::ImportExport.object_storage? - + def copy_project_uploads each_uploader do |uploader| next unless uploader.file - next if uploader.upload.local? # Already copied, using the old method - download_and_copy(uploader) + if uploader.upload.local? + next unless uploader.upload.exist? + + copy_files(uploader.absolute_path, File.join(uploads_export_path, uploader.upload.path)) + else + next unless Gitlab::ImportExport.object_storage? + + download_and_copy(uploader) + end end end diff --git a/spec/lib/gitlab/import_export/uploads_manager_spec.rb b/spec/lib/gitlab/import_export/uploads_manager_spec.rb index 9c3870a0af8..f799de18cd0 100644 --- a/spec/lib/gitlab/import_export/uploads_manager_spec.rb +++ b/spec/lib/gitlab/import_export/uploads_manager_spec.rb @@ -36,6 +36,38 @@ describe Gitlab::ImportExport::UploadsManager do expect(File).to exist(exported_file_path) end + + context 'with orphaned project upload files' do + let(:orphan_path) { File.join(FileUploader.absolute_base_dir(project), 'f93f088ddf492ffd950cf059002cbbb6', 'orphan.jpg') } + let(:exported_orphan_path) { "#{shared.export_path}/uploads/f93f088ddf492ffd950cf059002cbbb6/orphan.jpg" } + + before do + FileUtils.mkdir_p(File.dirname(orphan_path)) + FileUtils.touch(orphan_path) + end + + after do + File.delete(orphan_path) if File.exist?(orphan_path) + end + + it 'excludes orphaned upload files' do + manager.save + + expect(File).not_to exist(exported_orphan_path) + end + end + + context 'with an upload missing its file' do + before do + File.delete(upload.absolute_path) + end + + it 'does not cause errors' do + manager.save + + expect(shared.errors).to be_empty + end + end end context 'using object storage' do