From 3408051ad5c9bf68d7185c86fbee26c5c006ad12 Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 16 Jan 2023 10:55:47 +0900 Subject: [PATCH 01/17] docs: add PHPDoc types --- app/Config/Filters.php | 6 ++++++ system/Filters/Filters.php | 10 ++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/app/Config/Filters.php b/app/Config/Filters.php index 7b70c4fb3381..68600771f5cd 100644 --- a/app/Config/Filters.php +++ b/app/Config/Filters.php @@ -14,6 +14,9 @@ class Filters extends BaseConfig /** * Configures aliases for Filter classes to * make reading things nicer and simpler. + * + * @var array + * @phpstan-var array */ public array $aliases = [ 'csrf' => CSRF::class, @@ -26,6 +29,9 @@ class Filters extends BaseConfig /** * List of filter aliases that are always * applied before and after every request. + * + * @var array>>|array> + * @phpstan-var array>|array>> */ public array $globals = [ 'before' => [ diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 30e36a32c65f..3c4468110531 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -63,7 +63,7 @@ class Filters * The processed filters that will * be used to check against. * - * @var array + * @var array */ protected $filters = [ 'before' => [], @@ -74,7 +74,7 @@ class Filters * The collection of filters' class names that will * be used to execute in each position. * - * @var array + * @var array */ protected $filtersClass = [ 'before' => [], @@ -84,14 +84,16 @@ class Filters /** * Any arguments to be passed to filters. * - * @var array + * @var array> [name => params] + * @phpstan-var array> */ protected $arguments = []; /** * Any arguments to be passed to filtersClass. * - * @var array + * @var array [classname => arguments] + * @phpstan-var array>|null> */ protected $argumentsClass = []; From 4aabafdbab14b78cbf2f41f860632802f72b26b1 Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 16 Jan 2023 11:02:06 +0900 Subject: [PATCH 02/17] style: break long lines --- system/Filters/Filters.php | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 3c4468110531..010476aec665 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -172,7 +172,10 @@ public function run(string $uri, string $position = 'before') } if ($position === 'before') { - $result = $class->before($this->request, $this->argumentsClass[$className] ?? null); + $result = $class->before( + $this->request, + $this->argumentsClass[$className] ?? null + ); if ($result instanceof RequestInterface) { $this->request = $result; @@ -195,7 +198,11 @@ public function run(string $uri, string $position = 'before') } if ($position === 'after') { - $result = $class->after($this->request, $this->response, $this->argumentsClass[$className] ?? null); + $result = $class->after( + $this->request, + $this->response, + $this->argumentsClass[$className] ?? null + ); if ($result instanceof ResponseInterface) { $this->response = $result; From c242ed5dc1993269f269c7ce19a44ba053639b23 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 21 Jan 2023 17:37:58 +0900 Subject: [PATCH 03/17] refactor: extract getCleanName() method --- system/Filters/Filters.php | 39 ++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 010476aec665..3875c075d8ad 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -325,22 +325,18 @@ public function addFilter(string $class, ?string $alias = null, string $when = ' * after the filter name, followed by a comma-separated list of arguments that * are passed to the filter when executed. * + * @param string $name filter_name or filter_name:arguments like 'role:admin,manager' + * * @return Filters * * @deprecated Use enableFilters(). This method will be private. */ public function enableFilter(string $name, string $when = 'before') { - // Get parameters and clean name - if (strpos($name, ':') !== false) { - [$name, $params] = explode(':', $name); - - $params = explode(',', $params); - array_walk($params, static function (&$item) { - $item = trim($item); - }); - - $this->arguments[$name] = $params; + // Get arguments and clean name + [$name, $arguments] = $this->getCleanName($name); + if ($arguments !== []) { + $this->arguments[$name] = $arguments; } if (class_exists($name)) { @@ -363,6 +359,29 @@ public function enableFilter(string $name, string $when = 'before') return $this; } + /** + * Get clean name and arguments + * + * @param string $name filter_name or filter_name:arguments like 'role:admin,manager' + * + * @return array [name, arguments] + */ + private function getCleanName(string $name): array + { + $arguments = []; + + if (strpos($name, ':') !== false) { + [$name, $arguments] = explode(':', $name); + + $arguments = explode(',', $arguments); + array_walk($arguments, static function (&$item) { + $item = trim($item); + }); + } + + return [$name, $arguments]; + } + /** * Ensures that specific filters are on and enabled for the current request. * From bd62d40a4746d5e3c206eb35a8d716844d2ab3c0 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 21 Jan 2023 18:54:26 +0900 Subject: [PATCH 04/17] docs: add PHPDoc types --- system/Filters/Filters.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 3875c075d8ad..253e3ba9a792 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -365,6 +365,7 @@ public function enableFilter(string $name, string $when = 'before') * @param string $name filter_name or filter_name:arguments like 'role:admin,manager' * * @return array [name, arguments] + * @phpstan-return array{0: string, 1: list} */ private function getCleanName(string $name): array { @@ -389,6 +390,8 @@ private function getCleanName(string $name): array * after the filter name, followed by a comma-separated list of arguments that * are passed to the filter when executed. * + * @params array $names filter_name or filter_name:arguments like 'role:admin,manager' + * * @return Filters */ public function enableFilters(array $names, string $when = 'before') From ac730dddf43deed0275a00b4673e3b09f825501e Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 21 Jan 2023 18:55:08 +0900 Subject: [PATCH 05/17] docs: update comment --- system/Filters/Filters.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 253e3ba9a792..8dfd1a2082b4 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -531,8 +531,8 @@ protected function processAliasesToClass(string $position) } } - // 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. + // 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. // 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])); } From 2b923cd88aa3cbb4dfccc0d371b70683e131cea9 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 21 Jan 2023 19:59:36 +0900 Subject: [PATCH 06/17] feat: $filters can use filter arguments --- system/Filters/Filters.php | 33 +++++++++++++++++++++++++-- tests/system/Filters/FiltersTest.php | 34 ++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 8dfd1a2082b4..2e198cb89049 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -494,20 +494,49 @@ protected function processFilters(?string $uri = null) // Look for inclusion rules if (isset($settings['before'])) { $path = $settings['before']; + if ($this->pathApplies($uri, $path)) { - $this->filters['before'][] = $alias; + // Get arguments and clean name + [$name, $arguments] = $this->getCleanName($alias); + + $this->filters['before'][] = $name; + + $this->registerArguments($name, $arguments); } } if (isset($settings['after'])) { $path = $settings['after']; + if ($this->pathApplies($uri, $path)) { - $this->filters['after'][] = $alias; + // Get arguments and clean name + [$name, $arguments] = $this->getCleanName($alias); + + $this->filters['after'][] = $name; + + $this->registerArguments($name, $arguments); } } } } + /** + * @param string $name filter alias + * @param array $arguments filter arguments + */ + private function registerArguments(string $name, $arguments): void + { + if ($arguments !== []) { + $this->arguments[$name] = $arguments; + } + + $classNames = (array) $this->config->aliases[$name]; + + foreach ($classNames as $className) { + $this->argumentsClass[$className] = $this->arguments[$name] ?? null; + } + } + /** * Maps filter aliases to the equivalent filter classes * diff --git a/tests/system/Filters/FiltersTest.php b/tests/system/Filters/FiltersTest.php index f111f4b957f6..93c425eff395 100644 --- a/tests/system/Filters/FiltersTest.php +++ b/tests/system/Filters/FiltersTest.php @@ -783,6 +783,40 @@ public function testEnableFilter() $this->assertContains('google', $filters['before']); } + public function testFiltersWithArguments() + { + $_SERVER['REQUEST_METHOD'] = 'GET'; + + $config = [ + 'aliases' => ['role' => Role::class], + 'globals' => [ + ], + 'filters' => [ + 'role:admin,super' => [ + 'before' => ['admin/*'], + 'after' => ['admin/*'], + ], + ], + ]; + $filtersConfig = $this->createConfigFromArray(FiltersConfig::class, $config); + $filters = $this->createFilters($filtersConfig); + + $filters = $filters->initialize('admin/foo/bar'); + $found = $filters->getFilters(); + + $this->assertContains('role', $found['before']); + $this->assertSame(['admin', 'super'], $filters->getArguments('role')); + $this->assertSame(['role' => ['admin', 'super']], $filters->getArguments()); + + $response = $filters->run('admin/foo/bar', 'before'); + + $this->assertSame('admin;super', $response); + + $response = $filters->run('admin/foo/bar', 'after'); + + $this->assertSame('admin;super', $response->getBody()); + } + public function testEnableFilterWithArguments() { $_SERVER['REQUEST_METHOD'] = 'GET'; From 29fdf817e5d15bb38a9746d1c8fac6835ae3e5c7 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 21 Jan 2023 20:17:04 +0900 Subject: [PATCH 07/17] docs: add docs --- user_guide_src/source/changelogs/v4.4.0.rst | 1 + user_guide_src/source/incoming/filters.rst | 25 ++++++++++++++++--- .../source/incoming/filters/009.php | 2 ++ .../source/incoming/filters/012.php | 17 +++++++++++++ 4 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 user_guide_src/source/incoming/filters/012.php diff --git a/user_guide_src/source/changelogs/v4.4.0.rst b/user_guide_src/source/changelogs/v4.4.0.rst index 748829333165..713322198bc4 100644 --- a/user_guide_src/source/changelogs/v4.4.0.rst +++ b/user_guide_src/source/changelogs/v4.4.0.rst @@ -63,6 +63,7 @@ Others - **Auto Routing (Improved)**: Now you can use URI without a method name like ``product/15`` where ``15`` is an arbitrary number. See :ref:`controller-default-method-fallback` for details. +- **Filters:** Now you can use Filter Arguments with :ref:`$filters property `. Message Changes *************** diff --git a/user_guide_src/source/incoming/filters.rst b/user_guide_src/source/incoming/filters.rst index d768fa9879f7..944bb0c3e50d 100644 --- a/user_guide_src/source/incoming/filters.rst +++ b/user_guide_src/source/incoming/filters.rst @@ -139,15 +139,34 @@ a list of URI patterns that filter should apply to: .. literalinclude:: filters/009.php -Filter arguments -================= +Filter Arguments +================ -When configuring filters, additional arguments may be passed to a filter when setting up the route: +When configuring filters, additional arguments may be passed to a filter. + +Route +----- + +When setting up the route: .. literalinclude:: filters/010.php In this example, the array ``['dual', 'noreturn']`` will be passed in ``$arguments`` to the filter's ``before()`` and ``after()`` implementation methods. +.. _filter-arguments-filters: + +$filters +-------- + +When setting up the ``$filters``: + +.. literalinclude:: filters/012.php + +In this example, when the URI matches ``admin/*'``, the array ``['admin', 'superadmin']`` +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. + ****************** Confirming Filters ****************** diff --git a/user_guide_src/source/incoming/filters/009.php b/user_guide_src/source/incoming/filters/009.php index 162af6dcbaec..fd9119d21af5 100644 --- a/user_guide_src/source/incoming/filters/009.php +++ b/user_guide_src/source/incoming/filters/009.php @@ -6,6 +6,8 @@ class Filters extends BaseConfig { + // ... + public $filters = [ 'foo' => ['before' => ['admin/*'], 'after' => ['users/*']], 'bar' => ['before' => ['api/*', 'admin/*']], diff --git a/user_guide_src/source/incoming/filters/012.php b/user_guide_src/source/incoming/filters/012.php new file mode 100644 index 000000000000..799bd57fce7d --- /dev/null +++ b/user_guide_src/source/incoming/filters/012.php @@ -0,0 +1,17 @@ + ['before' => ['admin/*']], + 'permission:users.manage' => ['before' => ['admin/users/*']], + ]; + + // ... +} From a0827790c76d7ff4b38c395988d89c85d8842ca0 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 21 Jan 2023 20:50:36 +0900 Subject: [PATCH 08/17] docs: improve docs --- user_guide_src/source/changelogs/v4.4.0.rst | 2 +- user_guide_src/source/incoming/filters.rst | 22 +++++-------------- user_guide_src/source/incoming/routing.rst | 12 +++++++++- .../{filters/010.php => routing/067.php} | 0 4 files changed, 17 insertions(+), 19 deletions(-) rename user_guide_src/source/incoming/{filters/010.php => routing/067.php} (100%) diff --git a/user_guide_src/source/changelogs/v4.4.0.rst b/user_guide_src/source/changelogs/v4.4.0.rst index 713322198bc4..e3825c469530 100644 --- a/user_guide_src/source/changelogs/v4.4.0.rst +++ b/user_guide_src/source/changelogs/v4.4.0.rst @@ -63,7 +63,7 @@ Others - **Auto Routing (Improved)**: Now you can use URI without a method name like ``product/15`` where ``15`` is an arbitrary number. See :ref:`controller-default-method-fallback` for details. -- **Filters:** Now you can use Filter Arguments with :ref:`$filters property `. +- **Filters:** Now you can use Filter Arguments with :ref:`$filters property `. Message Changes *************** diff --git a/user_guide_src/source/incoming/filters.rst b/user_guide_src/source/incoming/filters.rst index 944bb0c3e50d..353661e510b6 100644 --- a/user_guide_src/source/incoming/filters.rst +++ b/user_guide_src/source/incoming/filters.rst @@ -139,26 +139,14 @@ a list of URI patterns that filter should apply to: .. literalinclude:: filters/009.php -Filter Arguments -================ - -When configuring filters, additional arguments may be passed to a filter. - -Route ------ - -When setting up the route: +.. _filters-filters-filter-arguments: -.. literalinclude:: filters/010.php - -In this example, the array ``['dual', 'noreturn']`` will be passed in ``$arguments`` to the filter's ``before()`` and ``after()`` implementation methods. - -.. _filter-arguments-filters: +Filter Arguments +---------------- -$filters --------- +.. versionadded:: 4.4.0 -When setting up the ``$filters``: +When configuring ``$filters``, additional arguments may be passed to a filter: .. literalinclude:: filters/012.php diff --git a/user_guide_src/source/incoming/routing.rst b/user_guide_src/source/incoming/routing.rst index fffa5cb1bdb8..f18edede419c 100644 --- a/user_guide_src/source/incoming/routing.rst +++ b/user_guide_src/source/incoming/routing.rst @@ -406,7 +406,7 @@ The value for the filter can be a string or an array of strings: * matching the aliases defined in **app/Config/Filters.php**. * filter classnames -See :doc:`Controller filters ` for more information on setting up filters. +See :doc:`Controller Filters ` for more information on setting up filters. .. Warning:: If you set filters to routes in **app/Config/Routes.php** (not in **app/Config/Filters.php**), it is recommended to disable Auto Routing (Legacy). @@ -446,6 +446,16 @@ You specify an array for the filter value: .. literalinclude:: routing/037.php +Filter Arguments +^^^^^^^^^^^^^^^^ + +Additional arguments may be passed to a filter: + +.. literalinclude:: routing/067.php + +In this example, the array ``['dual', 'noreturn']`` will be passed in ``$arguments`` +to the filter's ``before()`` and ``after()`` implementation methods. + .. _assigning-namespace: Assigning Namespace diff --git a/user_guide_src/source/incoming/filters/010.php b/user_guide_src/source/incoming/routing/067.php similarity index 100% rename from user_guide_src/source/incoming/filters/010.php rename to user_guide_src/source/incoming/routing/067.php From 9c36d01ff4b4120157485cd695e1d88548a886da Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 21 Jan 2023 21:34:38 +0900 Subject: [PATCH 09/17] docs: make @return more specific --- system/Filters/Filters.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 2e198cb89049..fa17d505fb62 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -327,7 +327,7 @@ public function addFilter(string $class, ?string $alias = null, string $when = ' * * @param string $name filter_name or filter_name:arguments like 'role:admin,manager' * - * @return Filters + * @return $this * * @deprecated Use enableFilters(). This method will be private. */ From 4d525eb78a3ab47322a9cb33f700b78da1301376 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 21 Jan 2023 21:40:24 +0900 Subject: [PATCH 10/17] feat: throws exception if the filter arguments already defined --- system/Filters/Filters.php | 21 ++++++++--- tests/system/Filters/FiltersTest.php | 53 ++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index fa17d505fb62..829ad565cb1a 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -11,6 +11,7 @@ namespace CodeIgniter\Filters; +use CodeIgniter\Exceptions\ConfigException; use CodeIgniter\Filters\Exceptions\FilterException; use CodeIgniter\HTTP\RequestInterface; use CodeIgniter\HTTP\ResponseInterface; @@ -84,8 +85,8 @@ class Filters /** * Any arguments to be passed to filters. * - * @var array> [name => params] - * @phpstan-var array> + * @var array|null> [name => params] + * @phpstan-var array|null> */ protected $arguments = []; @@ -337,6 +338,8 @@ public function enableFilter(string $name, string $when = 'before') [$name, $arguments] = $this->getCleanName($name); if ($arguments !== []) { $this->arguments[$name] = $arguments; + } else { + $this->arguments[$name] = null; } if (class_exists($name)) { @@ -514,7 +517,9 @@ protected function processFilters(?string $uri = null) $this->filters['after'][] = $name; - $this->registerArguments($name, $arguments); + // The arguments may have already been registered in the before filter. + // So disable check. + $this->registerArguments($name, $arguments, false); } } } @@ -523,10 +528,18 @@ protected function processFilters(?string $uri = null) /** * @param string $name filter alias * @param array $arguments filter arguments + * @param bool $check if true, check if already defined */ - private function registerArguments(string $name, $arguments): void + private function registerArguments(string $name, array $arguments, bool $check = true): void { if ($arguments !== []) { + if ($check && array_key_exists($name, $this->arguments)) { + throw new ConfigException( + '"' . $name . '" has already arguments: ' + . (($this->arguments[$name] === null) ? 'null' : implode(',', $this->arguments[$name])) + ); + } + $this->arguments[$name] = $arguments; } diff --git a/tests/system/Filters/FiltersTest.php b/tests/system/Filters/FiltersTest.php index 93c425eff395..9b01988e2e14 100644 --- a/tests/system/Filters/FiltersTest.php +++ b/tests/system/Filters/FiltersTest.php @@ -12,6 +12,7 @@ namespace CodeIgniter\Filters; use CodeIgniter\Config\Services; +use CodeIgniter\Exceptions\ConfigException; use CodeIgniter\Filters\Exceptions\FilterException; use CodeIgniter\Filters\fixtures\GoogleCurious; use CodeIgniter\Filters\fixtures\GoogleEmpty; @@ -817,6 +818,58 @@ public function testFiltersWithArguments() $this->assertSame('admin;super', $response->getBody()); } + public function testFilterWithArgumentsIsDefined() + { + $this->expectException(ConfigException::class); + $this->expectExceptionMessage('"role" has already arguments: admin,super'); + + $_SERVER['REQUEST_METHOD'] = 'GET'; + + $config = [ + 'aliases' => ['role' => Role::class], + 'globals' => [], + 'filters' => [ + 'role:admin,super' => [ + 'before' => ['admin/*'], + ], + 'role:super' => [ + 'before' => ['admin/user/*'], + ], + ], + ]; + $filtersConfig = $this->createConfigFromArray(FiltersConfig::class, $config); + $filters = $this->createFilters($filtersConfig); + + $filters->initialize('admin/user/bar'); + } + + public function testFilterWithoutArgumentsIsDefined() + { + $_SERVER['REQUEST_METHOD'] = 'GET'; + + $config = [ + 'aliases' => ['role' => Role::class], + 'globals' => [], + 'filters' => [ + 'role' => [ + 'before' => ['admin/*'], + ], + 'role:super' => [ + 'before' => ['admin/user/*'], + ], + ], + ]; + $filtersConfig = $this->createConfigFromArray(FiltersConfig::class, $config); + $filters = $this->createFilters($filtersConfig); + + $filters = $filters->initialize('admin/user/bar'); + $found = $filters->getFilters(); + + $this->assertContains('role', $found['before']); + $this->assertSame(['super'], $filters->getArguments('role')); + $this->assertSame(['role' => ['super']], $filters->getArguments()); + } + public function testEnableFilterWithArguments() { $_SERVER['REQUEST_METHOD'] = 'GET'; From e3ea3ed468eae1cc6cbd599447a0621b931f899f Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 23 Jan 2023 11:48:51 +0900 Subject: [PATCH 11/17] refactor: by rector --- system/Filters/Filters.php | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 829ad565cb1a..2dc0efc66104 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -335,12 +335,8 @@ public function addFilter(string $class, ?string $alias = null, string $when = ' public function enableFilter(string $name, string $when = 'before') { // Get arguments and clean name - [$name, $arguments] = $this->getCleanName($name); - if ($arguments !== []) { - $this->arguments[$name] = $arguments; - } else { - $this->arguments[$name] = null; - } + [$name, $arguments] = $this->getCleanName($name); + $this->arguments[$name] = ($arguments !== []) ? $arguments : null; if (class_exists($name)) { $this->config->aliases[$name] = $name; From b768f8f8b80d70082b5fe716bc2db643abfb73ab Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 23 Jan 2023 11:59:04 +0900 Subject: [PATCH 12/17] docs: add about route filter config --- user_guide_src/source/incoming/filters.rst | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/user_guide_src/source/incoming/filters.rst b/user_guide_src/source/incoming/filters.rst index 353661e510b6..1a80df9d091b 100644 --- a/user_guide_src/source/incoming/filters.rst +++ b/user_guide_src/source/incoming/filters.rst @@ -6,10 +6,12 @@ Controller Filters :local: :depth: 2 -Controller Filters allow you to perform actions either before or after the controllers execute. Unlike :doc:`events `, -you can choose the specific URIs in which the filters will be applied to. Incoming filters may +Controller Filters allow you to perform actions either before or after the controllers execute. Unlike :doc:`/extending/events`, +you can choose the specific URIs or routes in which the filters will be applied to. Before filters may modify the Request while after filters can act on and even modify the Response, allowing for a lot of flexibility -and power. Some common examples of tasks that might be performed with filters are: +and power. + +Some common examples of tasks that might be performed with filters are: * Performing CSRF protection on the incoming requests * Restricting areas of your site based upon their Role @@ -64,11 +66,13 @@ the final output, or even to filter the final output with a bad words filter. Configuring Filters ******************* -Once you've created your filters, you need to configure when they get run. This is done in **app/Config/Filters.php**. -This file contains four properties that allow you to configure exactly when the filters run. +Once you've created your filters, you need to configure when they get run. This is done in **app/Config/Filters.php** or **app/Config/Routes.php**. .. Note:: The safest way to apply filters is to :ref:`disable auto-routing `, and :ref:`set filters to routes `. +The **app/Config/Filters.php** file contains four properties that allow you to +configure exactly when the filters run. + .. Warning:: It is recommended that you should always add ``*`` at the end of a URI in the filter settings. Because a controller method might be accessible by different URLs than you think. For example, when :ref:`auto-routing-legacy` is enabled, if you have ``Blog::index``, From 5c46faaec0ae034e92cc298107a3018fd1c49052 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 31 Jan 2023 16:57:43 +0900 Subject: [PATCH 13/17] fix: exception message Co-authored-by: MGatner --- system/Filters/Filters.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 2dc0efc66104..4ef34431723f 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -531,7 +531,7 @@ private function registerArguments(string $name, array $arguments, bool $check = if ($arguments !== []) { if ($check && array_key_exists($name, $this->arguments)) { throw new ConfigException( - '"' . $name . '" has already arguments: ' + '"' . $name . '" already has arguments: ' . (($this->arguments[$name] === null) ? 'null' : implode(',', $this->arguments[$name])) ); } From 66083fa32491665a2a5de973b08e30e6b428788e Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 31 Jan 2023 17:06:45 +0900 Subject: [PATCH 14/17] test: update test --- tests/system/Filters/FiltersTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/system/Filters/FiltersTest.php b/tests/system/Filters/FiltersTest.php index 9b01988e2e14..88924de7bcff 100644 --- a/tests/system/Filters/FiltersTest.php +++ b/tests/system/Filters/FiltersTest.php @@ -821,7 +821,7 @@ public function testFiltersWithArguments() public function testFilterWithArgumentsIsDefined() { $this->expectException(ConfigException::class); - $this->expectExceptionMessage('"role" has already arguments: admin,super'); + $this->expectExceptionMessage('"role" already has arguments: admin,super'); $_SERVER['REQUEST_METHOD'] = 'GET'; From 9d664901fff60064ba62ab530a17373d7715fd9f Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 31 Jan 2023 17:28:47 +0900 Subject: [PATCH 15/17] test: add test --- tests/system/CodeIgniterTest.php | 41 ++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/tests/system/CodeIgniterTest.php b/tests/system/CodeIgniterTest.php index 1a5e50fc0775..16c7acd1a5e3 100644 --- a/tests/system/CodeIgniterTest.php +++ b/tests/system/CodeIgniterTest.php @@ -12,6 +12,7 @@ namespace CodeIgniter; use CodeIgniter\Config\Services; +use CodeIgniter\Exceptions\ConfigException; use CodeIgniter\HTTP\Response; use CodeIgniter\Router\RouteCollection; use CodeIgniter\Test\CIUnitTestCase; @@ -19,7 +20,7 @@ use CodeIgniter\Test\Mock\MockCodeIgniter; use Config\App; use Config\Cache; -use Config\Filters; +use Config\Filters as FiltersConfig; use Config\Modules; use Tests\Support\Filters\Customfilter; @@ -274,6 +275,42 @@ public function testControllersRunFilterByClassName() $this->resetServices(); } + public function testRegisterSameFilterTwiceWithDifferentArgument() + { + $this->expectException(ConfigException::class); + $this->expectExceptionMessage('"test-customfilter" already has arguments: null'); + + $_SERVER['argv'] = ['index.php', 'pages/about']; + $_SERVER['argc'] = 2; + + $_SERVER['REQUEST_URI'] = '/pages/about'; + + $routes = Services::routes(); + $routes->add( + 'pages/about', + static fn () => Services::incomingrequest()->getBody(), + // Set filter with no argument. + ['filter' => 'test-customfilter'] + ); + + $router = Services::router($routes, Services::incomingrequest()); + Services::injectMock('router', $router); + + /** @var FiltersConfig $filterConfig */ + $filterConfig = config('Filters'); + $filterConfig->filters = [ + // Set filter with argument. + 'test-customfilter:arg1' => [ + 'before' => ['pages/*'], + ], + ]; + Services::filters($filterConfig); + + $this->codeigniter->run(); + + $this->resetServices(); + } + public function testDisableControllerFilters() { $_SERVER['argv'] = ['index.php', 'pages/about']; @@ -666,7 +703,7 @@ public function testPageCacheSendSecureHeaders() $router = Services::router($routes, Services::incomingrequest()); Services::injectMock('router', $router); - /** @var Filters $filterConfig */ + /** @var FiltersConfig $filterConfig */ $filterConfig = config('Filters'); $filterConfig->globals['after'] = ['secureheaders']; Services::filters($filterConfig); From 472562d984f1c8940c18a22876f5510613f0fe3d Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 31 Jan 2023 17:31:11 +0900 Subject: [PATCH 16/17] test: improve test method --- tests/system/CodeIgniterTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/system/CodeIgniterTest.php b/tests/system/CodeIgniterTest.php index 16c7acd1a5e3..fa9b8e0f71d2 100644 --- a/tests/system/CodeIgniterTest.php +++ b/tests/system/CodeIgniterTest.php @@ -252,7 +252,7 @@ public function testControllersCanReturnDownloadResponseObject() $this->assertSame('some text', $output); } - public function testControllersRunFilterByClassName() + public function testRunExecuteFilterByClassName() { $_SERVER['argv'] = ['index.php', 'pages/about']; $_SERVER['argc'] = 2; From 46727ea30b4e14fcddcd0caa5c298f441f0104e2 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 31 Jan 2023 17:43:32 +0900 Subject: [PATCH 17/17] style: break long lines --- tests/system/CodeIgniterTest.php | 62 ++++++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 15 deletions(-) diff --git a/tests/system/CodeIgniterTest.php b/tests/system/CodeIgniterTest.php index fa9b8e0f71d2..daea86b8cc51 100644 --- a/tests/system/CodeIgniterTest.php +++ b/tests/system/CodeIgniterTest.php @@ -189,7 +189,10 @@ public function testControllersCanReturnString() // Inject mock router. $routes = Services::routes(); - $routes->add('pages/(:segment)', static fn ($segment) => 'You want to see "' . esc($segment) . '" page.'); + $routes->add( + 'pages/(:segment)', + static fn ($segment) => 'You want to see "' . esc($segment) . '" page.' + ); $router = Services::router($routes, Services::incomingrequest()); Services::injectMock('router', $router); @@ -261,7 +264,11 @@ public function testRunExecuteFilterByClassName() // Inject mock router. $routes = Services::routes(); - $routes->add('pages/about', static fn () => Services::incomingrequest()->getBody(), ['filter' => Customfilter::class]); + $routes->add( + 'pages/about', + static fn () => Services::incomingrequest()->getBody(), + ['filter' => Customfilter::class] + ); $router = Services::router($routes, Services::incomingrequest()); Services::injectMock('router', $router); @@ -743,8 +750,11 @@ public function testPageCacheSendSecureHeaders() * * @see https://github.com/codeigniter4/CodeIgniter4/pull/6410 */ - public function testPageCacheWithCacheQueryString($cacheQueryStringValue, int $expectedPagesInCache, array $testingUrls) - { + public function testPageCacheWithCacheQueryString( + $cacheQueryStringValue, + int $expectedPagesInCache, + array $testingUrls + ) { // Suppress command() output CITestStreamFilter::$buffer = ''; $outputStreamFilter = stream_filter_append(STDOUT, 'CITestStreamFilter'); @@ -766,7 +776,10 @@ public function testPageCacheWithCacheQueryString($cacheQueryStringValue, int $e $_SERVER['REQUEST_URI'] = '/' . $testingUrl; $routes = Services::routes(true); $routes->add($testingUrl, static function () { - CodeIgniter::cache(0); // Don't cache the page in the run() function because CodeIgniter class will create default $cacheConfig and overwrite settings from the dataProvider + // Don't cache the page in the run() function because CodeIgniter + // class will create default $cacheConfig and overwrite settings + // from the dataProvider + CodeIgniter::cache(0); $response = Services::response(); $string = 'This is a test page, to check cache configuration'; @@ -777,9 +790,11 @@ public function testPageCacheWithCacheQueryString($cacheQueryStringValue, int $e $router = Services::router($routes, Services::incomingrequest(null, false)); Services::injectMock('router', $router); - // Cache the page output using default caching function and $cacheConfig with value from the data provider + // Cache the page output using default caching function and $cacheConfig + // with value from the data provider $this->codeigniter->run(); - $this->codeigniter->cachePage($cacheConfig); // Cache the page using our own $cacheConfig confugration + // Cache the page using our own $cacheConfig confugration + $this->codeigniter->cachePage($cacheConfig); } // Calculate how much cached items exist in the cache after the test requests @@ -800,17 +815,34 @@ public function testPageCacheWithCacheQueryString($cacheQueryStringValue, int $e public function cacheQueryStringProvider(): array { $testingUrls = [ - 'test', // URL #1 - 'test?important_parameter=1', // URL #2 - 'test?important_parameter=2', // URL #3 - 'test?important_parameter=1¬_important_parameter=2', // URL #4 - 'test?important_parameter=1¬_important_parameter=2&another_not_important_parameter=3', // URL #5 + // URL #1 + 'test', + // URL #2 + 'test?important_parameter=1', + // URL #3 + 'test?important_parameter=2', + // URL #4 + 'test?important_parameter=1¬_important_parameter=2', + // URL #5 + 'test?important_parameter=1¬_important_parameter=2&another_not_important_parameter=3', ]; return [ - '$cacheQueryString=false' => [false, 1, $testingUrls], // We expect only 1 page in the cache, because when cacheQueryString is set to false, all GET parameter should be ignored, and page URI will be absolutely same "/test" string for all 5 requests - '$cacheQueryString=true' => [true, 5, $testingUrls], // We expect all 5 pages in the cache, because when cacheQueryString is set to true, all GET parameter should be processed as unique requests - '$cacheQueryString=array' => [['important_parameter'], 3, $testingUrls], // We expect only 3 pages in the cache, because when cacheQueryString is set to array with important parameters, we should ignore all parameters thats not in the array. Only URL #1, URL #2 and URL #3 should be cached. URL #4 and URL #5 is duplication of URL #2 (with value ?important_parameter=1), so they should not be processed as new unique requests and application should return already cached page for URL #2 + // We expect only 1 page in the cache, because when cacheQueryString + // is set to false, all GET parameter should be ignored, and page URI + // will be absolutely same "/test" string for all 5 requests + '$cacheQueryString=false' => [false, 1, $testingUrls], + // We expect all 5 pages in the cache, because when cacheQueryString + // is set to true, all GET parameter should be processed as unique requests + '$cacheQueryString=true' => [true, 5, $testingUrls], + // We expect only 3 pages in the cache, because when cacheQueryString + // is set to array with important parameters, we should ignore all + // parameters thats not in the array. Only URL #1, URL #2 and URL #3 + // should be cached. URL #4 and URL #5 is duplication of URL #2 + // (with value ?important_parameter=1), so they should not be processed + // as new unique requests and application should return already cached + // page for URL #2 + '$cacheQueryString=array' => [['important_parameter'], 3, $testingUrls], ]; } }