Skip to content

Commit e88e03a

Browse files
datamwebddevsr
andauthored
feat: make DebugToolbar smarter about detecting binary/streamed responses (#9862)
* feat: add native header detection to DebugToolbar * test: add infrastructure for mocking native header functions * test: add test for native header conflict detection * test: introduce NativeHeadersStack utility for native header testing * test: centralize native header function mocks * test: refactor existing tests to use NativeHeadersStack mocks * test: refactor NativeHeadersStack to use simplified header simulation * test: load mock once using setUpBeforeClass() * refactor: normalize native headers once for cleaner comparisons Co-authored-by: Denny Septian Panggabean <97607754+ddevsr@users.noreply.github.com> * refactor: improve readability of hasNativeHeaderConflict method * docs: update changelog for Toolbar native header detection fix --------- Co-authored-by: Denny Septian Panggabean <97607754+ddevsr@users.noreply.github.com>
1 parent a46eeee commit e88e03a

File tree

5 files changed

+221
-0
lines changed

5 files changed

+221
-0
lines changed

system/Debug/Toolbar.php

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,10 @@ public function prepare(?RequestInterface $request = null, ?ResponseInterface $r
372372
* @var IncomingRequest|null $request
373373
*/
374374
if (CI_DEBUG && ! is_cli()) {
375+
if ($this->hasNativeHeaderConflict()) {
376+
return;
377+
}
378+
375379
$app = service('codeigniter');
376380

377381
$request ??= service('request');
@@ -544,6 +548,32 @@ protected function format(string $data, string $format = 'html'): string
544548
return $output;
545549
}
546550

551+
/**
552+
* Checks if the native PHP headers indicate a non-HTML response
553+
* or if headers are already sent.
554+
*/
555+
protected function hasNativeHeaderConflict(): bool
556+
{
557+
// If headers are sent, we can't inject HTML.
558+
if (headers_sent()) {
559+
return true;
560+
}
561+
562+
// Native Header Inspection
563+
foreach (headers_list() as $header) {
564+
$lowerHeader = strtolower($header);
565+
566+
$isNonHtmlContent = str_starts_with($lowerHeader, 'content-type:') && ! str_contains($lowerHeader, 'text/html');
567+
$isAttachment = str_starts_with($lowerHeader, 'content-disposition:') && str_contains($lowerHeader, 'attachment');
568+
569+
if ($isNonHtmlContent || $isAttachment) {
570+
return true;
571+
}
572+
}
573+
574+
return false;
575+
}
576+
547577
/**
548578
* Determine if the toolbar should be disabled based on the request headers.
549579
*
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* This file is part of CodeIgniter 4 framework.
7+
*
8+
* (c) CodeIgniter Foundation <admin@codeigniter.com>
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\Test\Utilities;
15+
16+
/**
17+
* A utility class for simulating native PHP header handling in unit tests.
18+
*
19+
* @internal This class is for testing purposes only.
20+
*/
21+
final class NativeHeadersStack
22+
{
23+
/**
24+
* Simulates whether headers have been sent.
25+
*/
26+
public static bool $headersSent = false;
27+
28+
/**
29+
* Stores the list of headers.
30+
*
31+
* @var list<string>
32+
*/
33+
public static array $headers = [];
34+
35+
/**
36+
* Resets the header stack to defaults.
37+
* Call this in setUp() to ensure clean state between tests.
38+
*/
39+
public static function reset(): void
40+
{
41+
self::$headersSent = false;
42+
self::$headers = [];
43+
}
44+
45+
/**
46+
* Checks if a specific header exists in the stack.
47+
*
48+
* @param string $header The exact header string (e.g., 'Content-Type: text/html')
49+
*/
50+
public static function has(string $header): bool
51+
{
52+
return in_array($header, self::$headers, true);
53+
}
54+
55+
/**
56+
* Adds a header to the stack.
57+
*
58+
* @param string $header The header to add (e.g., 'Content-Type: text/html')
59+
*/
60+
public static function push(string $header): void
61+
{
62+
self::$headers[] = $header;
63+
}
64+
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* This file is part of CodeIgniter 4 framework.
7+
*
8+
* (c) CodeIgniter Foundation <admin@codeigniter.com>
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\Test\Utilities\NativeHeadersStack;
17+
18+
/**
19+
* Mock implementation of the native PHP `headers_sent()` function.
20+
*
21+
* Instead of checking the actual PHP output buffer, this function
22+
* checks the static property in NativeHeadersStack.
23+
*
24+
* @return bool True if headers are considered sent, false otherwise.
25+
*/
26+
function headers_sent(): bool
27+
{
28+
return NativeHeadersStack::$headersSent;
29+
}
30+
31+
/**
32+
* Mock implementation of the native PHP `headers_list()` function.
33+
*
34+
* Retrieves the array of headers stored in the NativeHeadersStack class
35+
* rather than the actual headers sent by the server.
36+
*
37+
* @return array The list of simulated headers.
38+
*/
39+
function headers_list(): array
40+
{
41+
return NativeHeadersStack::$headers;
42+
}

tests/system/Debug/ToolbarTest.php

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use CodeIgniter\HTTP\IncomingRequest;
2020
use CodeIgniter\HTTP\ResponseInterface;
2121
use CodeIgniter\Test\CIUnitTestCase;
22+
use CodeIgniter\Test\Utilities\NativeHeadersStack;
2223
use Config\Toolbar as ToolbarConfig;
2324
use PHPUnit\Framework\Attributes\BackupGlobals;
2425
use PHPUnit\Framework\Attributes\Group;
@@ -34,9 +35,20 @@ final class ToolbarTest extends CIUnitTestCase
3435
private ?IncomingRequest $request = null;
3536
private ?ResponseInterface $response = null;
3637

38+
public static function setUpBeforeClass(): void
39+
{
40+
parent::setUpBeforeClass();
41+
42+
// Load the mock once for the whole test class
43+
require_once SUPPORTPATH . 'Mock/MockNativeHeaders.php';
44+
}
45+
3746
protected function setUp(): void
3847
{
3948
parent::setUp();
49+
50+
NativeHeadersStack::reset();
51+
4052
Services::reset();
4153

4254
is_cli(false);
@@ -99,4 +111,76 @@ public function testPrepareInjectsNormallyWithoutIgnoredHeader(): void
99111
// Assertions
100112
$this->assertStringContainsString('id="debugbar_loader"', (string) $this->response->getBody());
101113
}
114+
115+
// -------------------------------------------------------------------------
116+
// Native Header Conflicts
117+
// -------------------------------------------------------------------------
118+
119+
public function testPrepareAbortsIfHeadersAlreadySent(): void
120+
{
121+
// Headers explicitly sent (e.g., echo before execution)
122+
NativeHeadersStack::$headersSent = true;
123+
124+
$this->request = service('incomingrequest', null, false);
125+
$this->response = service('response', null, false);
126+
$this->response->setBody('<html><body>Content</body></html>');
127+
128+
$toolbar = new Toolbar($this->config);
129+
$toolbar->prepare($this->request, $this->response);
130+
131+
// Must NOT inject because we can't modify the body safely
132+
$this->assertStringNotContainsString('id="debugbar_loader"', (string) $this->response->getBody());
133+
}
134+
135+
public function testPrepareAbortsIfNativeContentTypeIsNotHtml(): void
136+
{
137+
// A library (like Dompdf) set a PDF header directly
138+
NativeHeadersStack::push('Content-Type: application/pdf');
139+
140+
$this->request = service('incomingrequest', null, false);
141+
$this->response = service('response', null, false);
142+
// Even if the body looks like HTML (before rendering), the header says PDF
143+
$this->response->setBody('<html><body>Raw PDF Data</body></html>');
144+
145+
$toolbar = new Toolbar($this->config);
146+
$toolbar->prepare($this->request, $this->response);
147+
148+
// Must NOT inject into non-HTML content
149+
$this->assertStringNotContainsString('id="debugbar_loader"', (string) $this->response->getBody());
150+
}
151+
152+
public function testPrepareAbortsIfNativeContentDispositionIsAttachment(): void
153+
{
154+
// A file download (even if it is HTML)
155+
NativeHeadersStack::$headers = [
156+
'Content-Type: text/html',
157+
'Content-Disposition: attachment; filename="report.html"',
158+
];
159+
160+
$this->request = service('incomingrequest', null, false);
161+
$this->response = service('response', null, false);
162+
$this->response->setBody('<html><body>Downloadable Report</body></html>');
163+
164+
$toolbar = new Toolbar($this->config);
165+
$toolbar->prepare($this->request, $this->response);
166+
167+
// Must NOT inject into downloads
168+
$this->assertStringNotContainsString('id="debugbar_loader"', (string) $this->response->getBody());
169+
}
170+
171+
public function testPrepareWorksWithNativeHtmlHeader(): void
172+
{
173+
// Standard scenario where PHP header is text/html
174+
NativeHeadersStack::push('Content-Type: text/html; charset=UTF-8');
175+
176+
$this->request = service('incomingrequest', null, false);
177+
$this->response = service('response', null, false);
178+
$this->response->setBody('<html><body>Valid Page</body></html>');
179+
180+
$toolbar = new Toolbar($this->config);
181+
$toolbar->prepare($this->request, $this->response);
182+
183+
// Should inject normally
184+
$this->assertStringContainsString('id="debugbar_loader"', (string) $this->response->getBody());
185+
}
102186
}

user_guide_src/source/changelogs/v4.7.0.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ Libraries
247247
- **Time:** added methods ``Time::addCalendarMonths()`` and ``Time::subCalendarMonths()``
248248
- **Time:** Added ``Time::isPast()`` and ``Time::isFuture()`` convenience methods. See :ref:`isPast <time-comparing-two-times-isPast>` and :ref:`isFuture <time-comparing-two-times-isFuture>` for details.
249249
- **View:** Added the ability to override namespaced views (e.g., from modules/packages) by placing a matching file structure within the **app/Views/overrides** directory. See :ref:`Overriding Namespaced Views <views-overriding-namespaced-views>` for details.
250+
- **Toolbar:** Fixed an issue where the Debug Toolbar was incorrectly injected into responses generated by third-party libraries (e.g., Dompdf) that use native PHP headers instead of the framework's Response object.
250251

251252

252253
Commands

0 commit comments

Comments
 (0)