From e8d2c2579383897a1dd7f9debd359abe8ae8373d Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 20 Jul 2021 09:55:51 +0000 Subject: Add latest changes from gitlab-org/gitlab@14-1-stable-ee --- .../bulk_imports/file_download_service_spec.rb | 147 ++++++++++++++++++--- 1 file changed, 126 insertions(+), 21 deletions(-) (limited to 'spec/services/bulk_imports/file_download_service_spec.rb') diff --git a/spec/services/bulk_imports/file_download_service_spec.rb b/spec/services/bulk_imports/file_download_service_spec.rb index 0961ddce553..a24af9ae64d 100644 --- a/spec/services/bulk_imports/file_download_service_spec.rb +++ b/spec/services/bulk_imports/file_download_service_spec.rb @@ -4,26 +4,41 @@ require 'spec_helper' RSpec.describe BulkImports::FileDownloadService do describe '#execute' do + let_it_be(:allowed_content_types) { %w(application/gzip application/octet-stream) } + let_it_be(:file_size_limit) { 5.gigabytes } let_it_be(:config) { build(:bulk_import_configuration) } let_it_be(:content_type) { 'application/octet-stream' } + let_it_be(:content_disposition) { nil } let_it_be(:filename) { 'file_download_service_spec' } let_it_be(:tmpdir) { Dir.tmpdir } let_it_be(:filepath) { File.join(tmpdir, filename) } + let_it_be(:content_length) { 1000 } + + let(:chunk_double) { double('chunk', size: 100, code: 200) } - let(:chunk_double) { double('chunk', size: 1000, code: 200) } let(:response_double) do double( code: 200, success?: true, parsed_response: {}, headers: { - 'content-length' => 100, - 'content-type' => content_type + 'content-length' => content_length, + 'content-type' => content_type, + 'content-disposition' => content_disposition } ) end - subject { described_class.new(configuration: config, relative_url: '/test', dir: tmpdir, filename: filename) } + subject do + described_class.new( + configuration: config, + relative_url: '/test', + dir: tmpdir, + filename: filename, + file_size_limit: file_size_limit, + allowed_content_types: allowed_content_types + ) + end before do allow_next_instance_of(BulkImports::Clients::HTTP) do |client| @@ -54,7 +69,14 @@ RSpec.describe BulkImports::FileDownloadService do stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) double = instance_double(BulkImports::Configuration, url: 'https://localhost', access_token: 'token') - service = described_class.new(configuration: double, relative_url: '/test', dir: tmpdir, filename: filename) + service = described_class.new( + configuration: double, + relative_url: '/test', + dir: tmpdir, + filename: filename, + file_size_limit: file_size_limit, + allowed_content_types: allowed_content_types + ) expect { service.execute }.to raise_error(Gitlab::UrlBlocker::BlockedUrlError) end @@ -70,31 +92,46 @@ RSpec.describe BulkImports::FileDownloadService do context 'when content-length is not valid' do context 'when content-length exceeds limit' do - before do - stub_const("#{described_class}::FILE_SIZE_LIMIT", 1) - end + let(:file_size_limit) { 1 } it 'raises an error' do - expect { subject.execute }.to raise_error(described_class::ServiceError, 'Invalid content length') + expect { subject.execute }.to raise_error( + described_class::ServiceError, + 'File size 1000 Bytes exceeds limit of 1 Byte' + ) end end context 'when content-length is missing' do - let(:response_double) { double(success?: true, headers: { 'content-type' => content_type }) } + let(:content_length) { nil } it 'raises an error' do - expect { subject.execute }.to raise_error(described_class::ServiceError, 'Invalid content length') + expect { subject.execute }.to raise_error( + described_class::ServiceError, + 'Missing content-length header' + ) end end end - context 'when partially downloaded file exceeds limit' do - before do - stub_const("#{described_class}::FILE_SIZE_LIMIT", 150) + context 'when content-length is equals the file size limit' do + let(:content_length) { 150 } + let(:file_size_limit) { 150 } + + it 'does not raise an error' do + expect { subject.execute }.not_to raise_error end + end + + context 'when partially downloaded file exceeds limit' do + let(:content_length) { 151 } + let(:file_size_limit) { 150 } it 'raises an error' do - expect { subject.execute }.to raise_error(described_class::ServiceError, 'Invalid downloaded file') + expect { subject.execute }.to raise_error( + described_class::ServiceError, + 'File size 151 Bytes exceeds limit of 150 Bytes' + ) end end @@ -102,7 +139,10 @@ RSpec.describe BulkImports::FileDownloadService do let(:chunk_double) { double('chunk', size: 1000, code: 307) } it 'raises an error' do - expect { subject.execute }.to raise_error(described_class::ServiceError, 'File download error 307') + expect { subject.execute }.to raise_error( + described_class::ServiceError, + 'File download error 307' + ) end end @@ -110,23 +150,88 @@ RSpec.describe BulkImports::FileDownloadService do let_it_be(:symlink) { File.join(tmpdir, 'symlink') } before do - FileUtils.ln_s(File.join(tmpdir, filename), symlink) + FileUtils.ln_s(File.join(tmpdir, filename), symlink, force: true) end - subject { described_class.new(configuration: config, relative_url: '/test', dir: tmpdir, filename: 'symlink') } + subject do + described_class.new( + configuration: config, + relative_url: '/test', + dir: tmpdir, + filename: 'symlink', + file_size_limit: file_size_limit, + allowed_content_types: allowed_content_types + ) + end it 'raises an error and removes the file' do - expect { subject.execute }.to raise_error(described_class::ServiceError, 'Invalid downloaded file') + expect { subject.execute }.to raise_error( + described_class::ServiceError, + 'Invalid downloaded file' + ) expect(File.exist?(symlink)).to eq(false) end end context 'when dir is not in tmpdir' do - subject { described_class.new(configuration: config, relative_url: '/test', dir: '/etc', filename: filename) } + subject do + described_class.new( + configuration: config, + relative_url: '/test', + dir: '/etc', + filename: filename, + file_size_limit: file_size_limit, + allowed_content_types: allowed_content_types + ) + end it 'raises an error' do - expect { subject.execute }.to raise_error(described_class::ServiceError, 'Invalid target directory') + expect { subject.execute }.to raise_error( + described_class::ServiceError, + 'Invalid target directory' + ) + end + end + + context 'when using the remote filename' do + let_it_be(:filename) { nil } + + context 'when no filename is given' do + it 'raises an error when the filename is not provided in the request header' do + expect { subject.execute }.to raise_error( + described_class::ServiceError, + 'Remote filename not provided in content-disposition header' + ) + end + end + + context 'with a given filename' do + let_it_be(:content_disposition) { 'filename="avatar.png"' } + + it 'uses the given filename' do + expect(subject.execute).to eq(File.join(tmpdir, "avatar.png")) + end + end + + context 'when the filename is a path' do + let_it_be(:content_disposition) { 'filename="../../avatar.png"' } + + it 'raises an error when the filename is not provided in the request header' do + expect(subject.execute).to eq(File.join(tmpdir, "avatar.png")) + end + end + + context 'when the filename is longer the the limit' do + let_it_be(:content_disposition) { 'filename="../../xxx.b"' } + + before do + stub_const("#{described_class}::FILENAME_SIZE_LIMIT", 1) + end + + it 'raises an error when the filename is not provided in the request header' do + expect(subject.execute).to eq(File.join(tmpdir, "x.b")) + end end end end -- cgit v1.2.3