diff --git a/phpstan-baseline.php b/phpstan-baseline.php index 64829de6cc3c..d2066f33094d 100644 --- a/phpstan-baseline.php +++ b/phpstan-baseline.php @@ -877,18 +877,6 @@ 'count' => 3, 'path' => __DIR__ . '/system/Commands/Utilities/Environment.php', ]; -$ignoreErrors[] = [ - // identifier: missingType.iterableValue - 'message' => '#^Method CodeIgniter\\\\Commands\\\\Utilities\\\\FilterCheck\\:\\:addRequiredFilters\\(\\) has parameter \\$filters with no value type specified in iterable type array\\.$#', - 'count' => 1, - 'path' => __DIR__ . '/system/Commands/Utilities/FilterCheck.php', -]; -$ignoreErrors[] = [ - // identifier: missingType.iterableValue - 'message' => '#^Method CodeIgniter\\\\Commands\\\\Utilities\\\\FilterCheck\\:\\:addRequiredFilters\\(\\) return type has no value type specified in iterable type array\\.$#', - 'count' => 1, - 'path' => __DIR__ . '/system/Commands/Utilities/FilterCheck.php', -]; $ignoreErrors[] = [ // identifier: missingType.iterableValue 'message' => '#^Method CodeIgniter\\\\Commands\\\\Utilities\\\\Namespaces\\:\\:outputAllNamespaces\\(\\) has parameter \\$params with no value type specified in iterable type array\\.$#', diff --git a/system/Commands/Utilities/FilterCheck.php b/system/Commands/Utilities/FilterCheck.php index 56fcdb00b971..bb3811fc0ce8 100644 --- a/system/Commands/Utilities/FilterCheck.php +++ b/system/Commands/Utilities/FilterCheck.php @@ -73,13 +73,7 @@ class FilterCheck extends BaseCommand */ public function run(array $params) { - $tbody = []; - if (! isset($params[0], $params[1])) { - CLI::error('You must specify a HTTP verb and a route.'); - CLI::write(' Usage: ' . $this->usage); - CLI::write('Example: filter:check GET /'); - CLI::write(' filter:check PUT products/1'); - + if (! $this->checkParams($params)) { return EXIT_ERROR; } @@ -107,15 +101,38 @@ public function run(array $params) return EXIT_ERROR; } - $filters = $this->addRequiredFilters($filterCollector, $filters); + $this->showTable($filterCollector, $filters, $method, $route); + $this->showFilterClasses($filterCollector, $method, $route); - $tbody[] = [ - strtoupper($method), - $route, - implode(' ', $filters['before']), - implode(' ', $filters['after']), - ]; + return EXIT_SUCCESS; + } + /** + * @param array $params + */ + private function checkParams(array $params): bool + { + if (! isset($params[0], $params[1])) { + CLI::error('You must specify a HTTP verb and a route.'); + CLI::write(' Usage: ' . $this->usage); + CLI::write('Example: filter:check GET /'); + CLI::write(' filter:check PUT products/1'); + + return false; + } + + return true; + } + + /** + * @param array{before: list, after: list} $filters + */ + private function showTable( + FilterCollector $filterCollector, + array $filters, + string $method, + string $route + ): void { $thead = [ 'Method', 'Route', @@ -123,33 +140,60 @@ public function run(array $params) 'After Filters', ]; - CLI::table($tbody, $thead); + $required = $filterCollector->getRequiredFilters(); - return EXIT_SUCCESS; + $coloredRequired = $this->colorItems($required); + + $before = array_merge($coloredRequired['before'], $filters['before']); + $after = array_merge($filters['after'], $coloredRequired['after']); + + $tbody = []; + $tbody[] = [ + strtoupper($method), + $route, + implode(' ', $before), + implode(' ', $after), + ]; + + CLI::table($tbody, $thead); } - private function addRequiredFilters(FilterCollector $filterCollector, array $filters): array + /** + * Color all elements of the array. + * + * @param array $array + * + * @return array + */ + private function colorItems(array $array): array { - $output = []; + return array_map(function ($item) { + if (is_array($item)) { + return $this->colorItems($item); + } - $required = $filterCollector->getRequiredFilters(); + return CLI::color($item, 'yellow'); + }, $array); + } - $colored = []; + private function showFilterClasses( + FilterCollector $filterCollector, + string $method, + string $route + ): void { + $requiredFilterClasses = $filterCollector->getRequiredFilterClasses(); + $filterClasses = $filterCollector->getClasses($method, $route); - foreach ($required['before'] as $filter) { - $filter = CLI::color($filter, 'yellow'); - $colored[] = $filter; - } - $output['before'] = array_merge($colored, $filters['before']); + $coloredRequiredFilterClasses = $this->colorItems($requiredFilterClasses); - $colored = []; + $classList = [ + 'before' => array_merge($coloredRequiredFilterClasses['before'], $filterClasses['before']), + 'after' => array_merge($filterClasses['after'], $coloredRequiredFilterClasses['after']), + ]; - foreach ($required['after'] as $filter) { - $filter = CLI::color($filter, 'yellow'); - $colored[] = $filter; + foreach ($classList as $position => $classes) { + CLI::write(ucfirst($position) . ' Filter Classes:', 'cyan'); + CLI::write(implode(' → ', $classes)); } - $output['after'] = array_merge($filters['after'], $colored); - - return $output; } } diff --git a/system/Commands/Utilities/Routes/FilterCollector.php b/system/Commands/Utilities/Routes/FilterCollector.php index 002a529b953e..dd1015838851 100644 --- a/system/Commands/Utilities/Routes/FilterCollector.php +++ b/system/Commands/Utilities/Routes/FilterCollector.php @@ -43,7 +43,7 @@ public function __construct( * @param string $method HTTP verb like `GET`,`POST` or `CLI`. * @param string $uri URI path to find filters for * - * @return array{before: list, after: list} array of filter alias or classname + * @return array{before: list, after: list} array of alias/classname:args */ public function get(string $method, string $uri): array { @@ -79,10 +79,52 @@ public function get(string $method, string $uri): array return $finder->find($uri); } + /** + * Returns filter classes for the URI + * + * @param string $method HTTP verb like `GET`,`POST` or `CLI`. + * @param string $uri URI path to find filters for + * + * @return array{before: list, after: list} array of classname:args + */ + public function getClasses(string $method, string $uri): array + { + if ($method === strtolower($method)) { + @trigger_error( + 'Passing lowercase HTTP method "' . $method . '" is deprecated.' + . ' Use uppercase HTTP method like "' . strtoupper($method) . '".', + E_USER_DEPRECATED + ); + } + + /** + * @deprecated 4.5.0 + * @TODO Remove this in the future. + */ + $method = strtoupper($method); + + if ($method === 'CLI') { + return [ + 'before' => [], + 'after' => [], + ]; + } + + $request = Services::incomingrequest(null, false); + $request->setMethod($method); + + $router = $this->createRouter($request); + $filters = $this->createFilters($request); + + $finder = new FilterFinder($router, $filters); + + return $finder->findClasses($uri); + } + /** * Returns Required Filters * - * @return array{before: list, after: list} array of filter alias or classname + * @return array{before: list, after: list} array of aliases */ public function getRequiredFilters(): array { @@ -97,6 +139,24 @@ public function getRequiredFilters(): array return $finder->getRequiredFilters(); } + /** + * Returns Required Filter class list + * + * @return array{before: list, after: list} array of classnames + */ + public function getRequiredFilterClasses(): array + { + $request = Services::incomingrequest(null, false); + $request->setMethod(Method::GET); + + $router = $this->createRouter($request); + $filters = $this->createFilters($request); + + $finder = new FilterFinder($router, $filters); + + return $finder->getRequiredFilterClasses(); + } + private function createRouter(Request $request): Router { $routes = service('routes'); diff --git a/system/Commands/Utilities/Routes/FilterFinder.php b/system/Commands/Utilities/Routes/FilterFinder.php index 7971e5c1be84..f34ea26aa702 100644 --- a/system/Commands/Utilities/Routes/FilterFinder.php +++ b/system/Commands/Utilities/Routes/FilterFinder.php @@ -46,23 +46,20 @@ private function getRouteFilters(string $uri): array /** * @param string $uri URI path to find filters for * - * @return array{before: list, after: list} array of filter alias or classname + * @return array{before: list, after: list} array of alias/classname:args */ public function find(string $uri): array { $this->filters->reset(); - // Add route filters try { + // Add route filters $routeFilters = $this->getRouteFilters($uri); - $this->filters->enableFilters($routeFilters, 'before'); - $oldFilterOrder = config(Feature::class)->oldFilterOrder ?? false; if (! $oldFilterOrder) { $routeFilters = array_reverse($routeFilters); } - $this->filters->enableFilters($routeFilters, 'after'); $this->filters->initialize($uri); @@ -81,10 +78,66 @@ public function find(string $uri): array } } + /** + * @param string $uri URI path to find filters for + * + * @return array{before: list, after: list} array of classname:args + */ + public function findClasses(string $uri): array + { + $this->filters->reset(); + + try { + // Add route filters + $routeFilters = $this->getRouteFilters($uri); + $this->filters->enableFilters($routeFilters, 'before'); + $oldFilterOrder = config(Feature::class)->oldFilterOrder ?? false; + if (! $oldFilterOrder) { + $routeFilters = array_reverse($routeFilters); + } + $this->filters->enableFilters($routeFilters, 'after'); + + $this->filters->initialize($uri); + + $filterClassList = $this->filters->getFiltersClass(); + + $filterClasses = [ + 'before' => [], + 'after' => [], + ]; + + foreach ($filterClassList['before'] as $classInfo) { + $classWithArguments = ($classInfo[1] === []) ? $classInfo[0] + : $classInfo[0] . ':' . implode(',', $classInfo[1]); + + $filterClasses['before'][] = $classWithArguments; + } + + foreach ($filterClassList['after'] as $classInfo) { + $classWithArguments = ($classInfo[1] === []) ? $classInfo[0] + : $classInfo[0] . ':' . implode(',', $classInfo[1]); + + $filterClasses['after'][] = $classWithArguments; + } + + return $filterClasses; + } catch (RedirectException) { + return [ + 'before' => [], + 'after' => [], + ]; + } catch (BadRequestException|PageNotFoundException) { + return [ + 'before' => [''], + 'after' => [''], + ]; + } + } + /** * Returns Required Filters * - * @return array{before: list, after:list} + * @return array{before: list, after:list} array of aliases */ public function getRequiredFilters(): array { @@ -96,4 +149,31 @@ public function getRequiredFilters(): array 'after' => $requiredAfter, ]; } + + /** + * Returns Required Filter classes + * + * @return array{before: list, after:list} + */ + public function getRequiredFilterClasses(): array + { + $before = $this->filters->getRequiredClasses('before'); + $after = $this->filters->getRequiredClasses('after'); + + $requiredBefore = []; + $requiredAfter = []; + + foreach ($before as $classInfo) { + $requiredBefore[] = $classInfo[0]; + } + + foreach ($after as $classInfo) { + $requiredAfter[] = $classInfo[0]; + } + + return [ + 'before' => $requiredBefore, + 'after' => $requiredAfter, + ]; + } } diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 972257ac69ab..9cfd6bca793b 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -29,7 +29,7 @@ class Filters { /** - * The original config file + * The Config\Filters instance * * @var FiltersConfig */ @@ -50,15 +50,14 @@ class Filters protected $response; /** - * Handle to the modules config. + * The Config\Modules instance * * @var Modules */ protected $modules; /** - * Whether we've done initial processing - * on the filter lists. + * Whether we've done initial processing on the filter lists. * * @var bool */ @@ -94,8 +93,8 @@ class Filters ]; /** - * The collection of filter class names and its arguments to execute for the - * current request (URI path). + * The collection of filter classnames and their arguments to execute for + * the current request (URI path). * * This does not include "Required Filters". * @@ -163,10 +162,10 @@ public function __construct($config, RequestInterface $request, ResponseInterfac /** * If discoverFilters is enabled in Config then system will try to - * auto-discover custom filters files in Namespaces and allow access to - * the config object via the variable $filters as with the routes file + * auto-discover custom filters files in namespaces and allow access to + * the config object via the variable $filters as with the routes file. * - * Sample : + * Sample: * $filters->aliases['custom-auth'] = \Acme\Blob\Filters\BlobAuth::class; * * @deprecated 4.4.2 Use Registrar instead. @@ -204,8 +203,8 @@ public function setResponse(ResponseInterface $response) } /** - * Runs through all of the filters for the specified - * uri and position. + * Runs through all the filters (except "Required Filters") for the specified + * URI and position. * * @param string $uri URI path relative to baseURL * @phpstan-param 'before'|'after' $position @@ -309,42 +308,60 @@ private function createFilter(string $className): FilterInterface } /** - * Runs "Required Filters" for the specified position. - * - * @return RequestInterface|ResponseInterface|string|null + * Returns the "Required Filters" class list. * * @phpstan-param 'before'|'after' $position * - * @throws FilterException - * - * @internal + * @return list}> [[classname, arguments], ...] */ - public function runRequired(string $position = 'before') + public function getRequiredClasses(string $position): array { [$filters, $aliases] = $this->getRequiredFilters($position); if ($filters === []) { - return $position === 'before' ? $this->request : $this->response; + return []; } - $filterClasses = []; + $filterClassList = []; foreach ($filters as $alias) { if (is_array($aliases[$alias])) { foreach ($this->config->aliases[$alias] as $class) { - $filterClasses[$position][] = [$class, []]; + $filterClassList[] = [$class, []]; } } else { - $filterClasses[$position][] = [$aliases[$alias], []]; + $filterClassList[] = [$aliases[$alias], []]; } } + return $filterClassList; + } + + /** + * Runs "Required Filters" for the specified position. + * + * @phpstan-param 'before'|'after' $position + * + * @return RequestInterface|ResponseInterface|string|null + * + * @throws FilterException + * + * @internal + */ + public function runRequired(string $position = 'before') + { + $filterClassList = $this->getRequiredClasses($position); + + if ($filterClassList === []) { + return $position === 'before' ? $this->request : $this->response; + } + if ($position === 'before') { - return $this->runBefore($filterClasses[$position]); + return $this->runBefore($filterClassList); } // After - return $this->runAfter($filterClasses[$position]); + return $this->runAfter($filterClassList); } /** @@ -527,9 +544,11 @@ public function getFiltersClass(): array * MUST be called prior to initialize(); * Intended for use within routes files. * + * @phpstan-param 'before'|'after' $position + * * @return $this */ - public function addFilter(string $class, ?string $alias = null, string $when = 'before', string $section = 'globals') + public function addFilter(string $class, ?string $alias = null, string $position = 'before', string $section = 'globals') { $alias ??= md5($class); @@ -537,13 +556,13 @@ public function addFilter(string $class, ?string $alias = null, string $when = ' $this->config->{$section} = []; } - if (! isset($this->config->{$section}[$when])) { - $this->config->{$section}[$when] = []; + if (! isset($this->config->{$section}[$position])) { + $this->config->{$section}[$position] = []; } $this->config->aliases[$alias] = $class; - $this->config->{$section}[$when][] = $alias; + $this->config->{$section}[$position][] = $alias; return $this; } @@ -555,10 +574,11 @@ 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' - * or filter classname. + * @param string $name filter_name or filter_name:arguments like 'role:admin,manager' + * or filter classname. + * @phpstan-param 'before'|'after' $position */ - private function enableFilter(string $name, string $when = 'before'): void + private function enableFilter(string $name, string $position = 'before'): void { // Normalize the arguments. [$alias, $arguments] = $this->getCleanName($name); @@ -570,13 +590,13 @@ private function enableFilter(string $name, string $when = 'before'): void throw FilterException::forNoAlias($alias); } - if (! isset($this->filters[$when][$filter])) { - $this->filters[$when][] = $filter; + if (! isset($this->filters[$position][$filter])) { + $this->filters[$position][] = $filter; } // Since some filters like rate limiters rely on being executed once a request, // we filter em here. - $this->filters[$when] = array_unique($this->filters[$when]); + $this->filters[$position] = array_unique($this->filters[$position]); } /** @@ -798,13 +818,15 @@ protected function processFilters(?string $uri = null) /** * Maps filter aliases to the equivalent filter classes * + * @phpstan-param 'before'|'after' $position + * * @return void * * @throws FilterException */ protected function processAliasesToClass(string $position) { - $filterClasses = []; + $filterClassList = []; foreach ($this->filters[$position] as $filter) { // Get arguments and clean alias @@ -816,17 +838,17 @@ protected function processAliasesToClass(string $position) if (is_array($this->config->aliases[$alias])) { foreach ($this->config->aliases[$alias] as $class) { - $filterClasses[] = [$class, $arguments]; + $filterClassList[] = [$class, $arguments]; } } else { - $filterClasses[] = [$this->config->aliases[$alias], $arguments]; + $filterClassList[] = [$this->config->aliases[$alias], $arguments]; } } if ($position === 'before') { - $this->filtersClass[$position] = array_merge($filterClasses, $this->filtersClass[$position]); + $this->filtersClass[$position] = array_merge($filterClassList, $this->filtersClass[$position]); } else { - $this->filtersClass[$position] = array_merge($this->filtersClass[$position], $filterClasses); + $this->filtersClass[$position] = array_merge($this->filtersClass[$position], $filterClassList); } } diff --git a/tests/system/Commands/FilterCheckTest.php b/tests/system/Commands/FilterCheckTest.php index 0c26dd9bf349..3fffd9e3f72e 100644 --- a/tests/system/Commands/FilterCheckTest.php +++ b/tests/system/Commands/FilterCheckTest.php @@ -50,6 +50,13 @@ public function testFilterCheckDefinedRoute(): void '| GET | / | forcehttps pagecache | pagecache performance toolbar |', preg_replace('/\033\[.+?m/u', '', $this->getBuffer()) ); + $this->assertStringContainsString( + 'Before Filter Classes: +CodeIgniter\Filters\ForceHTTPS → CodeIgniter\Filters\PageCache +After Filter Classes: +CodeIgniter\Filters\PageCache → CodeIgniter\Filters\PerformanceMetrics → CodeIgniter\Filters\DebugToolbar', + preg_replace('/\033\[.+?m/u', '', $this->getBuffer()) + ); } public function testFilterCheckInvalidRoute(): void diff --git a/tests/system/Commands/Utilities/Routes/FilterFinderTest.php b/tests/system/Commands/Utilities/Routes/FilterFinderTest.php index d96571c6b2f2..a51c3db08634 100644 --- a/tests/system/Commands/Utilities/Routes/FilterFinderTest.php +++ b/tests/system/Commands/Utilities/Routes/FilterFinderTest.php @@ -154,6 +154,42 @@ public function testFindGlobalsAndRouteFilters(): void $this->assertSame($expected, $filters); } + public function testFindGlobalsAndRouteFiltersWithArguments(): void + { + $collection = $this->createRouteCollection(); + $collection->get('admin', ' AdminController::index', ['filter' => 'honeypot:arg1,arg2']); + $router = $this->createRouter($collection); + $filters = $this->createFilters(); + + $finder = new FilterFinder($router, $filters); + + $filters = $finder->find('admin'); + + $expected = [ + 'before' => ['csrf', 'honeypot:arg1,arg2'], + 'after' => ['honeypot:arg1,arg2', 'toolbar'], + ]; + $this->assertSame($expected, $filters); + } + + public function testFindClassesGlobalsAndRouteFiltersWithArguments(): void + { + $collection = $this->createRouteCollection(); + $collection->get('admin', ' AdminController::index', ['filter' => 'honeypot:arg1,arg2']); + $router = $this->createRouter($collection); + $filters = $this->createFilters(); + + $finder = new FilterFinder($router, $filters); + + $filters = $finder->findClasses('admin'); + + $expected = [ + 'before' => [CSRF::class, Honeypot::class . ':arg1,arg2'], + 'after' => [Honeypot::class . ':arg1,arg2', DebugToolbar::class], + ]; + $this->assertSame($expected, $filters); + } + public function testFindGlobalsAndRouteClassnameFilters(): void { $collection = $this->createRouteCollection(); diff --git a/user_guide_src/source/changelogs/v4.6.0.rst b/user_guide_src/source/changelogs/v4.6.0.rst index 4de1f8640248..cf2b19052d7c 100644 --- a/user_guide_src/source/changelogs/v4.6.0.rst +++ b/user_guide_src/source/changelogs/v4.6.0.rst @@ -117,6 +117,7 @@ Commands - The ``spark routes`` and ``spark filter:check`` commands now display filter arguments. +- The ``spark filter:check`` command now displays filter classnames. Testing ======= diff --git a/user_guide_src/source/incoming/filters.rst b/user_guide_src/source/incoming/filters.rst index 0697e706e09d..8e420bdfef7a 100644 --- a/user_guide_src/source/incoming/filters.rst +++ b/user_guide_src/source/incoming/filters.rst @@ -274,11 +274,19 @@ The output is like the following: .. code-block:: none - +--------+-------+----------------+---------------+ - | Method | Route | Before Filters | After Filters | - +--------+-------+----------------+---------------+ - | GET | / | | toolbar | - +--------+-------+----------------+---------------+ + +--------+-------+----------------------+-------------------------------+ + | Method | Route | Before Filters | After Filters | + +--------+-------+----------------------+-------------------------------+ + | GET | / | forcehttps pagecache | pagecache performance toolbar | + +--------+-------+----------------------+-------------------------------+ + + Before Filter Classes: + CodeIgniter\Filters\ForceHTTPS → CodeIgniter\Filters\PageCache + After Filter Classes: + CodeIgniter\Filters\PageCache → CodeIgniter\Filters\PerformanceMetrics → CodeIgniter\Filters\DebugToolbar + +.. note:: Since v4.6.0, filter arguments have been displayed in the output table. + Also, the actual filter classnames have been displayed in the end. You can also see the routes and filters by the ``spark routes`` command, but it might not show accurate filters when you use regular expressions for routes.