FIX: limit SvgSprite scan to string setting values

FIX: limit SvgSprite scan to string setting values

From a58c3ca430781ed5bd35e49510336c3a006b4621 Mon Sep 17 00:00:00 2001
From: Penar Musaraj <pmusaraj@gmail.com>
Date: Thu, 29 Nov 2018 18:36:48 -0500
Subject: [PATCH] FIX: limit SvgSprite scan to string setting values


diff --git a/lib/svg_sprite/svg_sprite.rb b/lib/svg_sprite/svg_sprite.rb
index 2854bd7..a651d3c 100644
--- a/lib/svg_sprite/svg_sprite.rb
+++ b/lib/svg_sprite/svg_sprite.rb
@@ -287,7 +287,7 @@ Discourse SVG subset of #{fa_license}
     site_setting_icons = []
 
     SiteSetting.settings_hash.select do |key, value|
-      if key.to_s.include?("_icon") && value.present?
+      if key.to_s.include?("_icon") && value.present? && value.class == String
         site_setting_icons |= value.split('|')
       end
     end
@@ -317,7 +317,7 @@ Discourse SVG subset of #{fa_license}
     # Theme.all includes default values
     Theme.all.each do |theme|
       settings = theme.cached_settings.each do |key, value|
-        if key.to_s.include?("_icon") && value.present?
+        if key.to_s.include?("_icon") && value.present? && value.class == String
           theme_icon_settings |= value.split('|')
         end
       end

GitHub

This commit has been mentioned on Discourse Meta. There might be relevant details there:

Instead:

if key.to_s.include?("_icon") && String === value

I like this as a general pattern plus it is quite a bit safer in many cases:

[10] pry(main)> class A < String; end; String === A.new
=> true
1 Like