From d1fc759ac49c618c6a735d5765c208c7148a805c Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Tue, 19 Oct 2021 17:12:29 +0530 Subject: [PATCH] FIX: remove 'crawl_images' site setting (#14646) --- config/environments/test.rb | 1 - config/site_settings.yml | 2 - ...092048_remove_crawl_images_site_setting.rb | 15 ++++++ lib/cooked_post_processor.rb | 3 -- .../advanced_user_narrative_spec.rb | 1 + .../new_user_narrative_spec.rb | 1 + .../track_selector_spec.rb | 1 + .../discourse-narrative-bot/spec/user_spec.rb | 1 + spec/components/cooked_post_processor_spec.rb | 51 +++++-------------- spec/components/post_revisor_spec.rb | 2 + spec/jobs/process_post_spec.rb | 3 ++ spec/jobs/pull_hotlinked_images_spec.rb | 16 +++++- spec/models/post_action_spec.rb | 1 + spec/models/quoted_post_spec.rb | 1 + spec/rails_helper.rb | 1 + spec/services/search_indexer_spec.rb | 1 - spec/services/user_anonymizer_spec.rb | 2 + spec/services/username_changer_spec.rb | 1 + spec/support/fast_image_helpers.rb | 7 +++ 19 files changed, 64 insertions(+), 47 deletions(-) create mode 100644 db/migrate/20211019092048_remove_crawl_images_site_setting.rb create mode 100644 spec/support/fast_image_helpers.rb diff --git a/config/environments/test.rb b/config/environments/test.rb index 502ae6b376d..ce5485047f3 100644 --- a/config/environments/test.rb +++ b/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) diff --git a/config/site_settings.yml b/config/site_settings.yml index 97c315166bd..7fb7f101eb5 100644 --- a/config/site_settings.yml +++ b/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 diff --git a/db/migrate/20211019092048_remove_crawl_images_site_setting.rb b/db/migrate/20211019092048_remove_crawl_images_site_setting.rb new file mode 100644 index 00000000000..63c28dbc64b --- /dev/null +++ b/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 diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb index 9624662c26e..1dae3a22dea 100644 --- a/lib/cooked_post_processor.rb +++ b/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. diff --git a/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/advanced_user_narrative_spec.rb b/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/advanced_user_narrative_spec.rb index c7fa4b7ee8e..5fce91fea83 100644 --- a/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/advanced_user_narrative_spec.rb +++ b/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 diff --git a/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/new_user_narrative_spec.rb b/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/new_user_narrative_spec.rb index e249d98a441..c421b620df0 100644 --- a/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/new_user_narrative_spec.rb +++ b/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 diff --git a/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/track_selector_spec.rb b/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/track_selector_spec.rb index e2e57290716..b93daf2facc 100644 --- a/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/track_selector_spec.rb +++ b/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\"}") diff --git a/plugins/discourse-narrative-bot/spec/user_spec.rb b/plugins/discourse-narrative-bot/spec/user_spec.rb index d2eb77384b6..de939c61726 100644 --- a/plugins/discourse-narrative-bot/spec/user_spec.rb +++ b/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 diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb index 61a5290e952..1ff425821c6 100644 --- a/spec/components/cooked_post_processor_spec.rb +++ b/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("

") @@ -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("

") + expect(cpp.doc.to_s).to eq("

") 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("

") + expect(cpp.doc.to_s).to eq("

") 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("

") + expect(cpp.doc.to_s).to eq("

") 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 diff --git a/spec/components/post_revisor_spec.rb b/spec/components/post_revisor_spec.rb index 48fc17e0f6e..207f2177830 100644 --- a/spec/components/post_revisor_spec.rb +++ b/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}) diff --git a/spec/jobs/process_post_spec.rb b/spec/jobs/process_post_spec.rb index 0bc38d6fde5..335af635f4c 100644 --- a/spec/jobs/process_post_spec.rb +++ b/spec/jobs/process_post_spec.rb @@ -37,6 +37,7 @@ describe Jobs::ProcessPost do it 'processes posts' do post = Fabricate(:post, raw: "") 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 diff --git a/spec/jobs/pull_hotlinked_images_spec.rb b/spec/jobs/pull_hotlinked_images_spec.rb index 3faa9195b81..1a67b1dbce8 100644 --- a/spec/jobs/pull_hotlinked_images_spec.rb +++ b/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: "") + 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: "") + 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: "") + 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: "") + 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: "test") + 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: "") + 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: "") + 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 BODY + stub_image_size 2.times do post.rebake! diff --git a/spec/models/post_action_spec.rb b/spec/models/post_action_spec.rb index 45bf725fdb2..82f7ea92f59 100644 --- a/spec/models/post_action_spec.rb +++ b/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 diff --git a/spec/models/quoted_post_spec.rb b/spec/models/quoted_post_spec.rb index 35082b5af27..b60722c5e45 100644 --- a/spec/models/quoted_post_spec.rb +++ b/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) diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index ab44d904ea9..61523bc2d7e 100644 --- a/spec/rails_helper.rb +++ b/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! diff --git a/spec/services/search_indexer_spec.rb b/spec/services/search_indexer_spec.rb index 1721b7b5518..dadcb567a12 100644 --- a/spec/services/search_indexer_spec.rb +++ b/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") diff --git a/spec/services/user_anonymizer_spec.rb b/spec/services/user_anonymizer_spec.rb index a150d1e1978..0ed06e82da5 100644 --- a/spec/services/user_anonymizer_spec.rb +++ b/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 diff --git a/spec/services/username_changer_spec.rb b/spec/services/username_changer_spec.rb index b23cbcf1d4d..65d0802b137 100644 --- a/spec/services/username_changer_spec.rb +++ b/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| diff --git a/spec/support/fast_image_helpers.rb b/spec/support/fast_image_helpers.rb new file mode 100644 index 00000000000..55ad54ba422 --- /dev/null +++ b/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