FIX: ensure local images use local CDN when uploads are stored on S3

FIX: ensure local images use local CDN when uploads are stored on S3

When the S3 store was enabled, we were only applying the S3 CDN. So all images stored locally, like the emojis, were never put on the local CDN.

Fixed a bunch of CookedPostProcessor test by adding a call to ‘optimize_urls’ in order to get final URLs.

I also removed the unnecessary PrettyText.add_s3_cdn method since this is already handled in the CookedPostProcessor.

diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb
index d87da05..300e481 100644
--- a/lib/cooked_post_processor.rb
+++ b/lib/cooked_post_processor.rb
@@ -389,7 +389,7 @@ class CookedPostProcessor
     if upload
       thumbnail = upload.thumbnail(w, h)
       if thumbnail && thumbnail.filesize.to_i < upload.filesize
-        img["src"] = UrlHelper.cook_url(thumbnail.url)
+        img["src"] = thumbnail.url
 
         srcset = +""
 
@@ -408,11 +408,11 @@ class CookedPostProcessor
           img["srcset"] = "#{UrlHelper.cook_url(img["src"])}#{srcset}" if srcset.present?
         end
       else
-        img["src"] = UrlHelper.cook_url(upload.url)
+        img["src"] = upload.url
       end
 
       if small_upload = loading_image(upload)
-        img["data-small-upload"] = UrlHelper.cook_url(small_upload.url)
+        img["data-small-upload"] = small_upload.url
       end
     end
 
@@ -588,8 +588,10 @@ class CookedPostProcessor
       end
     end
 
-    @doc.css("img[src]").each do |img|
-      img["src"] = UrlHelper.cook_url(img["src"].to_s)
+    %w{src data-small-upload}.each do |selector|
+      @doc.css("img[#{selector}]").each do |img|
+        img[selector] = UrlHelper.cook_url(img[selector].to_s)
+      end
     end
   end
 
diff --git a/lib/file_store/local_store.rb b/lib/file_store/local_store.rb
index ef36b37..988497c 100644
--- a/lib/file_store/local_store.rb
+++ b/lib/file_store/local_store.rb
@@ -46,8 +46,7 @@ module FileStore
     end
 
     def cdn_url(url)
-      return url if Discourse.asset_host.blank?
-      url.sub(Discourse.base_url_no_prefix, Discourse.asset_host)
+      UrlHelper.local_cdn_url(url)
     end
 
     def path_for(upload)
diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb
index bad230f..e3a89fe 100644
--- a/lib/pretty_text.rb
+++ b/lib/pretty_text.rb
@@ -254,10 +254,6 @@ module PrettyText
       add_rel_nofollow_to_user_content(doc)
     end
 
-    if SiteSetting.Upload.enable_s3_uploads && SiteSetting.Upload.s3_cdn_url.present?
-      add_s3_cdn(doc)
-    end
-
     if SiteSetting.enable_mentions
       add_mentions(doc, user_id: opts[:user_id])
     end
@@ -265,13 +261,6 @@ module PrettyText
     doc.to_html
   end
 
-  def self.add_s3_cdn(doc)
-    doc.css("img").each do |img|
-      next unless img["src"]
-      img["src"] = Discourse.store.cdn_url(img["src"])
-    end
-  end
-
   def self.add_rel_nofollow_to_user_content(doc)
     whitelist = []
 
diff --git a/lib/url_helper.rb b/lib/url_helper.rb
index 976a63d..51e9378 100644
--- a/lib/url_helper.rb
+++ b/lib/url_helper.rb
@@ -56,11 +56,20 @@ class UrlHelper
     no_cdn = SiteSetting.login_required || SiteSetting.prevent_anons_from_downloading_files
 
     url = absolute_without_cdn(url)
-    url = Discourse.store.cdn_url(url) unless is_attachment && no_cdn
+
+    unless is_attachment && no_cdn
+      url = Discourse.store.cdn_url(url)
+      url = local_cdn_url(url) if Discourse.store.external?
+    end
 
     schemaless(url)
   rescue URI::Error
     url
   end
 
+  def self.local_cdn_url(url)
+    return url if Discourse.asset_host.blank?
+    url.sub(Discourse.base_url_no_prefix, Discourse.asset_host)
+  end
+
 end
diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb
index 90c65db..5f7801d 100644
--- a/spec/components/cooked_post_processor_spec.rb
+++ b/spec/components/cooked_post_processor_spec.rb
@@ -1,5 +1,6 @@
 require "rails_helper"
 require "cooked_post_processor"
+require "file_store/s3_store"
 
 describe CookedPostProcessor do
   context "#post_process" do
@@ -304,6 +305,7 @@ describe CookedPostProcessor do
 
         cpp.add_to_size_cache(upload.url, 2000, 1500)
         cpp.post_process_images
