From f5c707c97ae626b5ae1632a1e83347f5e093ac0a Mon Sep 17 00:00:00 2001 From: Roman Rizzi Date: Thu, 18 Jul 2019 09:34:48 -0300 Subject: [PATCH] FEATURE: Gz to zip for exports (#7889) * Revert "Revert "FEATURE: admin/user exports are compressed using the zip format (#7784)"" This reverts commit f89bd555763bd61a130145d2eff6c2ee75fe6b06. * Replace .tar.zip with .zip --- Gemfile | 2 + Gemfile.lock | 2 + .../templates/modal/admin-install-theme.hbs | 2 +- app/controllers/admin/themes_controller.rb | 3 +- app/jobs/regular/export_csv_file.rb | 12 ++-- config/locales/client.en.yml | 2 +- config/locales/server.en.yml | 2 +- config/site_settings.yml | 2 +- lib/import_export/zip_utils.rb | 58 +++++++++++++++++++ lib/theme_store/tgz_exporter.rb | 10 +--- lib/theme_store/tgz_importer.rb | 14 ++++- .../theme_store/tgz_exporter_spec.rb | 15 +++-- .../theme_store/tgz_importer_spec.rb | 31 +++++++--- .../validators/upload_validator_spec.rb | 6 +- spec/requests/admin/themes_controller_spec.rb | 4 +- 15 files changed, 127 insertions(+), 38 deletions(-) create mode 100644 lib/import_export/zip_utils.rb diff --git a/Gemfile b/Gemfile index 6a621c456d4..ffa12dfe510 100644 --- a/Gemfile +++ b/Gemfile @@ -203,6 +203,8 @@ gem "sassc-rails" gem 'rotp' gem 'rqrcode' +gem 'rubyzip', require: false + gem 'sshkey', require: false gem 'rchardet', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 898e8b7658d..194ec34d1e7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -351,6 +351,7 @@ GEM guess_html_encoding (>= 0.0.4) nokogiri (>= 1.6.0) ruby_dep (1.5.0) + rubyzip (1.2.3) safe_yaml (1.0.5) sanitize (5.0.0) crass (~> 1.0.2) @@ -516,6 +517,7 @@ DEPENDENCIES rubocop ruby-prof ruby-readability + rubyzip sanitize sassc sassc-rails diff --git a/app/assets/javascripts/admin/templates/modal/admin-install-theme.hbs b/app/assets/javascripts/admin/templates/modal/admin-install-theme.hbs index 153dbc9e8f1..a41bfa93a94 100644 --- a/app/assets/javascripts/admin/templates/modal/admin-install-theme.hbs +++ b/app/assets/javascripts/admin/templates/modal/admin-install-theme.hbs @@ -44,7 +44,7 @@ {{#if local}}
-
+
{{i18n 'admin.customize.theme.import_file_tip'}}
{{/if}} diff --git a/app/controllers/admin/themes_controller.rb b/app/controllers/admin/themes_controller.rb index 5389269af87..4def5bd2149 100644 --- a/app/controllers/admin/themes_controller.rb +++ b/app/controllers/admin/themes_controller.rb @@ -88,7 +88,7 @@ class Admin::ThemesController < Admin::AdminController rescue RemoteTheme::ImportError => e render_json_error e.message end - elsif params[:bundle] || (params[:theme] && ["application/x-gzip", "application/gzip"].include?(params[:theme].content_type)) + elsif params[:bundle] || (params[:theme] && ["application/x-gzip", "application/gzip", "application/zip"].include?(params[:theme].content_type)) # params[:bundle] used by theme CLI. params[:theme] used by admin UI bundle = params[:bundle] || params[:theme] theme_id = params[:theme_id] @@ -252,6 +252,7 @@ class Admin::ThemesController < Admin::AdminController exporter = ThemeStore::TgzExporter.new(@theme) file_path = exporter.package_filename + headers['Content-Length'] = File.size(file_path).to_s send_data File.read(file_path), filename: File.basename(file_path), diff --git a/app/jobs/regular/export_csv_file.rb b/app/jobs/regular/export_csv_file.rb index e1b57398c1a..d8d3a7204ec 100644 --- a/app/jobs/regular/export_csv_file.rb +++ b/app/jobs/regular/export_csv_file.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'csv' +require 'zip' require_dependency 'system_message' require_dependency 'upload_creator' @@ -53,18 +54,19 @@ module Jobs # ensure directory exists FileUtils.mkdir_p(UserExport.base_directory) unless Dir.exists?(UserExport.base_directory) - # write to CSV file - CSV.open(absolute_path, "w") do |csv| + # Generate a compressed CSV file + csv_to_export = CSV.generate do |csv| csv << get_header if @entity != "report" public_send(export_method).each { |d| csv << d } end - # compress CSV file - system('gzip', '-5', absolute_path) + compressed_file_path = "#{absolute_path}.zip" + Zip::File.open(compressed_file_path, Zip::File::CREATE) do |zipfile| + zipfile.get_output_stream(file_name) { |f| f.puts csv_to_export } + end # create upload upload = nil - compressed_file_path = "#{absolute_path}.gz" if File.exist?(compressed_file_path) File.open(compressed_file_path) do |file| diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 4e01d15691b..47f8dacca23 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -3517,7 +3517,7 @@ en: delete_upload_confirm: "Delete this upload? (Theme CSS may stop working!)" import_web_tip: "Repository containing theme" import_web_advanced: "Advanced..." - import_file_tip: ".tar.gz or .dcstyle.json file containing theme" + import_file_tip: ".tar.gz, .zip, or .dcstyle.json file containing theme" is_private: "Theme is in a private git repository" remote_branch: "Branch name (optional)" public_key: "Grant the following public key access to the repo:" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 0a7a0af6098..6fa93ed7b29 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2671,7 +2671,7 @@ en: The above download link will be valid for 48 hours. - The data is compressed as a gzip archive. If the archive does not extract itself when you open it, use the tools recommended here: https://www.gzip.org/#faq4 + The data is compressed as a zip archive. If the archive does not extract itself when you open it, use the tool recommended here: https://www.7-zip.org/ csv_export_failed: title: "CSV Export Failed" diff --git a/config/site_settings.yml b/config/site_settings.yml index ebe51f6082e..3b5736ccd28 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1057,7 +1057,7 @@ files: list_type: compact export_authorized_extensions: hidden: true - default: "gz" + default: "zip" type: list list_type: compact responsive_post_image_sizes: diff --git a/lib/import_export/zip_utils.rb b/lib/import_export/zip_utils.rb new file mode 100644 index 00000000000..dc06e0efb07 --- /dev/null +++ b/lib/import_export/zip_utils.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'zip' + +class ZipUtils + def zip_directory(path, export_name) + zip_filename = "#{export_name}.zip" + absolute_path = "#{path}/#{export_name}" + entries = Dir.entries(absolute_path) - %w[. ..] + + Zip::File.open(zip_filename, Zip::File::CREATE) do |zipfile| + write_entries(entries, absolute_path, '', zipfile) + end + + "#{absolute_path}.zip" + end + + def unzip_directory(path, zip_filename, allow_non_root_folder: false) + Zip::File.open(zip_filename) do |zip_file| + root = root_folder_present?(zip_file, allow_non_root_folder) ? '' : 'unzipped/' + zip_file.each do |entry| + entry_path = File.join(path, "#{root}#{entry.name}") + FileUtils.mkdir_p(File.dirname(entry_path)) + entry.extract(entry_path) + end + end + end + + private + + def root_folder_present?(filenames, allow_non_root_folder) + filenames.map { |p| p.name.split('/').first }.uniq!.size == 1 || allow_non_root_folder + end + + # A helper method to make the recursion work. + def write_entries(entries, base_path, path, zipfile) + entries.each do |e| + zipfile_path = path == '' ? e : File.join(path, e) + disk_file_path = File.join(base_path, zipfile_path) + + if File.directory? disk_file_path + recursively_deflate_directory(disk_file_path, zipfile, base_path, zipfile_path) + else + put_into_archive(disk_file_path, zipfile, zipfile_path) + end + end + end + + def recursively_deflate_directory(disk_file_path, zipfile, base_path, zipfile_path) + zipfile.mkdir zipfile_path + subdir = Dir.entries(disk_file_path) - %w[. ..] + write_entries subdir, base_path, zipfile_path, zipfile + end + + def put_into_archive(disk_file_path, zipfile, zipfile_path) + zipfile.add(zipfile_path, disk_file_path) + end +end diff --git a/lib/theme_store/tgz_exporter.rb b/lib/theme_store/tgz_exporter.rb index 824a874ab94..75f5bff8963 100644 --- a/lib/theme_store/tgz_exporter.rb +++ b/lib/theme_store/tgz_exporter.rb @@ -56,14 +56,10 @@ class ThemeStore::TgzExporter end private + def export_package export_to_folder - Dir.chdir(@temp_folder) do - tar_filename = "#{@export_name}.tar" - Discourse::Utils.execute_command('tar', '--create', '--file', tar_filename, @export_name, failure_message: "Failed to tar theme.") - Discourse::Utils.execute_command('gzip', '-5', tar_filename, failure_message: "Failed to gzip archive.") - "#{@temp_folder}/#{tar_filename}.gz" - end - end + Dir.chdir(@temp_folder) { ZipUtils.new.zip_directory(@temp_folder, @export_name) } + end end diff --git a/lib/theme_store/tgz_importer.rb b/lib/theme_store/tgz_importer.rb index 5dfb0716e68..f0c88a4d1fd 100644 --- a/lib/theme_store/tgz_importer.rb +++ b/lib/theme_store/tgz_importer.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'import_export/zip_utils' + module ThemeStore; end class ThemeStore::TgzImporter @@ -13,8 +15,16 @@ class ThemeStore::TgzImporter def import! FileUtils.mkdir(@temp_folder) + Dir.chdir(@temp_folder) do - Discourse::Utils.execute_command("tar", "-xzvf", @filename, "--strip", "1") + if @filename.include?('.zip') + ZipUtils.new.unzip_directory(@temp_folder, @filename) + + # --strip 1 equivalent + FileUtils.mv(Dir.glob("#{@temp_folder}/*/*"), @temp_folder) + else + Discourse::Utils.execute_command("tar", "-xzvf", @filename, "--strip", "1") + end end rescue RuntimeError raise RemoteTheme::ImportError, I18n.t("themes.import_error.unpack_failed") @@ -44,7 +54,7 @@ class ThemeStore::TgzImporter def all_files Dir.chdir(@temp_folder) do - Dir.glob("**/*").reject { |f| File.directory?(f) } + Dir.glob("**/**").reject { |f| File.directory?(f) } end end diff --git a/spec/components/theme_store/tgz_exporter_spec.rb b/spec/components/theme_store/tgz_exporter_spec.rb index b66bf6e1b08..18266eeb263 100644 --- a/spec/components/theme_store/tgz_exporter_spec.rb +++ b/spec/components/theme_store/tgz_exporter_spec.rb @@ -2,6 +2,7 @@ require 'rails_helper' require 'theme_store/tgz_exporter' +require 'import_export/zip_utils' describe ThemeStore::TgzExporter do let!(:theme) do @@ -55,15 +56,17 @@ describe ThemeStore::TgzExporter do filename = exporter.package_filename FileUtils.cp(filename, dir) exporter.cleanup! - "#{dir}/discourse-header-icons.tar.gz" + "#{dir}/discourse-header-icons.zip" end it "exports the theme correctly" do package - Dir.chdir("#{dir}") do - `tar -xzf discourse-header-icons.tar.gz` - end - Dir.chdir("#{dir}/discourse-header-icons") do + file = 'discourse-header-icons.zip' + dest = 'discourse-header-icons' + Dir.chdir(dir) do + ZipUtils.new.unzip_directory(dir, file, allow_non_root_folder: true) + `rm #{file}` + folders = Dir.glob("**/*").reject { |f| File.file?(f) } expect(folders).to contain_exactly("assets", "common", "locales", "mobile") @@ -121,7 +124,7 @@ describe ThemeStore::TgzExporter do exporter = ThemeStore::TgzExporter.new(theme) filename = exporter.package_filename exporter.cleanup! - expect(filename).to end_with "/discourse-header-icons.tar.gz" + expect(filename).to end_with "/discourse-header-icons.zip" end end diff --git a/spec/components/theme_store/tgz_importer_spec.rb b/spec/components/theme_store/tgz_importer_spec.rb index 2986b1d2c9c..0aad92584e8 100644 --- a/spec/components/theme_store/tgz_importer_spec.rb +++ b/spec/components/theme_store/tgz_importer_spec.rb @@ -4,26 +4,41 @@ require 'rails_helper' require 'theme_store/tgz_importer' +require 'import_export/zip_utils' describe ThemeStore::TgzImporter do before do @temp_folder = "#{Pathname.new(Dir.tmpdir).realpath}/discourse_theme_#{SecureRandom.hex}" + + FileUtils.mkdir(@temp_folder) + Dir.chdir(@temp_folder) do + FileUtils.mkdir_p('test/a') + File.write("test/hello.txt", "hello world") + File.write("test/a/inner", "hello world inner") + end end after do FileUtils.rm_rf @temp_folder end - it "can import a simple theme" do - - FileUtils.mkdir(@temp_folder) - + it "can import a simple zipped theme" do Dir.chdir(@temp_folder) do - FileUtils.mkdir('test/') - File.write("test/hello.txt", "hello world") - FileUtils.mkdir('test/a') - File.write("test/a/inner", "hello world inner") + ZipUtils.new.zip_directory(@temp_folder, 'test') + FileUtils.rm_rf('test/') + end + importer = ThemeStore::TgzImporter.new("#{@temp_folder}/test.zip") + importer.import! + + expect(importer["hello.txt"]).to eq("hello world") + expect(importer["a/inner"]).to eq("hello world inner") + + importer.cleanup! + end + + it "can import a simple gzipped theme" do + Dir.chdir(@temp_folder) do `tar -cvzf test.tar.gz test/* 2> /dev/null` end diff --git a/spec/components/validators/upload_validator_spec.rb b/spec/components/validators/upload_validator_spec.rb index e9a3dfe601a..9348c85e79a 100644 --- a/spec/components/validators/upload_validator_spec.rb +++ b/spec/components/validators/upload_validator_spec.rb @@ -22,14 +22,14 @@ describe Validators::UploadValidator do it "allows 'gz' as extension when uploading export file" do SiteSetting.authorized_extensions = "" - expect(UploadCreator.new(csv_file, "#{filename}.gz", for_export: true).create_for(user.id)).to be_valid + expect(UploadCreator.new(csv_file, "#{filename}.zip", for_export: true).create_for(user.id)).to be_valid end it "allows uses max_export_file_size_kb when uploading export file" do SiteSetting.max_attachment_size_kb = "0" - SiteSetting.authorized_extensions = "gz" + SiteSetting.authorized_extensions = "zip" - expect(UploadCreator.new(csv_file, "#{filename}.gz", for_export: true).create_for(user.id)).to be_valid + expect(UploadCreator.new(csv_file, "#{filename}.zip", for_export: true).create_for(user.id)).to be_valid end describe 'when allow_staff_to_upload_any_file_in_pm is true' do diff --git a/spec/requests/admin/themes_controller_spec.rb b/spec/requests/admin/themes_controller_spec.rb index 4f32a63e013..43cc16cd8b8 100644 --- a/spec/requests/admin/themes_controller_spec.rb +++ b/spec/requests/admin/themes_controller_spec.rb @@ -51,10 +51,10 @@ describe Admin::ThemesController do expect(response.status).to eq(200) # Save the output in a temp file (automatically cleaned up) - file = Tempfile.new('archive.tar.gz') + file = Tempfile.new('archive.tar.zip') file.write(response.body) file.rewind - uploaded_file = Rack::Test::UploadedFile.new(file.path, "application/x-gzip") + uploaded_file = Rack::Test::UploadedFile.new(file.path, "application/zip") # Now import it again expect do