PHP Warning: Invalid argument supplied for foreach() in Image.php on line 155

[elkangaroo]

Requesting an image, which has not been cached yet, leads to a PHP warning:

request: "GET image.jpg?resize=h[100] HTTP/1.0"

PHP message: PHP Warning:  Invalid argument supplied for foreach() in ../vendor/meenie/Munee/src/Munee/Asset/Type/Image.php on line 155
PHP message: PHP Stack trace:
PHP message: PHP   1. {main}() ../www.static/assetic.php:0
PHP message: PHP   2. Munee\Dispatcher::run() ../www.static/assetic.php:16
PHP message: PHP   3. Munee\Asset\Type->init() ../vendor/meenie/Munee/src/Munee/Dispatcher.php:69
PHP message: PHP   4. Munee\Asset\Type\Image->setupFile() ../vendor/meenie/Munee/src/Munee/Asset/Type.php:131
PHP message: PHP   5. Munee\Asset\Type\Image->checkNumberOfAllowedFilters() ../vendor/meenie/Munee/src/Munee/Asset/Type/Image.php:95

This is because glob() may return false instead of an empty array which is then passed to foreach (leading to a warning):

On some systems it is impossible to distinguish between empty match and an error.
... from the PHP documentation (http://php.net/glob#refsect1-function.glob-returnvalues)

---

Here's my pull request which fixes this warning:

• Fixed "PHP Warning: Invalid argument supplied for foreach()" in Image::checkNumberOfAllowedFilters() when glob() returns false
• Moved Image::checkNumberOfAllowedFilters() and Image::checkReferrer() calls into Image::setupFile() function
• Removed duplicate code in Image::checkCache()

[meenie]

Thanks for this, I'll have a look now.

[elkangaroo]

This also fixes 404s on uncached images, if you set the option "numberOfAllowedFilters" to 1 (in the case where glob() returns false, because count(false) == 1).

[meenie]

Hi Alexander,

I haven't had the time to completely look at your pull request do to work commitments. I hope to get it all merged in this weekend :).

Just wondering though, the return from glob(), can you not just check to make sure it's an array with is_array() before trying to use it? And if it's not, then we know that it should be an empty array instead of false and change it to an empty array()? Then there isn't the need to make all the other changes to accommodate?

[elkangaroo]

can you not just check to make sure it's an array

That's exactly what I am doing :) (lines 157-159)

•    if (! is_array($cachedImages)) {
  

•        $cachedImages = array();
  

•    }
  

The other parts is more or less just refactoring (removing duplicate code, move filter checks out of "checkCache()" method, don't call "checkNumberOfAllowedFilters()" if there are filter requested).

I'll annotate them.

[meenie]

Well I feel silly now haha. I only had a very brief look ><. Okay well, I'll merge this in on the weekend.

Thanks again!

[elkangaroo]

Well I feel silly now haha. I only had a very brief look ><.

Haha don't feel silly! Thanks for having a look.

I hope the annotations help you see my intentions for the changes.

[meenie]

I've just went through it and these are great changes! Can't believe I didn't use parent::checkCache() for the placeholder before ><. Guess it was a rushed implementation when I put it in.

I'm just going to merge these now and create a new release.

Thanks a ton! Your pull requests are welcome any time 👍.

[meenie]

1.5.15 is now released :) - http://mun.ee/Changelog

Thanks again!