Commit 8b35bf71 authored by Dylan Griffith's avatar Dylan Griffith

Merge branch 'remove-safezip_use_rubyzip' into 'master'

Remove `safezip_use_rubyzip` feature flag

See merge request gitlab-org/gitlab!43554
parents aa67f5db 6e29c1de
---
name: safezip_use_rubyzip
introduced_by_url:
rollout_issue_url:
group:
type: development
default_enabled: true
...@@ -19,11 +19,7 @@ module SafeZip ...@@ -19,11 +19,7 @@ module SafeZip
def extract(opts = {}) def extract(opts = {})
params = SafeZip::ExtractParams.new(**opts) params = SafeZip::ExtractParams.new(**opts)
if Feature.enabled?(:safezip_use_rubyzip, default_enabled: true) extract_with_ruby_zip(params)
extract_with_ruby_zip(params)
else
legacy_unsafe_extract_with_system_zip(params)
end
end end
private private
...@@ -53,21 +49,5 @@ module SafeZip ...@@ -53,21 +49,5 @@ module SafeZip
.extract .extract
end end
end end
def legacy_unsafe_extract_with_system_zip(params)
# Requires UnZip at least 6.00 Info-ZIP.
# -n never overwrite existing files
args = %W(unzip -n -qq #{archive_path})
# We add * to end of directory, because we want to extract directory and all subdirectories
args += params.directories_wildcard
# Target directory where we extract
args += %W(-d #{params.extract_path})
unless system(*args)
raise Error, 'archive failed to extract'
end
end
end end
end end
...@@ -15,11 +15,7 @@ RSpec.describe SafeZip::Extract do ...@@ -15,11 +15,7 @@ RSpec.describe SafeZip::Extract do
describe '#extract' do describe '#extract' do
subject { object.extract(directories: directories, to: target_path) } subject { object.extract(directories: directories, to: target_path) }
shared_examples 'extracts archive' do |param| shared_examples 'extracts archive' do
before do
stub_feature_flags(safezip_use_rubyzip: param)
end
it 'does extract archive' do it 'does extract archive' do
subject subject
...@@ -28,11 +24,7 @@ RSpec.describe SafeZip::Extract do ...@@ -28,11 +24,7 @@ RSpec.describe SafeZip::Extract do
end end
end end
shared_examples 'fails to extract archive' do |param| shared_examples 'fails to extract archive' do
before do
stub_feature_flags(safezip_use_rubyzip: param)
end
it 'does not 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)
end end
...@@ -42,13 +34,7 @@ RSpec.describe SafeZip::Extract do ...@@ -42,13 +34,7 @@ RSpec.describe SafeZip::Extract do
context "when using #{name} archive" do context "when using #{name} archive" do
let(:archive_name) { name } let(:archive_name) { name }
context 'for RubyZip' do it_behaves_like 'extracts archive'
it_behaves_like 'extracts archive', true
end
context 'for UnZip' do
it_behaves_like 'extracts archive', false
end
end end
end end
...@@ -56,13 +42,7 @@ RSpec.describe SafeZip::Extract do ...@@ -56,13 +42,7 @@ RSpec.describe SafeZip::Extract do
context "when using #{name} archive" do context "when using #{name} archive" do
let(:archive_name) { name } let(:archive_name) { name }
context 'for RubyZip' do it_behaves_like 'fails to extract archive'
it_behaves_like 'fails to extract archive', true
end
context 'for UnZip (UNSAFE)' do
it_behaves_like 'extracts archive', false
end
end end
end end
...@@ -70,13 +50,7 @@ RSpec.describe SafeZip::Extract do ...@@ -70,13 +50,7 @@ RSpec.describe SafeZip::Extract do
let(:archive_name) { 'valid-simple.zip' } let(:archive_name) { 'valid-simple.zip' }
let(:directories) { %w(non/existing) } let(:directories) { %w(non/existing) }
context 'for RubyZip' do it_behaves_like 'fails to extract archive'
it_behaves_like 'fails to extract archive', true
end
context 'for UnZip' do
it_behaves_like 'fails to extract archive', false
end
end end
end end
end end
...@@ -16,8 +16,6 @@ RSpec.describe Projects::UpdatePagesService do ...@@ -16,8 +16,6 @@ RSpec.describe Projects::UpdatePagesService do
subject { described_class.new(project, build) } subject { described_class.new(project, build) }
before do before do
stub_feature_flags(safezip_use_rubyzip: true)
project.remove_pages project.remove_pages
end end
...@@ -104,10 +102,6 @@ RSpec.describe Projects::UpdatePagesService do ...@@ -104,10 +102,6 @@ RSpec.describe Projects::UpdatePagesService do
let(:file) { fixture_file_upload("spec/fixtures/pages_non_writeable.zip") } let(:file) { fixture_file_upload("spec/fixtures/pages_non_writeable.zip") }
context 'when using RubyZip' do context 'when using RubyZip' do
before do
stub_feature_flags(safezip_use_rubyzip: true)
end
it 'succeeds to extract' do it 'succeeds to extract' do
expect(execute).to eq(:success) expect(execute).to eq(:success)
expect(project.pages_metadatum).to be_deployed expect(project.pages_metadatum).to be_deployed
......
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