From b8f8acbecc5ef349180c2b47e88b02b7c933dac9 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Tue, 14 Jul 2026 20:37:22 +0200 Subject: [PATCH 1/6] refactor: simplify handleRequest method in CodeIgniter --- system/CodeIgniter.php | 25 ++++++++++++------------ tests/system/CodeIgniterTest.php | 33 ++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 13 deletions(-) diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php index a291809938e5..61f5faeab726 100644 --- a/system/CodeIgniter.php +++ b/system/CodeIgniter.php @@ -35,6 +35,7 @@ use Config\Cache; use Config\Feature; use Config\Kint as KintConfig; +use Config\Routing; use Config\Services; use Exception; use Kint; @@ -475,8 +476,7 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache if ($routeFilters !== null) { $filters->enableFilters($routeFilters, 'before'); - $oldFilterOrder = config(Feature::class)->oldFilterOrder ?? false; // @phpstan-ignore nullCoalesce.property - if (! $oldFilterOrder) { + if (! config(Feature::class)->oldFilterOrder) { $routeFilters = array_reverse($routeFilters); } @@ -501,13 +501,13 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache } $returned = $this->startController(); + $gathered = false; // If startController returned a Response (from an attribute or Closure), use it if ($returned instanceof ResponseInterface) { $this->gatherOutput($cacheConfig, $returned); - } - // Closure controller has run in startController(). - elseif (! is_callable($this->controller)) { + $gathered = true; + } elseif (! $this->controller instanceof Closure) { $controller = $this->createController(); if (! method_exists($controller, '_remap') && ! is_callable([$controller, $this->method], false)) { @@ -526,7 +526,9 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache // If $returned is a string, then the controller output something, // probably a view, instead of echoing it directly. Send it along // so it can be used with the output. - $this->gatherOutput($cacheConfig, $returned); + if (! $gathered) { + $this->gatherOutput($cacheConfig, $returned); + } if ($this->enableFilters) { /** @var Filters $filters */ @@ -543,7 +545,7 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache } } - // Execute controller attributes' after() methods AFTER framework filters + // Execute controller attributes' after() methods AFTER framework filters. if ((config('Routing')->useControllerAttributes ?? true) === true) { // @phpstan-ignore nullCoalesce.property $this->benchmark->start('route_attributes_after'); $this->response = $this->router->executeAfterAttributes($this->request, $this->response); @@ -560,8 +562,6 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache $this->storePreviousURL(current_url(true)); } - unset($uri); - return $this->response; } @@ -889,7 +889,7 @@ protected function startController() $this->benchmark->start('controller_constructor'); // Is it routed to a Closure? - if (is_object($this->controller) && ($this->controller::class === 'Closure')) { + if ($this->controller instanceof Closure) { $controller = $this->controller; return $controller(...$this->router->params()); @@ -909,8 +909,7 @@ protected function startController() } // Execute route attributes' before() methods - // This runs after routing/validation but BEFORE expensive controller instantiation - if ((config('Routing')->useControllerAttributes ?? true) === true) { // @phpstan-ignore nullCoalesce.property + if ((config(Routing::class)->useControllerAttributes ?? true) === true) { // @phpstan-ignore nullCoalesce.property $this->benchmark->start('route_attributes_before'); $attributeResponse = $this->router->executeBeforeAttributes($this->request); $this->benchmark->stop('route_attributes_before'); @@ -1082,7 +1081,7 @@ public function storePreviousURL($uri) return; } // Ignore AJAX requests - if (method_exists($this->request, 'isAJAX') && $this->request->isAJAX()) { + if ($this->request instanceof IncomingRequest && $this->request->isAJAX()) { return; } diff --git a/tests/system/CodeIgniterTest.php b/tests/system/CodeIgniterTest.php index e5d371bfb941..a48cfa387cf7 100644 --- a/tests/system/CodeIgniterTest.php +++ b/tests/system/CodeIgniterTest.php @@ -1309,4 +1309,37 @@ public function testResetForWorkerMode(): void $this->assertSame($csp->getStyleNonce(), RichRenderer::$css_nonce); $this->assertTrue(RichRenderer::$needs_pre_render); } + + public function testGatherOutputCalledOnceWhenControllerReturnsResponse(): void + { + $this->resetServices(); + + $superglobals = service('superglobals'); + $superglobals->setServer('argv', ['index.php', 'pages/test']); + $superglobals->setServer('argc', 2); + $superglobals->setServer('REQUEST_URI', '/pages/test'); + $superglobals->setServer('SCRIPT_NAME', '/index.php'); + + $routes = service('routes'); + $routes->add('pages/test', static fn () => service('response')->setBody('Test Body')); + + $config = new App(); + $codeigniter = new class ($config) extends MockCodeIgniter { + public int $gatherOutputCalls = 0; + + protected function gatherOutput(?Cache $cacheConfig = null, $returned = null): void + { + $this->gatherOutputCalls++; + parent::gatherOutput($cacheConfig, $returned); + } + }; + + ob_start(); + $codeigniter->run($routes); + ob_end_clean(); + + // When startController() returns a ResponseInterface (e.g. from a closure route), + // gatherOutput() must be called exactly once — not twice as in the original bug. + $this->assertSame(1, $codeigniter->gatherOutputCalls); + } } From 8ec5ca45aae867209e8cac0c824cfb44f229834c Mon Sep 17 00:00:00 2001 From: Bogdan Date: Wed, 15 Jul 2026 19:08:58 +0200 Subject: [PATCH 2/6] refactor: CodeIgniter class optimizations and cleanup --- system/CodeIgniter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php index 61f5faeab726..3c76a3ed0dde 100644 --- a/system/CodeIgniter.php +++ b/system/CodeIgniter.php @@ -909,7 +909,7 @@ protected function startController() } // Execute route attributes' before() methods - if ((config(Routing::class)->useControllerAttributes ?? true) === true) { // @phpstan-ignore nullCoalesce.property + if ((config('Routing')->useControllerAttributes ?? true)) { // @phpstan-ignore nullCoalesce.property $this->benchmark->start('route_attributes_before'); $attributeResponse = $this->router->executeBeforeAttributes($this->request); $this->benchmark->stop('route_attributes_before'); From a9cef9833c67f7c1bf8ae27b35a391fe616fa750 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Wed, 15 Jul 2026 19:11:53 +0200 Subject: [PATCH 3/6] fix: restore strict boolean check for useControllerAttributes --- system/CodeIgniter.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php index 3c76a3ed0dde..88c44bf82822 100644 --- a/system/CodeIgniter.php +++ b/system/CodeIgniter.php @@ -35,7 +35,6 @@ use Config\Cache; use Config\Feature; use Config\Kint as KintConfig; -use Config\Routing; use Config\Services; use Exception; use Kint; @@ -909,7 +908,7 @@ protected function startController() } // Execute route attributes' before() methods - if ((config('Routing')->useControllerAttributes ?? true)) { // @phpstan-ignore nullCoalesce.property + if ((config('Routing')->useControllerAttributes ?? true) === true) { // @phpstan-ignore nullCoalesce.property $this->benchmark->start('route_attributes_before'); $attributeResponse = $this->router->executeBeforeAttributes($this->request); $this->benchmark->stop('route_attributes_before'); From 56d7300ff0f02d66fc4a1a1c352759fac4665418 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Thu, 30 Jul 2026 22:43:18 +0200 Subject: [PATCH 4/6] fix: restore null coalesce for oldFilterOrder and add changelog entry --- CHANGELOG.md | 12 ++++++++++++ system/CodeIgniter.php | 3 ++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 82d3974606b0..6853ca8122ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Changelog +## [v4.7.5](https://github.com/codeigniter4/CodeIgniter4/tree/v4.7.5) (Unreleased) + +[Full Changelog](https://github.com/codeigniter4/CodeIgniter4/compare/v4.7.4...v4.7.5) + +### Refactoring + +* refactor: simplify and optimize handleRequest method in CodeIgniter by @gr8man in https://github.com/codeigniter4/CodeIgniter4/pull/10369 + +### Breaking Changes + +* **CodeIgniter:** `storePreviousURL()` no longer accepts arbitrary request objects with an `isAJAX()` method — only `IncomingRequest` instances are checked. This aligns with the fact that `isAJAX()` only exists on `IncomingRequest`, so CLI requests were never affected. If you rely on a custom request class that implements `isAJAX()` and passes it to `storePreviousURL()`, you must ensure it extends `IncomingRequest`. + ## [v4.7.4](https://github.com/codeigniter4/CodeIgniter4/tree/v4.7.4) (2026-07-07) [Full Changelog](https://github.com/codeigniter4/CodeIgniter4/compare/v4.7.3...v4.7.4) diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php index 88c44bf82822..2edb925f545f 100644 --- a/system/CodeIgniter.php +++ b/system/CodeIgniter.php @@ -475,7 +475,8 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache if ($routeFilters !== null) { $filters->enableFilters($routeFilters, 'before'); - if (! config(Feature::class)->oldFilterOrder) { + $oldFilterOrder = config(Feature::class)->oldFilterOrder ?? false; // @phpstan-ignore nullCoalesce.property + if (! $oldFilterOrder) { $routeFilters = array_reverse($routeFilters); } From a5a178c336aa2cbfd8a4a91803e981684bb55c70 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Thu, 30 Jul 2026 22:53:18 +0200 Subject: [PATCH 5/6] docs: add changelog entries for handleRequest refactor and BC break --- user_guide_src/source/changelogs/v4.7.5.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/user_guide_src/source/changelogs/v4.7.5.rst b/user_guide_src/source/changelogs/v4.7.5.rst index c83924b0d500..b6e04f41d27b 100644 --- a/user_guide_src/source/changelogs/v4.7.5.rst +++ b/user_guide_src/source/changelogs/v4.7.5.rst @@ -14,6 +14,8 @@ Release Date: Unreleased BREAKING ******** +- **CodeIgniter:** ``storePreviousURL()`` now checks ``$this->request instanceof IncomingRequest`` instead of using ``method_exists($this->request, 'isAJAX')``. Previously, any request object with an ``isAJAX()`` method was accepted; now only ``IncomingRequest`` instances are checked. If you use a custom request class that implements ``isAJAX()``, ensure it extends ``IncomingRequest``. + *************** Message Changes *************** @@ -22,6 +24,9 @@ Message Changes Changes ******* +- **CodeIgniter:** Fixed a bug where ``gatherOutput()`` could be called twice when ``startController()`` returned a ``ResponseInterface`` (e.g., from filter attributes or closure routes). +- **CodeIgniter:** Changed ``is_object($this->controller) && $this->controller::class === 'Closure'`` to ``$this->controller instanceof Closure`` for consistency and type safety. + ************ Deprecations ************ From f4ee26af0a81539551799ac4671f2007038c14fc Mon Sep 17 00:00:00 2001 From: Bogdan Date: Thu, 30 Jul 2026 22:54:10 +0200 Subject: [PATCH 6/6] revert: remove changelog entry from CHANGELOG.md (handled in user_guide) --- CHANGELOG.md | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6853ca8122ec..82d3974606b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,17 +1,5 @@ # Changelog -## [v4.7.5](https://github.com/codeigniter4/CodeIgniter4/tree/v4.7.5) (Unreleased) - -[Full Changelog](https://github.com/codeigniter4/CodeIgniter4/compare/v4.7.4...v4.7.5) - -### Refactoring - -* refactor: simplify and optimize handleRequest method in CodeIgniter by @gr8man in https://github.com/codeigniter4/CodeIgniter4/pull/10369 - -### Breaking Changes - -* **CodeIgniter:** `storePreviousURL()` no longer accepts arbitrary request objects with an `isAJAX()` method — only `IncomingRequest` instances are checked. This aligns with the fact that `isAJAX()` only exists on `IncomingRequest`, so CLI requests were never affected. If you rely on a custom request class that implements `isAJAX()` and passes it to `storePreviousURL()`, you must ensure it extends `IncomingRequest`. - ## [v4.7.4](https://github.com/codeigniter4/CodeIgniter4/tree/v4.7.4) (2026-07-07) [Full Changelog](https://github.com/codeigniter4/CodeIgniter4/compare/v4.7.3...v4.7.4)