search

php-opencloud / openstack

PHP SDK for OpenStack clouds
Apache License 2.0
222 stars 152 forks source link

Fix the memory limit check #404

Closed BelleNottelling closed 4 months ago

BelleNottelling commented 4 months ago

Closes #403

When you query ini_get('memory_limit') you don't always get an integer response, it returns exactly what is written inside of the PHP .ini file file which typically includes a shorthand value. (K, M, or G, all case-insensitive).

My server has the default PHP limit of 128M which was incorrectly being evaluated as not enough by the check.

This PR corrects the behavior by first evaluating the set memory limit, including the shorthand byte values and then returns it as an integer which can then be properly compared against the message's body size.

k0ka commented 4 months ago

Well, I don't think it even make sense to check memory_limit there. It looks like some old temporary fix. I guess it's better to show body only for jsons and truncate it by like 5kb.

BelleNottelling commented 4 months ago

Well, I don't think it even make sense to check memory_limit there. It looks like some old temporary fix. I guess it's better to show body only for jsons and truncate it by like 5kb.

I don't think it makes sense either, but when it comes to someone else's codebase (and one that I am unfamiliar with) I tend to lean towards fixing a bug rather than modifying existing behavior. I honestly am unable to think of any valid situation for this check to exist at all truthfully

Edit: tried to find when it was added via git blame to see if there was an explanation behind it, but it predates this code being on GitHub so I haven idea