FIX: Issues with custom icons in themes (#13732)

Fixes two issues:
- ignores invalid XML in custom icon sprite SVG file (and outputs an error if sprite was uploaded via admin UI)
- clears SVG sprite cache when deleting an `icons-sprite` upload in a theme

app/models/theme_field.rb

@@ -170,6 +170,29 @@ class ThemeField < ActiveRecord::Base
     [js_compiler.content, errors&.join("\n")]
   end
 
+  def validate_svg_sprite_xml
+    upload = Upload.find(self.upload_id) rescue nil
+
+    if Discourse.store.external?
+      external_copy = Discourse.store.download(upload) rescue nil
+      path = external_copy.try(:path)
+    else
+      path = Discourse.store.path_for(upload)
+    end
+
+    content = File.read(path)
+    error = nil
+
+    begin
+      svg_file = Nokogiri::XML(content) do |config|
+        config.options = Nokogiri::XML::ParseOptions::NOBLANKS
+      end
+    rescue => e
+      error = "Error with #{self.name}: #{e.inspect}"
+    end
+    error
+  end
+
   def raw_translation_data(internal: false)
     # Might raise ThemeTranslationParser::InvalidYaml
     ThemeTranslationParser.new(self, internal: internal).load
@@ -358,7 +381,7 @@ class ThemeField < ActiveRecord::Base
       self.compiler_version = Theme.compiler_version
     elsif svg_sprite_field?
       DB.after_commit { SvgSprite.expire_cache }
-      self.error = nil
+      self.error = validate_svg_sprite_xml
       self.value_baked = "baked"
       self.compiler_version = Theme.compiler_version
     end
@@ -554,6 +577,12 @@ class ThemeField < ActiveRecord::Base
     MessageBus.publish "/footer-change/#{theme.id}", self.value if theme && self.name == "footer"
   end
 
+  after_destroy do
+    if svg_sprite_field?
+      DB.after_commit { SvgSprite.expire_cache }
+    end
+  end
+
   private
 
   JAVASCRIPT_TYPES = %w(

lib/svg_sprite/svg_sprite.rb

@@ -351,10 +351,16 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL
     end
 
     custom_svg_sprites(theme_id).each do |item|
-      svg_file = Nokogiri::XML(item[:sprite]) do |config|
-        config.options = Nokogiri::XML::ParseOptions::NOBLANKS
+      begin
+        svg_file = Nokogiri::XML(item[:sprite]) do |config|
+          config.options = Nokogiri::XML::ParseOptions::NOBLANKS
+        end
+      rescue => e
+        Rails.logger.warn("Bad XML in custom sprite in theme with ID=#{theme_id}. Error info: #{e.inspect}")
       end
 
+      next if !svg_file
+
       svg_file.css("symbol").each do |sym|
         icon_id = prepare_symbol(sym, item[:filename])
 

spec/components/svg_sprite/svg_sprite_spec.rb

@@ -264,6 +264,19 @@ describe SvgSprite do
       expect(SvgSprite.bundle(theme.id)).to match(/my-custom-theme-icon/)
     end
 
+    it 'does not fail on bad XML in custom icon sprite' do
+      theme = Fabricate(:theme)
+      fname = "bad-xml-icon-sprite.svg"
+
+      upload = UploadCreator.new(file_from_fixtures(fname), fname, for_theme: true).create_for(-1)
+
+      theme.set_field(target: :common, name: SvgSprite.theme_sprite_variable_name, upload_id: upload.id, type: :theme_upload_var)
+      theme.save!
+
+      expect(Upload.where(id: upload.id)).to be_exist
+      expect(SvgSprite.bundle(theme.id)).to match(/arrow-down/)
+    end
+
     it 'includes custom icons in a child theme' do
       theme = Fabricate(:theme)
       fname = "custom-theme-icon-sprite.svg"

spec/fixtures/images/bad-xml-icon-sprite.svg

@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<svg width="0" height="0" class="hidden">
+  <symbol id="mytheme-icon-menu xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24">
+    <path d="M0 0h24v24H0V0z" fill="#9c9c9c"></path>
+  </symbol>
+</svg>

spec/models/theme_field_spec.rb

@@ -419,6 +419,17 @@ HTML
       theme_field.update(upload: Fabricate(:upload))
       expect(theme_field.value_baked).to eq(nil)
     end
+
+    it "clears SVG sprite cache when upload is deleted" do
+      fname = "custom-theme-icon-sprite.svg"
+      sprite = UploadCreator.new(file_from_fixtures(fname), fname, for_theme: true).create_for(-1)
+
+      theme_field.update(upload: sprite)
+      expect(SvgSprite.custom_svg_sprites(theme.id).size).to eq(1)
+
+      theme_field.destroy!
+      expect(SvgSprite.custom_svg_sprites(theme.id).size).to eq(0)
+    end
   end
 
 end