From dfbe9a1e3d8b86ce89da13fcab1723bd58b7a3b2 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 17 Oct 2023 16:35:29 +0900 Subject: [PATCH 01/32] feat: add PageCache filter --- system/Filters/PageCache.php | 74 ++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 system/Filters/PageCache.php diff --git a/system/Filters/PageCache.php b/system/Filters/PageCache.php new file mode 100644 index 000000000000..664ef5c7db6a --- /dev/null +++ b/system/Filters/PageCache.php @@ -0,0 +1,74 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\Filters; + +use CodeIgniter\Cache\ResponseCache; +use CodeIgniter\HTTP\CLIRequest; +use CodeIgniter\HTTP\DownloadResponse; +use CodeIgniter\HTTP\IncomingRequest; +use CodeIgniter\HTTP\RedirectResponse; +use CodeIgniter\HTTP\RequestInterface; +use CodeIgniter\HTTP\ResponseInterface; +use Config\Services; + +/** + * Page Cache filter + */ +class PageCache implements FilterInterface +{ + private ResponseCache $pageCache; + + public function __construct() + { + $this->pageCache = Services::responsecache(); + } + + /** + * Checks page cache and return if found. + * + * @param array|null $arguments + */ + public function before(RequestInterface $request, $arguments = null) + { + assert($request instanceof CLIRequest || $request instanceof IncomingRequest); + + $response = Services::response(); + + $cachedResponse = $this->pageCache->get($request, $response); + + if ($cachedResponse instanceof ResponseInterface) { + return $cachedResponse; + } + } + + /** + * Cache the page. + * + * @param array|null $arguments + * + * @return void + */ + public function after(RequestInterface $request, ResponseInterface $response, $arguments = null) + { + assert($request instanceof CLIRequest || $request instanceof IncomingRequest); + + if ( + ! $response instanceof DownloadResponse + && ! $response instanceof RedirectResponse + ) { + // Cache it without the performance metrics replaced + // so that we can have live speed updates along the way. + // Must be run after filters to preserve the Response headers. + $this->pageCache->make($request, $response); + } + } +} From 22f9c1d94099cb3cc5cf61aad763c4906769db25 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 17 Oct 2023 16:35:50 +0900 Subject: [PATCH 02/32] feat: add PerformanceMetrics filter --- system/Filters/PerformanceMetrics.php | 61 +++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 system/Filters/PerformanceMetrics.php diff --git a/system/Filters/PerformanceMetrics.php b/system/Filters/PerformanceMetrics.php new file mode 100644 index 000000000000..1c844f477dd5 --- /dev/null +++ b/system/Filters/PerformanceMetrics.php @@ -0,0 +1,61 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\Filters; + +use CodeIgniter\HTTP\RequestInterface; +use CodeIgniter\HTTP\ResponseInterface; +use Config\Services; + +/** + * Performance Metrics filter + */ +class PerformanceMetrics implements FilterInterface +{ + /** + * We don't need to do anything here. + * + * @param array|null $arguments + */ + public function before(RequestInterface $request, $arguments = null) + { + } + + /** + * Replaces the performance metrics. + * + * @param array|null $arguments + * + * @return void + */ + public function after(RequestInterface $request, ResponseInterface $response, $arguments = null) + { + $body = $response->getBody(); + + if ($body !== null) { + $benchmark = Services::timer(); + + $output = str_replace( + [ + '{elapsed_time}', + '{memory_usage}', + ], + [ + (string) $benchmark->getElapsedTime('total_execution'), + number_format(memory_get_peak_usage() / 1024 / 1024, 3), + ], + $body + ); + + $response->setBody($output); + } + } +} From 27b176c40ff431e25f419f937ab0fc5149433c08 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 17 Oct 2023 16:36:43 +0900 Subject: [PATCH 03/32] feat: add "Required Filters" and use PageCache and PerformanceMetrics Also changes benchmark points. --- app/Config/Filters.php | 24 ++++- system/CodeIgniter.php | 120 +++++++++++++-------- system/Debug/Toolbar/Collectors/Routes.php | 14 ++- system/Filters/Filters.php | 90 +++++++++++++++- 4 files changed, 199 insertions(+), 49 deletions(-) diff --git a/app/Config/Filters.php b/app/Config/Filters.php index 6ebe0f3100ac..b06bc4b32805 100644 --- a/app/Config/Filters.php +++ b/app/Config/Filters.php @@ -7,6 +7,8 @@ use CodeIgniter\Filters\DebugToolbar; use CodeIgniter\Filters\Honeypot; use CodeIgniter\Filters\InvalidChars; +use CodeIgniter\Filters\PageCache; +use CodeIgniter\Filters\PerformanceMetrics; use CodeIgniter\Filters\SecureHeaders; class Filters extends BaseConfig @@ -24,6 +26,27 @@ class Filters extends BaseConfig 'honeypot' => Honeypot::class, 'invalidchars' => InvalidChars::class, 'secureheaders' => SecureHeaders::class, + 'pagecache' => PageCache::class, + 'performance' => PerformanceMetrics::class, + ]; + + /** + * List of special required filters. + * + * The filters listed here is special. They are applied before and after + * other kinds of filters, and always applied even if a route does not exist. + * + * @var array> + */ + public array $required = [ + 'before' => [ + 'pagecache', + ], + 'after' => [ + 'pagecache', + 'performance', + 'toolbar', + ], ]; /** @@ -39,7 +62,6 @@ class Filters extends BaseConfig // 'invalidchars', ], 'after' => [ - 'toolbar', // 'honeypot', // 'secureheaders', ], diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php index 3e12e4e121b1..df1568560f20 100644 --- a/system/CodeIgniter.php +++ b/system/CodeIgniter.php @@ -17,6 +17,7 @@ use CodeIgniter\Events\Events; use CodeIgniter\Exceptions\FrameworkException; use CodeIgniter\Exceptions\PageNotFoundException; +use CodeIgniter\Filters\Filters; use CodeIgniter\HTTP\CLIRequest; use CodeIgniter\HTTP\DownloadResponse; use CodeIgniter\HTTP\Exceptions\RedirectException; @@ -339,30 +340,86 @@ public function run(?RouteCollectionInterface $routes = null, bool $returnRespon $this->getRequestObject(); $this->getResponseObject(); - try { - $this->forceSecureAccess(); + Events::trigger('pre_system'); - $this->response = $this->handleRequest($routes, config(Cache::class), $returnResponse); - } catch (ResponsableInterface|DeprecatedRedirectException $e) { - $this->outputBufferingEnd(); - if ($e instanceof DeprecatedRedirectException) { - $e = new RedirectException($e->getMessage(), $e->getCode(), $e); - } + $this->benchmark->stop('bootstrap'); - $this->response = $e->getResponse(); - } catch (PageNotFoundException $e) { - $this->response = $this->display404errors($e); - } catch (Throwable $e) { - $this->outputBufferingEnd(); + $this->benchmark->start('required_before_filters'); + // Start up the filters + $filters = Services::filters(); + // Run required before filters + $possibleResponse = $this->runRequiredBeforeFilters($filters); - throw $e; + // If a ResponseInterface instance is returned then send it back to the client and stop + if ($possibleResponse instanceof ResponseInterface) { + $this->response = $possibleResponse; + } else { + try { + $this->forceSecureAccess(); + + $this->response = $this->handleRequest($routes, config(Cache::class), $returnResponse); + } catch (ResponsableInterface|DeprecatedRedirectException $e) { + $this->outputBufferingEnd(); + if ($e instanceof DeprecatedRedirectException) { + $e = new RedirectException($e->getMessage(), $e->getCode(), $e); + } + + $this->response = $e->getResponse(); + } catch (PageNotFoundException $e) { + $this->response = $this->display404errors($e); + } catch (Throwable $e) { + $this->outputBufferingEnd(); + + throw $e; + } } + $this->runRequiredAfterFilters($filters); + if ($returnResponse) { return $this->response; } $this->sendResponse(); + + // Is there a post-system event? + Events::trigger('post_system'); + } + + /** + * Run required before filters. + */ + private function runRequiredBeforeFilters(Filters $filters): ?ResponseInterface + { + $possibleResponse = $filters->runRequired('before'); + $this->benchmark->stop('required_before_filters'); + + // If a ResponseInterface instance is returned then send it back to the client and stop + if ($possibleResponse instanceof ResponseInterface) { + return $possibleResponse; + } + + return null; + } + + /** + * Run required after filters. + */ + private function runRequiredAfterFilters(Filters $filters): void + { + $filters->setResponse($this->response); + + // After filter debug toolbar requires 'total_execution'. + $this->totalTime = $this->benchmark->getElapsedTime('total_execution'); + + // Run required after filters + $this->benchmark->start('required_after_filters'); + $response = $filters->runRequired('after'); + $this->benchmark->stop('required_after_filters'); + + if ($response instanceof ResponseInterface) { + $this->response = $response; + } } /** @@ -405,20 +462,11 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache return $this->response->setStatusCode(405)->setBody('Method Not Allowed'); } - Events::trigger('pre_system'); - - // Check for a cached page. Execution will stop - // if the page has been cached. - if (($response = $this->displayCache($cacheConfig)) instanceof ResponseInterface) { - return $response; - } - $routeFilters = $this->tryToRouteIt($routes); $uri = $this->request->getPath(); if ($this->enableFilters) { - // Start up the filters $filters = Services::filters(); // If any filters were specified within the routes file, @@ -478,9 +526,6 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache $filters = Services::filters(); $filters->setResponse($this->response); - // After filter debug toolbar requires 'total_execution'. - $this->totalTime = $this->benchmark->getElapsedTime('total_execution'); - // Run "after" filters $this->benchmark->start('after_filters'); $response = $filters->run($uri, 'after'); @@ -496,11 +541,6 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache ! $this->response instanceof DownloadResponse && ! $this->response instanceof RedirectResponse ) { - // Cache it without the performance metrics replaced - // so that we can have live speed updates along the way. - // Must be run after filters to preserve the Response headers. - $this->pageCache->make($this->request, $this->response); - // Save our current URI as the previous URI in the session // for safer, more accurate use with `previous_url()` helper function. $this->storePreviousURL(current_url(true)); @@ -508,9 +548,6 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache unset($uri); - // Is there a post-system event? - Events::trigger('post_system'); - return $this->response; } @@ -668,6 +705,7 @@ protected function forceSecureAccess($duration = 31_536_000) * * @throws Exception * + * @deprecated 4.5.0 PageCache required filter is used. No longer used. * @deprecated 4.4.2 The parameter $config is deprecated. No longer used. */ public function displayCache(Cache $config) @@ -750,6 +788,8 @@ protected function generateCacheName(Cache $config): string /** * Replaces the elapsed_time and memory_usage tag. + * + * @deprecated 4.5.0 PerformanceMetrics required filter is used. No longer used. */ public function displayPerformanceMetrics(string $output): string { @@ -774,6 +814,8 @@ public function displayPerformanceMetrics(string $output): string */ protected function tryToRouteIt(?RouteCollectionInterface $routes = null) { + $this->benchmark->start('routing'); + if ($routes === null) { $routes = Services::routes()->loadRoutes(); } @@ -783,9 +825,6 @@ protected function tryToRouteIt(?RouteCollectionInterface $routes = null) $uri = $this->request->getPath(); - $this->benchmark->stop('bootstrap'); - $this->benchmark->start('routing'); - $this->outputBufferingStart(); $this->controller = $this->router->handle($uri); @@ -1052,13 +1091,6 @@ public function spoofRequestMethod() */ protected function sendResponse() { - // Update the performance metrics - $body = $this->response->getBody(); - if ($body !== null) { - $output = $this->displayPerformanceMetrics($body); - $this->response->setBody($output); - } - $this->response->send(); } diff --git a/system/Debug/Toolbar/Collectors/Routes.php b/system/Debug/Toolbar/Collectors/Routes.php index 0420c19b92dd..46aa91644004 100644 --- a/system/Debug/Toolbar/Collectors/Routes.php +++ b/system/Debug/Toolbar/Collectors/Routes.php @@ -64,9 +64,17 @@ public function display(): array try { $method = new ReflectionMethod($router->controllerName(), $router->methodName()); } catch (ReflectionException $e) { - // If we're here, the method doesn't exist - // and is likely calculated in _remap. - $method = new ReflectionMethod($router->controllerName(), '_remap'); + try { + // If we're here, the method doesn't exist + // and is likely calculated in _remap. + $method = new ReflectionMethod($router->controllerName(), '_remap'); + } catch (ReflectionException) { + // If we're here, page cache is returned. The router is not executed. + return [ + 'matchedRoute' => [], + 'routes' => [], + ]; + } } } diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 9019c317aa1d..50312dfa1e89 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -224,6 +224,94 @@ public function run(string $uri, string $position = 'before') return $position === 'before' ? $this->request : $this->response; } + /** + * Runs required filters for the specified position. + * + * @return RequestInterface|ResponseInterface|string|null + * + * @throws FilterException + */ + public function runRequired(string $position = 'before') + { + // Set the toolbar filter to the last position to be executed + if ( + in_array('toolbar', $this->config->required['after'], true) + && ($count = count($this->config->required['after'])) > 1 + && $this->config->required['after'][$count - 1] !== 'toolbar' + ) { + array_splice( + $this->config->required['after'], + array_search('toolbar', $this->config->required['after'], true), + 1 + ); + $this->config->required['after'][] = 'toolbar'; + } + + $filtersClass = []; + + foreach ($this->config->required[$position] as $alias) { + if (! array_key_exists($alias, $this->config->aliases)) { + throw FilterException::forNoAlias($alias); + } + + if (is_array($this->config->aliases[$alias])) { + $filtersClass[$position] = array_merge($filtersClass[$position], $this->config->aliases[$alias]); + } else { + $filtersClass[$position][] = $this->config->aliases[$alias]; + } + } + + foreach ($filtersClass[$position] as $className) { + $class = new $className(); + + if (! $class instanceof FilterInterface) { + throw FilterException::forIncorrectInterface(get_class($class)); + } + + if ($position === 'before') { + $result = $class->before( + $this->request, + $this->argumentsClass[$className] ?? null + ); + + if ($result instanceof RequestInterface) { + $this->request = $result; + + continue; + } + + // If the response object was sent back, + // then send it and quit. + if ($result instanceof ResponseInterface) { + // short circuit - bypass any other filters + return $result; + } + // Ignore an empty result + if (empty($result)) { + continue; + } + + return $result; + } + + if ($position === 'after') { + $result = $class->after( + $this->request, + $this->response, + $this->argumentsClass[$className] ?? null + ); + + if ($result instanceof ResponseInterface) { + $this->response = $result; + + continue; + } + } + } + + return $position === 'before' ? $this->request : $this->response; + } + /** * Runs through our list of filters provided by the configuration * object to get them ready for use, including getting uri masks @@ -665,7 +753,7 @@ protected function processAliasesToClass(string $position) private function pathApplies(string $uri, $paths) { // empty path matches all - if (empty($paths)) { + if ($paths === '' || $paths === []) { return true; } From fe2880773bb142c4959a6ddf51b72a44648e6331 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 17 Oct 2023 17:10:20 +0900 Subject: [PATCH 04/32] test: update existing tests --- tests/system/Commands/FilterCheckTest.php | 2 +- tests/system/Commands/RoutesTest.php | 134 +++++++++--------- .../AutoRouteCollectorTest.php | 8 +- .../Utilities/Routes/FilterCollectorTest.php | 1 - tests/system/Test/FilterTestTraitTest.php | 3 +- 5 files changed, 74 insertions(+), 74 deletions(-) diff --git a/tests/system/Commands/FilterCheckTest.php b/tests/system/Commands/FilterCheckTest.php index a0b74099f818..65063e5d77e1 100644 --- a/tests/system/Commands/FilterCheckTest.php +++ b/tests/system/Commands/FilterCheckTest.php @@ -45,7 +45,7 @@ public function testFilterCheckDefinedRoute(): void command('filter:check get /'); $this->assertStringContainsString( - '|GET|/||toolbar|', + '|GET|/|||', str_replace(' ', '', $this->getBuffer()) ); } diff --git a/tests/system/Commands/RoutesTest.php b/tests/system/Commands/RoutesTest.php index 9cd24cbe0f6b..92ddb89763fd 100644 --- a/tests/system/Commands/RoutesTest.php +++ b/tests/system/Commands/RoutesTest.php @@ -61,16 +61,16 @@ public function testRoutesCommand(): void +---------+---------+---------------+----------------------------------------+----------------+---------------+ | Method | Route | Name | Handler | Before Filters | After Filters | +---------+---------+---------------+----------------------------------------+----------------+---------------+ - | GET | / | » | \App\Controllers\Home::index | | toolbar | - | GET | closure | » | (Closure) | | toolbar | - | GET | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | HEAD | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | POST | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | PUT | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | DELETE | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | OPTIONS | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | TRACE | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | CONNECT | testing | testing-index | \App\Controllers\TestController::index | | toolbar | + | GET | / | » | \App\Controllers\Home::index | | | + | GET | closure | » | (Closure) | | | + | GET | testing | testing-index | \App\Controllers\TestController::index | | | + | HEAD | testing | testing-index | \App\Controllers\TestController::index | | | + | POST | testing | testing-index | \App\Controllers\TestController::index | | | + | PUT | testing | testing-index | \App\Controllers\TestController::index | | | + | DELETE | testing | testing-index | \App\Controllers\TestController::index | | | + | OPTIONS | testing | testing-index | \App\Controllers\TestController::index | | | + | TRACE | testing | testing-index | \App\Controllers\TestController::index | | | + | CONNECT | testing | testing-index | \App\Controllers\TestController::index | | | | CLI | testing | testing-index | \App\Controllers\TestController::index | | | +---------+---------+---------------+----------------------------------------+----------------+---------------+ EOL; @@ -87,16 +87,16 @@ public function testRoutesCommandSortByHandler(): void +---------+---------+---------------+----------------------------------------+----------------+---------------+ | Method | Route | Name | Handler ↓ | Before Filters | After Filters | +---------+---------+---------------+----------------------------------------+----------------+---------------+ - | GET | closure | » | (Closure) | | toolbar | - | GET | / | » | \App\Controllers\Home::index | | toolbar | - | GET | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | HEAD | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | POST | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | PUT | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | DELETE | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | OPTIONS | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | TRACE | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | CONNECT | testing | testing-index | \App\Controllers\TestController::index | | toolbar | + | GET | closure | » | (Closure) | | | + | GET | / | » | \App\Controllers\Home::index | | | + | GET | testing | testing-index | \App\Controllers\TestController::index | | | + | HEAD | testing | testing-index | \App\Controllers\TestController::index | | | + | POST | testing | testing-index | \App\Controllers\TestController::index | | | + | PUT | testing | testing-index | \App\Controllers\TestController::index | | | + | DELETE | testing | testing-index | \App\Controllers\TestController::index | | | + | OPTIONS | testing | testing-index | \App\Controllers\TestController::index | | | + | TRACE | testing | testing-index | \App\Controllers\TestController::index | | | + | CONNECT | testing | testing-index | \App\Controllers\TestController::index | | | | CLI | testing | testing-index | \App\Controllers\TestController::index | | | +---------+---------+---------------+----------------------------------------+----------------+---------------+ EOL; @@ -114,17 +114,17 @@ public function testRoutesCommandHostHostname() +---------+---------+---------------+----------------------------------------+----------------+---------------+ | Method | Route | Name | Handler | Before Filters | After Filters | +---------+---------+---------------+----------------------------------------+----------------+---------------+ - | GET | / | » | \App\Controllers\Blog::index | | toolbar | - | GET | closure | » | (Closure) | | toolbar | - | GET | all | » | \App\Controllers\AllDomain::index | | toolbar | - | GET | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | HEAD | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | POST | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | PUT | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | DELETE | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | OPTIONS | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | TRACE | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | CONNECT | testing | testing-index | \App\Controllers\TestController::index | | toolbar | + | GET | / | » | \App\Controllers\Blog::index | | | + | GET | closure | » | (Closure) | | | + | GET | all | » | \App\Controllers\AllDomain::index | | | + | GET | testing | testing-index | \App\Controllers\TestController::index | | | + | HEAD | testing | testing-index | \App\Controllers\TestController::index | | | + | POST | testing | testing-index | \App\Controllers\TestController::index | | | + | PUT | testing | testing-index | \App\Controllers\TestController::index | | | + | DELETE | testing | testing-index | \App\Controllers\TestController::index | | | + | OPTIONS | testing | testing-index | \App\Controllers\TestController::index | | | + | TRACE | testing | testing-index | \App\Controllers\TestController::index | | | + | CONNECT | testing | testing-index | \App\Controllers\TestController::index | | | | CLI | testing | testing-index | \App\Controllers\TestController::index | | | +---------+---------+---------------+----------------------------------------+----------------+---------------+ EOL; @@ -142,17 +142,17 @@ public function testRoutesCommandHostSubdomain() +---------+---------+---------------+----------------------------------------+----------------+---------------+ | Method | Route | Name | Handler | Before Filters | After Filters | +---------+---------+---------------+----------------------------------------+----------------+---------------+ - | GET | / | » | \App\Controllers\Sub::index | | toolbar | - | GET | closure | » | (Closure) | | toolbar | - | GET | all | » | \App\Controllers\AllDomain::index | | toolbar | - | GET | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | HEAD | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | POST | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | PUT | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | DELETE | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | OPTIONS | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | TRACE | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | CONNECT | testing | testing-index | \App\Controllers\TestController::index | | toolbar | + | GET | / | » | \App\Controllers\Sub::index | | | + | GET | closure | » | (Closure) | | | + | GET | all | » | \App\Controllers\AllDomain::index | | | + | GET | testing | testing-index | \App\Controllers\TestController::index | | | + | HEAD | testing | testing-index | \App\Controllers\TestController::index | | | + | POST | testing | testing-index | \App\Controllers\TestController::index | | | + | PUT | testing | testing-index | \App\Controllers\TestController::index | | | + | DELETE | testing | testing-index | \App\Controllers\TestController::index | | | + | OPTIONS | testing | testing-index | \App\Controllers\TestController::index | | | + | TRACE | testing | testing-index | \App\Controllers\TestController::index | | | + | CONNECT | testing | testing-index | \App\Controllers\TestController::index | | | | CLI | testing | testing-index | \App\Controllers\TestController::index | | | +---------+---------+---------------+----------------------------------------+----------------+---------------+ EOL; @@ -174,19 +174,19 @@ public function testRoutesCommandAutoRouteImproved(): void +------------+--------------------------------+---------------+-----------------------------------------------------+----------------+---------------+ | Method | Route | Name | Handler | Before Filters | After Filters | +------------+--------------------------------+---------------+-----------------------------------------------------+----------------+---------------+ - | GET | / | » | \App\Controllers\Home::index | | toolbar | - | GET | closure | » | (Closure) | | toolbar | - | GET | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | HEAD | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | POST | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | PUT | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | DELETE | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | OPTIONS | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | TRACE | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | CONNECT | testing | testing-index | \App\Controllers\TestController::index | | toolbar | + | GET | / | » | \App\Controllers\Home::index | | | + | GET | closure | » | (Closure) | | | + | GET | testing | testing-index | \App\Controllers\TestController::index | | | + | HEAD | testing | testing-index | \App\Controllers\TestController::index | | | + | POST | testing | testing-index | \App\Controllers\TestController::index | | | + | PUT | testing | testing-index | \App\Controllers\TestController::index | | | + | DELETE | testing | testing-index | \App\Controllers\TestController::index | | | + | OPTIONS | testing | testing-index | \App\Controllers\TestController::index | | | + | TRACE | testing | testing-index | \App\Controllers\TestController::index | | | + | CONNECT | testing | testing-index | \App\Controllers\TestController::index | | | | CLI | testing | testing-index | \App\Controllers\TestController::index | | | - | GET(auto) | newautorouting[/..] | | \Tests\Support\Controllers\Newautorouting::getIndex | | toolbar | - | POST(auto) | newautorouting/save/../..[/..] | | \Tests\Support\Controllers\Newautorouting::postSave | | toolbar | + | GET(auto) | newautorouting[/..] | | \Tests\Support\Controllers\Newautorouting::getIndex | | | + | POST(auto) | newautorouting/save/../..[/..] | | \Tests\Support\Controllers\Newautorouting::postSave | | | +------------+--------------------------------+---------------+-----------------------------------------------------+----------------+---------------+ EOL; $this->assertStringContainsString($expected, $this->getBuffer()); @@ -205,20 +205,20 @@ public function testRoutesCommandRouteLegacy(): void +---------+------------------+---------------+----------------------------------------+----------------+---------------+ | Method | Route | Name | Handler | Before Filters | After Filters | +---------+------------------+---------------+----------------------------------------+----------------+---------------+ - | GET | / | » | \App\Controllers\Home::index | | toolbar | - | GET | closure | » | (Closure) | | toolbar | - | GET | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | HEAD | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | POST | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | PUT | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | DELETE | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | OPTIONS | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | TRACE | testing | testing-index | \App\Controllers\TestController::index | | toolbar | - | CONNECT | testing | testing-index | \App\Controllers\TestController::index | | toolbar | + | GET | / | » | \App\Controllers\Home::index | | | + | GET | closure | » | (Closure) | | | + | GET | testing | testing-index | \App\Controllers\TestController::index | | | + | HEAD | testing | testing-index | \App\Controllers\TestController::index | | | + | POST | testing | testing-index | \App\Controllers\TestController::index | | | + | PUT | testing | testing-index | \App\Controllers\TestController::index | | | + | DELETE | testing | testing-index | \App\Controllers\TestController::index | | | + | OPTIONS | testing | testing-index | \App\Controllers\TestController::index | | | + | TRACE | testing | testing-index | \App\Controllers\TestController::index | | | + | CONNECT | testing | testing-index | \App\Controllers\TestController::index | | | | CLI | testing | testing-index | \App\Controllers\TestController::index | | | - | auto | / | | \App\Controllers\Home::index | | toolbar | - | auto | home | | \App\Controllers\Home::index | | toolbar | - | auto | home/index[/...] | | \App\Controllers\Home::index | | toolbar | + | auto | / | | \App\Controllers\Home::index | | | + | auto | home | | \App\Controllers\Home::index | | | + | auto | home/index[/...] | | \App\Controllers\Home::index | | | +---------+------------------+---------------+----------------------------------------+----------------+---------------+ EOL; $this->assertStringContainsString($expected, $this->getBuffer()); diff --git a/tests/system/Commands/Utilities/Routes/AutoRouterImproved/AutoRouteCollectorTest.php b/tests/system/Commands/Utilities/Routes/AutoRouterImproved/AutoRouteCollectorTest.php index e705dad48869..3b502f328efc 100644 --- a/tests/system/Commands/Utilities/Routes/AutoRouterImproved/AutoRouteCollectorTest.php +++ b/tests/system/Commands/Utilities/Routes/AutoRouterImproved/AutoRouteCollectorTest.php @@ -66,7 +66,7 @@ public function testGetFilterMatches(): void '', '\\Tests\\Support\\Controllers\\Newautorouting::getIndex', '', - 'toolbar', + '', ], 1 => [ 'POST(auto)', @@ -74,7 +74,7 @@ public function testGetFilterMatches(): void '', '\\Tests\\Support\\Controllers\\Newautorouting::postSave', 'honeypot', - 'toolbar', + '', ], ]; $this->assertSame($expected, $routes); @@ -94,7 +94,7 @@ public function testGetFilterDoesNotMatch(): void '', '\\Tests\\Support\\Controllers\\Newautorouting::getIndex', '', - 'toolbar', + '', ], 1 => [ 'POST(auto)', @@ -102,7 +102,7 @@ public function testGetFilterDoesNotMatch(): void '', '\\Tests\\Support\\Controllers\\Newautorouting::postSave', '', - 'toolbar', + '', ], ]; $this->assertSame($expected, $routes); diff --git a/tests/system/Commands/Utilities/Routes/FilterCollectorTest.php b/tests/system/Commands/Utilities/Routes/FilterCollectorTest.php index 1363d468a49d..14e9c37819e9 100644 --- a/tests/system/Commands/Utilities/Routes/FilterCollectorTest.php +++ b/tests/system/Commands/Utilities/Routes/FilterCollectorTest.php @@ -36,7 +36,6 @@ public function testGet(): void 'before' => [ ], 'after' => [ - 'toolbar', ], ]; $this->assertSame($expected, $filters); diff --git a/tests/system/Test/FilterTestTraitTest.php b/tests/system/Test/FilterTestTraitTest.php index dda0ca0d3265..d51915922935 100644 --- a/tests/system/Test/FilterTestTraitTest.php +++ b/tests/system/Test/FilterTestTraitTest.php @@ -39,6 +39,7 @@ protected function setUp(): void // Apply the Custom Filter $this->filtersConfig->aliases['test-customfilter'] = Customfilter::class; $this->filtersConfig->globals['before'] = ['test-customfilter']; + $this->filtersConfig->globals['after'] = ['secureheaders']; } public function testDidRunTraitSetUp(): void @@ -110,7 +111,7 @@ public function testGetFiltersForRoute(): void public function testAssertFilter(): void { $this->assertFilter('/', 'before', 'test-customfilter'); - $this->assertFilter('/', 'after', 'toolbar'); + $this->assertFilter('/', 'after', 'secureheaders'); } public function testAssertNotFilter(): void From e0d80e8c0e799c1e349055b312b4cfc1653373ac Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 17 Oct 2023 17:29:28 +0900 Subject: [PATCH 05/32] fix: add check if filters are empty --- system/Filters/Filters.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 50312dfa1e89..edc38aab99f1 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -233,6 +233,10 @@ public function run(string $uri, string $position = 'before') */ public function runRequired(string $position = 'before') { + if (! isset($this->config->required[$position]) || $this->config->required[$position] === []) { + return $position === 'before' ? $this->request : $this->response; + } + // Set the toolbar filter to the last position to be executed if ( in_array('toolbar', $this->config->required['after'], true) From 7b9805e67ea335575da5921a647db2a025de757d Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 18 Oct 2023 11:37:37 +0900 Subject: [PATCH 06/32] fix: move event point post_system before sending Response Keep the current 4.4 behavior. --- system/CodeIgniter.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php index df1568560f20..901c0112eceb 100644 --- a/system/CodeIgniter.php +++ b/system/CodeIgniter.php @@ -376,14 +376,14 @@ public function run(?RouteCollectionInterface $routes = null, bool $returnRespon $this->runRequiredAfterFilters($filters); + // Is there a post-system event? + Events::trigger('post_system'); + if ($returnResponse) { return $this->response; } $this->sendResponse(); - - // Is there a post-system event? - Events::trigger('post_system'); } /** From a010895306699d6758d68036a86502c43680d1ef Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 18 Oct 2023 13:38:06 +0900 Subject: [PATCH 07/32] refactor: extract methods --- system/Filters/Filters.php | 99 ++++++++++++++++++++++++++------------ 1 file changed, 67 insertions(+), 32 deletions(-) diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index edc38aab99f1..88e4375400f3 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -173,55 +173,90 @@ public function run(string $uri, string $position = 'before') { $this->initialize(strtolower($uri)); - foreach ($this->filtersClass[$position] as $className) { + if ($position === 'before') { + $result = $this->runBefore($this->filtersClass[$position]); + + // If the response object was sent back, + // then send it and quit. + if ($result instanceof ResponseInterface) { + // short circuit - bypass any other filters + return $result; + } + + return $result; + } + + if ($position === 'after') { + $result = $this->runAfter($this->filtersClass[$position]); + } + + return $result; + } + + /** + * @return RequestInterface|ResponseInterface|string + */ + private function runBefore(array $filterClasses) + { + foreach ($filterClasses as $className) { $class = new $className(); if (! $class instanceof FilterInterface) { throw FilterException::forIncorrectInterface(get_class($class)); } - if ($position === 'before') { - $result = $class->before( - $this->request, - $this->argumentsClass[$className] ?? null - ); - - if ($result instanceof RequestInterface) { - $this->request = $result; + $result = $class->before( + $this->request, + $this->argumentsClass[$className] ?? null + ); - continue; - } + if ($result instanceof RequestInterface) { + $this->request = $result; - // If the response object was sent back, - // then send it and quit. - if ($result instanceof ResponseInterface) { - // short circuit - bypass any other filters - return $result; - } - // Ignore an empty result - if (empty($result)) { - continue; - } + continue; + } + // If the response object was sent back, + // then send it and quit. + if ($result instanceof ResponseInterface) { + // short circuit - bypass any other filters return $result; } - if ($position === 'after') { - $result = $class->after( - $this->request, - $this->response, - $this->argumentsClass[$className] ?? null - ); + // Ignore an empty result + if (empty($result)) { + continue; + } - if ($result instanceof ResponseInterface) { - $this->response = $result; + return $result; + } - continue; - } + return $this->request; + } + + private function runAfter(array $filterClasses): ResponseInterface + { + foreach ($filterClasses as $className) { + $class = new $className(); + + if (! $class instanceof FilterInterface) { + throw FilterException::forIncorrectInterface(get_class($class)); + } + + $result = $class->after( + $this->request, + $this->response, + $this->argumentsClass[$className] ?? null + ); + + if ($result instanceof ResponseInterface) { + $this->response = $result; + + continue; } } - return $position === 'before' ? $this->request : $this->response; + return $this->response; } /** From 7cd279789f88a9e384f644883b1a8111b50ee93d Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 18 Oct 2023 13:50:50 +0900 Subject: [PATCH 08/32] refactor: use private methods --- system/Filters/Filters.php | 52 +++++++------------------------------- 1 file changed, 9 insertions(+), 43 deletions(-) diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 88e4375400f3..aeea51e5b722 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -300,55 +300,21 @@ public function runRequired(string $position = 'before') } } - foreach ($filtersClass[$position] as $className) { - $class = new $className(); - - if (! $class instanceof FilterInterface) { - throw FilterException::forIncorrectInterface(get_class($class)); - } - - if ($position === 'before') { - $result = $class->before( - $this->request, - $this->argumentsClass[$className] ?? null - ); - - if ($result instanceof RequestInterface) { - $this->request = $result; - - continue; - } - - // If the response object was sent back, - // then send it and quit. - if ($result instanceof ResponseInterface) { - // short circuit - bypass any other filters - return $result; - } - // Ignore an empty result - if (empty($result)) { - continue; - } + if ($position === 'before') { + $result = $this->runBefore($filterClasses[$position]); + // If the response object was sent back, + // then send it and quit. + if ($result instanceof ResponseInterface) { + // short circuit - bypass any other filters return $result; } - if ($position === 'after') { - $result = $class->after( - $this->request, - $this->response, - $this->argumentsClass[$className] ?? null - ); - - if ($result instanceof ResponseInterface) { - $this->response = $result; - - continue; - } - } + return $result; } - return $position === 'before' ? $this->request : $this->response; + // After + return $this->runAfter($filterClasses[$position]); } /** From 43c3a0122ec3bad63fbf99df1abb908b6d28066c Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 18 Oct 2023 13:53:22 +0900 Subject: [PATCH 09/32] refactor: rename variable name --- system/Filters/Filters.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index aeea51e5b722..e71f94dba303 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -286,7 +286,7 @@ public function runRequired(string $position = 'before') $this->config->required['after'][] = 'toolbar'; } - $filtersClass = []; + $filterClasses = []; foreach ($this->config->required[$position] as $alias) { if (! array_key_exists($alias, $this->config->aliases)) { @@ -294,9 +294,9 @@ public function runRequired(string $position = 'before') } if (is_array($this->config->aliases[$alias])) { - $filtersClass[$position] = array_merge($filtersClass[$position], $this->config->aliases[$alias]); + $filterClasses[$position] = array_merge($filterClasses[$position], $this->config->aliases[$alias]); } else { - $filtersClass[$position][] = $this->config->aliases[$alias]; + $filterClasses[$position][] = $this->config->aliases[$alias]; } } @@ -717,7 +717,7 @@ private function registerArguments(string $name, array $arguments, bool $check = */ protected function processAliasesToClass(string $position) { - $filtersClass = []; + $filterClasses = []; foreach ($this->filters[$position] as $alias => $rules) { if (is_numeric($alias) && is_string($rules)) { @@ -729,18 +729,18 @@ protected function processAliasesToClass(string $position) } if (is_array($this->config->aliases[$alias])) { - $filtersClass = [...$filtersClass, ...$this->config->aliases[$alias]]; + $filterClasses = [...$filterClasses, ...$this->config->aliases[$alias]]; } else { - $filtersClass[] = $this->config->aliases[$alias]; + $filterClasses[] = $this->config->aliases[$alias]; } } - // when using enableFilter() we already write the class name in $filtersClass as well as the + // when using enableFilter() we already write the class name in $filterClasses 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]); + $this->filtersClass[$position] = array_merge($filterClasses, $this->filtersClass[$position]); } else { - $this->filtersClass[$position] = array_merge($this->filtersClass[$position], $filtersClass); + $this->filtersClass[$position] = array_merge($this->filtersClass[$position], $filterClasses); } // Since some filters like rate limiters rely on being executed once a request we filter em here. From 005b95fc2d0b750d724e2914dc4ea7f75cf550a3 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 18 Oct 2023 14:07:11 +0900 Subject: [PATCH 10/32] refactor: fix PHPStan errors --- phpstan-baseline.php | 2 +- system/Filters/Filters.php | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/phpstan-baseline.php b/phpstan-baseline.php index 9c06aa1d95b4..edfef74d0ef9 100644 --- a/phpstan-baseline.php +++ b/phpstan-baseline.php @@ -1923,7 +1923,7 @@ ]; $ignoreErrors[] = [ 'message' => '#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#', - 'count' => 2, + 'count' => 1, 'path' => __DIR__ . '/system/Filters/Filters.php', ]; $ignoreErrors[] = [ diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index e71f94dba303..fa92c34adf06 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -164,6 +164,7 @@ public function setResponse(ResponseInterface $response) * uri and position. * * @param string $uri URI path relative to baseURL + * @phpstan-param 'before'|'after' $position * * @return RequestInterface|ResponseInterface|string|null * @@ -186,11 +187,8 @@ public function run(string $uri, string $position = 'before') return $result; } - if ($position === 'after') { - $result = $this->runAfter($this->filtersClass[$position]); - } - - return $result; + // After + return $this->runAfter($this->filtersClass[$position]); } /** @@ -264,6 +262,8 @@ private function runAfter(array $filterClasses): ResponseInterface * * @return RequestInterface|ResponseInterface|string|null * + * @phpstan-param 'before'|'after' $position + * * @throws FilterException */ public function runRequired(string $position = 'before') From a4dd6696bb001cabb63c10a452a2a83557540c56 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 18 Oct 2023 14:32:46 +0900 Subject: [PATCH 11/32] refactor: remove meaningless code --- system/Filters/Filters.php | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index fa92c34adf06..7787f9e5240b 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -175,16 +175,7 @@ public function run(string $uri, string $position = 'before') $this->initialize(strtolower($uri)); if ($position === 'before') { - $result = $this->runBefore($this->filtersClass[$position]); - - // If the response object was sent back, - // then send it and quit. - if ($result instanceof ResponseInterface) { - // short circuit - bypass any other filters - return $result; - } - - return $result; + return $this->runBefore($this->filtersClass[$position]); } // After @@ -301,16 +292,7 @@ public function runRequired(string $position = 'before') } if ($position === 'before') { - $result = $this->runBefore($filterClasses[$position]); - - // If the response object was sent back, - // then send it and quit. - if ($result instanceof ResponseInterface) { - // short circuit - bypass any other filters - return $result; - } - - return $result; + return $this->runBefore($filterClasses[$position]); } // After From c0d775103ddebb7b36798c3add0bf0235a8a795a Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 19 Oct 2023 09:19:52 +0900 Subject: [PATCH 12/32] fix: forceSecureAccess() position It was before the event `pre_system`. --- system/CodeIgniter.php | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php index 901c0112eceb..663c278cc2f9 100644 --- a/system/CodeIgniter.php +++ b/system/CodeIgniter.php @@ -340,6 +340,20 @@ public function run(?RouteCollectionInterface $routes = null, bool $returnRespon $this->getRequestObject(); $this->getResponseObject(); + try { + $this->forceSecureAccess(); + } catch (RedirectException $e) { + $this->response = $e->getResponse(); + + if ($returnResponse) { + return $this->response; + } + + $this->sendResponse(); + + return; + } + Events::trigger('pre_system'); $this->benchmark->stop('bootstrap'); @@ -355,8 +369,6 @@ public function run(?RouteCollectionInterface $routes = null, bool $returnRespon $this->response = $possibleResponse; } else { try { - $this->forceSecureAccess(); - $this->response = $this->handleRequest($routes, config(Cache::class), $returnResponse); } catch (ResponsableInterface|DeprecatedRedirectException $e) { $this->outputBufferingEnd(); From df9a5f80b33221b9dd754b2f50b195cdfbf2eb41 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 3 Nov 2023 11:44:48 +0900 Subject: [PATCH 13/32] docs: add @return --- system/Filters/PageCache.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/system/Filters/PageCache.php b/system/Filters/PageCache.php index 664ef5c7db6a..d6fa407f5a0d 100644 --- a/system/Filters/PageCache.php +++ b/system/Filters/PageCache.php @@ -36,6 +36,8 @@ public function __construct() * Checks page cache and return if found. * * @param array|null $arguments + * + * @return ResponseInterface|void */ public function before(RequestInterface $request, $arguments = null) { From 2a4e587c3b2c539c69d68403f3a418f615096aac Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 5 Nov 2023 09:14:32 +0900 Subject: [PATCH 14/32] docs: fix by proofreading Co-authored-by: MGatner --- app/Config/Filters.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Config/Filters.php b/app/Config/Filters.php index b06bc4b32805..874a19a21a7b 100644 --- a/app/Config/Filters.php +++ b/app/Config/Filters.php @@ -33,7 +33,7 @@ class Filters extends BaseConfig /** * List of special required filters. * - * The filters listed here is special. They are applied before and after + * The filters listed here are special. They are applied before and after * other kinds of filters, and always applied even if a route does not exist. * * @var array> From db582a2e676d2fc569a8a9f80f0913695b4b7f4a Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 5 Nov 2023 09:15:26 +0900 Subject: [PATCH 15/32] refactor: use single iteration Co-authored-by: MGatner --- system/Filters/Filters.php | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 7787f9e5240b..6f5e9026b52c 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -264,19 +264,25 @@ public function runRequired(string $position = 'before') } // Set the toolbar filter to the last position to be executed - if ( - in_array('toolbar', $this->config->required['after'], true) - && ($count = count($this->config->required['after'])) > 1 - && $this->config->required['after'][$count - 1] !== 'toolbar' - ) { - array_splice( - $this->config->required['after'], - array_search('toolbar', $this->config->required['after'], true), - 1 - ); - $this->config->required['after'][] = 'toolbar'; + $afters = []; + $found = false; + + foreach ($this->config->required['after'] as $alias) { + if ($alias === 'toolbar') { + $found = true; + + continue; + } + + $afters[] = $alias; } + if ($found) { + $afters[] = 'toolbar'; + } + + $this->config->required['after'] = $afters; + $filterClasses = []; foreach ($this->config->required[$position] as $alias) { From a4998c2029f33ecf478dee0962d4cc96c798f52d Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 3 Nov 2023 11:45:27 +0900 Subject: [PATCH 16/32] refactor: move forceSecureAccess() to ForceHTTPS filter --- app/Config/App.php | 2 +- app/Config/Filters.php | 3 ++ system/CodeIgniter.php | 16 +------- system/Filters/ForceHTTPS.php | 63 ++++++++++++++++++++++++++++++++ tests/system/CodeIgniterTest.php | 4 +- 5 files changed, 72 insertions(+), 16 deletions(-) create mode 100644 system/Filters/ForceHTTPS.php diff --git a/app/Config/App.php b/app/Config/App.php index 6ae678625e7b..8c6fbf47e0d7 100644 --- a/app/Config/App.php +++ b/app/Config/App.php @@ -130,7 +130,7 @@ class App extends BaseConfig * If true, this will force every request made to this application to be * made via a secure connection (HTTPS). If the incoming request is not * secure, the user will be redirected to a secure version of the page - * and the HTTP Strict Transport Security header will be set. + * and the HTTP Strict Transport Security (HSTS) header will be set. */ public bool $forceGlobalSecureRequests = false; diff --git a/app/Config/Filters.php b/app/Config/Filters.php index 874a19a21a7b..5786eeefcdc3 100644 --- a/app/Config/Filters.php +++ b/app/Config/Filters.php @@ -5,6 +5,7 @@ use CodeIgniter\Config\BaseConfig; use CodeIgniter\Filters\CSRF; use CodeIgniter\Filters\DebugToolbar; +use CodeIgniter\Filters\ForceHTTPS; use CodeIgniter\Filters\Honeypot; use CodeIgniter\Filters\InvalidChars; use CodeIgniter\Filters\PageCache; @@ -26,6 +27,7 @@ class Filters extends BaseConfig 'honeypot' => Honeypot::class, 'invalidchars' => InvalidChars::class, 'secureheaders' => SecureHeaders::class, + 'forcehttps' => ForceHTTPS::class, 'pagecache' => PageCache::class, 'performance' => PerformanceMetrics::class, ]; @@ -40,6 +42,7 @@ class Filters extends BaseConfig */ public array $required = [ 'before' => [ + 'forcehttps', 'pagecache', ], 'after' => [ diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php index 663c278cc2f9..c3975303b547 100644 --- a/system/CodeIgniter.php +++ b/system/CodeIgniter.php @@ -340,20 +340,6 @@ public function run(?RouteCollectionInterface $routes = null, bool $returnRespon $this->getRequestObject(); $this->getResponseObject(); - try { - $this->forceSecureAccess(); - } catch (RedirectException $e) { - $this->response = $e->getResponse(); - - if ($returnResponse) { - return $this->response; - } - - $this->sendResponse(); - - return; - } - Events::trigger('pre_system'); $this->benchmark->stop('bootstrap'); @@ -700,6 +686,8 @@ protected function getResponseObject() * should be enforced for this URL. * * @return void + * + * @deprecated 4.5.0 No longer used. Moved to ForceHTTPS filter. */ protected function forceSecureAccess($duration = 31_536_000) { diff --git a/system/Filters/ForceHTTPS.php b/system/Filters/ForceHTTPS.php new file mode 100644 index 000000000000..dd56b7d86263 --- /dev/null +++ b/system/Filters/ForceHTTPS.php @@ -0,0 +1,63 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\Filters; + +use CodeIgniter\HTTP\Exceptions\RedirectException; +use CodeIgniter\HTTP\RequestInterface; +use CodeIgniter\HTTP\ResponseInterface; +use Config\App; +use Config\Services; + +/** + * Force HTTPS filter + */ +class ForceHTTPS implements FilterInterface +{ + /** + * Force Secure Site Access? If the config value 'forceGlobalSecureRequests' + * is true, will enforce that all requests to this site are made through + * HTTPS. Will redirect the user to the current page with HTTPS, as well + * as set the HTTP Strict Transport Security (HSTS) header for those browsers + * that support it. + * + * @param array|null $arguments + * + * @return ResponseInterface|void + */ + public function before(RequestInterface $request, $arguments = null) + { + $config = config(App::class); + + if ($config->forceGlobalSecureRequests !== true) { + return; + } + + $response = Services::response(); + + try { + force_https(YEAR, $request, $response); + } catch (RedirectException $e) { + return $e->getResponse(); + } + } + + /** + * We don't have anything to do here. + * + * @param array|null $arguments + * + * @return void + */ + public function after(RequestInterface $request, ResponseInterface $response, $arguments = null) + { + } +} diff --git a/tests/system/CodeIgniterTest.php b/tests/system/CodeIgniterTest.php index 5d12863604f6..ef9a42cc2f33 100644 --- a/tests/system/CodeIgniterTest.php +++ b/tests/system/CodeIgniterTest.php @@ -440,8 +440,10 @@ public function testRunForceSecure(): void $_SERVER['argv'] = ['index.php', '/']; $_SERVER['argc'] = 2; - $config = new App(); + $filterConfig = config(FiltersConfig::class); + $filterConfig->required['before'][] = 'forcehttps'; + $config = config(App::class); $config->forceGlobalSecureRequests = true; $codeigniter = new MockCodeIgniter($config); From 2c497d9231e7a5c9743202dad5fe338057f4fdf1 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 5 Nov 2023 11:09:17 +0900 Subject: [PATCH 17/32] docs: add comments to Config\Filters --- app/Config/Filters.php | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/app/Config/Filters.php b/app/Config/Filters.php index 5786eeefcdc3..2062bfab5d6f 100644 --- a/app/Config/Filters.php +++ b/app/Config/Filters.php @@ -38,17 +38,22 @@ class Filters extends BaseConfig * The filters listed here are special. They are applied before and after * other kinds of filters, and always applied even if a route does not exist. * - * @var array> + * Filters set by default provide framework functionality. If removed, + * those functions will no longer work. + * + * @see https://codeigniter.com/user_guide/incoming/filters.html#provided-filters + * + * @var array> */ public array $required = [ 'before' => [ - 'forcehttps', - 'pagecache', + 'forcehttps', // Force Global Secure Requests + 'pagecache', // Web Page Caching ], 'after' => [ - 'pagecache', - 'performance', - 'toolbar', + 'pagecache', // Web Page Caching + 'performance', // Performance Metrics + 'toolbar', // Debug Toolbar ], ]; From 10b6cadaa014077bcf036007dd1dff3c0289a30e Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 5 Nov 2023 11:34:51 +0900 Subject: [PATCH 18/32] docs: add about Required Filters --- user_guide_src/source/incoming/filters.rst | 27 ++++++++++++++++++- .../source/incoming/filters/013.php | 23 ++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 user_guide_src/source/incoming/filters/013.php diff --git a/user_guide_src/source/incoming/filters.rst b/user_guide_src/source/incoming/filters.rst index ab3f5b08fcb5..934a53137807 100644 --- a/user_guide_src/source/incoming/filters.rst +++ b/user_guide_src/source/incoming/filters.rst @@ -116,10 +116,33 @@ You can combine multiple filters into one alias, making complex sets of filters You should define as many aliases as you need. +.. _filters-required: + +$required +--------- + +.. versionadded:: 4.5.0 + +The second section allows you to define **Required Filters**. +They are special filters that are applied to every request made by the +framework. They are applied before and after other kinds of filters that are +explained below. + +.. note:: The Required Filters are always executed even if a route does not exist. + +You should take care with how many you use here, since it could have performance +implications to have too many run on every request. But the filters set by default +provide framework functionality. If removed, those functions will no longer work. +See :ref:`provided-filters` for details. + +Filters can be specified by adding their alias to either the ``before`` or ``after`` array: + +.. literalinclude:: filters/013.php + $globals -------- -The second section allows you to define any filters that should be applied to every valid request made by the framework. +The third section allows you to define any filters that should be applied to every valid 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 run on every request. @@ -244,6 +267,8 @@ 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. See :ref:`URI Routing ` for details. +.. _provided-filters: + **************** Provided Filters **************** diff --git a/user_guide_src/source/incoming/filters/013.php b/user_guide_src/source/incoming/filters/013.php new file mode 100644 index 000000000000..5e7154bd4c9e --- /dev/null +++ b/user_guide_src/source/incoming/filters/013.php @@ -0,0 +1,23 @@ + [ + 'forcehttps', // Force Global Secure Requests + 'pagecache', // Web Page Caching + ], + 'after' => [ + 'pagecache', // Web Page Caching + 'performance', // Performance Metrics + 'toolbar', // Debug Toolbar + ], + ]; + + // ... +} From 91994fa5ec393dc69db6498f29a98e5ebb1fe409 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 5 Nov 2023 11:35:22 +0900 Subject: [PATCH 19/32] docs: update Filter Execution Order --- user_guide_src/source/incoming/filters.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/user_guide_src/source/incoming/filters.rst b/user_guide_src/source/incoming/filters.rst index 934a53137807..b1787ae6e79c 100644 --- a/user_guide_src/source/incoming/filters.rst +++ b/user_guide_src/source/incoming/filters.rst @@ -225,8 +225,10 @@ Filter Execution Order Filters are executed in the following order: -- **Before Filters**: globals → methods → filters → route -- **After Filters**: route → filters → globals +- **Before Filters**: required → globals → methods → filters → route +- **After Filters**: route → filters → globals → required + +.. note:: The *required* filters can be used since v4.5.0. .. 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 From 7646a44bedfbb5bcc36f1d76969dca37a3dfe659 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 5 Nov 2023 11:35:59 +0900 Subject: [PATCH 20/32] docs: add about ForceHTTPS and PerformanceMetrics --- user_guide_src/source/incoming/filters.rst | 48 +++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/user_guide_src/source/incoming/filters.rst b/user_guide_src/source/incoming/filters.rst index b1787ae6e79c..736323d773c5 100644 --- a/user_guide_src/source/incoming/filters.rst +++ b/user_guide_src/source/incoming/filters.rst @@ -275,10 +275,56 @@ See :ref:`URI Routing ` for details. Provided Filters **************** -The filters bundled with CodeIgniter4 are: :doc:`Honeypot <../libraries/honeypot>`, :ref:`CSRF `, ``InvalidChars``, ``SecureHeaders``, and :ref:`DebugToolbar `. +The filters bundled with CodeIgniter4 are: + +- ``csrf`` => :ref:`CSRF ` +- ``toolbar`` => :ref:`DebugToolbar ` +- ``honeypot`` => :doc:`Honeypot <../libraries/honeypot>` +- ``invalidchars`` => :ref:`invalidchars` +- ``secureheaders`` => :ref:`secureheaders` +- ``forcehttps`` => :ref:`forcehttps` +- ``pagecache`` => :doc:`PageCache <../general/caching>` +- ``performance`` => :ref:`performancemetrics` .. note:: The filters are executed in the order defined in the config file. However, if enabled, ``DebugToolbar`` is always executed last because it should be able to capture everything that happens in the other filters. +.. _forcehttps: + +ForceHTTPS +========== + +.. versionadded:: 4.5.0 + +This filter provides the "Force Global Secure Requests" feature. + +If you set ``Config\App:$forceGlobalSecureRequests`` to true, this will force +every request made to this application to be made via a secure connection (HTTPS). +If the incoming request is not secure, the user will be redirected to a secure +version of the page and the HTTP Strict Transport Security (HSTS) header will be +set. + +.. _performancemetrics: + +PerformanceMetrics +================== + +.. versionadded:: 4.5.0 + +This filter provides the pseudo-variables for performance metrics. + +If you would like to display the total elapsed time from the moment CodeIgniter +starts to the moment right before the final output is sent to the browser, +simply place this pseudo-variable in one of your view:: + + {elapsed_time} + +If you would like to show your memory usage in your view files, use this +pseudo-variable:: + + {memory_usage} + +If you don't need this feature, remove ``'performance'`` from ``$required['after']``. + .. _invalidchars: InvalidChars From 0cc0654e8d1440d88a5d4ec828f33bd37ebd9cfa Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 9 Nov 2023 13:54:21 +0900 Subject: [PATCH 21/32] docs: change TOC depth to 3 --- user_guide_src/source/incoming/filters.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/user_guide_src/source/incoming/filters.rst b/user_guide_src/source/incoming/filters.rst index 736323d773c5..73646f1d3921 100644 --- a/user_guide_src/source/incoming/filters.rst +++ b/user_guide_src/source/incoming/filters.rst @@ -4,7 +4,7 @@ Controller Filters .. contents:: :local: - :depth: 2 + :depth: 3 Controller Filters allow you to perform actions either before or after the controllers execute. Unlike :doc:`events <../extending/events>`, you can choose the specific URIs or routes in which the filters will be applied to. Before filters may From 57fb443ebf31b166a68ed7f5b3309c7aad9c9c96 Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 9 Nov 2023 13:55:20 +0900 Subject: [PATCH 22/32] docs: update filter property name --- user_guide_src/source/testing/debugging.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/user_guide_src/source/testing/debugging.rst b/user_guide_src/source/testing/debugging.rst index 6c589ead52aa..af8e2b28c63c 100644 --- a/user_guide_src/source/testing/debugging.rst +++ b/user_guide_src/source/testing/debugging.rst @@ -70,7 +70,9 @@ constant ``CI_DEBUG`` is defined and its value is truthy. This is defined in the .. note:: The Debug Toolbar is not displayed when your ``baseURL`` setting (in **app/Config/App.php** or ``app.baseURL`` in **.env**) does not match your actual URL. The toolbar itself is displayed as an :doc:`After Filter `. You can stop it from ever -running by removing it from the ``$globals`` property of **app/Config/Filters.php**. +running by removing ``'toolbar'`` from the ``$required`` (or ``$globals``) property of **app/Config/Filters.php**. + +.. note:: Prior to v4.5.0, the toolbar was set to ``$globals`` by default. Choosing What to Show --------------------- From af366ad9530dbc45937cdee16599ba263b71e3be Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 9 Nov 2023 13:57:04 +0900 Subject: [PATCH 23/32] docs: add changelog --- user_guide_src/source/changelogs/v4.5.0.rst | 29 +++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/user_guide_src/source/changelogs/v4.5.0.rst b/user_guide_src/source/changelogs/v4.5.0.rst index e4d72f5a1b08..7a4ffb2fdde7 100644 --- a/user_guide_src/source/changelogs/v4.5.0.rst +++ b/user_guide_src/source/changelogs/v4.5.0.rst @@ -255,6 +255,35 @@ Libraries Helpers and Functions ===================== +Controller Filters +================== + +The :ref:`Required Filters ` has been introduced. They are new +special filters that are applied before and after other kinds of filters, and +always applied even if a route does not exist. + +The following existing functionalities have been reimplemented as Required Filters. + +- :ref:`Force Global Secure Requests ` +- :doc:`../general/caching` +- :ref:`performancemetrics` +- :ref:`the-debug-toolbar` + +The Benchmark **Timers** used by Debug Toolbar now collect *Required Before Filters* +and *Required After Filters* data. + +The benchmark points have been changed: + +- Before + + - ``bootstrap``: Creating Request and Response objects, Event ``pre_system``, Instantiating RouteCollection object, Loading Routes files, Instantiating Router object, + - ``routing``: Routing, +- After + + - ``bootstrap``: Creating Request and Response objects, Event ``pre_system``. + - ``required_before_filters``: Instantiating Filters object, Running *Required Before Filters*. + - ``routing``: Instantiating RouteCollection object, Loading Routes files, Instantiating Router object, Routing, + Others ====== From cb2516ab0e0da28bbfcd7d61cc9bd41b642f05ec Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 9 Nov 2023 14:16:48 +0900 Subject: [PATCH 24/32] docs: add upgrade --- user_guide_src/source/changelogs/v4.5.0.rst | 6 ++-- .../source/installation/upgrade_450.rst | 34 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/user_guide_src/source/changelogs/v4.5.0.rst b/user_guide_src/source/changelogs/v4.5.0.rst index 7a4ffb2fdde7..a6a85dd25e66 100644 --- a/user_guide_src/source/changelogs/v4.5.0.rst +++ b/user_guide_src/source/changelogs/v4.5.0.rst @@ -255,8 +255,10 @@ Libraries Helpers and Functions ===================== -Controller Filters -================== +.. _v450-required-filters: + +Required Filters +================ The :ref:`Required Filters ` has been introduced. They are new special filters that are applied before and after other kinds of filters, and diff --git a/user_guide_src/source/installation/upgrade_450.rst b/user_guide_src/source/installation/upgrade_450.rst index 6f756d0f8f7e..36b749b1a6f6 100644 --- a/user_guide_src/source/installation/upgrade_450.rst +++ b/user_guide_src/source/installation/upgrade_450.rst @@ -15,6 +15,40 @@ Please refer to the upgrade instructions corresponding to your installation meth Mandatory File Changes ********************** +Config Files +============ + +app/Config/Filters.php +---------------------- + +Required Filters have been added, so add that settings. See +:ref:`Upgrading ` for details. + +Add the following items in the ``$aliases`` property:: + + public array $aliases = [ + // ... + 'forcehttps' => \CodeIgniter\Filters\ForceHTTPS::class, + 'pagecache' => \CodeIgniter\Filters\PageCache::class, + 'performance' => \CodeIgniter\Filters\PerformanceMetrics::class, + ]; + +Add the new property ``$required``, and set the following:: + + public array $required = [ + 'before' => [ + 'forcehttps', // Force Global Secure Requests + 'pagecache', // Web Page Caching + ], + 'after' => [ + 'pagecache', // Web Page Caching + 'performance', // Performance Metrics + 'toolbar', // Debug Toolbar + ], + ]; + +Remove ``'toolbar'`` from the ``$global['after']``. + Breaking Changes **************** From 8aa9169abbb2d60aefccfc138b59c5ecd542428c Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 19 Nov 2023 09:28:09 +0900 Subject: [PATCH 25/32] docs: fix by proofreading Co-authored-by: MGatner --- user_guide_src/source/changelogs/v4.5.0.rst | 2 +- user_guide_src/source/incoming/filters.rst | 2 +- user_guide_src/source/installation/upgrade_450.rst | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/user_guide_src/source/changelogs/v4.5.0.rst b/user_guide_src/source/changelogs/v4.5.0.rst index a6a85dd25e66..09e64936179f 100644 --- a/user_guide_src/source/changelogs/v4.5.0.rst +++ b/user_guide_src/source/changelogs/v4.5.0.rst @@ -260,7 +260,7 @@ Helpers and Functions Required Filters ================ -The :ref:`Required Filters ` has been introduced. They are new +New :ref:`Required Filters ` have been introduced. They are special filters that are applied before and after other kinds of filters, and always applied even if a route does not exist. diff --git a/user_guide_src/source/incoming/filters.rst b/user_guide_src/source/incoming/filters.rst index 73646f1d3921..f33f94508670 100644 --- a/user_guide_src/source/incoming/filters.rst +++ b/user_guide_src/source/incoming/filters.rst @@ -314,7 +314,7 @@ This filter provides the pseudo-variables for performance metrics. If you would like to display the total elapsed time from the moment CodeIgniter starts to the moment right before the final output is sent to the browser, -simply place this pseudo-variable in one of your view:: +simply place this pseudo-variable in one of your views:: {elapsed_time} diff --git a/user_guide_src/source/installation/upgrade_450.rst b/user_guide_src/source/installation/upgrade_450.rst index 36b749b1a6f6..b43eeaa75a7c 100644 --- a/user_guide_src/source/installation/upgrade_450.rst +++ b/user_guide_src/source/installation/upgrade_450.rst @@ -21,7 +21,7 @@ Config Files app/Config/Filters.php ---------------------- -Required Filters have been added, so add that settings. See +Required Filters have been added, so add that setting. See :ref:`Upgrading ` for details. Add the following items in the ``$aliases`` property:: From 523e414a868aa8b5f404ab1133f2d85c5ba2956a Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 19 Nov 2023 09:30:41 +0900 Subject: [PATCH 26/32] docs: add @internal --- system/Filters/Filters.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 6f5e9026b52c..528c65e66b48 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -256,6 +256,8 @@ private function runAfter(array $filterClasses): ResponseInterface * @phpstan-param 'before'|'after' $position * * @throws FilterException + * + * @internal */ public function runRequired(string $position = 'before') { From 71cc57ab3b70da6ffeef4ec2f0a91197afe59cad Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 19 Nov 2023 10:06:04 +0900 Subject: [PATCH 27/32] config: add system/Config/Filters.php --- app/Config/Filters.php | 12 ++-- system/Config/Filters.php | 112 +++++++++++++++++++++++++++++++++++++ system/Filters/Filters.php | 3 +- 3 files changed, 121 insertions(+), 6 deletions(-) create mode 100644 system/Config/Filters.php diff --git a/app/Config/Filters.php b/app/Config/Filters.php index 2062bfab5d6f..35b3bf64eac1 100644 --- a/app/Config/Filters.php +++ b/app/Config/Filters.php @@ -2,7 +2,7 @@ namespace Config; -use CodeIgniter\Config\BaseConfig; +use CodeIgniter\Config\Filters as BaseFilters; use CodeIgniter\Filters\CSRF; use CodeIgniter\Filters\DebugToolbar; use CodeIgniter\Filters\ForceHTTPS; @@ -12,14 +12,16 @@ use CodeIgniter\Filters\PerformanceMetrics; use CodeIgniter\Filters\SecureHeaders; -class Filters extends BaseConfig +class Filters extends BaseFilters { /** * Configures aliases for Filter classes to * make reading things nicer and simpler. * - * @var array> [filter_name => classname] - * or [filter_name => [classname1, classname2, ...]] + * @var array> + * + * [filter_name => classname] + * or [filter_name => [classname1, classname2, ...]] */ public array $aliases = [ 'csrf' => CSRF::class, @@ -43,7 +45,7 @@ class Filters extends BaseConfig * * @see https://codeigniter.com/user_guide/incoming/filters.html#provided-filters * - * @var array> + * @var array{before: list, after: list} */ public array $required = [ 'before' => [ diff --git a/system/Config/Filters.php b/system/Config/Filters.php new file mode 100644 index 000000000000..535f7d9d6663 --- /dev/null +++ b/system/Config/Filters.php @@ -0,0 +1,112 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\Config; + +use CodeIgniter\Filters\CSRF; +use CodeIgniter\Filters\DebugToolbar; +use CodeIgniter\Filters\ForceHTTPS; +use CodeIgniter\Filters\Honeypot; +use CodeIgniter\Filters\InvalidChars; +use CodeIgniter\Filters\PageCache; +use CodeIgniter\Filters\PerformanceMetrics; +use CodeIgniter\Filters\SecureHeaders; + +/** + * Filters configuration + */ +class Filters extends BaseConfig +{ + /** + * Configures aliases for Filter classes to + * make reading things nicer and simpler. + * + * @var array> + * + * [filter_name => classname] + * or [filter_name => [classname1, classname2, ...]] + */ + public array $aliases = [ + 'csrf' => CSRF::class, + 'toolbar' => DebugToolbar::class, + 'honeypot' => Honeypot::class, + 'invalidchars' => InvalidChars::class, + 'secureheaders' => SecureHeaders::class, + 'forcehttps' => ForceHTTPS::class, + 'pagecache' => PageCache::class, + 'performance' => PerformanceMetrics::class, + ]; + + /** + * List of special required filters. + * + * The filters listed here are special. They are applied before and after + * other kinds of filters, and always applied even if a route does not exist. + * + * Filters set by default provide framework functionality. If removed, + * those functions will no longer work. + * + * @see https://codeigniter.com/user_guide/incoming/filters.html#provided-filters + * + * @var array{before: list, after: list} + */ + public array $required = [ + 'before' => [ + 'forcehttps', // Force Global Secure Requests + 'pagecache', // Web Page Caching + ], + 'after' => [ + 'pagecache', // Web Page Caching + 'performance', // Performance Metrics + 'toolbar', // Debug Toolbar + ], + ]; + + /** + * List of filter aliases that are always + * applied before and after every request. + * + * @var array>>|array> + */ + public array $globals = [ + 'before' => [ + // 'honeypot', + // 'csrf', + // 'invalidchars', + ], + 'after' => [ + // 'honeypot', + // 'secureheaders', + ], + ]; + + /** + * List of filter aliases that works on a + * particular HTTP method (GET, POST, etc.). + * + * Example: + * 'post' => ['foo', 'bar'] + * + * If you use this, you should disable auto-routing because auto-routing + * permits any HTTP method to access a controller. Accessing the controller + * with a method you don't expect could bypass the filter. + */ + public array $methods = []; + + /** + * List of filter aliases that should run on any + * before or after URI patterns. + * + * Example: + * 'isLoggedIn' => ['before' => ['account/*', 'profiles/*']] + */ + public array $filters = []; +} diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 528c65e66b48..b3399bb3d37b 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -11,6 +11,7 @@ namespace CodeIgniter\Filters; +use CodeIgniter\Config\Filters as BaseFiltersConfig; use CodeIgniter\Exceptions\ConfigException; use CodeIgniter\Filters\Exceptions\FilterException; use CodeIgniter\HTTP\RequestInterface; @@ -141,7 +142,7 @@ private function discoverFilters(): void $className = $locator->getClassname($file); // Don't include our main Filter config again... - if ($className === FiltersConfig::class) { + if ($className === FiltersConfig::class || $className === BaseFiltersConfig::class) { continue; } From baeea0ff24091f0ed58f928bb7d7c166e69128bd Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 19 Nov 2023 10:44:01 +0900 Subject: [PATCH 28/32] refactor: extract setToolbarToLast() --- system/Filters/Filters.php | 54 +++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index b3399bb3d37b..63aa86e16b70 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -266,25 +266,10 @@ public function runRequired(string $position = 'before') return $position === 'before' ? $this->request : $this->response; } - // Set the toolbar filter to the last position to be executed - $afters = []; - $found = false; - - foreach ($this->config->required['after'] as $alias) { - if ($alias === 'toolbar') { - $found = true; - - continue; - } - - $afters[] = $alias; - } - - if ($found) { - $afters[] = 'toolbar'; } - $this->config->required['after'] = $afters; + // Set the toolbar filter to the last position to be executed + $this->config->required['after'] = $this->setToolbarToLast($this->config->required['after']); $filterClasses = []; @@ -308,6 +293,33 @@ public function runRequired(string $position = 'before') return $this->runAfter($filterClasses[$position]); } + /** + * Set the toolbar filter to the last position to be executed. + * + * @param list $filters `after` filter array + */ + private function setToolbarToLast(array $filters): array + { + $afters = []; + $found = false; + + foreach ($filters as $alias) { + if ($alias === 'toolbar') { + $found = true; + + continue; + } + + $afters[] = $alias; + } + + if ($found) { + $afters[] = 'toolbar'; + } + + return $afters; + } + /** * Runs through our list of filters provided by the configuration * object to get them ready for use, including getting uri masks @@ -342,13 +354,7 @@ public function initialize(?string $uri = null) } // Set the toolbar filter to the last position to be executed - if (in_array('toolbar', $this->filters['after'], true) - && ($count = count($this->filters['after'])) > 1 - && $this->filters['after'][$count - 1] !== 'toolbar' - ) { - array_splice($this->filters['after'], array_search('toolbar', $this->filters['after'], true), 1); - $this->filters['after'][] = 'toolbar'; - } + $this->filters['after'] = $this->setToolbarToLast($this->filters['after']); $this->processAliasesToClass('before'); $this->processAliasesToClass('after'); From 64d69658745f2f52893250a8316065f9efc99f50 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 19 Nov 2023 11:11:56 +0900 Subject: [PATCH 29/32] feat: to work even if Config\Filters is not updated --- system/Filters/Filters.php | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 63aa86e16b70..420de1834c8f 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -262,26 +262,36 @@ private function runAfter(array $filterClasses): ResponseInterface */ public function runRequired(string $position = 'before') { - if (! isset($this->config->required[$position]) || $this->config->required[$position] === []) { - return $position === 'before' ? $this->request : $this->response; + // For backward compatibility. For users who do not update Config\Filters. + if (! isset($this->config->required[$position])) { + $baseConfig = config(BaseFiltersConfig::class); + $filters = $baseConfig->required[$position]; + $aliases = $baseConfig->aliases; + } else { + $filters = $this->config->required[$position]; + $aliases = $this->config->aliases; } + if ($filters === []) { + return $position === 'before' ? $this->request : $this->response; } - // Set the toolbar filter to the last position to be executed - $this->config->required['after'] = $this->setToolbarToLast($this->config->required['after']); + if ($position === 'after') { + // Set the toolbar filter to the last position to be executed + $filters = $this->setToolbarToLast($filters); + } $filterClasses = []; - foreach ($this->config->required[$position] as $alias) { - if (! array_key_exists($alias, $this->config->aliases)) { + foreach ($filters as $alias) { + if (! array_key_exists($alias, $aliases)) { throw FilterException::forNoAlias($alias); } - if (is_array($this->config->aliases[$alias])) { - $filterClasses[$position] = array_merge($filterClasses[$position], $this->config->aliases[$alias]); + if (is_array($aliases[$alias])) { + $filterClasses[$position] = array_merge($filterClasses[$position], $aliases[$alias]); } else { - $filterClasses[$position][] = $this->config->aliases[$alias]; + $filterClasses[$position][] = $aliases[$alias]; } } From 121756162a2515988257f7f63367a479b6d1e258 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 19 Nov 2023 11:39:59 +0900 Subject: [PATCH 30/32] fix: toolbar does not work in globals filters --- system/CodeIgniter.php | 6 +++--- system/Filters/Filters.php | 14 ++++++++++---- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php index c3975303b547..d71f1661564e 100644 --- a/system/CodeIgniter.php +++ b/system/CodeIgniter.php @@ -407,9 +407,6 @@ private function runRequiredAfterFilters(Filters $filters): void { $filters->setResponse($this->response); - // After filter debug toolbar requires 'total_execution'. - $this->totalTime = $this->benchmark->getElapsedTime('total_execution'); - // Run required after filters $this->benchmark->start('required_after_filters'); $response = $filters->runRequired('after'); @@ -760,6 +757,9 @@ public function cachePage(Cache $config) */ public function getPerformanceStats(): array { + // After filter debug toolbar requires 'total_execution'. + $this->totalTime = $this->benchmark->getElapsedTime('total_execution'); + return [ 'startTime' => $this->startTime, 'totalTime' => $this->totalTime, diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 420de1834c8f..17f5f88a0660 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -277,8 +277,13 @@ public function runRequired(string $position = 'before') } if ($position === 'after') { - // Set the toolbar filter to the last position to be executed - $filters = $this->setToolbarToLast($filters); + if (in_array('toolbar', $this->filters['after'], true)) { + // It was already run in globals filters. So remove it. + $filters = $this->setToolbarToLast($filters, true); + } else { + // Set the toolbar filter to the last position to be executed + $filters = $this->setToolbarToLast($filters); + } } $filterClasses = []; @@ -307,8 +312,9 @@ public function runRequired(string $position = 'before') * Set the toolbar filter to the last position to be executed. * * @param list $filters `after` filter array + * @param bool $remove if true, remove `toolbar` filter */ - private function setToolbarToLast(array $filters): array + private function setToolbarToLast(array $filters, bool $remove = false): array { $afters = []; $found = false; @@ -323,7 +329,7 @@ private function setToolbarToLast(array $filters): array $afters[] = $alias; } - if ($found) { + if ($found && ! $remove) { $afters[] = 'toolbar'; } From 3ed4329b6e2c2d31c392d266c86bf581717abc56 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 19 Nov 2023 11:54:48 +0900 Subject: [PATCH 31/32] docs: app/Config/Filters.php changes are not mandatory --- .../source/installation/upgrade_450.rst | 72 ++++++++++--------- 1 file changed, 38 insertions(+), 34 deletions(-) diff --git a/user_guide_src/source/installation/upgrade_450.rst b/user_guide_src/source/installation/upgrade_450.rst index b43eeaa75a7c..c81ad21ab1d7 100644 --- a/user_guide_src/source/installation/upgrade_450.rst +++ b/user_guide_src/source/installation/upgrade_450.rst @@ -15,40 +15,6 @@ Please refer to the upgrade instructions corresponding to your installation meth Mandatory File Changes ********************** -Config Files -============ - -app/Config/Filters.php ----------------------- - -Required Filters have been added, so add that setting. See -:ref:`Upgrading ` for details. - -Add the following items in the ``$aliases`` property:: - - public array $aliases = [ - // ... - 'forcehttps' => \CodeIgniter\Filters\ForceHTTPS::class, - 'pagecache' => \CodeIgniter\Filters\PageCache::class, - 'performance' => \CodeIgniter\Filters\PerformanceMetrics::class, - ]; - -Add the new property ``$required``, and set the following:: - - public array $required = [ - 'before' => [ - 'forcehttps', // Force Global Secure Requests - 'pagecache', // Web Page Caching - ], - 'after' => [ - 'pagecache', // Web Page Caching - 'performance', // Performance Metrics - 'toolbar', // Debug Toolbar - ], - ]; - -Remove ``'toolbar'`` from the ``$global['after']``. - Breaking Changes **************** @@ -251,6 +217,44 @@ and it is recommended that you merge the updated versions with your application: Config ------ +app/Config/Filters.php +^^^^^^^^^^^^^^^^^^^^^^ + +Required Filters have been added, so the following changes were made. See also +:ref:`Upgrading `. + +The base class has been changed:: + + class Filters extends \CodeIgniter\Config\Filters + +The following items are added in the ``$aliases`` property:: + + public array $aliases = [ + // ... + 'forcehttps' => \CodeIgniter\Filters\ForceHTTPS::class, + 'pagecache' => \CodeIgniter\Filters\PageCache::class, + 'performance' => \CodeIgniter\Filters\PerformanceMetrics::class, + ]; + +A new property ``$required`` is added, and set as the following:: + + public array $required = [ + 'before' => [ + 'forcehttps', // Force Global Secure Requests + 'pagecache', // Web Page Caching + ], + 'after' => [ + 'pagecache', // Web Page Caching + 'performance', // Performance Metrics + 'toolbar', // Debug Toolbar + ], + ]; + +The ``'toolbar'`` in the ``$global['after']`` was removed. + +Others +^^^^^^ + - app/Config/Database.php - The default value of ``charset`` in ``$default`` has been change to ``utf8mb4``. - The default value of ``DBCollat`` in ``$default`` has been change to ``utf8mb4_general_ci``. From 709b9f2e0c86a9d51a9291e7f235111081a085eb Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 20 Nov 2023 06:11:59 +0900 Subject: [PATCH 32/32] docs: add @phpstan-ignore-line --- 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 17f5f88a0660..25ca9a9ec5f7 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -264,7 +264,7 @@ public function runRequired(string $position = 'before') { // For backward compatibility. For users who do not update Config\Filters. if (! isset($this->config->required[$position])) { - $baseConfig = config(BaseFiltersConfig::class); + $baseConfig = config(BaseFiltersConfig::class); // @phpstan-ignore-line $filters = $baseConfig->required[$position]; $aliases = $baseConfig->aliases; } else {