[Vips] Handle 0 byte size files

lib/active_storage_validations/metadata.rb

@@ -4,29 +4,18 @@ class InvalidImageError < StandardError; end
 
     attr_reader :file
 
+    DEFAULT_IMAGE_PROCESSOR = :mini_magick.freeze
+
     def initialize(file)
       require_image_processor
       @file = file
     end
 
-    def image_processor
-      Rails.application.config.active_storage.variant_processor
-    end
-
-    def exception_class
-      if image_processor == :vips && defined?(Vips)
-        Vips::Error
-      elsif defined?(MiniMagick)
-        MiniMagick::Error
-      end
-    end
-
-    def require_image_processor
-      if image_processor == :vips
-        require 'vips' unless defined?(Vips)
-      else
-        require 'mini_magick' unless defined?(MiniMagick)
-      end
+    def valid?
+      read_image
+      true
+    rescue InvalidImageError
+      false
     end
 
     def metadata
@@ -42,14 +31,31 @@ def metadata
       {}
     end
 
-    def valid?
-      read_image
-      true
-    rescue InvalidImageError
-      false
+    private
+
+    def image_processor
+      # Rails returns nil for default image processor, because it is set in an after initiliaze callback
+      # https://github.com/rails/rails/blob/89d8569abe2564c8187debf32dd3b4e33d6ad983/activestorage/lib/active_storage/engine.rb
+      Rails.application.config.active_storage.variant_processor || DEFAULT_IMAGE_PROCESSOR
     end
 
-    private
+    def require_image_processor
+      case image_processor
+      when :vips then require 'vips' unless defined?(Vips)
+      when :mini_magick then require 'mini_magick' unless defined?(MiniMagick)
+      end
+    end
+
+    def exception_class
+      case image_processor
+      when :vips then Vips::Error
+      when :mini_magick then MiniMagick::Error
+      end
+    end
+
+    def vips_image_processor?
+      image_processor == :vips
+    end
 
     def read_image
       is_string = file.is_a?(String)
@@ -95,8 +101,16 @@ def read_image
     end
 
     def new_image_from_path(path)
-      if image_processor == :vips && defined?(Vips) && (Vips::get_suffixes.include?(File.extname(path).downcase) || !Vips::respond_to?(:vips_foreign_get_suffixes))
-        Vips::Image.new_from_file(path)
+      if vips_image_processor? && (Vips::get_suffixes.include?(File.extname(path).downcase) || !Vips::respond_to?(:vips_foreign_get_suffixes))
+        begin
+          Vips::Image.new_from_file(path)
+        rescue exception_class
+          # We handle cases where an error is raised when reading the file
+          # because Vips can throw errors rather than returning false
+          # We stumble upon this issue while reading 0 byte size file
+          # https://github.com/janko/image_processing/issues/97
+          false
+        end
       elsif defined?(MiniMagick)
         MiniMagick::Image.new(path)
       end
@@ -105,13 +119,13 @@ def new_image_from_path(path)
     def valid_image?(image)
       return false unless image
 
-      image_processor == :vips && image.is_a?(Vips::Image) ? image.avg : image.valid?
+      vips_image_processor? && image.is_a?(Vips::Image) ? image.avg : image.valid?
     rescue exception_class
       false
     end
 
     def rotated_image?(image)
-      if image_processor == :vips && image.is_a?(Vips::Image)
+      if vips_image_processor? && image.is_a?(Vips::Image)
         image.get('exif-ifd0-Orientation').include?('Right-top') ||
           image.get('exif-ifd0-Orientation').include?('Left-bottom')
       else

lib/active_storage_validations/processable_image_validator.rb

@@ -10,6 +10,10 @@ class ProcessableImageValidator < ActiveModel::EachValidator # :nodoc
     include Errorable
     include Symbolizable
 
+    ERROR_TYPES = %i[
+      image_not_processable
+    ].freeze
+
     if Rails.gem_version >= Gem::Version.new('6.0.0')
       def validate_each(record, attribute, _value)
         return true unless record.send(attribute).attached?
@@ -22,7 +26,7 @@ def validate_each(record, attribute, _value)
         files.each do |file|
           if !Metadata.new(file).valid?
             errors_options = initialize_error_options(options, file)
-            add_error(record, attribute, :image_not_processable, **errors_options) unless Metadata.new(file).valid?
+            add_error(record, attribute, ERROR_TYPES.first , **errors_options) unless Metadata.new(file).valid?
           end
         end
       end
@@ -36,7 +40,7 @@ def validate_each(record, attribute, _value)
         files.each do |file|
           if !Metadata.new(file).valid?
             errors_options = initialize_error_options(options, file)
-            add_error(record, attribute, :image_not_processable, **errors_options) unless Metadata.new(file).valid?
+            add_error(record, attribute, ERROR_TYPES.first , **errors_options) unless Metadata.new(file).valid?
           end
         end
       end