diff --git a/app/models/upload.rb b/app/models/upload.rb index 8e2150541f9..6c693730184 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -67,7 +67,7 @@ class Upload < ActiveRecord::Base def self.create_for(user_id, file, filename, filesize, options = {}) DistributedMutex.synchronize("upload_#{user_id}_#{filename}") do # do some work on images - if FileHelper.is_image?(filename) + if FileHelper.is_image?(filename) && system("identify '#{file.path}' >/dev/null 2>&1") if filename =~ /\.svg$/i svg = Nokogiri::XML(file).at_css("svg") w = svg["width"].to_i @@ -122,7 +122,7 @@ class Upload < ActiveRecord::Base upload = find_by(sha1: sha1) # make sure the previous upload has not failed - if upload && upload.url.blank? + if upload && (upload.url.blank? || is_dimensionless_image?(filename, upload.width, upload.height)) upload.destroy upload = nil end @@ -141,8 +141,9 @@ class Upload < ActiveRecord::Base upload.height = height upload.origin = options[:origin][0...1000] if options[:origin] - if FileHelper.is_image?(filename) && (upload.width == 0 || upload.height == 0) + if is_dimensionless_image?(filename, upload.width, upload.height) upload.errors.add(:base, I18n.t("upload.images.size_not_found")) + return upload end return upload unless upload.save @@ -162,6 +163,10 @@ class Upload < ActiveRecord::Base end end + def self.is_dimensionless_image?(filename, width, height) + FileHelper.is_image?(filename) && (width.blank? || width == 0 || height.blank? || height == 0) + end + def self.get_from_url(url) return if url.blank? # we store relative urls, so we need to remove any host/cdn diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index 2a0ceaf22c9..4a57897e771 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -19,6 +19,13 @@ describe UploadsController do }) end + let(:fake_jpg) do + ActionDispatch::Http::UploadedFile.new({ + filename: 'fake.jpg', + tempfile: file_from_fixtures("fake.jpg") + }) + end + let(:text_file) do ActionDispatch::Http::UploadedFile.new({ filename: 'LICENSE.TXT', @@ -118,6 +125,20 @@ describe UploadsController do expect(response).to_not be_success end + it 'returns an error when it could not determine the dimensions of an image' do + Jobs.expects(:enqueue).with(:create_thumbnails, anything).never + + message = MessageBus.track_publish do + xhr :post, :create, file: fake_jpg, type: "composer" + end.first + + expect(response.status).to eq 200 + + expect(message.channel).to eq("/uploads/composer") + expect(message.data["errors"]).to be + expect(message.data["errors"][0]).to eq(I18n.t("upload.images.size_not_found")) + end + end end diff --git a/spec/fixtures/images/fake.jpg b/spec/fixtures/images/fake.jpg new file mode 100644 index 00000000000..e59fa523ee0 Binary files /dev/null and b/spec/fixtures/images/fake.jpg differ