From b023d88b096d566093f05d54eeebb97c53bbd94c Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 1 Apr 2022 12:03:14 +1100 Subject: [PATCH] FIX: Abort theme creation if unable to create uploads (#16336) Previous to this change if any of the assets were not allowed extensions they would simply be silently ignored, this could lead to broken themes that are very hard to debug --- app/models/remote_theme.rb | 5 +++++ config/locales/server.en.yml | 1 + spec/requests/admin/themes_controller_spec.rb | 10 ++++++++++ 3 files changed, 16 insertions(+) diff --git a/app/models/remote_theme.rb b/app/models/remote_theme.rb index 10190ff064d..21f6bef275b 100644 --- a/app/models/remote_theme.rb +++ b/app/models/remote_theme.rb @@ -162,6 +162,11 @@ class RemoteTheme < ActiveRecord::Base new_path = "#{File.dirname(path)}/#{SecureRandom.hex}#{File.extname(path)}" File.rename(path, new_path) # OptimizedImage has strict file name restrictions, so rename temporarily upload = UploadCreator.new(File.open(new_path), File.basename(relative_path), for_theme: true).create_for(theme.user_id) + + if !upload.errors.empty? + raise ImportError, I18n.t("themes.import_error.upload", name: name, errors: upload.errors.full_messages.join(",")) + end + updated_fields << theme.set_field(target: :common, name: name, type: :theme_upload_var, upload_id: upload.id) end end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index daf859e9807..fe6edcf91b5 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -72,6 +72,7 @@ en: unrecognized_extension: "Unrecognized file extension: %{extension}" import_error: generic: An error occurred while importing that theme + upload: "Error creating upload asset: %{name}. %{errors}" about_json: "Import Error: about.json does not exist, or is invalid. Are you sure this is a Discourse Theme?" about_json_values: "about.json contains invalid values: %{errors}" modifier_values: "about.json modifiers contain invalid values: %{errors}" diff --git a/spec/requests/admin/themes_controller_spec.rb b/spec/requests/admin/themes_controller_spec.rb index 13f6d2499cf..90aa47d0fd9 100644 --- a/spec/requests/admin/themes_controller_spec.rb +++ b/spec/requests/admin/themes_controller_spec.rb @@ -151,6 +151,16 @@ describe Admin::ThemesController do expect(UserHistory.where(action: UserHistory.actions[:change_theme]).count).to eq(1) end + it 'fails to import with an error if uploads are not allowed' do + SiteSetting.theme_authorized_extensions = "nothing" + + expect do + post "/admin/themes/import.json", params: { theme: theme_archive } + end.to change { Theme.count }.by (0) + + expect(response.status).to eq(422) + end + it 'imports a theme from an archive' do _existing_theme = Fabricate(:theme, name: "Header Icons")