Fixed Minor logic error in $container['config']

[PaulB]

Affected version

2.1.10 Patch 2

In XF\App#initialize, definition for $container['config']:

$container['config'] = function (Container $c)
{
    $default = $c['config.default'];
    $file = $c['config.file'];
    $legacyFile = $c['config.legacyFile'];

    if (file_exists($file))
    {
        $config = [];
        require($file);

        $config = array_replace_recursive($default, $config);
        $config['exists'] = true;
        $config['legacyExists'] = null;
    }
    else
    {
        if (file_exists($legacyFile))
        {
            $config = [];
            require($legacyFile);

            $config = array_replace_recursive($default, $config);
            $config['legacyExists'] = true;
        }
        else
        {
            // LOGIC ERROR HERE
            $config['legacyExists'] = false;
            $config = $default;
        }
    }

    // ...

    return $config;
};

Notice the logic when neither $file nor $legacyFile exist: $config is overwritten $default after being set to $config = ['legacyExists' => false];. Those two lines should be swapped.

In theory, this could result in an exception being thrown in setup, though I haven't attempted to run the relevant code:

function setup(array $options = [])
{
    $config = $this->container('config');

    if (!$config['exists'])
    {
        if ($config['legacyExists'])  // EXCEPTION HERE
        {

 

[PaulB]

Assuming 2.2 targets PHP 7.2, I suggest the following fix:

$container['config'] = function (Container $c): array
{
    $default = $c['config.default'];
    $file = $c['config.file'];
    $legacyFile = $c['config.legacyFile'];

    $hasConfig = file_exists($file);
    $hasLegacy = !$hasConfig && file_exists($legacyFile);

    $config = [];

    if ($hasConfig)
    {
        require $file;
    }
    else if ($hasLegacy)
    {
        require $legacyFile;
    }

    $config = array_replace_recursive($default, $config);

    $config['exists'] = $hasConfig;
    $config['legacyExists'] = $hasConfig ? null : $hasLegacyConfig;

    // if there's a specific session cache specified, force this to be enabled
    if (!empty($config['cache']['context']['sessions']))
    {
        $config['cache']['sessions'] = true;
    }

    foreach ($config['container'] AS $key => $value)
    {
        $c[$key] = $value;
    }

    return $config;
};

Changes:

1. Reduce duplicate code
2. Add PHP 7 return type hint
3. Avoid nested if-else-if-else when an if-elseif would suffice
4. Avoid making require look like a function call--it has different precedence from a function call, and treating it like a function can lead to subtle bugs.

 

[XF Bug Bot]

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.0 Beta 3).

Change log:

Fix minor logic error when populating the default config.

There may be a delay before changes are rolled out to the XenForo Community.