Add processable image validator

[kukicola]

Content type validation isn't enough to ensure that what users upload is an image and can be manipulated by MiniMagick or Vips. So my idea is to add additional validator which can check if a file can be processed by the library.

For simplicity, I used Metadata class which has necessary methods. It probably should be extracted to separate class but I didn't want to make a huge refactor along the way

It should solve the following issues:
#91
#62
#130 (partially, since it's only for images, for all other files we can extend ContentTypeValidator to read a file and use Marcel::MimeType.for)

[igorkasyanchuk]

@gr8bit could you please help me with a review? Difficult to focus on this right now because of current situation in Ukraine

[gr8bit]

Hi @kukicola, I dealt with this exact subject (I think) a few days ago and I'd like your opinion on this issue: rails/marcel#76
I overloaded the Marcel::MimeType.for method in an initializer for this purpose, changing the one line from the ticket above.

[kukicola]

@gr8bit I'm not sure if we should call it a Marcel issue. Those fallbacks are probably there for some reason. If it was up to me I would suggest making for_data public (to keep current for behavior) and using it in ActiveStorage instead.

Anyway, I would treat that as a separate thing. Minetype validation is still not enough here. Consider a file with magicbytes matching PNG file but the rest of the file is garbage (corrupted image, random bytes, malicious code, whatever). Validation should also fail in this case. That's why calling image.avg (vips) or image.valid? (minimagick) is important here - to check if file can be processed by library

[gr8bit]

@kukicola Thank you for the feedback, I'll incorporate that in the issue.
As for this PR, what I understood: you want to make sure an attachment really is a "processable image" for Vips/ImageMagick, so for types they cannot handle (might be webp or jpeg xl depending on your processor) they should fail validation. Right?
I'd like that feature a lot. I'm not sure about the name ("real") though... it might be a real image which the image processor of your choice just cannot process (unhandled file type). What do you think about naming it "processable" instead of "real"?

[kukicola]

@gr8bit Yes but it can be used also just to check if the image is safe to display somewhere (even without processing). I agree, that real is not a perfect name, I was working on it late in the evening and didn't have better ideas :). What about valid_image to avoid suggesting that user needs to process the image?

[gr8bit]

@gr8bit Yes but it can be used also just to check if the image is safe to display somewhere (even without processing). I agree, that real is not a perfect name, I was working on it late in the evening and didn't have better ideas :). What about valid_image to avoid suggesting that user needs to process the image?

I fully understand that. :D My point of view on it is: you cannot know if the user can actually display it - it might be something that ImageMagick can handle but the user's browser (or "application") cannot (some new formats or even aged ones like Ghostscript). So I was thinking: the only thing we can ensure using Vips/IM is: they can handle it, that's why think processable might be a good fit.
In terms of "valid": all images that we can process are definitely valid - but not all valid images we can definitely process. ;)

[kukicola]

ok I agree, I'll update PR to "processable" later today

[kukicola]

@gr8bit done, please take a look

[gr8bit]

Suggested change

	has_one_attached :another_image
	has_one_attached :another_image

[gr8bit]

I think we should add a negative test here as well, ensuring that a processable_image: false will accept the .tar file as png as suggested by file extension and type.

[gr8bit]

@gr8bit done, please take a look

You're fast, I tried my best to keep up. ;)

[kukicola]

I have a lot of time today :) fixes done

[gr8bit]

@igorkasyanchuk everything looks fine in my opinion, this PR could be merged.

[igorkasyanchuk]

very good, I have checked your discussion, and to me all looks good.