FIX: Stop secure media URLs being censored too liberally in emails (#8817)

For example /t/ URLs were being replaced if they contained secure-media-uploads so if you made a topic called "Secure Media Uploads Are Cool" the View Topic link in the user notifications would be stripped out.

Refactored code so this secure URL detection happens in one place.

app/models/upload.rb

@@ -128,7 +128,9 @@ class Upload < ActiveRecord::Base
   end
 
   def self.secure_media_url?(url)
-    url.include?(SECURE_MEDIA_ROUTE)
+    # we do not want to exclude topic links that for whatever reason
+    # have secure-media-uploads in the URL e.g. /t/secure-media-uploads-are-cool/223452
+    url.include?(SECURE_MEDIA_ROUTE) && !url.include?("/t/") && FileHelper.is_supported_media?(url)
   end
 
   def self.signed_url_from_secure_media_url(url)

lib/email/styles.rb

@@ -288,15 +288,14 @@ module Email
 
     def replace_secure_media_urls
       @fragment.css('[href]').each do |a|
-        if a['href'][/#{Upload::SECURE_MEDIA_ROUTE}/]
+        if Upload.secure_media_url?(a['href'])
           a.add_next_sibling "<p class='secure-media-notice'>#{I18n.t("emails.secure_media_placeholder")}</p>"
           a.remove
         end
       end
 
-      @fragment.search('img').each do |img|
-        next unless img['src']
-        if img['src'][/#{Upload::SECURE_MEDIA_ROUTE}/]
+      @fragment.search('img[src]').each do |img|
+        if Upload.secure_media_url?(img['src'])
           img.add_next_sibling "<p class='secure-media-notice'>#{I18n.t("emails.secure_media_placeholder")}</p>"
           img.remove
         end

lib/pretty_text.rb

@@ -388,7 +388,7 @@ module PrettyText
 
   def self.strip_secure_media(doc)
     doc.css("a[href]").each do |a|
-      if a["href"].include?("/#{Upload::SECURE_MEDIA_ROUTE}/") && FileHelper.is_supported_media?(a["href"])
+      if Upload.secure_media_url?(a["href"])
         target = %w(video audio).include?(a&.parent&.parent&.name) ? a.parent.parent : a
         target.replace "<p class='secure-media-notice'>#{I18n.t("emails.secure_media_placeholder")}</p>"
       end

spec/components/email/styles_spec.rb

@@ -212,6 +212,10 @@ describe Email::Styles do
       expect(frag.at('p.secure-media-notice')).to be_present
       expect(frag.at('img')).not_to be_present
     end
-  end
 
+    it "does not replace topic links with secure-media-uploads in the name" do
+      frag = html_fragment("<a href=\"#{Discourse.base_url}\/t/secure-media-uploads/235723\">Visit Topic</a>")
+      expect(frag.at('p.secure-media-notice')).not_to be_present
+    end
+  end
 end