Skip to content

fix: add check for duplicate Registrar Auto-Discovery runs #9073

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion system/Config/BaseConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace CodeIgniter\Config;

use CodeIgniter\Exceptions\ConfigException;
use CodeIgniter\Exceptions\RuntimeException;
use Config\Encryption;
use Config\Modules;
Expand Down Expand Up @@ -45,12 +46,22 @@ class BaseConfig
public static bool $override = true;

/**
* Has module discovery happened yet?
* Has module discovery completed?
*
* @var bool
*/
protected static $didDiscovery = false;

/**
* Is module discovery running or not?
*/
protected static bool $discovering = false;

/**
* The processing Registrar file for error message.
*/
protected static string $registrarFile = '';

/**
* The modules configuration.
*
Expand Down Expand Up @@ -230,10 +241,24 @@ protected function registerProperties()
}

if (! static::$didDiscovery) {
// Discovery must be completed before the first instantiation of any Config class.
if (static::$discovering) {
throw new ConfigException(
'During Auto-Discovery of Registrars,'
. ' "' . static::class . '" executes Auto-Discovery again.'
. ' "' . clean_path(static::$registrarFile) . '" seems to have bad code.'
);
}

static::$discovering = true;

$locator = service('locator');
$registrarsFiles = $locator->search('Config/Registrar.php');

foreach ($registrarsFiles as $file) {
// Saves the file for error message.
static::$registrarFile = $file;

$className = $locator->findQualifiedNameFromPath($file);

if ($className === false) {
Expand All @@ -244,6 +269,7 @@ protected function registerProperties()
}

static::$didDiscovery = true;
static::$discovering = false;
}

$shortName = (new ReflectionClass($this))->getShortName();
Expand Down
7 changes: 7 additions & 0 deletions user_guide_src/source/changelogs/v4.6.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ The ``Filters`` class has been changed to allow multiple runs of the same filter
with different arguments in before or after. See
:ref:`Upgrading Guide <upgrade-460-filters-changes>` for details.

Registrars
----------

Added check to prevent Auto-Discovery of Registrars from running twice. If it is
executed twice, an exception will be thrown. See
:ref:`upgrade-460-registrars-with-dirty-hack`.

.. _v460-interface-changes:

Interface Changes
Expand Down
22 changes: 22 additions & 0 deletions user_guide_src/source/installation/upgrade_460.rst
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,28 @@ See :ref:`ChangeLog <v460-behavior-changes-exceptions>` for details.

If you have code that catches these exceptions, change the exception classes.

.. _upgrade-460-registrars-with-dirty-hack:

Registrars with Dirty Hack
==========================

To prevent Auto-Discovery of :ref:`registrars` from running twice, when a registrar
class is loaded or instantiated, if it instantiates a Config class (which extends
``CodeIgniter\Config\BaseConfig``), ``ConfigException`` will be raised.

This is because if Auto-Discovery of Registrars is performed twice, duplicate
values may be added to properties of Config classes.

All registrar classes (**Config/Registrar.php** in all namespaces) must be modified
so that they do not instantiate any Config class when loaded or instantiated.

If the packages/modules you are using provide such registrar classes, the registrar
classes in the packages/modules need to be fixed.

The following is an example of code that will no longer work:

.. literalinclude:: upgrade_460/001.php

Interface Changes
=================

Expand Down
33 changes: 33 additions & 0 deletions user_guide_src/source/installation/upgrade_460/001.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

namespace CodeIgniter\Shield\Config;

use Config\App;

class Registrar
{
public function __construct()
{
$config = new App(); // Bad. When this class is instantiated, Config\App will be instantiated.

// Does something.
}

public static function Pager(): array
{
return [
'templates' => [
'module_pager' => 'MyModule\Views\Pager',
],
];
}

public static function hack(): void
{
$config = config('Cache');

// Does something.
}
}

Registrar::hack(); // Bad. When this class is loaded, Config\Cache will be instantiated.
Loading