From 6133dad86970951094bdb4a603bc80c4968a9667 Mon Sep 17 00:00:00 2001 From: Guilherme Mansur Date: Tue, 23 Apr 2019 16:11:10 -0400 Subject: Don't fail ImageAnalyzer on unsupported types Fix: #36065 The IamgeAnalyzer passes a image to ImageMagick without checking if the image is supported by ImageMagick. This patch checks that image is supported and if not logs an error and returns an empty hash instead of raising an error. This is the same error handling we do when we encounter a LoadError when mini_magick is not installed. --- activestorage/CHANGELOG.md | 5 +++++ .../lib/active_storage/analyzer/image_analyzer.rb | 15 +++++++++++---- activestorage/test/analyzer/image_analyzer_test.rb | 8 ++++++++ 3 files changed, 24 insertions(+), 4 deletions(-) (limited to 'activestorage') diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index d524ecf7d6..ff5dd884e3 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,3 +1,8 @@ +* Don't raise when analyzing an unsupported image type by ImageMagick + Fixes: #36065 + + *Guilherme Mansur* + * Permit generating variants of BMP images. *Younes Serraj* diff --git a/activestorage/lib/active_storage/analyzer/image_analyzer.rb b/activestorage/lib/active_storage/analyzer/image_analyzer.rb index 3b39de91be..c8bc8fe953 100644 --- a/activestorage/lib/active_storage/analyzer/image_analyzer.rb +++ b/activestorage/lib/active_storage/analyzer/image_analyzer.rb @@ -25,17 +25,24 @@ module ActiveStorage { width: image.width, height: image.height } end end - rescue LoadError - logger.info "Skipping image analysis because the mini_magick gem isn't installed" - {} end private def read_image download_blob_to_tempfile do |file| require "mini_magick" - yield MiniMagick::Image.new(file.path) + image = MiniMagick::Image.new(file.path) + + if image.valid? + yield image + else + logger.info "Skipping image analysis because ImageMagick doesn't support the file" + {} + end end + rescue LoadError + logger.info "Skipping image analysis because the mini_magick gem isn't installed" + {} end def rotated_image?(image) diff --git a/activestorage/test/analyzer/image_analyzer_test.rb b/activestorage/test/analyzer/image_analyzer_test.rb index 55bb5e7280..73438c15ab 100644 --- a/activestorage/test/analyzer/image_analyzer_test.rb +++ b/activestorage/test/analyzer/image_analyzer_test.rb @@ -29,4 +29,12 @@ class ActiveStorage::Analyzer::ImageAnalyzerTest < ActiveSupport::TestCase assert_equal 792, metadata[:width] assert_equal 584, metadata[:height] end + + test "analyzing an unsupported image type" do + blob = create_blob(data: "bad", filename: "bad_file.bad", content_type: "image/bad_type") + metadata = extract_metadata_from(blob) + + assert_nil metadata[:width] + assert_nil metadata[:heigh] + end end -- cgit v1.2.3