salesagility / SuiteCRM

SuiteCRM - Open source CRM for the world
https://www.suitecrm.com
GNU Affero General Public License v3.0
4.39k stars 2.06k forks source link

check_php_version() in utils.php can never return 1 #6581

Closed connorshea closed 5 years ago

connorshea commented 5 years ago

In include/utils.php, this line exists:

https://github.com/salesagility/SuiteCRM/blob/2479923a1a71f45143ef8614c7899df11574fbb9/include/utils.php#L3205

I thought that line was pretty weird, so I investigated further and found that at some point in the 7.8.x hotfix cycle a merge caused the code to change from this:

// Everything else is fair game
return 1;

to this:

// Everything else is fair gamereturn 1;

See the diff here: https://github.com/salesagility/SuiteCRM/commit/720f4384a7742d8067d7ddf3242bf3c5b246c96a#diff-b97be8484e5a5757aa6d4372838d140eR3145

The overall function now looks like this, and never returns 1:

/**
 * Will check if a given PHP version string is accepted or not.
 * Do not pass in any pararameter to default to a check against the
 * current environment's PHP version.
 *
 * @param string Version to check against, defaults to the current environment's.
 *
 * @return integer1 if version is greater than the recommended PHP version,
 * 0 if version is between minimun and recomended PHP versions,
 * -1 otherwise (less than minimum or buggy version)
 */
function check_php_version($sys_php_version = '')
{
    if ($sys_php_version === '') {
        $sys_php_version = constant('PHP_VERSION');
    }
    // versions below MIN_PHP_VERSION are not accepted, so return early.
    if (version_compare($sys_php_version, constant('SUITECRM_PHP_MIN_VERSION'), '<') === true) {
        return - 1;
    }
    // If there are some bug ridden versions, we should include them here
    // and check immediately for one of this versions
    $bug_php_versions = array();
    foreach ($bug_php_versions as $v) {
        if (version_compare($sys_php_version, $v, '=') === true) {
            return -1;
        }
    }
    //If the checked version is between the minimum and recommended versions, return 0
    if (version_compare($sys_php_version, constant('SUITECRM_PHP_REC_VERSION'), '<') === true) {
        return 0;
    }
    // Everything else is fair gamereturn 1;
}

Suggested Fix

Add a newline so the return 1 isn't part of a comment.

samus-aran commented 5 years ago

Thanks for that catch. This is one of the reasons we have no stop merging 7.8.x-hotfix into hotfix.