Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 6 months ago

#52226 closed defect (bug) (fixed)

PHP 8 issue: Fatal error when error_reporting is disabled

Reported by: fijisunshine's profile fijisunshine Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.8 Priority: normal
Severity: normal Version: 5.6
Component: Bootstrap/Load Keywords: php8 has-patch
Focuses: Cc:

Description

I had posted this in the Support > Everything else WordPress forum and the moderator, James Huff, asked me to create a ticket with you.

For security reasons, I've always disabled the function "error_reporting" in php.ini unless I'm debugging. Disabling this function works great in php 7.4 – WordPress doesn't output error messages to the error log, it just outputs a message to the error log that error_reporting is disabled for security reasons.

But in php 8.0, I'm getting this error:

PHP Fatal error: Uncaught Error: Call to undefined function error_reporting() in … /wp-load.php:24

In php 7.4, it wouldn't be a fatal error. It would just say that error_reporting is disabled for security reasons.

Can you update WordPress so that it won't have fatal errors when error_reporting is disabled in php.ini for security reasons? Thank you

Attachments (1)

52226.diff (1.0 KB) - added by peterwilsoncc 4 years ago.

Download all attachments as: .zip

Change History (15)

@peterwilsoncc
4 years ago

#1 @peterwilsoncc
4 years ago

  • Keywords php8 added

@fijisunshine Thanks for the report.

When disabling core PHP functions in the php.ini file it's probably best to add a dummy version in your wp-config.php file so fatal errors don't occur within WordPress, themes or plugins.

<?php
// wp-config.php

if ( ! function_exists( 'error_reporting' ) {
  function error_reporting() {}
}

As the first WordPress call to error_reporting() occurs before wp-config.php is loaded, I think it's reasonable to add a function_exists() check to that, as proposed in 52226.diff.

I wouldn't want to add the conditionals throughout or dummy functions to WordPress as modifying php.ini to disable PHP Core functions is an advanced use case, is slightly out of warranty and solved with additional configuration at run time.

#2 @mukesh27
4 years ago

#52225 was marked as a duplicate.

#3 @ayeshrajans
4 years ago

Thanks for creating this issue, @fijisunshine - it indeed brings more attention than the forum.

I find it interesting that error_reporting function is disabled. While it can enable error reporting, it could also be used to hide them. I also think the _display_ of errors is information exposure vulnerability - not the fact that they are reported and logged. But I'm digressing.

The fatal errors are because in PHP 8.0, disabled function are not registered at all, and allows to be redeclared as well. See PHP 8.0: Disabled functions behave as if they do not exist.

Redeclaring a dummy function for disabled functions is only possible in PHP 8.0 as well.

Patch above by @peterwilsoncc looks great, although I wonder if we could use ini_set as a fall-back , provided it's available:

<?php
/*
 * The error_reporting() function can be disabled in php.ini and may be done so for
 * security purposes. On systems with this function disabled it's best to add a dummy
 * function to the wp-config.php file but as this call to the function is run prior
 * to wp-config.php loading, it is wrapped in a function_exists() check.
 */
if ( function_exists( 'error_reporting' ) ) {
        error_reporting( E_CORE_ERROR | E_CORE_WARNING | E_COMPILE_ERROR | E_ERROR | E_WARNING | E_PARSE | E_USER_ERROR | E_USER_WARNING | E_RECOVERABLE_ERROR );
}
elseif ( function_exists( 'ini_set' ) ) {
        ini_set( 'error_reporting',  E_CORE_ERROR | E_CORE_WARNING | E_COMPILE_ERROR | E_ERROR | E_WARNING | E_PARSE | E_USER_ERROR | E_USER_WARNING | E_RECOVERABLE_ERROR );
}

#4 @fijisunshine
4 years ago

Thank you all for your excellent feedback.

FYI I tried @peterwilsoncc's suggestion in wp-config.php and I still get the same fatal error in php 8.0. As he said, "...the first WordPress call to error_reporting() occurs before wp-config.php is loaded..."

Thanks again

#5 @fijisunshine
4 years ago

Thank you again for your great feedback. Did we ever find a solution? i.e. a permanent solution, or a temporary fix until it's implemented in WordPress.

Thanks again

#6 @SergeyBiryukov
4 years ago

  • Milestone changed from Awaiting Review to 5.8

#7 @SergeyBiryukov
4 years ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 50447:

Bootstrap/Load: Check if the error_reporting() function exists in wp-load.php.

This avoids a fatal error on PHP 8 if error_reporting() is disabled in php.ini.

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 a function_exists() check.

Props peterwilsoncc, fijisunshine, ayeshrajans.
Fixes #52226.

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


4 years ago

#9 @Boniu91
4 years ago

Confirmed with PHP Version 8.0.0 and WP 5.8-alpha-50784

#10 @justinahinon
4 years ago

Tested with WordPress 5.8-alpha-50427-src and PHP 8.0.0.

Disabled error_reporting .

I get no fatal error.

#11 @peterwilsoncc
4 years ago

  • Component changed from Security to Bootstrap/Load

This ticket was mentioned in PR #7197 on WordPress/wordpress-develop by @gansbrest.


6 months ago
#12

  • Keywords has-patch added

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 here https://core.trac.wordpress.org/ticket/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/

Track ticket: https://core.trac.wordpress.org/ticket/61873

@gansbrest commented on PR #7197:


6 months ago
#13

@martinkrcho added comments blocks for consistency with wp-load.php checks.

@SergeyBiryukov commented on PR #7197:


6 months ago
#14

Thanks for the PR! Merged in r58905.

Note: See TracTickets for help on using tickets.