Fixed Small Bug in getBytesFromPhpConfigValue


New member
Affected version
So I mistyped my Units in my php.ini. I wrote 50MB, not 50M, but had to look through the source code until I realized what my error was. While looking I found this code that tries to fix this kind of mistake. But it can't handle this mistake, because $units will always be one character, in case of 50M it will be "M" and in case of 50MB it will be "B". In my personal opinion it would be nice if MB units generated a warning message.

    protected static function getBytesFromPhpConfigValue($configParam)
            $units = strtoupper(substr($configValue, -1));
            $value = intval($configValue);

            // note that KB, MB and GB are not actually valid in PHP config, but are frequently encountered

            switch ($units)
                case 'K':
                case 'KB':
                    return $value * 1024;
                case 'M':
                case 'MB':
                    return $value * 1048576;
                case 'G':
                case 'GB':
                    return $value * 1073741824;
                    return $value;
Think you need something like:

$units = strtoupper(substr($configValue, -1));
if (strtoupper($units) == 'B')
    $units = strtoupper(substr($configValue, -2));

Otherwise the KB/MB/GB branches of the switch never match. This won’t prevent something weird like 1000000B being misread but logically it ends up working out the same as it falls through to the default case which is bytes anyway.
imho, if at all, $units == 'B' should just throw an exception because you cannot assume "it has to be 'MB' if we find a 'B'" - it could also be KB or GB.
Also, I'm not sure how PHP handles an invalid configuration - in any way screaming loud could make such an error more visible. Masking it by assuming stuff would not help.
That wasn’t what either the OP’s suggestion or my follow-up did - the OP suggestion tries to match KB or K (and the other combinations) but it’s only ever fed the last letter. Mine took that, and also added “if the last letter was a B, get the second to last letter as well) and feed that into the calculation” so it would handle both.
Thank you for reporting this issue, it has now been resolved. We are aiming to include any changes that have been made in a future XF release (2.2.12).

Change log:
More accurate way of parsing byte values from PHP config values.
There may be a delay before changes are rolled out to the XenForo Community.
Top Bottom