-
Notifications
You must be signed in to change notification settings - Fork 2k
feat: make DebugToolbar smarter about detecting binary/streamed responses #9862
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 4.7
Are you sure you want to change the base?
Conversation
|
I haven't investigated this problem, but my first impression is that we should rather make it as simple as possible. We can check for We can add a new method to the public function withoutDebugbar()
{
if (CI_DEBUG) {
$this->setHeader('Debugbar-Skip', 'true');
}
return $this;
}This won't resolve anything automatically, but it will provide developers with the tools to remove the Debugbar when needed. |
|
@michalsn Thanks for the feedback! I completely agree that keeping things simple and explicit is key. The However, the specific problem this PR aims to solve is related to third-party libraries (like Dompdf, ...) that bypass the framework's In these scenarios: The developer often doesn't interact with I’ve done my best to explain the topic as clearly and completely as possible, even though putting it into words is a bit challenging for me. <?php
namespace App\Controllers;
class Home extends BaseController
{
public function index()
{
if (! headers_sent()) {
header('Content-Type: application/pdf');
header('X-Library: Dompdf');
}
$ciHeaders = $this->response->headers();
$nativeHeaders = headers_list();
header_remove('Content-Type');
header('Content-Type: text/plain');
echo "=== REALITY CHECK ===\n";
echo "What PHP Native says (Actual Output):\n";
foreach ($nativeHeaders as $h) {
echo ' - ' . $h . "\n";
}
echo "\n=== CI4 DELUSION ===\n";
echo "What \$this->response thinks (Framework Object):\n";
if (empty($ciHeaders)) {
echo " - (Empty / Default text/html)\n";
} else {
foreach ($ciHeaders as $h) {
echo ' - ' . $h->getValueLine() . "\n";
}
}
exit;
}
}From my perspective, this PR is now ready for review. The changelogs will be uploaded later. |
michalsn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I understand the problem now. I would like to reorganize some of the code to make it more structured and reusable.
michalsn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - we need a changelog entry.
Co-authored-by: Denny Septian Panggabean <[email protected]>
Description
This PR enhances the
DebugToolbar's detection logic to support third-party libraries that bypass the framework's Response object (e.g., Dompdf).Currently, CodeIgniter relies solely on its internal
Responseobject to decide whether to inject the toolbar. However, many libraries output content directly using native PHPheader()andecho. In these cases, the Toolbar assumes the content is HTML and injects itself, which is often undesirable for binary streams.I've implemented a Native State Awareness mechanism in
Toolbar::prepare(). The toolbar now acts as a secondary guard, validating the environment's actual state before injection.This issue has been raised before by several community members on the forum. At the time, we were unable to clearly identify the root cause, as the behavior only manifests when third-party libraries bypass the framework’s abstraction layer and interact directly with native PHP output mechanisms.
After revisiting the problem with a deeper analysis of the runtime behavior, I was able to trace the issue to a mismatch between the framework’s assumed response state and the actual native PHP state. Based on that insight, this PR introduces a more resilient detection mechanism that addresses not only the originally reported cases, but also a broader class of similar issues.
See: https://forum.codeigniter.com/showthread.php?tid=82461
Checklist: