From dac923379a91b821690ac234a6a01717a3669c20 Mon Sep 17 00:00:00 2001
From: Martin Brennan <mjrbrennan@gmail.com>
Date: Mon, 17 Feb 2020 12:30:47 +1000
Subject: [PATCH] FIX: Never mark uploads based on regular emoji secure (#8973)

Sometimes PullHotlinkedImages pulls down a site emoji and creates a new upload record for it. In the cases where these happen the upload is not created via the normal path that custom emoji follows, so we need to check in UploadSecurity whether the origin of the upload is based on a regular site emoji. If it is we never want to mark it as secure (we don't want emoji not accessible from other posts because of secure media).

This only became apparent because the uploads:ensure_correct_acl rake task uses UploadSecurity to check whether an upload should be secure, which would have marked a whole bunch of regular-old-emojis as secure.
---
 lib/upload_security.rb     | 11 ++++++++---
 spec/models/upload_spec.rb |  8 ++++++++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/lib/upload_security.rb b/lib/upload_security.rb
index 816377d5265..79e162734b7 100644
--- a/lib/upload_security.rb
+++ b/lib/upload_security.rb
@@ -28,7 +28,7 @@ class UploadSecurity
   private
 
   def uploading_in_public_context?
-    @upload.for_theme || @upload.for_site_setting || @upload.for_gravatar || public_type? || used_for_custom_emoji?
+    @upload.for_theme || @upload.for_site_setting || @upload.for_gravatar || public_type? || used_for_custom_emoji? || based_on_regular_emoji?
   end
 
   def supported_media?
@@ -72,7 +72,12 @@ class UploadSecurity
   end
 
   def used_for_custom_emoji?
-    return false if @upload.id.blank?
-    CustomEmoji.exists?(upload_id: @upload.id)
+    @upload.id.present? && CustomEmoji.exists?(upload_id: @upload.id)
+  end
+
+  def based_on_regular_emoji?
+    return false if @upload.origin.blank?
+    uri = URI.parse(@upload.origin)
+    Emoji.all.map(&:url).include?("#{uri.path}?#{uri.query}")
   end
 end
diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb
index 2b7479c06ef..73ce096ba82 100644
--- a/spec/models/upload_spec.rb
+++ b/spec/models/upload_spec.rb
@@ -431,6 +431,14 @@ describe Upload do
 
         expect(upload.reload.secure).to eq(false)
       end
+
+      it 'does not mark an upload whose origin matches a regular emoji as secure (sometimes emojis are downloaded in pull_hotlinked_images)' do
+        SiteSetting.login_required = true
+        grinning = Emoji.all.last
+        upload.update!(secure: false, origin: "http://localhost:3000#{grinning.url}")
+        expect { upload.update_secure_status }
+          .not_to change { upload.secure }
+      end
     end
   end