diff --git a/app/Config/Feature.php b/app/Config/Feature.php index 8ed8e01a0158..39a37676183c 100644 --- a/app/Config/Feature.php +++ b/app/Config/Feature.php @@ -13,4 +13,9 @@ class Feature extends BaseConfig * Use improved new auto routing instead of the default legacy version. */ public bool $autoRoutesImproved = false; + + /** + * Use filter execution order in 4.4 or before. + */ + public bool $oldFilterOrder = false; } diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php index da24156b654f..22f86b5d4c7e 100644 --- a/system/CodeIgniter.php +++ b/system/CodeIgniter.php @@ -31,6 +31,7 @@ use CodeIgniter\Router\Router; use Config\App; use Config\Cache; +use Config\Feature; use Config\Kint as KintConfig; use Config\Services; use Exception; @@ -446,7 +447,7 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache return $response; } - $routeFilter = $this->tryToRouteIt($routes); + $routeFilters = $this->tryToRouteIt($routes); $uri = $this->determinePath(); @@ -456,9 +457,14 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache // If any filters were specified within the routes file, // we need to ensure it's active for the current request - if ($routeFilter !== null) { - $filters->enableFilters($routeFilter, 'before'); - $filters->enableFilters($routeFilter, 'after'); + if ($routeFilters !== null) { + $filters->enableFilters($routeFilters, 'before'); + + if (! config(Feature::class)->oldFilterOrder) { + $routeFilters = array_reverse($routeFilters); + } + + $filters->enableFilters($routeFilters, 'after'); } // Run "before" filters diff --git a/system/Commands/Utilities/Routes/FilterFinder.php b/system/Commands/Utilities/Routes/FilterFinder.php index 40a834df52f1..cc1f6d2b05f4 100644 --- a/system/Commands/Utilities/Routes/FilterFinder.php +++ b/system/Commands/Utilities/Routes/FilterFinder.php @@ -15,6 +15,7 @@ use CodeIgniter\Filters\Filters; use CodeIgniter\HTTP\Exceptions\RedirectException; use CodeIgniter\Router\Router; +use Config\Feature; use Config\Services; /** @@ -52,7 +53,13 @@ public function find(string $uri): array // Add route filters try { $routeFilters = $this->getRouteFilters($uri); + $this->filters->enableFilters($routeFilters, 'before'); + + if (! config(Feature::class)->oldFilterOrder) { + $routeFilters = array_reverse($routeFilters); + } + $this->filters->enableFilters($routeFilters, 'after'); $this->filters->initialize($uri); diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 33101aa512db..ad74b016acaf 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -15,6 +15,7 @@ use CodeIgniter\Filters\Exceptions\FilterException; use CodeIgniter\HTTP\RequestInterface; use CodeIgniter\HTTP\ResponseInterface; +use Config\Feature; use Config\Filters as FiltersConfig; use Config\Modules; use Config\Services; @@ -245,9 +246,15 @@ public function initialize(?string $uri = null) return $this; } - $this->processGlobals($uri); - $this->processMethods(); - $this->processFilters($uri); + if (config(Feature::class)->oldFilterOrder) { + $this->processGlobals($uri); + $this->processMethods(); + $this->processFilters($uri); + } else { + $this->processFilters($uri); + $this->processMethods(); + $this->processGlobals($uri); + } // Set the toolbar filter to the last position to be executed if (in_array('toolbar', $this->filters['after'], true) @@ -436,6 +443,8 @@ protected function processGlobals(?string $uri = null) // Add any global filters, unless they are excluded for this URI $sets = ['before', 'after']; + $filters = []; + foreach ($sets as $set) { if (isset($this->config->globals[$set])) { // look at each alias in the group @@ -455,11 +464,23 @@ protected function processGlobals(?string $uri = null) } if ($keep) { - $this->filters[$set][] = $alias; + $filters[$set][] = $alias; } } } } + + if (isset($filters['before'])) { + if (config(Feature::class)->oldFilterOrder) { + $this->filters['before'] = array_merge($this->filters['before'], $filters['before']); + } else { + $this->filters['before'] = array_merge($filters['before'], $this->filters['before']); + } + } + + if (isset($filters['after'])) { + $this->filters['after'] = array_merge($this->filters['after'], $filters['after']); + } } /** @@ -477,7 +498,11 @@ protected function processMethods() $method = strtolower($this->request->getMethod()) ?? 'cli'; if (array_key_exists($method, $this->config->methods)) { - $this->filters['before'] = array_merge($this->filters['before'], $this->config->methods[$method]); + if (config(Feature::class)->oldFilterOrder) { + $this->filters['before'] = array_merge($this->filters['before'], $this->config->methods[$method]); + } else { + $this->filters['before'] = array_merge($this->config->methods[$method], $this->filters['before']); + } } } @@ -497,6 +522,8 @@ protected function processFilters(?string $uri = null) $uri = strtolower(trim($uri, '/ ')); // Add any filters that apply to this URI + $filters = []; + foreach ($this->config->filters as $alias => $settings) { // Look for inclusion rules if (isset($settings['before'])) { @@ -506,7 +533,7 @@ protected function processFilters(?string $uri = null) // Get arguments and clean name [$name, $arguments] = $this->getCleanName($alias); - $this->filters['before'][] = $name; + $filters['before'][] = $name; $this->registerArguments($name, $arguments); } @@ -519,7 +546,7 @@ protected function processFilters(?string $uri = null) // Get arguments and clean name [$name, $arguments] = $this->getCleanName($alias); - $this->filters['after'][] = $name; + $filters['after'][] = $name; // The arguments may have already been registered in the before filter. // So disable check. @@ -527,6 +554,22 @@ protected function processFilters(?string $uri = null) } } } + + if (isset($filters['before'])) { + if (config(Feature::class)->oldFilterOrder) { + $this->filters['before'] = array_merge($this->filters['before'], $filters['before']); + } else { + $this->filters['before'] = array_merge($filters['before'], $this->filters['before']); + } + } + + if (isset($filters['after'])) { + if (! config(Feature::class)->oldFilterOrder) { + $filters['after'] = array_reverse($filters['after']); + } + + $this->filters['after'] = array_merge($this->filters['after'], $filters['after']); + } } /** @@ -563,6 +606,8 @@ private function registerArguments(string $name, array $arguments, bool $check = */ protected function processAliasesToClass(string $position) { + $filtersClass = []; + foreach ($this->filters[$position] as $alias => $rules) { if (is_numeric($alias) && is_string($rules)) { $alias = $rules; @@ -573,14 +618,20 @@ protected function processAliasesToClass(string $position) } if (is_array($this->config->aliases[$alias])) { - $this->filtersClass[$position] = array_merge($this->filtersClass[$position], $this->config->aliases[$alias]); + $filtersClass = array_merge($filtersClass, $this->config->aliases[$alias]); } else { - $this->filtersClass[$position][] = $this->config->aliases[$alias]; + $filtersClass[] = $this->config->aliases[$alias]; } } // when using enableFilter() we already write the class name in $filtersClass as well as the // alias in $filters. This leads to duplicates when using route filters. + if ($position === 'before') { + $this->filtersClass[$position] = array_merge($filtersClass, $this->filtersClass[$position]); + } else { + $this->filtersClass[$position] = array_merge($this->filtersClass[$position], $filtersClass); + } + // Since some filters like rate limiters rely on being executed once a request we filter em here. $this->filtersClass[$position] = array_values(array_unique($this->filtersClass[$position])); } diff --git a/tests/system/Commands/Utilities/Routes/FilterFinderTest.php b/tests/system/Commands/Utilities/Routes/FilterFinderTest.php index 652102976a19..c59c9b9aee8e 100644 --- a/tests/system/Commands/Utilities/Routes/FilterFinderTest.php +++ b/tests/system/Commands/Utilities/Routes/FilterFinderTest.php @@ -23,6 +23,7 @@ use CodeIgniter\Router\Router; use CodeIgniter\Test\CIUnitTestCase; use CodeIgniter\Test\ConfigFromArrayTrait; +use Config\Feature; use Config\Filters as FiltersConfig; use Config\Modules; use Config\Routing; @@ -145,7 +146,7 @@ public function testFindGlobalsAndRouteFilters(): void $filters = $finder->find('admin'); $expected = [ - 'before' => ['honeypot', 'csrf'], + 'before' => ['csrf', 'honeypot'], 'after' => ['honeypot', 'toolbar'], ]; $this->assertSame($expected, $filters); @@ -163,7 +164,7 @@ public function testFindGlobalsAndRouteClassnameFilters(): void $filters = $finder->find('admin'); $expected = [ - 'before' => [InvalidChars::class, 'csrf'], + 'before' => ['csrf', InvalidChars::class], 'after' => [InvalidChars::class, 'toolbar'], ]; $this->assertSame($expected, $filters); @@ -181,8 +182,135 @@ public function testFindGlobalsAndRouteMultipleFilters(): void $filters = $finder->find('admin'); $expected = [ - 'before' => ['honeypot', InvalidChars::class, 'csrf'], - 'after' => ['honeypot', InvalidChars::class, 'toolbar'], + 'before' => ['csrf', 'honeypot', InvalidChars::class], + 'after' => [InvalidChars::class, 'honeypot', 'toolbar'], + ]; + $this->assertSame($expected, $filters); + } + + public function testFilterOrder() + { + $collection = $this->createRouteCollection([]); + $collection->get('/', ' Home::index', ['filter' => ['route1', 'route2']]); + $router = $this->createRouter($collection); + $filters = $this->createFilters([ + 'aliases' => [ + 'global1' => 'Dummy', + 'global2' => 'Dummy', + 'method1' => 'Dummy', + 'method2' => 'Dummy', + 'filter1' => 'Dummy', + 'filter2' => 'Dummy', + 'route1' => 'Dummy', + 'route2' => 'Dummy', + ], + 'globals' => [ + 'before' => [ + 'global1', + 'global2', + ], + 'after' => [ + 'global2', + 'global1', + ], + ], + 'methods' => [ + 'get' => ['method1', 'method2'], + ], + 'filters' => [ + 'filter1' => ['before' => '*', 'after' => '*'], + 'filter2' => ['before' => '*', 'after' => '*'], + ], + ]); + + $finder = new FilterFinder($router, $filters); + + $filters = $finder->find('/'); + + $expected = [ + 'before' => [ + 'global1', + 'global2', + 'method1', + 'method2', + 'filter1', + 'filter2', + 'route1', + 'route2', + ], + 'after' => [ + 'route2', + 'route1', + 'filter2', + 'filter1', + 'global2', + 'global1', + ], + ]; + $this->assertSame($expected, $filters); + } + + public function testFilterOrderWithOldFilterOrder() + { + $feature = config(Feature::class); + $feature->oldFilterOrder = true; + + $collection = $this->createRouteCollection([]); + $collection->get('/', ' Home::index', ['filter' => ['route1', 'route2']]); + $router = $this->createRouter($collection); + $filters = $this->createFilters([ + 'aliases' => [ + 'global1' => 'Dummy', + 'global2' => 'Dummy', + 'method1' => 'Dummy', + 'method2' => 'Dummy', + 'filter1' => 'Dummy', + 'filter2' => 'Dummy', + 'route1' => 'Dummy', + 'route2' => 'Dummy', + ], + 'globals' => [ + 'before' => [ + 'global1', + 'global2', + ], + 'after' => [ + 'global1', + 'global2', + ], + ], + 'methods' => [ + 'get' => ['method1', 'method2'], + ], + 'filters' => [ + 'filter1' => ['before' => '*', 'after' => '*'], + 'filter2' => ['before' => '*', 'after' => '*'], + ], + ]); + + $finder = new FilterFinder($router, $filters); + + $filters = $finder->find('/'); + + $expected = [ + 'before' => [ + 'route1', + 'route2', + 'global1', + 'global2', + 'method1', + 'method2', + 'filter1', + 'filter2', + ], + 'after' => [ + 'route1', + 'route2', + 'global1', + 'global2', + 'filter1', + 'filter2', + ], ]; $this->assertSame($expected, $filters); } diff --git a/tests/system/Filters/FiltersTest.php b/tests/system/Filters/FiltersTest.php index efe47300fd49..7faf127f4647 100644 --- a/tests/system/Filters/FiltersTest.php +++ b/tests/system/Filters/FiltersTest.php @@ -406,8 +406,8 @@ public function testProcessMethodProcessesCombinedAfterForToolbar(): void $expected = [ 'before' => ['bar'], 'after' => [ - 'bazg', 'foof', + 'bazg', 'toolbar', ], ]; @@ -1049,8 +1049,8 @@ public function testMatchesURICaseInsensitively(): void 'frak', ], 'after' => [ - 'baz', 'frak', + 'baz', ], ]; $this->assertSame($expected, $filters->initialize($uri)->getFilters()); diff --git a/user_guide_src/source/changelogs/v4.5.0.rst b/user_guide_src/source/changelogs/v4.5.0.rst index 49b2fd6f4ac5..3f509d73431f 100644 --- a/user_guide_src/source/changelogs/v4.5.0.rst +++ b/user_guide_src/source/changelogs/v4.5.0.rst @@ -21,6 +21,15 @@ BREAKING Behavior Changes ================ +Filter Execution Order +---------------------- + +The order in which Controller Filters are executed has changed. See +:ref:`Upgrading Guide ` for details. + +Others +------ + - **Logger:** The :php:func:`log_message()` function and the logger methods in ``CodeIgniter\Log\Logger`` now do not return ``bool`` values. The return types have been fixed to ``void`` to follow the PSR-3 interface. diff --git a/user_guide_src/source/incoming/filters.rst b/user_guide_src/source/incoming/filters.rst index 3965dce51b0e..5292841d895f 100644 --- a/user_guide_src/source/incoming/filters.rst +++ b/user_guide_src/source/incoming/filters.rst @@ -85,11 +85,11 @@ There are two ways to configure filters when they get run. One is done in If you want to specify filter to a specific route, use **app/Config/Routes.php** and see :ref:`URI Routing `. -The filters that are specified to a route (in **app/Config/Routes.php**) are -executed before the filters specified in **app/Config/Filters.php**. - .. Note:: The safest way to apply filters is to :ref:`disable auto-routing `, and :ref:`set filters to routes `. +app/Config/Filters.php +====================== + The **app/Config/Filters.php** file contains four properties that allow you to configure exactly when the filters run. @@ -99,7 +99,7 @@ configure exactly when the filters run. it can be accessible with ``blog``, ``blog/index``, and ``blog/index/1``, etc. $aliases -======== +-------- The ``$aliases`` array is used to associate a simple name with one or more fully-qualified class names that are the filters to run: @@ -117,7 +117,7 @@ You can combine multiple filters into one alias, making complex sets of filters You should define as many aliases as you need. $globals -======== +-------- The second section allows you to define any filters that should be applied to every request made by the framework. You should take care with how many you use here, since it could have performance implications to have too many @@ -126,7 +126,7 @@ run on every request. Filters can be specified by adding their alias to either t .. literalinclude:: filters/005.php Except for a Few URIs ---------------------- +^^^^^^^^^^^^^^^^^^^^^ There are times where you want to apply a filter to almost every request, but have a few that should be left alone. One common example is if you need to exclude a few URI's from the CSRF protection filter to allow requests from @@ -143,7 +143,7 @@ URI paths, you can use an array of URI path patterns: .. literalinclude:: filters/007.php $methods -======== +-------- .. Warning:: If you use ``$methods`` filters, you should :ref:`disable Auto Routing (Legacy) ` because :ref:`auto-routing-legacy` permits any HTTP method to access a controller. @@ -161,7 +161,7 @@ In addition to the standard HTTP methods, this also supports one special case: ` all requests that were run from the command line. $filters -======== +-------- This property is an array of filter aliases. For each alias, you can specify ``before`` and ``after`` arrays that contain a list of URI path (relative to BaseURL) patterns that filter should apply to: @@ -171,7 +171,7 @@ a list of URI path (relative to BaseURL) patterns that filter should apply to: .. _filters-filters-filter-arguments: Filter Arguments ----------------- +^^^^^^^^^^^^^^^^ .. versionadded:: 4.4.0 @@ -184,6 +184,26 @@ will be passed in ``$arguments`` to the ``group`` filter's ``before()`` methods. When the URI matches ``admin/users/*'``, the array ``['users.manage']`` will be passed in ``$arguments`` to the ``permission`` filter's ``before()`` methods. +.. _filter-execution-order: + +Filter Execution Order +====================== + +.. important:: Starting with v4.5.0, the order in which filters are executed has + changed. If you wish to maintain the same execution order as in previous versions, + you must set ``true`` to ``Config\Feature::$oldFilterOrder``. + +Filters are executed in the following order: + +- **Before Filters**: globals → methods → filters → route +- **After Filters**: route → filters → globals + +.. note:: Prior to v4.5.0, the filters that are specified to a route + (in **app/Config/Routes.php**) are executed before the filters specified in + **app/Config/Filters.php**. And the After Filters in Route filters and Filters + filters execution order were not reversed. + See :ref:`Upgrading Guide ` for details. + ****************** Confirming Filters ****************** diff --git a/user_guide_src/source/installation/upgrade_450.rst b/user_guide_src/source/installation/upgrade_450.rst index 74fcf24a9e0e..7a4fbc0b29da 100644 --- a/user_guide_src/source/installation/upgrade_450.rst +++ b/user_guide_src/source/installation/upgrade_450.rst @@ -25,6 +25,53 @@ Some method signature changes have been made. Classes that extend them should update their APIs to reflect the changes. See :ref:`ChangeLog ` for details. +.. _upgrade-450-filter-execution-order: + +Filter Execution Order +====================== + +The order in which Controller Filters are executed has changed. +If you wish to maintain the same execution order as in previous versions, set +``true`` to ``Config\Feature::$oldFilterOrder``. See also :ref:`filter-execution-order`. + +1. The order of execution of filter groups has been changed. + + Before Filters:: + + Previous: route → globals → methods → filters + Now: globals → methods → filters → route + + After Filters:: + + Previous: route → globals → filters + Now: route → filters → globals + +2. The After Filters in *Route* filters and *Filters* filters execution order is now +reversed. + + When you have the following configuration: + + .. code-block:: php + + // In app/Config/Routes.php + $routes->get('/', 'Home::index', ['filter' => ['route1', 'route2']]); + + // In app/Config/Filters.php + public array $filters = [ + 'filter1' => ['before' => '*', 'after' => '*'], + 'filter2' => ['before' => '*', 'after' => '*'], + ]; + + Before Filters:: + + Previous: route1 → route2 → filter1 → filter2 + Now: filter1 → filter2 → route1 → route2 + + After Filters:: + + Previous: route1 → route2 → filter1 → filter2 + Now: route2 → route1 → filter2 → filter1 + Removed Deprecated Items ========================