igorkasyanchuk/active_storage_validations

"foobar is not a valid image" for valid image (error only in production)

marckohlbrugge opened this issue · 9 comments

I have the following validation in my model:

  validates :input_image,
    attached: true,
    content_type: ["image/png", "image/jpeg"],
    dimension: {
      width: { min: 10, max: 5000 },
      height: { min: 10, max: 5000 }
    },
    size: { less_than: 25.megabytes }

When trying to attach an image that fits these criteria, it works fine on localhost (disk storage). But when trying the same image in production (using S3), I get the error Input message is not a valid image. Unfortunately it does not specify what's not valid about it.

Any suggestions on how to debug this?

I'm using rails (7.0.4), and active_storage_validations (0.9.8).

Hm, my semi-educated guess is: you're either
a) using an asynchronous job runner in production but not locally which delays image analyzation and therefore the adding of the width and height metadata the dimension validator needs or
b) missing a Vips/ImageMagick processor for the uploaded files in question.

I'd try to reproduce the error using an own "development" S3 bucket locally.

a) using an asynchronous job runner in production but not locally which delays image analyzation and therefore the adding of the width and height metadata the dimension validator needs

That was it!

I have removed the dimensions validation (and kept the rest), and now it works as expected. Thank you!

I guess I can use something like fastimage in a before_validation callback to set the height and width. Perhaps also worth adding as an option to this gem, as I believe analyzing an attachment asynchronously is the default in Rails?

There's a mechanism for this already somewhere in ActiveStorage, I remember seeing some logic like "was the picture analyzed yet? if not, do it right away." in the AS code a few months ago. I will dive back into that.

There's a mechanism for this already somewhere in ActiveStorage, I remember seeing some logic like "was the picture analyzed yet? if not, do it right away." in the AS code a few months ago. I will dive back into that.

I think this gem by already do this. The problem might be my own code disabling analyzers altogether.

(So please don't spend time looking into this just for me.)

I'm running into a similar issue in a different codebase.

The validations work fine in development. But on production I always get "[…] is not a valid image"

This codebase uses Amazon S3 in both development and production. I haven't disabled any analyzers or done any custom ActiveStorage configuration:

Rails.application.config.active_storage.analyzers
=> [ActiveStorage::Analyzer::ImageAnalyzer::Vips, ActiveStorage::Analyzer::ImageAnalyzer::ImageMagick, ActiveStorage::Analyzer::VideoAnalyzer, ActiveStorage::Analyzer::AudioAnalyzer]

I did notice when I checked one of my activestorage attachments, it had no width/height set:

User.first.twitter_avatar.metadata
=> {"identified"=>true, "analyzed"=>true}

But after running .analyze on it, it did:

User.first.twitter_avatar.metadata
=> {"identified"=>true, "analyzed"=>true, "width"=>400, "height"=>400}

So my current thesis is that when the gem tries to validate the image size, there's no sufficient metadata and it raises the error.

However, it's my understanding that the gem manually called .analyze:

# Analyze file first if not analyzed to get all required metadata.
file.analyze; file.reload unless file.analyzed?

Any idea what might be going on here?

Okay, I think I tracked down the issue!

My server uses vips-8.7.4 whereas my local development machine has vips-8.13.2

It turns out that Vips.get_suffixes will always return an empty array for versions below 8.8. See https://www.rubydoc.info/gems/ruby-vips/Vips.get_suffixes

Which means that the following code…

image = if image_processor == :vips && defined?(Vips) && Vips::get_suffixes.include?(File.extname(tempfile.path).downcase)
Vips::Image.new_from_file(tempfile.path)
elsif defined?(MiniMagick)
MiniMagick::Image.new(tempfile.path)
end
else
image = if image_processor == :vips && defined?(Vips) && Vips::get_suffixes.include?(File.extname(read_file_path).downcase)
Vips::Image.new_from_file(read_file_path)
elsif defined?(MiniMagick)
MiniMagick::Image.new(read_file_path)
end
end
if image && valid_image?(image)
yield image
else
logger.info "Skipping image analysis because ImageMagick or Vips doesn't support the file"
{}

…will always return an empty hash for the metadata.

Which means the aspect ratio (and dimensions, etc) cannot be checked and thus not validated.

I think that A) the README should stipulate that libvips 8.8 or higher is required, or B) the code would need to be rewritten not to rely on 8.8+ functionality.

@marckohlbrugge I've made a PR based on your recommendation #185 . It works for me - it would be helpful if you could try it too

@marckohlbrugge I've made a PR based on your recommendation #185 . It works for me - it would be helpful if you could try it too

It works!

Would love to see this get merged into the main repo

Hi all,
I am closing this issue since #185 has been merged and released in June.
Let's me know if you need anything else,