Skip to content

Commit 0d52f5a

Browse files
datamwebmichalsn
andauthored
feat: prevent Maximum call stack size exceeded on client-managed requests (codeigniter4#9852)
* feat: skip HTML/JS injection for partial requests * tests: add test for skip html/js injection in development mode * docs: Add explanation for skipping Debug Toolbar injection on AJAX-like requests * tests: try fix test * style: fix code style * refactor: remove unnecessary parts * feat: add MockCommon for shared test helpers and mocks * tests: refactor tests to use MockCommon helpers * fix: rector & cs * refactor: update test and other * refactor: internal cleanup and tooling alignment * tests: fix is_cli state * refactor: stop iterating headers after first match * fix: suppress RemoveExtraParametersRector false-positive for is_cli override * Update app/Config/Toolbar.php Co-authored-by: Michal Sniatala <[email protected]> * Update system/Debug/Toolbar.php Co-authored-by: Michal Sniatala <[email protected]> * refactor: improve by shouldDisableToolbar to check header values * test: fix test * refactor: simplify toolbar disable logic and remove unnecessary flags * docs: update method signature changes * refactor: add a backward-compatible fallback --------- Co-authored-by: Michal Sniatala <[email protected]>
1 parent e2fc524 commit 0d52f5a

File tree

6 files changed

+183
-8
lines changed

6 files changed

+183
-8
lines changed

app/Config/Toolbar.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,4 +119,29 @@ class Toolbar extends BaseConfig
119119
public array $watchedExtensions = [
120120
'php', 'css', 'js', 'html', 'svg', 'json', 'env',
121121
];
122+
123+
/**
124+
* --------------------------------------------------------------------------
125+
* Ignored HTTP Headers
126+
* --------------------------------------------------------------------------
127+
*
128+
* CodeIgniter Debug Toolbar normally injects HTML and JavaScript into every
129+
* HTML response. This is correct for full page loads, but it breaks requests
130+
* that expect only a clean HTML fragment.
131+
*
132+
* Libraries like HTMX, Unpoly, and Hotwire (Turbo) update parts of the page or
133+
* manage navigation on the client side. Injecting the Debug Toolbar into their
134+
* responses can cause invalid HTML, duplicated scripts, or JavaScript errors
135+
* (such as infinite loops or "Maximum call stack size exceeded").
136+
*
137+
* Any request containing one of the following headers is treated as a
138+
* client-managed or partial request, and the Debug Toolbar injection is skipped.
139+
*
140+
* @var array<string, string|null>
141+
*/
142+
public array $disableOnHeaders = [
143+
'X-Requested-With' => 'xmlhttprequest', // AJAX requests
144+
'HX-Request' => 'true', // HTMX requests
145+
'X-Up-Version' => null, // Unpoly partial requests
146+
];
122147
}

rector.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
use Rector\EarlyReturn\Rector\If_\RemoveAlwaysElseRector;
3232
use Rector\EarlyReturn\Rector\Return_\PreparedValueToEarlyReturnRector;
3333
use Rector\Php70\Rector\FuncCall\RandomFunctionRector;
34+
use Rector\Php71\Rector\FuncCall\RemoveExtraParametersRector;
3435
use Rector\Php80\Rector\Class_\ClassPropertyAssignToConstructorPromotionRector;
3536
use Rector\Php81\Rector\FuncCall\NullToStrictStringFuncCallArgRector;
3637
use Rector\PHPUnit\CodeQuality\Rector\Class_\YieldDataProviderRector;
@@ -107,6 +108,11 @@
107108
__DIR__ . '/system/HTTP/Response.php',
108109
],
109110

111+
// Exclude test file because `is_cli()` is mocked and Rector might remove needed parameters.
112+
RemoveExtraParametersRector::class => [
113+
__DIR__ . '/tests/system/Debug/ToolbarTest.php',
114+
],
115+
110116
// check on constant compare
111117
UnwrapFutureCompatibleIfPhpVersionRector::class => [
112118
__DIR__ . '/system/Autoloader/Autoloader.php',

system/Boot.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,9 @@ public static function bootTest(Paths $paths): void
144144
static::loadDotEnv($paths);
145145
static::loadEnvironmentBootstrap($paths, false);
146146

147+
static::loadCommonFunctionsMock();
147148
static::loadCommonFunctions();
149+
148150
static::loadAutoloader();
149151
static::setExceptionHandler();
150152
static::initializeKint();
@@ -260,6 +262,11 @@ protected static function loadCommonFunctions(): void
260262
require_once SYSTEMPATH . 'Common.php';
261263
}
262264

