feat: prevent Maximum call stack size exceeded on client-managed requests
#9852
Conversation
|
@samsonasik , could you please advise on the best way to fix this rector issue in ToolbarTest.php? |
michalsn
left a comment
michalsn
left a comment
There was a problem hiding this comment.
As for the Rector error, we probably will have to skip the RemoveExtraParametersRector rule for this test. But maybe there is a better way - I'm not an expert.
I agree. I’ve disabled |
michalsn
left a comment
michalsn
left a comment
There was a problem hiding this comment.
Thanks for the updates! I have a few final thoughts.
| 'HX-Request', // HTMX partial requests | ||
| 'X-Up-Version', // Unpoly partial requests |
There was a problem hiding this comment.
| 'HX-Request', // HTMX partial requests | |
| 'X-Up-Version', // Unpoly partial requests | |
| 'X-Requested-With' => 'xmlhttprequest', // AJAX requests | |
| 'HX-Request' => 'true', // HTMX requests | |
| 'X-Up-Version' => null, // Unpoly partial requests |
Since we need to loop over these values, we can also add AJAX requests here. Additionally, we can check not only for the presence of the header but also for the value, which is important in some cases. The null value indicates that we are only interested in the header presence.
| * Indicates if the current request is a custom AJAX-like request | ||
| * (HTMX, Unpoly, Turbo, etc.) that expects clean HTML fragments. | ||
| */ | ||
| protected bool $isCustomAjax = false; |
| if ($request->hasHeader($header)) { | ||
| $this->isCustomAjax = true; | ||
| break; | ||
| } |
There was a problem hiding this comment.
Since we will need more complex checks, this can be delegated to a separate method.
| // then we send headers saying where to find the debug data | ||
| // for this response | ||
| if ($request->isAJAX() || ! str_contains($format, 'html')) { | ||
| if ($request->isAJAX() || ! str_contains($format, 'html') || $this->isCustomAjax) { |
There was a problem hiding this comment.
| if ($request->isAJAX() || ! str_contains($format, 'html') || $this->isCustomAjax) { | |
| if ($this->isDisabled || ! str_contains($format, 'html')) { |
This way, we can make this check a bit simpler.
| RemoveExtraParametersRector::class => [ | ||
| __DIR__ . '/tests/system/Debug/ToolbarTest.php', | ||
| ], |
There was a problem hiding this comment.
Can we add above that this is required because of is_cli() mocking?
2 participants
Description
The Debug Toolbar injects HTML and JavaScript into every HTML response by default, which works for full page loads but causes issues for client-managed or partial requests (such as those from HTMX, Unpoly, or Hotwire Turbo) that expect clean HTML fragments. This can result in invalid HTML, duplicated scripts, or JavaScript errors like “Maximum call stack size exceeded.” To address this, support was added to skip Debug Toolbar HTML/JS injection for requests containing specific headers (e.g. HX-Request, X-Up-Version), while still preserving Debugbar response headers for network-level debugging.
I don’t consider this PR a new feature. I believe it could have been to the develop branch, but since it introduces the
$disableOnHeadersproperty, I only PR’d it to the 4.7 branch.Checklist: