Skip to content

Commit d636a61

Browse files
committed
fix: prevent gatherOutput from being called twice when controller returns Response
1 parent b7321d3 commit d636a61

3 files changed

Lines changed: 53 additions & 19 deletions

File tree

system/CodeIgniter.php

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -505,29 +505,29 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache
505505
// If startController returned a Response (from an attribute or Closure), use it
506506
if ($returned instanceof ResponseInterface) {
507507
$this->gatherOutput($cacheConfig, $returned);
508-
}
509-
// Closure controller has run in startController().
510-
elseif (! is_callable($this->controller)) {
511-
$controller = $this->createController();
508+
} else {
509+
// Closure controller has run in startController().
510+
if (! is_callable($this->controller)) {
511+
$controller = $this->createController();
512512

513-
if (! method_exists($controller, '_remap') && ! is_callable([$controller, $this->method], false)) {
514-
throw PageNotFoundException::forMethodNotFound($this->method);
515-
}
513+
if (! method_exists($controller, '_remap') && ! is_callable([$controller, $this->method], false)) {
514+
throw PageNotFoundException::forMethodNotFound($this->method);
515+
}
516516

517-
// Is there a "post_controller_constructor" event?
518-
Events::trigger('post_controller_constructor');
517+
// Is there a "post_controller_constructor" event?
518+
Events::trigger('post_controller_constructor');
519519

520-
$returned = $this->runController($controller);
521-
} else {
522-
$this->benchmark->stop('controller_constructor');
523-
$this->benchmark->stop('controller');
524-
}
525-
526-
// If $returned is a string, then the controller output something,
527-
// probably a view, instead of echoing it directly. Send it along
528-
// so it can be used with the output.
529-
$this->gatherOutput($cacheConfig, $returned);
520+
$returned = $this->runController($controller);
521+
} else {
522+
$this->benchmark->stop('controller_constructor');
523+
$this->benchmark->stop('controller');
524+
}
530525

526+
// If $returned is a string, then the controller output something,
527+
// probably a view, instead of echoing it directly. Send it along
528+
// so it can be used with the output.
529+
$this->gatherOutput($cacheConfig, $returned);
530+
}
531531
if ($this->enableFilters) {
532532
/** @var Filters $filters */
533533
$filters = service('filters');

tests/system/CodeIgniterTest.php

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1308,4 +1308,37 @@ public function testResetForWorkerMode(): void
13081308
$this->assertSame($csp->getStyleNonce(), RichRenderer::$css_nonce);
13091309
$this->assertTrue(RichRenderer::$needs_pre_render);
13101310
}
1311+
1312+
public function testGatherOutputCalledOnceWhenControllerReturnsResponse(): void
1313+
{
1314+
$this->resetServices();
1315+
1316+
$superglobals = service('superglobals');
1317+
$superglobals->setServer('argv', ['index.php', 'pages/test']);
1318+
$superglobals->setServer('argc', 2);
1319+
$superglobals->setServer('REQUEST_URI', '/pages/test');
1320+
$superglobals->setServer('SCRIPT_NAME', '/index.php');
1321+
1322+
$routes = service('routes');
1323+
$routes->add('pages/test', static fn () => service('response')->setBody('Test Body'));
1324+
1325+
$config = new App();
1326+
$codeigniter = new class ($config) extends MockCodeIgniter {
1327+
public int $gatherOutputCalls = 0;
1328+
1329+
protected function gatherOutput(?Cache $cacheConfig = null, $returned = null): void
1330+
{
1331+
$this->gatherOutputCalls++;
1332+
parent::gatherOutput($cacheConfig, $returned);
1333+
}
1334+
};
1335+
1336+
ob_start();
1337+
$codeigniter->run($routes);
1338+
ob_end_clean();
1339+
1340+
// When startController() returns a ResponseInterface (e.g. from a closure route),
1341+
// gatherOutput() must be called exactly once — not twice as in the original bug.
1342+
$this->assertSame(1, $codeigniter->gatherOutputCalls);
1343+
}
13111344
}

user_guide_src/source/changelogs/v4.7.5.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ Bugs Fixed
3737
- **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.
3838
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.
3939
- **CLIRequest:** Fixed a bug where ``parseCommand()`` could throw a TypeError when ``argv`` is missing.
40+
- **CodeIgniter:** Fixed a bug where ``gatherOutput()`` could be called twice when ``startController()`` returned a ``ResponseInterface`` (e.g., from filter attributes or closure routes).
4041
- **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.
4142
- **Helpers:** Fixed a bug where ``get_dir_file_info()`` returned incomplete entries for subdirectories and missing files instead of omitting them.
4243
- **Honeypot:** Fixed a bug where bot detection returned an HTTP 500 response instead of 403 (Forbidden).

0 commit comments

Comments
 (0)