[#41188] - Fix error page rendering with unclosed Output buffers
- Fixed in Code Base
- 24 Jul 2023
- Medium
- Build: 4.3-dev
- # 41188
- Diff
- Fedik:error-ob-clean-4x
User tests: Successful: Unsuccessful:
Summary of Changes
In some cases the error page is rendered incorrectly, when exception happen while rendering.
Testing Instructions
Applly patch.
Add an error throw new \Exception('That an error'); in to layouts/joomla/content/info_block/hits.php
On the site open the article detail view.
Also can try to add an error in to the text field layout layouts/joomla/form/field/text.php
And try to edit the Article.
Actual result BEFORE applying this Pull Request
You get broken page:
Half article are rendered,
And then an error is rendered
Expected result AFTER applying this Pull Request
Only error is rendered
Link to documentations
Please select:
- No documentation changes for docs.joomla.org needed
- No documentation changes for manual.joomla.org needed
joomla-cms-bot
- | Category | ⇒ | Libraries |
| Status | New | ⇒ | Pending |
| Title |
|
||||||
Or maybe 
I will try, will mark it as draft for now
Okay, I will keep it as it is. There 100+ places where we use output buffering, some are in the layouts file.
It is more practical to close all at once before error rendering.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/41188.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/41188.
| Status | Pending | ⇒ | Ready to Commit |
| Labels |
Added:
bug
PR-4.3-dev
?
|
||
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/41188.
| Labels |
Added:
?
|
||
obuisard
- | Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-07-24 20:43:53 |
| Closed_By | ⇒ | obuisard |
I think it would be good in core for us to close the buffers using the finally keyword. So run a try/finally (you don't actually need to have a catch) and putting this in the Layout, HtmlView classes (does Document use it too when we include the template?) would also be a smart idea I guess.
I mean I'm not against this but this should be a method of last resort rather than the go to thing.