From f2da630c19dc04e8e17d2d5eb3bb84692229e4de Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Thu, 14 Nov 2019 13:20:16 +0000
Subject: [PATCH] FIX: SVG Sprite version hash should be based on bundle result

This version hash is used for the filename, and so browsers/CDNs cache based on it. Previously the version hash was based only on the list of requested icons. This can cause issues in a couple of situations, most commonly when developing themes with custom icons:

- A requested icon does not exist, and then later is added to the theme. The bundle output changes, but the hash did not
- The SVG content of an icon changes, but the name of the icon does not. The bundle output changes, but the hash did not
---
 lib/svg_sprite/svg_sprite.rb                  |  2 +-
 spec/components/svg_sprite/svg_sprite_spec.rb | 31 +++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/lib/svg_sprite/svg_sprite.rb b/lib/svg_sprite/svg_sprite.rb
index 0408fc2817d..da8ddf72d5f 100644
--- a/lib/svg_sprite/svg_sprite.rb
+++ b/lib/svg_sprite/svg_sprite.rb
@@ -239,7 +239,7 @@ module SvgSprite
 
   def self.version(theme_ids = [])
     get_set_cache("version_#{Theme.transform_ids(theme_ids).join(',')}") do
-      Digest::SHA1.hexdigest(all_icons(theme_ids).join('|'))
+      Digest::SHA1.hexdigest(bundle(theme_ids))
     end
   end
 
diff --git a/spec/components/svg_sprite/svg_sprite_spec.rb b/spec/components/svg_sprite/svg_sprite_spec.rb
index c4177cd6561..2885699b4a4 100644
--- a/spec/components/svg_sprite/svg_sprite_spec.rb
+++ b/spec/components/svg_sprite/svg_sprite_spec.rb
@@ -49,6 +49,37 @@ describe SvgSprite do
     expect(version1).not_to eq(version2)
   end
 
+  it 'version should be based on bundled output, not requested icons' do
+    theme = Fabricate(:theme)
+    fname = "custom-theme-icon-sprite.svg"
+    upload = UploadCreator.new(file_from_fixtures(fname), fname, for_theme: true).create_for(-1)
+
+    version1 = SvgSprite.version([theme.id])
+    bundle1 = SvgSprite.bundle([theme.id])
+
+    SiteSetting.svg_icon_subset = "my-custom-theme-icon"
+
+    version2 = SvgSprite.version([theme.id])
+    bundle2 = SvgSprite.bundle([theme.id])
+
+    # The contents of the bundle should not change, because the icon does not actually exist
+    expect(bundle1).to eq(bundle2)
+    # Therefore the version hash should not change
+    expect(version1).to eq(version2)
+
+    # Now add the icon to the theme
+    theme.set_field(target: :common, name: SvgSprite.theme_sprite_variable_name, upload_id: upload.id, type: :theme_upload_var)
+    theme.save!
+
+    version3 = SvgSprite.version([theme.id])
+    bundle3 = SvgSprite.bundle([theme.id])
+
+    # The version/bundle should be updated
+    expect(bundle3).not_to match(bundle2)
+    expect(version3).not_to match(version2)
+    expect(bundle3).to match(/my-custom-theme-icon/)
+  end
+
   it 'strips whitespace when processing icons' do
     Fabricate(:badge, name: 'Custom Icon Badge', icon: '  fab fa-facebook-messenger  ')
     expect(SvgSprite.all_icons).to include("fab-facebook-messenger")