+        cpp.optimize_urls
 
         html = cpp.html
 
@@ -318,6 +320,8 @@ describe CookedPostProcessor do
         cpp = CookedPostProcessor.new(post)
         cpp.add_to_size_cache(upload.url, 2000, 1500)
         cpp.post_process_images
+        cpp.optimize_urls
+
         html = cpp.html
 
         expect(html).to include(%Q|data-small-upload="//cdn.localhost/uploads/default/10x10.png"|)
@@ -343,6 +347,7 @@ describe CookedPostProcessor do
         cpp = CookedPostProcessor.new(post)
         cpp.add_to_size_cache(upload.url, 200, 4000)
         cpp.post_process_images
+        cpp.optimize_urls
 
         expect(cpp.html).to_not include('srcset="')
       end
@@ -360,7 +365,10 @@ describe CookedPostProcessor do
       let(:post) { Fabricate(:post_with_image_urls) }
       let(:cpp) { CookedPostProcessor.new(post, image_sizes: image_sizes) }
 
-      before { cpp.post_process_images }
+      before do
+        cpp.post_process_images
+        cpp.optimize_urls
+      end
 
       context "valid" do
         let(:image_sizes) { { "http://foo.bar/image.png" => { "width" => 111, "height" => 222 } } }
@@ -422,9 +430,14 @@ describe CookedPostProcessor do
         FileStore::BaseStore.any_instance.expects(:get_depth_for).returns(0)
 
         cpp.post_process_images
-        expect(cpp.html).to match_html "<p><div class=\"lightbox-wrapper\"><a class=\"lightbox\" href=\"/uploads/default/original/1X/1234567890123456.jpg\" data-download-href=\"/uploads/default/#{upload.sha1}\" title=\"logo.png\"><img src=\"//test.localhost/uploads/default/optimized/1X/#{upload.sha1}_#{OptimizedImage::VERSION}_690x788.png\" width=\"690\" height=\"788\"><div class=\"meta\">
-<span class=\"filename\">logo.png</span><span class=\"informations\">1750×2000 1.21 KB</span><span class=\"expand\"></span>
-</div></a></div></p>"
+        cpp.optimize_urls
+
+        expect(cpp.html).to match_html <<~HTML
+          <p><div class="lightbox-wrapper"><a class="lightbox" href="//test.localhost/uploads/default/original/1X/1234567890123456.jpg" data-download-href="//test.localhost/uploads/default/#{upload.sha1}" title="logo.png"><img src="//test.localhost/uploads/default/optimized/1X/#{upload.sha1}_#{OptimizedImage::VERSION}_690x788.png" width="690" height="788"><div class="meta">
+          <span class="filename">logo.png</span><span class="informations">1750×2000 1.21 KB</span><span class="expand"></span>
+          </div></a></div></p>
+        HTML
+
         expect(cpp).to be_dirty
       end
 
@@ -439,8 +452,11 @@ describe CookedPostProcessor do
         it 'should not add lightbox' do
           cpp.post_process_oneboxes
           cpp.post_process_images
+          cpp.optimize_urls
 
-          expect(cpp.html).to match_html("<p><img class=\"onebox\" src=\"/uploads/default/original/1X/1234567890123456.jpg\" width=\"690\"\ height=\"788\"></p>")
+          expect(cpp.html).to match_html <<~HTML
+            <p><img class="onebox" src="//test.localhost/uploads/default/original/1X/1234567890123456.jpg" width="690" height="788"></p>
+          HTML
         end
       end
 
@@ -451,8 +467,11 @@ describe CookedPostProcessor do
 
         it 'should not add lightbox' do
           cpp.post_process_images
+          cpp.optimize_urls
 
-          expect(cpp.html).to match_html("<p><img src=\"/uploads/default/original/1X/1234567890123456.svg\" width=\"690\"\ height=\"788\"></p>")
+          expect(cpp.html).to match_html <<~HTML
+            <p><img src="//test.localhost/uploads/default/original/1X/1234567890123456.svg" width="690" height="788"></p>
+          HTML
         end
 
         describe 'when image src is an URL' do
@@ -463,6 +482,7 @@ describe CookedPostProcessor do
           it 'should not add lightbox' do
             SiteSetting.crawl_images = true
             cpp.post_process_images
+            cpp.optimize_urls
 
             expect(cpp.html).to match_html("<p><img src=\"http://test.discourse/uploads/default/original/1X/1234567890123456.svg?somepamas\" width=\"690\"\ height=\"788\"></p>")
           end
@@ -515,9 +535,14 @@ describe CookedPostProcessor do
 
       it "crops the image" do
         cpp.post_process_images

[... diff too long, it was truncated ...]

GitHub sha: 664e90bd

1 Like