#61873 closed defect (bug) (fixed)
Check if error_reporting function exists, otherwise breaks php8 installs when disabled in the ini config
Reported by: | gansbrest | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | 6.7 | Priority: | normal |
Severity: | normal | Version: | trunk |
Component: | Administration | Keywords: | has-patch |
Focuses: | administration | Cc: |
Description (last modified by )
Short summary of the problem: When error_reporting
function is disabled on the hosting provider, it breaks admin area due to the fact that wp-admin/load-styles.php
and wp-admin/load-scripts.php
are triggering it without checking.
Some work has been done to resolve the problem on #52226
Unfortunately it doesn't address the issue completely as it only adds checks to the wp-load.php
file.
Here is the great post that summarizes problem with error_reporting
in WP for some additional context: https://phil.lavin.me.uk/2022/02/wordpress-changing-error_reporting-level/
Change History (11)
#3
@
4 weeks ago
Hi and welcome!
Could you remove the link to ticket 52226 from the PR summary, and edit the text surrounding it? The pull request automatically connected itself to the first Trac ticket mentioned there, and I think editing the summary would correct that.
#4
in reply to:
↑ description
@
4 weeks ago
Hi there, thanks for the ticket!
Replying to gansbrest:
Short summary of the problem: When
error_reporting
function is disabled on the hosting provider, it breaks admin area due to the fact thatwp-admin/load-styles.php
andwp-admin/load-scripts.php
are triggering it without checking.
Some work has been done to resolve the problem on #52226
Unfortunately it doesn't address the issue completely as it only adds checks to the
wp-load.php
file.
As noted in the commit message for [50447]:
On systems with this function disabled, it's best to add a dummy function to the
wp-config.php
file, as there are multiple other calls in core or plugins.
However, as this call to the function is run prior to
wp-config.php
loading, it is now wrapped in afunction_exists()
check.
There are quite a few other error_reporting()
calls in core, a quick search shows 21 matches in 11 files. However, defining an empty function in wp-config.php
should resolve the issue without changing all those instances:
function error_reporting() {}
Afunction_exists()
check was only added to wp-load.php
is because it runs before wp-config.php
is loaded.
#5
follow-up:
↓ 9
@
4 weeks ago
@SergeyBiryukov just wanted to clarify couple things.
I do use dummy function in wp-config.php, the problem still remains on admin pages (you can test it yourself by disabling error_reporting in the ini and navigating to /wp-admin pages).
Here is the example of direct call from wp-admin area:
GET /wp-admin/load-styles.php?c=1&dir=ltr&load%5Bchunk_0%5D=dashicons,admin-bar,common,forms,admin-menu,dashboard,list-tables,edit,revisions,media,themes,about,nav-menus,wp-pointer,widgets&load%5Bchunk_1%5D=,site-icon,l10n,buttons,wp-auth-check,wp-color-picker&ver=6.5.4
As you can see it calls /wp-admin/load-styles.php
directly (similar to load-scripts.php
calls).
If we look inside of the load-styles/scripts.php
files (see my pull request above) we can notice that error_reporting is called before wp-config.php
is initialized (hence the need for the PR).
It just throws fatal errors for each style or script.
I had to patch my WP installations with the additional function_exists
checks I submitted in the PR here to get rid of fatal errors and restore functionality on PHP 8 setups ( with dummy function in wp-config.php of course ).
p.s. I did go through all of the remaining error_reporting calls in core and none of them seems to be breaking running websites with dummy error_reporting definition in the wp-config.php.
#6
@
4 weeks ago
- Focuses php-compatibility removed
- Severity changed from major to normal
Just noting here for the record that adding error_reporting
to the disabled functions is definitely not good practice and doesn't improve security at all.
#7
@
4 weeks ago
Adding the extra checks to load-styles.php
and load-scripts.php
seems sensible to me.
I completely agree with @jrf that disabling error_reporting
function is not good practice. For me personally it's one of those weird edge cases WordPress "should" work around because it runs 43% of the web.
#8
@
4 weeks ago
Agree with all of you that disabling error_reporting
is definitely not the most common practice, but I've seen more than a few cases where it's in fact disabled.
In addition to that it's actually pretty convenient way to precisely control error_reporting
via ini
config in one place vs chasing error_reporting
declarations across different modules.
Personally I don't see a harm if we add couple additional function_exists
checks in the core, since we already did it for the wp-load.php
, but open to other suggestions.
#9
in reply to:
↑ 5
@
4 weeks ago
- Milestone changed from Awaiting Review to 6.7
- Owner set to SergeyBiryukov
- Status changed from new to accepted
Replying to gansbrest:
If we look inside of the
load-styles/scripts.php
files (see my pull request above) we can notice that error_reporting is called beforewp-config.php
is initialized (hence the need for the PR).
Ah yes, I missed that, thanks for clarifying! The PR makes sense to me.
Here is PR with the fix: https://github.com/WordPress/wordpress-develop/pull/7197