From 1e9ce51151d57a68cac3c6c2747f85c99dc4095a Mon Sep 17 00:00:00 2001 From: Blake Erickson Date: Wed, 15 Jul 2020 19:15:53 -0600 Subject: [PATCH] FIX: Prevent thumbnail gen if image too large (#10247) It's possible through an import or other means to have images larger than the current max allowed image size in the db. If this happens the thumbnail generation job will keep running indefinitely trying to download a new copy of the original but discarding it because it is larger than the max_file_size eventually causing this error `Job exception: undefined method `path' for nil:NilClass` because the newly downloaded image is now nil. This fix stops the enqueuing of the `GenerateTopicThumbnails` job for all images that happen to be larger than the max image size. --- app/models/topic.rb | 3 +++ spec/models/topic_thumbnail_spec.rb | 13 +++++++++++++ 2 files changed, 16 insertions(+) diff --git a/app/models/topic.rb b/app/models/topic.rb index 4088ab4ded2..d78533fcd8b 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -52,6 +52,7 @@ class Topic < ActiveRecord::Base def thumbnail_info(enqueue_if_missing: false, extra_sizes: []) return nil unless original = image_upload + return nil unless original.filesize < SiteSetting.max_image_size_kb return nil unless original.read_attribute(:width) && original.read_attribute(:height) infos = [] @@ -94,6 +95,7 @@ class Topic < ActiveRecord::Base def generate_thumbnails!(extra_sizes: []) return nil unless SiteSetting.create_thumbnails return nil unless original = image_upload + return nil unless original.filesize < SiteSetting.max_image_size_kb return nil unless original.width && original.height extra_sizes = [] unless extra_sizes.kind_of?(Array) @@ -111,6 +113,7 @@ class Topic < ActiveRecord::Base if thumbnail.nil? && image_upload && SiteSetting.create_thumbnails && + image_upload.filesize < SiteSetting.max_image_size_kb && enqueue_if_missing && Discourse.redis.set(thumbnail_job_redis_key([]), 1, nx: true, ex: 1.minute) Jobs.enqueue(:generate_topic_thumbnails, { topic_id: id }) diff --git a/spec/models/topic_thumbnail_spec.rb b/spec/models/topic_thumbnail_spec.rb index 84c8298a824..92481241c4a 100644 --- a/spec/models/topic_thumbnail_spec.rb +++ b/spec/models/topic_thumbnail_spec.rb @@ -5,6 +5,9 @@ describe "TopicThumbnail" do let(:upload1) { Fabricate(:image_upload, width: 5000, height: 5000) } let(:topic) { Fabricate(:topic, image_upload: upload1) } + let(:upload2) { Fabricate(:image_upload, width: 5000, height: 5000, filesize: 8000) } + let(:topic2) { Fabricate(:topic, image_upload: upload2) } + before do SiteSetting.create_thumbnails = true topic.generate_thumbnails!(extra_sizes: nil) @@ -15,6 +18,16 @@ describe "TopicThumbnail" do expect(topic.topic_thumbnails.length).to eq(1) end + it "does not enque job if original image is too large" do + SiteSetting.create_thumbnails = true + topic2.generate_thumbnails!(extra_sizes: nil) + + TopicThumbnail.ensure_consistency! + topic2.reload + + expect(topic2.topic_thumbnails.length).to eq(0) + end + it "cleans up deleted uploads" do upload1.delete