diff --git a/app/models/upload.rb b/app/models/upload.rb index 7bd583f2a6c..8ffbbe99c7e 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -373,6 +373,10 @@ class Upload < ActiveRecord::Base raise "Calculated dominant color but unable to parse output:\n#{data}" if color.nil? color + rescue OpenURI::HTTPError => e + # Some issue with downloading the image from a remote store. + # Assume the upload is broken and save an empty string to prevent re-evaluation + "" rescue Discourse::Utils::CommandError => e # Timeout or unable to parse image # This can happen due to bad user input - ignore and save diff --git a/lib/file_store/base_store.rb b/lib/file_store/base_store.rb index eec747ba080..8b4c41cf6f4 100644 --- a/lib/file_store/base_store.rb +++ b/lib/file_store/base_store.rb @@ -116,6 +116,8 @@ module FileStore follow_redirect: true ) + return nil if file.nil? + cache_file(file, filename) file = get_from_cache(filename) end diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index a2441540ccc..fb4e6487a09 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -703,7 +703,7 @@ RSpec.describe Upload do expect(invalid_image.dominant_color).to eq(nil) end - it "correctly handles download failures" do + it "correctly handles error when file is too large to download" do white_image.stubs(:local?).returns(true) Discourse.store.stubs(:download).returns(nil) @@ -712,6 +712,15 @@ RSpec.describe Upload do expect(invalid_image.dominant_color).to eq("") end + it "correctly handles error when file has HTTP error" do + white_image.stubs(:local?).returns(true) + Discourse.store.stubs(:download).raises(OpenURI::HTTPError) + + expect(invalid_image.dominant_color).to eq(nil) + expect(invalid_image.dominant_color(calculate_if_missing: true)).to eq("") + expect(invalid_image.dominant_color).to eq("") + end + it "is validated for length" do u = Fabricate(:upload)