FIX: Do not use SVGs for twitter:image metadata (#16973)

Twitter does not allow SVGs to be used for twitter:image
metadata (see https://developer.twitter.com/en/docs/twitter-for-websites/cards/overview/markup)
so we should fall back to the site logo if the image option
provided to `crawlable_meta_data` or SiteSetting.site_twitter_summary_large_image_url
is an SVG, and do not add the meta tag for twitter:image at all
if the site logo is an SVG.

app/assets/javascripts/admin/addon/templates/components/site-settings/upload.hbs

@@ -6,3 +6,4 @@
   id=(concat "site-setting-image-uploader-" setting.setting)
 }}
 <div class="desc">{{html-safe setting.description}}</div>
+{{setting-validation-message message=validationMessage}}

app/assets/javascripts/admin/addon/templates/components/site-settings/uploaded-image-list.hbs

@@ -1,2 +1,3 @@
 {{d-button label="admin.site_settings.uploaded_image_list.label" action=(action "showUploadModal") actionParam=(hash value=value setting=setting)}}
 <div class="desc">{{html-safe setting.description}}</div>
+{{setting-validation-message message=validationMessage}}

app/helpers/application_helper.rb

@@ -289,15 +289,7 @@ module ApplicationHelper
     result << tag(:meta, property: 'og:site_name', content: SiteSetting.title)
     result << tag(:meta, property: 'og:type', content: 'website')
 
-    if opts[:twitter_summary_large_image].present?
+    result = generate_twitter_card_metadata(opts, result)
-      result << tag(:meta, name: 'twitter:card', content: "summary_large_image")
-      result << tag(:meta, name: "twitter:image", content: opts[:twitter_summary_large_image])
-    elsif opts[:image].present?
-      result << tag(:meta, name: 'twitter:card', content: "summary")
-      result << tag(:meta, name: "twitter:image", content: opts[:image])
-    else
-      result << tag(:meta, name: 'twitter:card', content: "summary")
-    end
     result << tag(:meta, property: "og:image", content: opts[:image]) if opts[:image].present?
 
     [:url, :title, :description].each do |property|
@@ -326,6 +318,29 @@ module ApplicationHelper
     result.join("\n")
   end
 
