Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FIX: Correctly handle HTTP errors during dominant color calculation (#18565) 

The previous fix in e83d35d6 was incorrect, and the stub in the test was never actually hit. This commit moves the error handling to the right place and updates the specs to ensure the stub is always used.
This commit is contained in:
David Taylor 2022-10-12 15:50:44 +01:00 committed by GitHub
parent 899ed039b7
commit 76c86a4269
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 17 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
app/models/upload.rb
View File

@ -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

20
spec/models/upload_spec.rb
View File

@ -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