summaryrefslogtreecommitdiff
path: root/spec/lib
diff options
context:
space:
mode:
Diffstat (limited to 'spec/lib')
-rw-r--r--spec/lib/gitlab/ci/artifact_file_reader_spec.rb124
-rw-r--r--spec/lib/safe_zip/entry_spec.rb35
-rw-r--r--spec/lib/safe_zip/extract_params_spec.rb35
-rw-r--r--spec/lib/safe_zip/extract_spec.rb55
4 files changed, 193 insertions, 56 deletions
diff --git a/spec/lib/gitlab/ci/artifact_file_reader_spec.rb b/spec/lib/gitlab/ci/artifact_file_reader_spec.rb
index e982f0eb015..813dc15e79f 100644
--- a/spec/lib/gitlab/ci/artifact_file_reader_spec.rb
+++ b/spec/lib/gitlab/ci/artifact_file_reader_spec.rb
@@ -10,71 +10,117 @@ RSpec.describe Gitlab::Ci::ArtifactFileReader do
subject { described_class.new(job).read(path) }
context 'when job has artifacts and metadata' do
- let!(:artifacts) { create(:ci_job_artifact, :archive, job: job) }
- let!(:metadata) { create(:ci_job_artifact, :metadata, job: job) }
+ shared_examples 'extracting job artifact archive' do
+ it 'returns the content at the path' do
+ is_expected.to be_present
+ expect(YAML.safe_load(subject).keys).to contain_exactly('rspec', 'time', 'custom')
+ end
- it 'returns the content at the path' do
- is_expected.to be_present
- expect(YAML.safe_load(subject).keys).to contain_exactly('rspec', 'time', 'custom')
- end
+ context 'when path does not exist' do
+ let(:path) { 'file/does/not/exist.txt' }
+ let(:expected_error) do
+ "Path `#{path}` does not exist inside the `#{job.name}` artifacts archive!"
+ end
- context 'when path does not exist' do
- let(:path) { 'file/does/not/exist.txt' }
- let(:expected_error) do
- "Path `#{path}` does not exist inside the `#{job.name}` artifacts archive!"
+ it 'raises an error' do
+ expect { subject }.to raise_error(described_class::Error, expected_error)
+ end
end
- it 'raises an error' do
- expect { subject }.to raise_error(described_class::Error, expected_error)
+ context 'when path points to a directory' do
+ let(:path) { 'other_artifacts_0.1.2' }
+ let(:expected_error) do
+ "Path `#{path}` was expected to be a file but it was a directory!"
+ end
+
+ it 'raises an error' do
+ expect { subject }.to raise_error(described_class::Error, expected_error)
+ end
end
- end
- context 'when path points to a directory' do
- let(:path) { 'other_artifacts_0.1.2' }
- let(:expected_error) do
- "Path `#{path}` was expected to be a file but it was a directory!"
+ context 'when path is nested' do
+ # path exists in ci_build_artifacts.zip
+ let(:path) { 'other_artifacts_0.1.2/doc_sample.txt' }
+
+ it 'returns the content at the nested path' do
+ is_expected.to be_present
+ end
end
- it 'raises an error' do
- expect { subject }.to raise_error(described_class::Error, expected_error)
+ context 'when artifact archive size is greater than the limit' do
+ let(:expected_error) do
+ "Artifacts archive for job `#{job.name}` is too large: max 1 KB"
+ end
+
+ before do
+ stub_const("#{described_class}::MAX_ARCHIVE_SIZE", 1.kilobyte)
+ end
+
+ it 'raises an error' do
+ expect { subject }.to raise_error(described_class::Error, expected_error)
+ end
end
- end
- context 'when path is nested' do
- # path exists in ci_build_artifacts.zip
- let(:path) { 'other_artifacts_0.1.2/doc_sample.txt' }
+ context 'when metadata entry shows size greater than the limit' do
+ let(:expected_error) do
+ "Artifacts archive for job `#{job.name}` is too large: max 5 MB"
+ end
- it 'returns the content at the nested path' do
- is_expected.to be_present
+ before do
+ expect_next_instance_of(Gitlab::Ci::Build::Artifacts::Metadata::Entry) do |entry|
+ expect(entry).to receive(:total_size).and_return(10.megabytes)
+ end
+ end
+
+ it 'raises an error' do
+ expect { subject }.to raise_error(described_class::Error, expected_error)
+ end
end
end
- context 'when artifact archive size is greater than the limit' do
- let(:expected_error) do
- "Artifacts archive for job `#{job.name}` is too large: max 1 KB"
- end
+ context 'when job artifact is on local storage' do
+ let!(:artifacts) { create(:ci_job_artifact, :archive, job: job) }
+ let!(:metadata) { create(:ci_job_artifact, :metadata, job: job) }
+
+ it_behaves_like 'extracting job artifact archive'
+ end
+ context 'when job artifact is on remote storage' do
before do
- stub_const("#{described_class}::MAX_ARCHIVE_SIZE", 1.kilobyte)
+ stub_artifacts_object_storage
+ stub_request(:get, %r{https://artifacts.+ci_build_artifacts\.zip})
+ .to_return(
+ status: 200,
+ body: File.open(Rails.root.join('spec/fixtures/ci_build_artifacts.zip')),
+ headers: {}
+ )
+ stub_request(:get, %r{https://artifacts.+ci_build_artifacts_metadata})
+ .to_return(
+ status: 200,
+ body: File.open(Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz')),
+ headers: {}
+ )
end
- it 'raises an error' do
- expect { subject }.to raise_error(described_class::Error, expected_error)
- end
+ let!(:artifacts) { create(:ci_job_artifact, :archive, :remote_store, job: job) }
+ let!(:metadata) { create(:ci_job_artifact, :metadata, :remote_store, job: job) }
+
+ it_behaves_like 'extracting job artifact archive'
end
- context 'when metadata entry shows size greater than the limit' do
- let(:expected_error) do
- "Artifacts archive for job `#{job.name}` is too large: max 5 MB"
- end
+ context 'when extracting job artifact raises entry size error' do
+ let!(:artifacts) { create(:ci_job_artifact, :archive, job: job) }
+ let!(:metadata) { create(:ci_job_artifact, :metadata, job: job) }
before do
- expect_next_instance_of(Gitlab::Ci::Build::Artifacts::Metadata::Entry) do |entry|
- expect(entry).to receive(:total_size).and_return(10.megabytes)
+ allow_next_instance_of(SafeZip::Extract, anything) do |extractor|
+ allow(extractor).to receive(:extract).and_raise(SafeZip::Extract::EntrySizeError)
end
end
it 'raises an error' do
+ expected_error = "Path `#{path}` has invalid size in the zip!"
+
expect { subject }.to raise_error(described_class::Error, expected_error)
end
end
diff --git a/spec/lib/safe_zip/entry_spec.rb b/spec/lib/safe_zip/entry_spec.rb
index 9929b8073a0..8d49e2bcece 100644
--- a/spec/lib/safe_zip/entry_spec.rb
+++ b/spec/lib/safe_zip/entry_spec.rb
@@ -5,12 +5,13 @@ require 'spec_helper'
RSpec.describe SafeZip::Entry do
let(:target_path) { Dir.mktmpdir('safe-zip') }
let(:directories) { %w(public folder/with/subfolder) }
- let(:params) { SafeZip::ExtractParams.new(directories: directories, to: target_path) }
+ let(:files) { %w(public/index.html public/assets/image.png) }
+ let(:params) { SafeZip::ExtractParams.new(directories: directories, files: files, to: target_path) }
let(:entry) { described_class.new(zip_archive, zip_entry, params) }
let(:entry_name) { 'public/folder/index.html' }
let(:entry_path_dir) { File.join(target_path, File.dirname(entry_name)) }
- let(:entry_path) { File.join(target_path, entry_name) }
+ let(:entry_path) { File.join(File.realpath(target_path), entry_name) }
let(:zip_archive) { double }
let(:zip_entry) do
@@ -28,7 +29,7 @@ RSpec.describe SafeZip::Entry do
describe '#path_dir' do
subject { entry.path_dir }
- it { is_expected.to eq(target_path + '/public/folder') }
+ it { is_expected.to eq(File.realpath(target_path) + '/public/folder') }
end
describe '#exist?' do
@@ -51,6 +52,9 @@ RSpec.describe SafeZip::Entry do
subject { entry.extract }
context 'when entry does not match the filtered directories' do
+ let(:directories) { %w(public folder/with/subfolder) }
+ let(:files) { [] }
+
using RSpec::Parameterized::TableSyntax
where(:entry_name) do
@@ -70,7 +74,30 @@ RSpec.describe SafeZip::Entry do
end
end
- context 'when entry does exist' do
+ context 'when entry does not match the filtered files' do
+ let(:directories) { [] }
+ let(:files) { %w(public/index.html public/assets/image.png) }
+
+ using RSpec::Parameterized::TableSyntax
+
+ where(:entry_name) do
+ [
+ 'assets/folder/index.html',
+ 'public/../folder/index.html',
+ 'public/../../../../../index.html',
+ '../../../../../public/index.html',
+ '/etc/passwd'
+ ]
+ end
+
+ with_them do
+ it 'does not extract file' do
+ is_expected.to be_falsey
+ end
+ end
+ end
+
+ context 'when there is an existing extracted entry' do
before do
create_entry
end
diff --git a/spec/lib/safe_zip/extract_params_spec.rb b/spec/lib/safe_zip/extract_params_spec.rb
index 880c4358663..0ebfb7430c5 100644
--- a/spec/lib/safe_zip/extract_params_spec.rb
+++ b/spec/lib/safe_zip/extract_params_spec.rb
@@ -4,8 +4,10 @@ require 'spec_helper'
RSpec.describe SafeZip::ExtractParams do
let(:target_path) { Dir.mktmpdir("safe-zip") }
- let(:params) { described_class.new(directories: directories, to: target_path) }
+ let(:real_target_path) { File.realpath(target_path) }
+ let(:params) { described_class.new(directories: directories, files: files, to: target_path) }
let(:directories) { %w(public folder/with/subfolder) }
+ let(:files) { %w(public/index.html public/assets/image.png) }
after do
FileUtils.remove_entry_secure(target_path)
@@ -14,13 +16,13 @@ RSpec.describe SafeZip::ExtractParams do
describe '#extract_path' do
subject { params.extract_path }
- it { is_expected.to eq(target_path) }
+ it { is_expected.to eq(real_target_path) }
end
describe '#matching_target_directory' do
using RSpec::Parameterized::TableSyntax
- subject { params.matching_target_directory(target_path + path) }
+ subject { params.matching_target_directory(real_target_path + path) }
where(:path, :result) do
'/public/index.html' | '/public/'
@@ -30,7 +32,7 @@ RSpec.describe SafeZip::ExtractParams do
end
with_them do
- it { is_expected.to eq(result ? target_path + result : nil) }
+ it { is_expected.to eq(result ? real_target_path + result : nil) }
end
end
@@ -38,7 +40,7 @@ RSpec.describe SafeZip::ExtractParams do
subject { params.target_directories }
it 'starts with target_path' do
- is_expected.to all(start_with(target_path + '/'))
+ is_expected.to all(start_with(real_target_path + '/'))
end
it 'ends with / for all paths' do
@@ -53,4 +55,27 @@ RSpec.describe SafeZip::ExtractParams do
is_expected.to all(end_with('/*'))
end
end
+
+ describe '#matching_target_file' do
+ using RSpec::Parameterized::TableSyntax
+
+ subject { params.matching_target_file(real_target_path + path) }
+
+ where(:path, :result) do
+ '/public/index.html' | true
+ '/non/existing/path' | false
+ '/public/' | false
+ '/folder/with/index.html' | false
+ end
+
+ with_them do
+ it { is_expected.to eq(result) }
+ end
+ end
+
+ context 'when directories and files are empty' do
+ it 'is invalid' do
+ expect { described_class.new(to: target_path) }.to raise_error(ArgumentError, /directories or files are required/)
+ end
+ end
end
diff --git a/spec/lib/safe_zip/extract_spec.rb b/spec/lib/safe_zip/extract_spec.rb
index 443430b267d..c727475e271 100644
--- a/spec/lib/safe_zip/extract_spec.rb
+++ b/spec/lib/safe_zip/extract_spec.rb
@@ -5,6 +5,7 @@ require 'spec_helper'
RSpec.describe SafeZip::Extract do
let(:target_path) { Dir.mktmpdir('safe-zip') }
let(:directories) { %w(public) }
+ let(:files) { %w(public/index.html) }
let(:object) { described_class.new(archive) }
let(:archive) { Rails.root.join('spec', 'fixtures', 'safe_zip', archive_name) }
@@ -13,20 +14,36 @@ RSpec.describe SafeZip::Extract do
end
describe '#extract' do
- subject { object.extract(directories: directories, to: target_path) }
+ subject { object.extract(directories: directories, files: files, to: target_path) }
shared_examples 'extracts archive' do
- it 'does extract archive' do
- subject
+ context 'when specifying directories' do
+ subject { object.extract(directories: directories, to: target_path) }
- expect(File.exist?(File.join(target_path, 'public', 'index.html'))).to eq(true)
- expect(File.exist?(File.join(target_path, 'source'))).to eq(false)
+ it 'does extract archive' do
+ subject
+
+ expect(File.exist?(File.join(target_path, 'public', 'index.html'))).to eq(true)
+ expect(File.exist?(File.join(target_path, 'public', 'assets', 'image.png'))).to eq(true)
+ expect(File.exist?(File.join(target_path, 'source'))).to eq(false)
+ end
+ end
+
+ context 'when specifying files' do
+ subject { object.extract(files: files, to: target_path) }
+
+ it 'does extract archive' do
+ subject
+
+ expect(File.exist?(File.join(target_path, 'public', 'index.html'))).to eq(true)
+ expect(File.exist?(File.join(target_path, 'public', 'assets', 'image.png'))).to eq(false)
+ end
end
end
shared_examples 'fails to extract archive' do
it 'does not extract archive' do
- expect { subject }.to raise_error(SafeZip::Extract::Error)
+ expect { subject }.to raise_error(SafeZip::Extract::Error, including(error_message))
end
end
@@ -38,9 +55,18 @@ RSpec.describe SafeZip::Extract do
end
end
- %w(invalid-symlink-does-not-exist.zip invalid-symlinks-outside.zip).each do |name|
- context "when using #{name} archive" do
+ context 'when zip files are invalid' do
+ using RSpec::Parameterized::TableSyntax
+
+ where(:name, :message) do
+ 'invalid-symlink-does-not-exist.zip' | 'does not exist'
+ 'invalid-symlinks-outside.zip' | 'Symlink cannot be created'
+ 'invalid-unexpected-large.zip' | 'larger when inflated'
+ end
+
+ with_them do
let(:archive_name) { name }
+ let(:error_message) { message }
it_behaves_like 'fails to extract archive'
end
@@ -49,6 +75,19 @@ RSpec.describe SafeZip::Extract do
context 'when no matching directories are found' do
let(:archive_name) { 'valid-simple.zip' }
let(:directories) { %w(non/existing) }
+ let(:error_message) { 'No entries extracted' }
+
+ subject { object.extract(directories: directories, to: target_path) }
+
+ it_behaves_like 'fails to extract archive'
+ end
+
+ context 'when no matching files are found' do
+ let(:archive_name) { 'valid-simple.zip' }
+ let(:files) { %w(non/existing) }
+ let(:error_message) { 'No entries extracted' }
+
+ subject { object.extract(files: files, to: target_path) }
it_behaves_like 'fails to extract archive'
end