+  def generate_twitter_card_metadata(opts, result)
+    img_url = opts[:twitter_summary_large_image].present? ? \
+      opts[:twitter_summary_large_image] :
+      opts[:image]
+
+    # Twitter does not allow SVGs, see https://developer.twitter.com/en/docs/twitter-for-websites/cards/overview/markup
+    if img_url.ends_with?(".svg")
+      img_url = SiteSetting.site_logo_url.ends_with?(".svg") ? nil : SiteSetting.site_logo_url
+    end
+
+    if opts[:twitter_summary_large_image].present? && img_url.present?
+      result << tag(:meta, name: 'twitter:card', content: "summary_large_image")
+      result << tag(:meta, name: "twitter:image", content: img_url)
+    elsif opts[:image].present? && img_url.present?
+      result << tag(:meta, name: 'twitter:card', content: "summary")
+      result << tag(:meta, name: "twitter:image", content: img_url)
+    else
+      result << tag(:meta, name: 'twitter:card', content: "summary")
+    end
+
+    result
+  end
+
   def render_sitelinks_search_tag
     json = {
       '@context' => 'http://schema.org',

config/locales/server.en.yml

@@ -220,6 +220,7 @@ en:
       slow_down_crawler_user_agent_must_be_at_least_3_characters: "User agents must be at least 3 characters long to avoid incorrectly rate limiting human users."
       slow_down_crawler_user_agent_cannot_be_popular_browsers: "You cannot add any of the following values to the setting: %{values}."
       strip_image_metadata_cannot_be_disabled_if_composer_media_optimization_image_enabled: "You cannot disable strip image metadata if 'composer media optimization image enabled' is enabled. Disable 'composer media optimization image enabled' before disabling strip image metadata."
+      twitter_summary_large_image_no_svg: "Twitter summary images used for twitter:image metadata cannot be an .svg image."
     conflicting_google_user_id: 'The Google Account ID for this account has changed; staff intervention is required for security reasons. Please contact staff and point them to <br><a href="https://meta.discourse.org/t/76575">https://meta.discourse.org/t/76575</a>'
     onebox:
       invalid_address: "Sorry, we were unable to generate a preview for this web page, because the server '%{hostname}' could not be found. Instead of a preview, only a link will appear in your post. :cry:"
@@ -1559,7 +1560,7 @@ en:
     favicon: "A favicon for your site, see <a href='https://en.wikipedia.org/wiki/Favicon' target='_blank'>https://en.wikipedia.org/wiki/Favicon</a>. To work correctly over a CDN it must be a png. Will be resized to 32x32. If left blank, large_icon will be used."
     apple_touch_icon: "Icon used for Apple touch devices. Will be automatically resized to 180x180. If left blank, large_icon will be used."
     opengraph_image: "Default opengraph image, used when the page has no other suitable image. If left blank, large_icon will be used"
-    twitter_summary_large_image: "Twitter card 'summary large image' (should be at least 280 in width, and at least 150 in height). If left blank, regular card metadata is generated using the opengraph_image."
+    twitter_summary_large_image: "Twitter card 'summary large image' (should be at least 280 in width, and at least 150 in height, cannot be .svg). If left blank, regular card metadata is generated using the opengraph_image, as long as that is not also a .svg"
 
     notification_email: "The from: email address used when sending all essential system emails. The domain specified here must have SPF, DKIM and reverse PTR records set correctly for email to arrive."
     email_custom_headers: "A pipe-delimited list of custom email headers"

lib/site_settings/validations.rb

@@ -245,6 +245,12 @@ module SiteSettings::Validations
     validate_error :strip_image_metadata_cannot_be_disabled_if_composer_media_optimization_image_enabled
   end
 
+  def validate_twitter_summary_large_image(new_val)
+    return if new_val.blank?
+    return if !Upload.exists?(id: new_val, extension: "svg")
+    validate_error :twitter_summary_large_image_no_svg
+  end
+
   private
 
   def validate_bucket_setting(setting_name, upload_bucket, backup_bucket)

spec/helpers/application_helper_spec.rb

@@ -467,6 +467,45 @@ describe ApplicationHelper do
 
         expect(helper.crawlable_meta_data).to include(Upload.find(SiteIconManager::SKETCH_LOGO_ID).url)
       end
+
+      it "does not allow SVG images for twitter:image, falls back to site logo or nothing if site logo is SVG too" do
+        SiteSetting.logo = Fabricate(:upload, url: '/images/d-logo-sketch.png')
+        SiteSetting.opengraph_image = Fabricate(:upload,
+          url: '/images/og-image.png'
+        )
+
+        expect(helper.crawlable_meta_data).to include(<<~HTML)
+        <meta name=\"twitter:image\" content=\"#{SiteSetting.site_opengraph_image_url}\" />
+        HTML
+
+        SiteSetting.opengraph_image = Fabricate(:upload,
+          url: '/images/og-image.svg'
+        )
+
+        expect(helper.crawlable_meta_data).to include(<<~HTML)
+        <meta name=\"twitter:image\" content=\"#{SiteSetting.site_logo_url}\" />
+        HTML
+
+        SiteSetting.twitter_summary_large_image = Fabricate(:upload,
+          url: '/images/twitter.png'
+        )
+
+        expect(helper.crawlable_meta_data).to include(<<~HTML)
+        <meta name=\"twitter:image\" content=\"#{SiteSetting.site_twitter_summary_large_image_url}\" />
+        HTML
+
+        SiteSetting.twitter_summary_large_image = Fabricate(:upload,
+          url: '/images/twitter.svg'
+        )
+
+        expect(helper.crawlable_meta_data).to include(<<~HTML)
+        <meta name=\"twitter:image\" content=\"#{SiteSetting.site_logo_url}\" />
+        HTML
+
+        SiteSetting.logo = Fabricate(:upload, url: '/images/d-logo-sketch.svg')
+
+        expect(helper.crawlable_meta_data).not_to include("twitter:image")
+      end
     end
   end
 

spec/lib/site_settings/validations_spec.rb

@@ -402,4 +402,16 @@ describe SiteSettings::Validations do
       end
     end
   end
+
+  describe "#twitter_summary_large_image" do
+    it "does not allow SVG image files" do
+      upload = Fabricate(:upload, url: '/images/logo-dark.svg', extension: "svg")
+      expect { subject.validate_twitter_summary_large_image(upload.id) }.to raise_error(
+        Discourse::InvalidParameters, I18n.t("errors.site_settings.twitter_summary_large_image_no_svg")
+      )
+      upload.update!(url: '/images/logo-dark.png', extension: 'png')
+      expect { subject.validate_twitter_summary_large_image(upload.id) }.not_to raise_error
+      expect { subject.validate_twitter_summary_large_image(nil) }.not_to raise_error
+    end
+  end
 end