search

tinify / wordpress-plugin

Speed up your WordPress website. Optimize your JPEG and PNG images automatically with TinyPNG.
GNU General Public License v2.0
88 stars 36 forks source link

PHP notice: Undefined index: file in ..\tiny-compress-images\src\class-tiny-image.php on line 48 #12

Closed jerturowetz closed 7 years ago

jerturowetz commented 7 years ago

In the plugin-created Compression column of my media library I'm getting the following notice for PDFs and some other custom file types:

Undefined index: file in ..\tiny-compress-images\src\class-tiny-image.php on line 48

I assumed that this issue occurs because parse_wp_metadata() isn't checking if this is of the allowed file types before moving forward (assuming of course the allowed file types will have 'file' set), it seems only to check that $this->wp_metadata be an array.

I thought of adding isset($this->wp_metadata['file']) to line 45 but I end up breaking has_been_compressed() among other things...

tinify-support commented 7 years ago

Hi there!

Thank you for contacting us!

That sounds very strange. Which version of the plugin are you using? The plugin should not display any information in the Compression column at all if you upload a non png/jpeg file.

If you are using the latest version of the plugin, could you perhaps share the PDF with us so we can give it a try?

Kind regards, Simon Wahlstrom

jerturowetz commented 7 years ago

Version 2.2.2

I don't think it's necessary to send you the file as I did a bit of digging since last we spoke and I think I found the issue. It occured when PDF auto-gen thumbnails were forcibly removed and the plugin tried to optimize them (due to my implementation of an S3 bucket for migrating static files).

I gather the PDF thumbnails being images trigger the the plugins is-it-an-image test. Regardless of this assumption, should attachment information be stored as metadata, but not have a 'file' key & info, parse_wp_metadata() in src\class-tiny-image.php will cause this error.

While probably not an issue for many, I think it would still be better practice to explicitly check for the file key rather than assuming it exists when the attachment's meta is an array. To this end I've gone ahead and added the check to parse_wp_metadata and detect_duplicates and will issue a pull request for your review.

tinify-support commented 7 years ago

Hello again,

Awesome that you found the problem!

We will absolutely have a look at your pull request and see if there's anything else we need to improve to prevent this in the future.

Kind regards,

Simon Wahlstrom

tinify/wordpress-plugin on April 14, 2017 at 2:34am wrote:

Version 2.2.2

I don't think it's necessary to send you the file as I did a bit of digging since last we spoke and I think I found the issue. It occured when PDF auto-gen thumbnails were forcibly removed and the plugin tried to optimize them (due to my implementation of an S3 bucket for migrating static files).

I gather the PDF thumbnails being images trigger the the plugins is-it-an-image test. Regardless of this assumption, should attachment information be stored as metadata, but not have a 'file' key & info, parse_wp_metadata() in src\class-tiny-image.php will cause this error.

While probably not an issue for many, I think it would still be better practice to explicitly check for the file key rather than assuming it exists when the attachment's meta is an array. To this end I've gone ahead and added the check to parse_wp_metadata and detect_duplicates and will issue a pull request for your review.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub ( https://github.com/tinify/wordpress-plugin/issues/12#issuecomment-294055830 ) , or mute the thread ( https://github.com/notifications/unsubscribe-auth/AVWuQChrh6qOsbJ1wsZOA9L-HJTjRGlSks5rvr8zgaJpZM4M6fvB ) .

TinyPNG Support on April 13, 2017 at 11:40am wrote:

Hi there!

Thank you for contacting us!

That sounds very strange. Which version of the plugin are you using? The plugin should not display any information in the Compression column at all if you upload a non png/jpeg file.

If you are using the latest version of the plugin, could you perhaps share the PDF with us so we can give it a try?

tinify/wordpress-plugin on April 11, 2017 at 8:46pm wrote:

In the plugin-created Compression column of my media library I'm getting the following notice for PDFs and some other custom file types:

Undefined index: file in ..\tiny-compress-images\src\class-tiny-image.php on line 48

I assumed that this issue occurs because parse_wp_metadata() isn't checking if this is of the allowed file types before moving forward (assuming of course the allowed file types will have 'file' set), it seems only to check that $this->wp_metadata be an array.

I thought of adding isset($this->wp_metadata['file']) to line 45 but I end up breaking has_been_compressed() among other things...

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub ( https://github.com/tinify/wordpress-plugin/issues/12 ), or mute the thread ( https://github.com/notifications/unsubscribe-auth/AVWuQOChvb-clIIb7B1nqEFgc_T9mLK_ks5ru8qAgaJpZM4M6fvB ) .

jerturowetz commented 7 years ago

Killer, let me know if there's any where I could improve it too (it didn't pass the Travis build and I'm wondering why as I didn't really do much :P )