modxcms / revolution

MODX Revolution - Content Management Framework
https://modx.com/
GNU General Public License v2.0
1.36k stars 529 forks source link

Handling .ico files in media sources #16420

Open MaticSulc opened 1 year ago

MaticSulc commented 1 year ago

MODX 3.0.3-pl, PHP 8.1.17.

When adding a favicon.ico file in the website root, media source preview doesn't work for it and it keeps filling the error log. PHP warning: exif_read_data(favicon.ico): File not supported

I can simply make the mediasource ignore it, by adding it in the skipFiles setting, but since .ico isn't under imageExtensions, I don't expect it to generate a preview. Can't replicate this in MODX 2.x.x.

halftrainedharry commented 1 year ago

but since .ico isn't under imageExtensions, I don't expect it to generate a preview.

It generates a preview because the string 'image' is in the mime type.

https://github.com/modxcms/revolution/blob/69a7d6d14f84151ced04a552678657456c05b9cc/core/src/Revolution/Sources/modMediaSource.php#L2414-L2417


The same PHP warning occurs also for other image types (like e.g. PNG or WebP) when PHP 8 is used. Probably the code has to be changed here, to run exif_read_data only for image types that support it.

https://github.com/modxcms/revolution/blob/69a7d6d14f84151ced04a552678657456c05b9cc/core/src/Revolution/Sources/modMediaSource.php#L2260


There is also a similar topic in the MODX forum: https://community.modx.com/t/exif-read-data-file-not-supported/6572