From c2719cabe1c861ec2b55e8c4ac3ae1c808612d63 Mon Sep 17 00:00:00 2001 From: Jeremy Stanley Date: Wed, 18 Feb 2015 11:33:40 -0700 Subject: [PATCH] don't create thumbnails for very large images test plan: 1. enable new files 2. upload a sane-sized image file (jpg/gif/png) with less than 100 million pixels 3. within a minute or so, a thumbnail image should be generated for the image, and visible after the new files page refreshes 4. upload an image with more than 100 million pixels (the size of the file in bytes isn't relevant; you can use 100mpx.png, attached to the ticket, which is a 12800x8000 image which compresses down to 13K because it's solid blackness). Canvas should not generate a thumbnail for it. A generic icon should be used (not a solid black square). fixes CNVS-17995 Change-Id: Ie5ed1f57de65fa105cb3f23ceb0b895b676375fe Reviewed-on: https://gerrit.instructure.com/48973 Tested-by: Jenkins Reviewed-by: James Williams QA-Review: Jahnavi Yetukuri Product-Review: Jeremy Stanley --- app/models/attachment.rb | 1 - app/models/thumbnail.rb | 3 ++- gems/attachment_fu/lib/attachment_fu.rb | 2 ++ .../processors/mini_magick_processor.rb | 2 ++ spec/factories/attachment_factory.rb | 7 ++++++- spec/fixtures/100mpx.png | Bin 0 -> 12675 bytes spec/models/attachment_spec.rb | 18 ++++++++++++++++++ 7 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 spec/fixtures/100mpx.png diff --git a/app/models/attachment.rb b/app/models/attachment.rb index 0da7bf3fd33..ef6766729e6 100644 --- a/app/models/attachment.rb +++ b/app/models/attachment.rb @@ -223,7 +223,6 @@ class Attachment < ActiveRecord::Base # try an infer encoding if it would be useful to do so send_later(:infer_encoding) if self.encoding.nil? && self.content_type =~ /text/ && self.context_type != 'SisBatch' if respond_to?(:process_attachment_with_processing, true) && thumbnailable? && !attachment_options[:thumbnails].blank? && parent_id.nil? - temp_file = temp_path || create_temp_file self.class.attachment_options[:thumbnails].each { |suffix, size| send_later_if_production(:create_thumbnail_size, suffix) } end end diff --git a/app/models/thumbnail.rb b/app/models/thumbnail.rb index b870118f940..6473457d12f 100644 --- a/app/models/thumbnail.rb +++ b/app/models/thumbnail.rb @@ -30,7 +30,8 @@ class Thumbnail < ActiveRecord::Base :storage => (Attachment.local_storage? ? :file_system : :s3), :path_prefix => Attachment.file_store_config['path_prefix'], :s3_access => :private, - :keep_profile => true + :keep_profile => true, + :thumbnail_max_image_size_pixels => Setting.get('thumbnail_max_image_size_pixels', 100_000_000) ) before_save :set_namespace diff --git a/gems/attachment_fu/lib/attachment_fu.rb b/gems/attachment_fu/lib/attachment_fu.rb index 2a984eb2c3a..ae9a1ca9ea6 100644 --- a/gems/attachment_fu/lib/attachment_fu.rb +++ b/gems/attachment_fu/lib/attachment_fu.rb @@ -281,6 +281,8 @@ module AttachmentFu # :nodoc: res = self.create_or_update_thumbnail(tmp, target_size.to_s, actual_size) rescue AWS::S3::Errors::NoSuchKey => e logger.warn("error when trying to make thumbnail for attachment_id: #{self.id} (the image probably doesn't exist on s3) error details: #{e.inspect}") + rescue ThumbnailError => e + logger.warn("error creating thumbnail for attachment_id #{self.id}: #{e.inspect}") ensure tmp.unlink if tmp end diff --git a/gems/attachment_fu/lib/attachment_fu/processors/mini_magick_processor.rb b/gems/attachment_fu/lib/attachment_fu/processors/mini_magick_processor.rb index e728ef116dc..a6866b2c026 100644 --- a/gems/attachment_fu/lib/attachment_fu/processors/mini_magick_processor.rb +++ b/gems/attachment_fu/lib/attachment_fu/processors/mini_magick_processor.rb @@ -28,6 +28,8 @@ module AttachmentFu # :nodoc: def process_attachment_with_processing return unless process_attachment_without_processing with_image do |img| + max_image_size = attachment_options[:thumbnail_max_image_size_pixels] + raise ThumbnailError.new("source image too large") if max_image_size && img[:width] * img[:height] > max_image_size resize_image_or_thumbnail! img self.width = img[:width] if respond_to?(:width) self.height = img[:height] if respond_to?(:height) diff --git a/spec/factories/attachment_factory.rb b/spec/factories/attachment_factory.rb index a745fbd4336..e3312b5ea45 100644 --- a/spec/factories/attachment_factory.rb +++ b/spec/factories/attachment_factory.rb @@ -81,10 +81,15 @@ def stub_png_data(filename = 'test my file? hai!&.png', data = nil) end def jpeg_data_frd - fixture_path = '/test_image.jpg' + fixture_path = 'test_image.jpg' fixture_file_upload(fixture_path, 'image/jpeg', true) end +def one_hundred_megapixels_of_highly_compressed_png_data + fixture_path = '100mpx.png' + fixture_file_upload(fixture_path, 'image/png', true) +end + def crocodocable_attachment_model(opts={}) attachment_model({:content_type => 'application/pdf'}.merge(opts)) end diff --git a/spec/fixtures/100mpx.png b/spec/fixtures/100mpx.png new file mode 100644 index 0000000000000000000000000000000000000000..1653f5b6e1d258319f05746c1c39db5a9e63789a GIT binary patch literal 12675 zcmeIuyGjE=6b9gbLqv(BvJirxO$3XD*%i?Y7Orkc7D><$!Op}aiijw(5e*h=Vxvt` zT8LFzZ>?3d^$ooABH{yhpDiuy4g6EgnG0v;obO;Jod|~_Apl_~X=eaaE&C@lHRE$^ zZb*gTvX!!c%X;@mzC-N?3du|gs0;$$KJccFyeFV6fae?_kAU8d$t%qjoSHjwQ Xab;=e?}X01zFptKiKp$e$z1gZ jpeg_data_frd, :filename => 'ok.jpg' + expect(att.thumbnail).not_to be_nil + expect(att.thumbnail.width).not_to be_nil + end + + it 'does not create thumbnails for larger images' do + att = @course.attachments.create! :uploaded_data => one_hundred_megapixels_of_highly_compressed_png_data, :filename => '3vil.png' + expect(att.thumbnail).to be_nil + end + end + context "notifications" do before :once do course_model(:workflow_state => "available")