FIX: Do not attempt to pull_hotlinked on emoji images when CDN enabled (#10091)

FIX: Do not attempt to pull_hotlinked on emoji images when CDN enabled (#10091)

diff --git a/app/jobs/regular/pull_hotlinked_images.rb b/app/jobs/regular/pull_hotlinked_images.rb
index 9472da0..d147a5a 100644
--- a/app/jobs/regular/pull_hotlinked_images.rb
+++ b/app/jobs/regular/pull_hotlinked_images.rb
@@ -171,10 +171,13 @@ module Jobs
       # make sure we actually have a url
       return false unless src.present?
 
-      # If file is on the forum or CDN domain or already has the
-      # secure media url
-      if Discourse.store.has_been_uploaded?(src) || src =~ /\A\/[^\/]/i || Upload.secure_media_url?(src)
-        return false if src =~ /\/images\/emoji\//
+      local_bases = [
+        Discourse.base_url,
+        Discourse.asset_host,
+      ].compact.map { |s| normalize_src(s) }
+
+      if Discourse.store.has_been_uploaded?(src) || normalize_src(src).start_with?(*local_bases) || src =~ /\A\/[^\/]/i
+        return false if !(src =~ /\/uploads\// || Upload.secure_media_url?(src))
 
         # Someone could hotlink a file from a different site on the same CDN,
         # so check whether we have it in this database
diff --git a/spec/jobs/pull_hotlinked_images_spec.rb b/spec/jobs/pull_hotlinked_images_spec.rb
index f0531b8..5072213 100644
--- a/spec/jobs/pull_hotlinked_images_spec.rb
+++ b/spec/jobs/pull_hotlinked_images_spec.rb
@@ -360,6 +360,35 @@ describe Jobs::PullHotlinkedImages do
       end
     end
 
+    it "returns false for emoji" do
+      src = Emoji.url_for("testemoji.png")
+      expect(subject.should_download_image?(src)).to eq(false)
+    end
+
+    it "returns false for emoji when app and S3 CDNs configured" do
+      set_cdn_url "https://mydomain.cdn/test"
+      SiteSetting.s3_upload_bucket = "some-bucket-on-s3"
+      SiteSetting.s3_access_key_id = "s3-access-key-id"
+      SiteSetting.s3_secret_access_key = "s3-secret-access-key"
+      SiteSetting.s3_cdn_url = "https://s3.cdn.com"
+      SiteSetting.enable_s3_uploads = true
+
+      src = UrlHelper.cook_url(Emoji.url_for("testemoji.png"))
+      expect(subject.should_download_image?(src)).to eq(false)
+    end
+
+    it "returns false for plugin assets" do
+      src = UrlHelper.cook_url("/plugins/discourse-amazing-plugin/myasset.png")
+      puts "src is #{src}"
+      expect(subject.should_download_image?(src)).to eq(false)
+    end
+
+    it "returns false for local non-uploaded files" do
+      src = UrlHelper.cook_url("/mycustomroute.png")
+      puts "src is #{src}"
+      expect(subject.should_download_image?(src)).to eq(false)
+    end
+
     context "when download_remote_images_to_local? is false" do
       before do
         SiteSetting.download_remote_images_to_local = false
@@ -370,11 +399,6 @@ describe Jobs::PullHotlinkedImages do
         expect(subject.should_download_image?(src)).to eq(true)
       end
 
-      it "returns false for emoji" do
-        src = Emoji.url_for("testemoji.png")
-        expect(subject.should_download_image?(src)).to eq(false)
-      end
-
       it 'returns false for valid remote URLs' do
         expect(subject.should_download_image?("http://meta.discourse.org")).to eq(false)
       end

GitHub sha: 17c4f76e

1 Like

This commit appears in #10091 which was approved by eviltrout. It was merged by davidtaylorhq.

Looks like you left a couple puts statements in here @davidtaylorhq

1 Like

Thanks, removed in c5078e5

1 Like