Fix mem limit

[seansan]

Described here: https://magento.com/tech-resources/bug-tracking/issue/index/id/1540/

[sreichel]

Original Post:

Steps to reproduce:

in app/code/core/Mage/Catalog/Model/Product/Image.php, on line 186,

if (!isSet($memoryLimit[0])) {

incorrectly refers to $memoryLimit as an array with index [0]. The variable is not an array, so the if statement always evaluates to true and the memoryLimit is ALWAYS interpreted to be 128M regardless of the setting in php.ini

if (!isSet($memoryLimit[0])) {
    $memoryLimit = "128M";
}

Corrected code below:

if (!isSet($memoryLimit)) {
    $memoryLimit = "128M";
}

Expected Result:

Memory limit should be pulled from php.ini from ini

Actual Result:

always evaluates to true and uses 128M hardcoded setting from code.

---

@seansan This is not a bug ... it's absolutly corret! $memoryLimit[0] gives you the first char of $memoryLimit string. If ini_get('memory_limit') returns an empty string limit is set to 128M.

With this PR !isSet($memoryLimit) would be false because it is set - it's just an empty string - and return value would be string '' (length=0)

See: http://php.net/manual/en/function.isset.php (Example #1)

So please check code before PR :)

[LeeSaferite]

I don't think this is doing what you think it's doing.
$memoryLimit = trim(strtoupper(ini_get('memory_limit'))); will always set the variable. As far as I can tell, empty($memoryLimit) is the proper check in this case.

[seansan]

Maybe keep it the way it is?

[sreichel]

For me ... I see no need to change this.

[colinmollenhour]

The value of isset() on an empty string is true. This should be if (empty($memoryLimit)) {.

php > $a = '';
php > var_dump(isset($a));
php shell code:1:
bool(true)

[seansan]

@sreichel right you are So inderdaad seems to be correct (or at least checking for that first char)

…

On Tue, 13 Jun 2017 at 18:46, sreichel ***@***.***> wrote: Original Post: *Steps to reproduce:* in app/code/core/Mage/Catalog/Model/Product/Image.php, on line 186, if (!isSet($memoryLimit[0])) { incorrectly refers to $memoryLimit as an array with index [0]. The variable is not an array, so the if statement always evaluates to true and the memoryLimit is ALWAYS interpreted to be 128M regardless of the setting in php.ini if (!isSet($memoryLimit[0])) { $memoryLimit = "128M"; } Corrected code below: if (!isSet($memoryLimit)) { $memoryLimit = "128M"; } *Expected Result:* Memory limit should be pulled from php.ini from ini *Actual Result:* always evaluates to true and uses 128M hardcoded setting from code. ------------------------------ @seansan <https://github.com/seansan> This is not a bug ... it's absolutly corret! $memoryLimit[0] gives you the first char of $memoryLimit *string*. If ini_get('memory_limit') returns an empty string limit is set to 128M. With this PR !isSet($memoryLimit) would be false because it is set - it's just an empty string - and return value would be string '' (length=0) See: http://php.net/manual/en/function.isset.php (Example #1 <#1>) So please check code before PR :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#286 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAn0a9DXhYydaHKrly5mE09hNOdNyf_Sks5sDrzmgaJpZM4N4hy5> .