FIX: remove 'crawl_images' site setting (#14646)

config/environments/test.rb

@@ -65,7 +65,6 @@ Discourse::Application.configure do
       s.set_regardless_of_locale(:min_post_length, 5)
       s.set_regardless_of_locale(:min_first_post_length, 5)
       s.set_regardless_of_locale(:min_personal_message_post_length, 10)
-      s.set_regardless_of_locale(:crawl_images, false)
       s.set_regardless_of_locale(:download_remote_images_to_local, false)
       s.set_regardless_of_locale(:unique_posts_mins, 0)
       s.set_regardless_of_locale(:max_consecutive_replies, 0)

config/site_settings.yml

@@ -1293,8 +1293,6 @@ files:
     default: "1|1.5|2"
     type: list
     list_type: compact
-  crawl_images:
-    default: true
   max_image_width:
     client: true
     default: 690

db/migrate/20211019092048_remove_crawl_images_site_setting.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class RemoveCrawlImagesSiteSetting < ActiveRecord::Migration[6.1]
+  def up
+    execute <<~SQL
+      DELETE
+      FROM site_settings
+      WHERE name = 'crawl_images';
+    SQL
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

lib/cooked_post_processor.rb

@@ -296,9 +296,6 @@ class CookedPostProcessor
 
     return unless is_valid_image_url?(absolute_url)
 
-    # we can *always* crawl our own images
-    return unless SiteSetting.crawl_images? || Discourse.store.has_been_uploaded?(url)
-
     @size_cache[url] = FastImage.size(absolute_url)
   rescue Zlib::BufError, URI::Error, OpenSSL::SSL::SSLError
     # FastImage.size raises BufError for some gifs, leave it.

plugins/discourse-narrative-bot/spec/discourse_narrative_bot/advanced_user_narrative_spec.rb

@@ -26,6 +26,7 @@ RSpec.describe DiscourseNarrativeBot::AdvancedUserNarrative do
   fab!(:reset_trigger) { DiscourseNarrativeBot::TrackSelector.reset_trigger }
 
   before do
+    stub_image_size
     Jobs.run_immediately!
     SiteSetting.discourse_narrative_bot_enabled = true
   end

plugins/discourse-narrative-bot/spec/discourse_narrative_bot/new_user_narrative_spec.rb

@@ -29,6 +29,7 @@ describe DiscourseNarrativeBot::NewUserNarrative do
   fab!(:reset_trigger) { DiscourseNarrativeBot::TrackSelector.reset_trigger }
 
   before do
+    stub_image_size
     Jobs.run_immediately!
     SiteSetting.discourse_narrative_bot_enabled = true
   end

plugins/discourse-narrative-bot/spec/discourse_narrative_bot/track_selector_spec.rb

@@ -17,6 +17,7 @@ describe DiscourseNarrativeBot::TrackSelector do
   end
 
   before do
+    stub_image_size
     stub_request(:get, "http://api.forismatic.com/api/1.0/?format=json&lang=en&method=getQuote").
       to_return(status: 200, body: "{\"quoteText\":\"Be Like Water\",\"quoteAuthor\":\"Bruce Lee\"}")
 

plugins/discourse-narrative-bot/spec/user_spec.rb

@@ -15,6 +15,7 @@ describe User do
   end
 
   before do
+    stub_image_size
     Jobs.run_immediately!
     SiteSetting.discourse_narrative_bot_enabled = true
   end

spec/components/cooked_post_processor_spec.rb

@@ -345,6 +345,7 @@ describe CookedPostProcessor do
         let(:cpp) { CookedPostProcessor.new(post, image_sizes: image_sizes) }
 
         before do
+          stub_image_size
           cpp.post_process
         end
 
@@ -462,7 +463,6 @@ describe CookedPostProcessor do
             it 'should not add lightbox' do
               FastImage.expects(:size).returns([1750, 2000])
 
-              SiteSetting.crawl_images = true
               cpp.post_process
 
               expect(cpp.html).to match_html("<p><img src=\"http://test.discourse/#{upload_path}/original/1X/1234567890123456.svg?somepamas\" width=\"690\"\ height=\"788\"></p>")
@@ -526,6 +526,7 @@ describe CookedPostProcessor do
             it "does not create thumbnails or optimize images" do
               CookedPostProcessor.any_instance.expects(:optimize_image!).never
               Upload.any_instance.expects(:create_thumbnail!).never
+              stub_image_size
               cpp.post_process
 
               expect(cpp.html).not_to match_html cooked_html
@@ -828,21 +829,18 @@ describe CookedPostProcessor do
 
     it "resizes when only width is specified" do
       img = { 'src' => 'http://foo.bar/image3.png', 'width' => 100 }
-      SiteSetting.crawl_images = true
       FastImage.expects(:size).returns([200, 400])
       expect(cpp.get_size_from_attributes(img)).to eq([100, 200])
     end
 
     it "resizes when only height is specified" do
       img = { 'src' => 'http://foo.bar/image3.png', 'height' => 100 }
-      SiteSetting.crawl_images = true
       FastImage.expects(:size).returns([100, 300])
       expect(cpp.get_size_from_attributes(img)).to eq([33, 100])
     end
 
     it "doesn't raise an error with a weird url" do
       img = { 'src' => nil, 'height' => 100 }
-      SiteSetting.crawl_images = true
       expect(cpp.get_size_from_attributes(img)).to be_nil
     end
 
@@ -876,39 +874,10 @@ describe CookedPostProcessor do
     end
 
     it "caches the results" do
-      SiteSetting.crawl_images = true
       FastImage.expects(:size).returns([200, 400])
       cpp.get_size("http://foo.bar/image3.png")
       expect(cpp.get_size("http://foo.bar/image3.png")).to eq([200, 400])
     end
-
-    context "when crawl_images is disabled" do
-
-      before do
-        SiteSetting.crawl_images = false
-      end
-
-      it "doesn't call FastImage" do
-        FastImage.expects(:size).never
-        expect(cpp.get_size("http://foo.bar/image1.png")).to eq(nil)
-      end
-
-      it "is always allowed to crawl our own images" do
-        store = stub
-        Discourse.expects(:store).returns(store).at_least_once
-        store.expects(:has_been_uploaded?).returns(true)
-        FastImage.expects(:size).returns([100, 200])
-        expect(cpp.get_size("http://foo.bar/image2.png")).to eq([100, 200])
-      end
-
-      it "returns nil if FastImage can't get the original size" do
-        Discourse.store.class.any_instance.expects(:has_been_uploaded?).returns(true)
-        FastImage.expects(:size).returns(nil)
-        expect(cpp.get_size("http://foo.bar/image3.png")).to eq(nil)
-      end
-
-    end
-
   end
 
   context "#is_valid_image_url?" do
@@ -1078,15 +1047,17 @@ describe CookedPostProcessor do
         post.save_custom_fields
 
         cpp = CookedPostProcessor.new(post, invalidate_oneboxes: true)
+        stub_image_size(width: 100, height: 200)
         cpp.post_process_oneboxes
 
-        expect(cpp.doc.to_s).to eq("<p><img class=\"onebox\" src=\"#{upload.url}\" width=\"\" height=\"\"></p>")
+        expect(cpp.doc.to_s).to eq("<p><img class=\"onebox\" src=\"#{upload.url}\" width=\"100\" height=\"200\"></p>")
 
         upload.destroy!
         cpp = CookedPostProcessor.new(post, invalidate_oneboxes: true)
+        stub_image_size(width: 100, height: 200)
         cpp.post_process_oneboxes
 
-        expect(cpp.doc.to_s).to eq("<p><img class=\"onebox\" src=\"#{image_url}\" width=\"\" height=\"\"></p>")
+        expect(cpp.doc.to_s).to eq("<p><img class=\"onebox\" src=\"#{image_url}\" width=\"100\" height=\"200\"></p>")
         Oneboxer.unstub(:onebox)
       end
 
@@ -1112,9 +1083,10 @@ describe CookedPostProcessor do
           UrlHelper.expects(:cook_url).with(upload.url, secure: true).returns(cooked_url)
 
           cpp = CookedPostProcessor.new(post, invalidate_oneboxes: true)
+          stub_image_size(width: 100, height: 200)
           cpp.post_process_oneboxes
 
-          expect(cpp.doc.to_s).to eq("<p><img class=\"onebox\" src=\"#{cooked_url}\" width=\"\" height=\"\"></p>")
+          expect(cpp.doc.to_s).to eq("<p><img class=\"onebox\" src=\"#{cooked_url}\" width=\"100\" height=\"200\"></p>")
         end
       end
     end
@@ -1206,8 +1178,6 @@ describe CookedPostProcessor do
   context "#post_process_oneboxes with square image" do
 
     it "generates a onebox-avatar class" do
-      SiteSetting.crawl_images = true
-
       url = 'https://square-image.com/onebox'
 
       body = <<~HTML
@@ -1226,7 +1196,7 @@ describe CookedPostProcessor do
 
       # not an ideal stub but shipping the whole image to fast image can add
       # a lot of cost to this test
-      FastImage.stubs(:size).returns([200, 200])
+      stub_image_size(width: 200, height: 200)
 
       post = Fabricate.build(:post, raw: url)
       cpp = CookedPostProcessor.new(post, invalidate_oneboxes: true)
@@ -1817,10 +1787,13 @@ describe CookedPostProcessor do
       Fabricate(:post, topic: topic, post_type: Post.types[:small_action])
       reply = PostCreator.create!(topic.user, topic_id: topic.id, raw: raw)
 
+      stub_image_size
       CookedPostProcessor.new(reply).post_process
       expect(reply.raw).to eq(raw)
 
       PostRevisor.new(reply).revise!(Discourse.system_user, raw: raw, edit_reason: "put back full quote")
+
+      stub_image_size
       CookedPostProcessor.new(reply).post_process(new_post: true)
       expect(reply.raw).to eq("and this is the third reply")
     end

spec/components/post_revisor_spec.rb

@@ -1185,6 +1185,7 @@ describe PostRevisor do
         end
 
         it "updates the upload secure status, which is secure by default from the composer. set to false for a public topic" do
+          stub_image_size
           subject.revise!(user, raw: <<~RAW)
               This is a post with a secure upload
               ![image5](#{image5.short_url})
@@ -1196,6 +1197,7 @@ describe PostRevisor do
 
         it "does not update the upload secure status, which is secure by default from the composer for a private" do
           post.topic.update(category: Fabricate(:private_category,  group: Fabricate(:group)))
+          stub_image_size
           subject.revise!(user, raw: <<~RAW)
               This is a post with a secure upload
               ![image5](#{image5.short_url})

spec/jobs/process_post_spec.rb

@@ -37,6 +37,7 @@ describe Jobs::ProcessPost do
     it 'processes posts' do
       post = Fabricate(:post, raw: "<img src='#{Discourse.base_url_no_prefix}/awesome/picture.png'>")
       expect(post.cooked).to match(/http/)
+      stub_image_size
 
       Jobs::ProcessPost.new.execute(post_id: post.id)
       post.reload
@@ -53,6 +54,7 @@ describe Jobs::ProcessPost do
     it "extracts links to quoted posts" do
       quoted_post = Fabricate(:post, raw: "This is a post with a link to https://www.discourse.org", post_number: 42)
       post.update_columns(raw: "This quote is the best\n\n[quote=\"#{quoted_post.user.username}, topic:#{quoted_post.topic_id}, post:#{quoted_post.post_number}\"]\n#{quoted_post.excerpt}\n[/quote]")
+      stub_image_size
       # when creating a quote, we also create the reflexion link
       expect { Jobs::ProcessPost.new.execute(post_id: post.id) }.to change { TopicLink.count }.by(2)
     end
@@ -60,6 +62,7 @@ describe Jobs::ProcessPost do
     it "extracts links to oneboxed topics" do
       oneboxed_post = Fabricate(:post)
       post.update_columns(raw: "This post is the best\n\n#{oneboxed_post.full_url}")
+      stub_image_size
       # when creating a quote, we also create the reflexion link
       expect { Jobs::ProcessPost.new.execute(post_id: post.id) }.to change { TopicLink.count }.by(2)
     end

spec/jobs/pull_hotlinked_images_spec.rb

@@ -31,7 +31,6 @@ describe Jobs::PullHotlinkedImages do
       "#{Discourse.base_url}/#{upload_path}/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png"
     )
 
-    SiteSetting.crawl_images = true
     SiteSetting.download_remote_images_to_local = true
     SiteSetting.max_image_size_kb = 2
     SiteSetting.download_remote_images_threshold = 0
@@ -62,6 +61,7 @@ describe Jobs::PullHotlinkedImages do
 
     it 'replaces images' do
       post = Fabricate(:post, raw: "<img src='#{image_url}'>")
+      stub_image_size
 
       expect do
         Jobs::PullHotlinkedImages.new.execute(post_id: post.id)
@@ -73,6 +73,7 @@ describe Jobs::PullHotlinkedImages do
 
     it 'removes downloaded images when they are no longer needed' do
       post = Fabricate(:post, raw: "<img src='#{image_url}'>")
+      stub_image_size
       post.rebake!
       post.reload
       expect(post.post_uploads.count).to eq(1)
@@ -85,6 +86,7 @@ describe Jobs::PullHotlinkedImages do
 
     it 'replaces images again after edit' do
       post = Fabricate(:post, raw: "<img src='#{image_url}'>")
+      stub_image_size
 
       expect do
         post.rebake!
@@ -104,6 +106,7 @@ describe Jobs::PullHotlinkedImages do
 
     it 'replaces encoded image urls' do
       post = Fabricate(:post, raw: "<img src='#{encoded_image_url}'>")
+      stub_image_size
       expect do
         Jobs::PullHotlinkedImages.new.execute(post_id: post.id)
       end.to change { Upload.count }.by(1)
@@ -143,6 +146,7 @@ describe Jobs::PullHotlinkedImages do
     it 'replaces correct image URL' do
       url = image_url.sub("/2e/Longcat1.png", '')
       post = Fabricate(:post, raw: "[Images](#{url})\n![](#{image_url})")
+      stub_image_size
 
       expect do
         Jobs::PullHotlinkedImages.new.execute(post_id: post.id)
@@ -154,6 +158,7 @@ describe Jobs::PullHotlinkedImages do
     it 'replaces images without protocol' do
       url = image_url.sub(/^https?\:/, '')
       post = Fabricate(:post, raw: "<img alt='test' src='#{url}'>")
+      stub_image_size
 
       expect do
         Jobs::PullHotlinkedImages.new.execute(post_id: post.id)
@@ -166,6 +171,7 @@ describe Jobs::PullHotlinkedImages do
       url = image_url.sub(/\.[a-zA-Z0-9]+$/, '')
       stub_request(:get, url).to_return(body: png, headers: { "Content-Type" => "image/png" })
       post = Fabricate(:post, raw: "<img src='#{url}'>")
+      stub_image_size
 
       expect do
         Jobs::PullHotlinkedImages.new.execute(post_id: post.id)
@@ -182,6 +188,7 @@ describe Jobs::PullHotlinkedImages do
         .to_return(status: 200, body: file_from_fixtures("smallest.png"))
 
       post = Fabricate(:post, raw: "<img src='#{url}'>")
+      stub_image_size
 
       expect { Jobs::PullHotlinkedImages.new.execute(post_id: post.id) }
         .to change { Upload.count }.by(1)
@@ -261,6 +268,7 @@ describe Jobs::PullHotlinkedImages do
       ![abcde](#{image_url} 'some test')
       ![](#{image_url} 'some test')
       MD
+      stub_image_size
 
       expect { Jobs::PullHotlinkedImages.new.execute(post_id: post.id) }
         .to change { Upload.count }.by(1)
@@ -281,6 +289,7 @@ describe Jobs::PullHotlinkedImages do
       ![some test](#{image_url})
       ![some test 2]("#{image_url})
       MD
+      stub_image_size
 
       expect { Jobs::PullHotlinkedImages.new.execute(post_id: post.id) }
         .to change { Upload.count }.by(1)
@@ -296,6 +305,7 @@ describe Jobs::PullHotlinkedImages do
       #{image_url}
       [/img]
       MD
+      stub_image_size
 
       expect { Jobs::PullHotlinkedImages.new.execute(post_id: post.id) }
         .to change { Upload.count }.by(1)
@@ -336,6 +346,8 @@ describe Jobs::PullHotlinkedImages do
 
       it 'replaces image src' do
         post = Fabricate(:post, raw: "#{url}")
+        stub_image_size
+
         post.rebake!
         post.reload
 
@@ -345,6 +357,7 @@ describe Jobs::PullHotlinkedImages do
 
       it 'associates uploads correctly' do
         post = Fabricate(:post, raw: "#{url}")
+        stub_image_size
         post.rebake!
         post.reload
 
@@ -364,6 +377,7 @@ describe Jobs::PullHotlinkedImages do
         <img src='#{broken_image_url}'>
         <a href='#{url}'><img src='#{large_image_url}'></a>
         BODY
+        stub_image_size
 
         2.times do
           post.rebake!

spec/models/post_action_spec.rb

@@ -900,6 +900,7 @@ describe PostAction do
       Jobs.run_immediately!
       post = Fabricate(:post)
       user = Fabricate(:user)
+      stub_image_size
       result = PostActionCreator.create(user, post, :spam, message: "WAT")
       topic = result.post_action.related_post.topic
       reviewable = result.reviewable

spec/models/quoted_post_spec.rb

@@ -22,6 +22,7 @@ describe QuotedPost do
 
       #{post3.full_url}
     RAW
+    stub_image_size
 
     post4 = create_post(topic: topic, raw: raw, post_number: 4, reply_to_post_number: post3.post_number)
 

spec/rails_helper.rb

@@ -182,6 +182,7 @@ RSpec.configure do |config|
   config.include SidekiqHelpers
   config.include UploadsHelpers
   config.include OneboxHelpers
+  config.include FastImageHelpers
   config.mock_framework = :mocha
   config.order = 'random'
   config.infer_spec_type_from_file_location!

spec/services/search_indexer_spec.rb

@@ -225,7 +225,6 @@ describe SearchIndexer do
 
     it 'should not include lightbox in search' do
       Jobs.run_immediately!
-      SiteSetting.crawl_images = true
       SiteSetting.max_image_width = 1
 
       stub_request(:get, "https://meta.discourse.org/some.png")

spec/services/user_anonymizer_spec.rb

@@ -153,6 +153,8 @@ describe UserAnonymizer do
 
       topic = Fabricate(:topic, user: user)
       quoted_post = create_post(user: user, topic: topic, post_number: 1, raw: "quoted post")
+
+      stub_image_size
       post = create_post(raw: <<~RAW)
         Lorem ipsum
 

spec/services/username_changer_spec.rb

@@ -107,6 +107,7 @@ describe UsernameChanger do
       end
 
       def create_post_and_change_username(args = {}, &block)
+        stub_image_size
         post = create_post(args.merge(topic_id: topic.id))
 
         args.delete(:revisions)&.each do |revision|

spec/support/fast_image_helpers.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+module FastImageHelpers
+  def stub_image_size(width: nil, height: nil)
+    FastImage.stubs(:size).returns([width, height])
+  end
+end