diff --git a/app/models/upload.rb b/app/models/upload.rb index 8ffbbe99c7e..813c5e57e91 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -340,7 +340,13 @@ class Upload < ActiveRecord::Base if local? Discourse.store.path_for(self) else - Discourse.store.download(self)&.path + begin + Discourse.store.download(self)&.path + 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 + nil + end end if local_path.nil? @@ -373,10 +379,6 @@ 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/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index fb4e6487a09..642061904e3 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -704,21 +704,21 @@ RSpec.describe Upload do end it "correctly handles error when file is too large to download" do - white_image.stubs(:local?).returns(true) - Discourse.store.stubs(:download).returns(nil) + white_image.stubs(:local?).returns(false) + FileStore::LocalStore.any_instance.stubs(:download).returns(nil).once - 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("") + expect(white_image.dominant_color).to eq(nil) + expect(white_image.dominant_color(calculate_if_missing: true)).to eq("") + expect(white_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) + white_image.stubs(:local?).returns(false) + FileStore::LocalStore.any_instance.stubs(:download).raises(OpenURI::HTTPError.new("Error", nil)).once - 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("") + expect(white_image.dominant_color).to eq(nil) + expect(white_image.dominant_color(calculate_if_missing: true)).to eq("") + expect(white_image.dominant_color).to eq("") end it "is validated for length" do