265+
protected static function loadCommonFunctionsMock(): void
266+
{
267+
require_once SYSTEMPATH . 'Test/Mock/MockCommon.php';
268+
}
269+
263270
/**
264271
* The autoloader allows all the pieces to work together in the framework.
265272
* We have to load it here, though, so that the config files can use the

system/Debug/Toolbar.php

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -365,10 +365,8 @@ protected function roundTo(float $number, int $increments = 5): float
365365

366366
/**
367367
* Prepare for debugging.
368-
*
369-
* @return void
370368
*/
371-
public function prepare(?RequestInterface $request = null, ?ResponseInterface $response = null)
369+
public function prepare(?RequestInterface $request = null, ?ResponseInterface $response = null): void
372370
{
373371
/**
374372
* @var IncomingRequest|null $request
@@ -385,7 +383,7 @@ public function prepare(?RequestInterface $request = null, ?ResponseInterface $r
385383
return;
386384
}
387385

388-
$toolbar = service('toolbar', config(ToolbarConfig::class));
386+
$toolbar = service('toolbar', $this->config);
389387
$stats = $app->getPerformanceStats();
390388
$data = $toolbar->run(
391389
$stats['startTime'],
@@ -410,7 +408,7 @@ public function prepare(?RequestInterface $request = null, ?ResponseInterface $r
410408
// Non-HTML formats should not include the debugbar
411409
// then we send headers saying where to find the debug data
412410
// for this response
413-
if ($request->isAJAX() || ! str_contains($format, 'html')) {
411+
if ($this->shouldDisableToolbar($request) || ! str_contains($format, 'html')) {
414412
$response->setHeader('Debugbar-Time', "{$time}")
415413
->setHeader('Debugbar-Link', site_url("?debugbar_time={$time}"));
416414

@@ -454,10 +452,8 @@ public function prepare(?RequestInterface $request = null, ?ResponseInterface $r
454452
* Inject debug toolbar into the response.
455453
*
456454
* @codeCoverageIgnore
457-
*
458-
* @return void
459455
*/
460-
public function respond()
456+
public function respond(): void
461457
{
462458
if (ENVIRONMENT === 'testing') {
463459
return;
@@ -547,4 +543,37 @@ protected function format(string $data, string $format = 'html'): string
547543

548544
return $output;
549545
}
546+
547+
/**
548+
* Determine if the toolbar should be disabled based on the request headers.
549+
*
550+
* This method allows checking both the presence of headers and their expected values.
551+
* Useful for AJAX, HTMX, Unpoly, Turbo, etc., where partial HTML responses are expected.
552+
*
553+
* @return bool True if any header condition matches; false otherwise.
554+
*/
555+
private function shouldDisableToolbar(IncomingRequest $request): bool
556+
{
557+
// Fallback for older installations where the config option is missing (e.g. after upgrading from a previous version).
558+
$headers = $this->config->disableOnHeaders ?? ['X-Requested-With' => 'xmlhttprequest'];
559+
560+
foreach ($headers as $headerName => $expectedValue) {
561+
if (! $request->hasHeader($headerName)) {
562+
continue; // header not present, skip
563+
}
564+
565+
// If expectedValue is null, only presence is enough
566+
if ($expectedValue === null) {
567+
return true;
568+
}
569+
570+
$headerValue = strtolower($request->getHeaderLine($headerName));
571+
572+
if ($headerValue === strtolower($expectedValue)) {
573+
return true;
574+
}
575+
}
576+
577+
return false;
578+
}
550579
}

tests/system/Debug/ToolbarTest.php

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* This file is part of CodeIgniter 4 framework.
7+
*
8+
* (c) CodeIgniter Foundation <[email protected]>
9+
*
10+
* For the full copyright and license information, please view
11+
* the LICENSE file that was distributed with this source code.
12+
*/
13+
14+
namespace CodeIgniter\Debug;
15+
16+
use CodeIgniter\CodeIgniter;
17+
use CodeIgniter\Config\Factories;
18+
use CodeIgniter\Config\Services;
19+
use CodeIgniter\HTTP\IncomingRequest;
20+
use CodeIgniter\HTTP\ResponseInterface;
21+
use CodeIgniter\Test\CIUnitTestCase;
22+
use Config\Toolbar as ToolbarConfig;
23+
use PHPUnit\Framework\Attributes\BackupGlobals;
24+
use PHPUnit\Framework\Attributes\Group;
25+
26+
/**
27+
* @internal
28+
*/
29+
#[BackupGlobals(true)]
30+
#[Group('Others')]
31+
final class ToolbarTest extends CIUnitTestCase
32+
{
33+
private ToolbarConfig $config;
34+
private ?IncomingRequest $request = null;
35+
private ?ResponseInterface $response = null;
36+
37+
protected function setUp(): void
38+
{
39+
parent::setUp();
40+
Services::reset();
41+
42+
is_cli(false);
43+
44+
$this->config = new ToolbarConfig();
45+
46+
// Mock CodeIgniter core service to provide performance stats
47+
$app = $this->createMock(CodeIgniter::class);
48+
$app->method('getPerformanceStats')->willReturn([
49+
'startTime' => microtime(true),
50+
'totalTime' => 0.05,
51+
]);
52+
Services::injectMock('codeigniter', $app);
53+
}
54+
55+
protected function tearDown(): void
56+
{
57+
// Restore is_cli state
58+
is_cli(true);
59+
60+
parent::tearDown();
61+
}
62+
63+
public function testPrepareRespectsDisableOnHeaders(): void
64+
{
65+
// Set up the new configuration property
66+
$this->config->disableOnHeaders = ['HX-Request' => 'true'];
67+
Factories::injectMock('config', 'Toolbar', $this->config);
68+
69+
// Initialize Request with the custom header
70+
$this->request = service('incomingrequest', null, false);
71+
$this->request->setHeader('HX-Request', 'true');
72+
73+
// Initialize Response
74+
$this->response = service('response', null, false);
75+
$this->response->setBody('<html><body>Content</body></html>');
76+
$this->response->setHeader('Content-Type', 'text/html');
77+
78+
$toolbar = new Toolbar($this->config);
79+
$toolbar->prepare($this->request, $this->response);
80+
81+
// Assertions
82+
$this->assertTrue($this->response->hasHeader('Debugbar-Time'));
83+
$this->assertStringNotContainsString('id="debugbar_loader"', (string) $this->response->getBody());
84+
}
85+
86+
public function testPrepareInjectsNormallyWithoutIgnoredHeader(): void
87+
{
88+
$this->config->disableOnHeaders = ['HX-Request' => 'true'];
89+
Factories::injectMock('config', 'Toolbar', $this->config);
90+
91+
$this->request = service('incomingrequest', null, false);
92+
$this->response = service('response', null, false);
93+
$this->response->setBody('<html><body>Content</body></html>');
94+
$this->response->setHeader('Content-Type', 'text/html');
95+
96+
$toolbar = new Toolbar($this->config);
97+
$toolbar->prepare($this->request, $this->response);
98+
99+
// Assertions
100+
$this->assertStringContainsString('id="debugbar_loader"', (string) $this->response->getBody());
101+
}
102+
}

user_guide_src/source/changelogs/v4.7.0.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,9 @@ Method Signature Changes
141141
- ``clean()``
142142
- ``getCacheInfo()``
143143
- ``getMetaData()``
144+
- Added native return types to ``CodeIgniter\Debug\Toolbar`` methods:
145+
- ``prepare(): void``
146+
- ``respond(): void``
144147

145148
Removed Deprecated Items
146149
========================
@@ -230,6 +233,8 @@ Changes
230233
- **Cookie:** The ``CookieInterface::EXPIRES_FORMAT`` has been changed to ``D, d M Y H:i:s T`` to follow the recommended format in RFC 7231.
231234
- **Format:** Added support for configuring ``json_encode()`` maximum depth via ``Config\Format::$jsonEncodeDepth``.
232235
- **Paths:** Added support for changing the location of the ``.env`` file via the ``Paths::$envDirectory`` property.
236+
- **Toolbar:** Added ``$disableOnHeaders`` property to **app/Config/Toolbar.php**.
237+
233238

234239
************
235240
Deprecations
@@ -245,6 +250,7 @@ Bugs Fixed
245250

246251
- **Cookie:** The ``CookieInterface::SAMESITE_STRICT``, ``CookieInterface::SAMESITE_LAX``, and ``CookieInterface::SAMESITE_NONE`` constants are now written in ucfirst style to be consistent with usage in the rest of the framework.
247252
- **Cache:** Changed ``WincacheHandler::increment()`` and ``WincacheHandler::decrement()`` to return ``bool`` instead of ``mixed``.
253+
- **Toolbar:** Fixed **Maximum call stack size exceeded** crash when AJAX-like requests (HTMX, Turbo, Unpoly, etc.) were made on pages with Debug Toolbar enabled.
248254

249255
See the repo's
250256
`CHANGELOG.md <https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md>`_

0 commit comments

Comments
 (0)