Merge pull request #9073 from kenjis/check-for-duplicate-registrar-discovery

fix: add check for duplicate Registrar Auto-Discovery runs

Author: kenjis

system/Config/BaseConfig.php

@@ -11,6 +11,7 @@
 
 namespace CodeIgniter\Config;
 
+use CodeIgniter\Exceptions\ConfigException;
 use CodeIgniter\Exceptions\RuntimeException;
 use Config\Encryption;
 use Config\Modules;
@@ -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.
      *
@@ -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) {
@@ -244,6 +269,7 @@ protected function registerProperties()
             }
 
             static::$didDiscovery = true;
+            static::$discovering  = false;
         }
 
         $shortName = (new ReflectionClass($this))->getShortName();

user_guide_src/source/changelogs/v4.6.0.rst

@@ -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

user_guide_src/source/installation/upgrade_460.rst

@@ -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
 =================
 

user_guide_src/source/installation/upgrade_460/001.php

@@ -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.