"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
:
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…
active_storage_validations/lib/active_storage_validations/metadata.rb
Lines 66 to 83 in c439869
…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