active_storage_validations icon indicating copy to clipboard operation
active_storage_validations copied to clipboard

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

Open marckohlbrugge opened this issue 2 years ago • 1 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).

marckohlbrugge avatar Oct 11 '22 07:10 marckohlbrugge

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.

gr8bit avatar Oct 13 '22 01:10 gr8bit

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?

marckohlbrugge avatar Oct 14 '22 11:10 marckohlbrugge

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.

gr8bit avatar Oct 24 '22 21:10 gr8bit

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.)

marckohlbrugge avatar Oct 26 '22 12:10 marckohlbrugge

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:

https://github.com/igorkasyanchuk/active_storage_validations/blob/c439869a6c43389c70266e9cec5f06d17b9fed6c/lib/active_storage_validations/aspect_ratio_validator.rb#L40-L41

Any idea what might be going on here?

marckohlbrugge avatar Nov 18 '22 09:11 marckohlbrugge

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…

https://github.com/igorkasyanchuk/active_storage_validations/blob/c439869a6c43389c70266e9cec5f06d17b9fed6c/lib/active_storage_validations/metadata.rb#L66-L83

…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 avatar Nov 18 '22 09:11 marckohlbrugge

@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

iainbeeston avatar Feb 15 '23 15:02 iainbeeston

@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

marckohlbrugge avatar Feb 26 '23 09:02 marckohlbrugge

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,

Mth0158 avatar Nov 21 '23 10:11 Mth0158