From d636a615e806137a8a88f9b4fffd78321e503cf5 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Wed, 2 Sep 2026 21:14:59 +0200 Subject: [PATCH] fix: prevent gatherOutput from being called twice when controller returns Response --- system/CodeIgniter.php | 38 ++++++++++----------- tests/system/CodeIgniterTest.php | 33 ++++++++++++++++++ user_guide_src/source/changelogs/v4.7.5.rst | 1 + 3 files changed, 53 insertions(+), 19 deletions(-) diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php index 177edbd8d753..ab4c647049ba 100644 --- a/system/CodeIgniter.php +++ b/system/CodeIgniter.php @@ -505,29 +505,29 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache // 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)) { - $controller = $this->createController(); + } else { + // Closure controller has run in startController(). + if (! is_callable($this->controller)) { + $controller = $this->createController(); - if (! method_exists($controller, '_remap') && ! is_callable([$controller, $this->method], false)) { - throw PageNotFoundException::forMethodNotFound($this->method); - } + if (! method_exists($controller, '_remap') && ! is_callable([$controller, $this->method], false)) { + throw PageNotFoundException::forMethodNotFound($this->method); + } - // Is there a "post_controller_constructor" event? - Events::trigger('post_controller_constructor'); + // Is there a "post_controller_constructor" event? + Events::trigger('post_controller_constructor'); - $returned = $this->runController($controller); - } else { - $this->benchmark->stop('controller_constructor'); - $this->benchmark->stop('controller'); - } - - // 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); + $returned = $this->runController($controller); + } else { + $this->benchmark->stop('controller_constructor'); + $this->benchmark->stop('controller'); + } + // 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 ($this->enableFilters) { /** @var Filters $filters */ $filters = service('filters'); diff --git a/tests/system/CodeIgniterTest.php b/tests/system/CodeIgniterTest.php index c5b3863ee203..da65ffd4a22d 100644 --- a/tests/system/CodeIgniterTest.php +++ b/tests/system/CodeIgniterTest.php @@ -1308,4 +1308,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); + } } diff --git a/user_guide_src/source/changelogs/v4.7.5.rst b/user_guide_src/source/changelogs/v4.7.5.rst index 7cd8169a5d34..b4010a6f96ca 100644 --- a/user_guide_src/source/changelogs/v4.7.5.rst +++ b/user_guide_src/source/changelogs/v4.7.5.rst @@ -37,6 +37,7 @@ Bugs Fixed - **CLI:** Fixed a bug where pressing backspace in a ``CLI::prompt()`` erased the prompt text when the ``readline`` extension is enabled. The prompt is now passed to ``readline()`` so line redraws repaint it. ANSI color codes in the prompt (e.g., option defaults) are wrapped in readline's non-printing markers under GNU readline so cursor positioning stays accurate. - **CLIRequest:** Fixed a bug where ``parseCommand()`` could throw a TypeError when ``argv`` is missing. +- **CodeIgniter:** Fixed a bug where ``gatherOutput()`` could be called twice when ``startController()`` returned a ``ResponseInterface`` (e.g., from filter attributes or closure routes). - **Content Security Policy:** Fixed a bug where empty ``Content-Security-Policy``, ``Content-Security-Policy-Report-Only``, and ``Reporting-Endpoints`` response headers were generated when no corresponding values existed. - **Helpers:** Fixed a bug where ``get_dir_file_info()`` returned incomplete entries for subdirectories and missing files instead of omitting them. - **Honeypot:** Fixed a bug where bot detection returned an HTTP 500 response instead of 403 (Forbidden).