#61873 closed defect (bug) (fixed)
Check if error_reporting function exists, otherwise breaks php8 installs when disabled in the ini config
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 6.7 | Priority: | normal |
| Severity: | normal | Version: | 6.7 |
| 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
@
16 months 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
@
16 months ago
Hi there, thanks for the ticket!
Replying to gansbrest:
Short summary of the problem: When
error_reportingfunction is disabled on the hosting provider, it breaks admin area due to the fact thatwp-admin/load-styles.phpandwp-admin/load-scripts.phpare 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.phpfile.
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.phpfile, as there are multiple other calls in core or plugins.
However, as this call to the function is run prior to
wp-config.phploading, 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
@
16 months 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
@
16 months 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
@
16 months 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
@
16 months 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
@
16 months 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.phpfiles (see my pull request above) we can notice that error_reporting is called beforewp-config.phpis 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