WordPress.org

Make WordPress Core

Opened 11 months ago

Last modified 8 months ago

#48693 accepted defect (bug)

Silence ini_set caused error or check if function_exists first

Reported by: drazon Owned by: SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version: 5.3
Component: Bootstrap/Load Keywords: needs-testing has-patch
Focuses: Cc:

Description

https://core.trac.wordpress.org/browser/tags/5.3/src/wp-includes/load.php#L352

should be either

<?php
@ini_set( 'display_errors', 0 );

to silence the error

or

<?php
if ( function_exists( 'ini_set' ) ) {
ini_set( 'display_errors', 0 );
}

for the same reason as https://core.trac.wordpress.org/browser/tags/5.3/src/wp-admin/includes/class-wp-debug-data.php#L627

The same login could be applied to other ini_set lines in the wp_debug_mode() function

Attachments (1)

48693.diff (1.4 KB) - added by AkSDvP 9 months ago.
Added a condition check to check whether if "ini_set" core function is exists, then only include "ini_set" statement. Or else suppress the errors.

Download all attachments as: .zip

Change History (16)

#1 @drazon
11 months ago

  • Keywords reporter-feedback added; dev-feedback removed

#2 @drazon
11 months ago

  • Keywords reporter-feedback removed

#3 @SergeyBiryukov
11 months ago

  • Component changed from General to Bootstrap/Load

Hi there, welcome to WordPress Trac! Thanks for the report.

Related: [45611/trunk/src/wp-includes/load.php], which removed error suppression for ini_set() here.

Also related: [29330] / #26772, #33112, #37680.

Could you provide some more details about the environment and issue you're seeing? Is ini_set() disabled via PHP's disable_functions setting or in some other way?

#4 @drazon
11 months ago

thank you for your reply, I can see the error suppression removal. Maybe the if ( function_exists( 'ini_set' ) ) is a more elegant way at this point as it is being in other cases in the core.

Yes ini_set() is disabled at the php.ini disable_functions. I use PHP 7.3. The problem is that in production environments where

disable_functions = "ini_set, ....."
display_errors = 0
WP_DEBUG = false


the PHP error_log gets flooded with unnecessary warnings.

#6 @autotutorial
11 months ago

I'll answer you here and not in your other topic, add_filter or apply_filter is not available in the context of wp-config.php.
I customized the debug based on my preferences, if ini_set works set WP_DEBUG_DISPLAY, default to false, check reserved words in WP_DEBUG_LOG and react.
Create a folder in /wp-content/plugins/testsuper/
add a testsuper.php file to this folder
the plugin name is located at the beginning of the php file.
See if you solve it?

<?php
/**
 * Plugin Name: testsuper
 * Plugin URI: http://www.mywebsite.com/testsuper
 * Description: The very first plugin that I have ever created.
 * Version: 1.0
 * Author: Your Name
 * Author URI: http://www.mywebsite.com
 */
defined( 'ABSPATH' ) or die( 'No script kiddies please!' );
add_filter( 'enable_wp_debug_mode_checks', '__return_false' );
if ( WP_DEBUG ) {
error_reporting( E_ALL );
$display_conf = 0;
  if ( defined( 'WP_DEBUG_DISPLAY' ) && WP_DEBUG_DISPLAY == true )
    $display_conf = 1;
  if( is_string( WP_DEBUG_LOG ) && strlen(WP_DEBUG_LOG) > 5 ) {
    $log_path = WP_DEBUG_LOG;
  } elseif (!empty( WP_DEBUG_LOG ) ) {
    $log_path = WP_CONTENT_DIR . '/debug.log';
  } else {
    $log_path = false;
  }
  if( $log_path ) {
    @ini_set( 'log_errors', 1 );
      if( !@ini_set( 'error_log', $log_path ) )
        error_log('error unknow'."\n",3,$log_path);
  }
} else {
  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 );
}
if ( defined( 'XMLRPC_REQUEST' ) || defined( 'REST_REQUEST' ) || ( defined( 'WP_INSTALLING' ) && WP_INSTALLING ) || wp_doing_ajax() || wp_is_json_request() )
$display_config = 0;
@ini_set( 'display_startup_errors', $display_conf );
@ini_set( 'display_errors', $display_conf );

Hi guys I hope you are okay, sorry I believe that null !== WP_DEBUG_DISPLAY confuses ideas, when I think of null I believe in an undefined constant, frankly there is not the first WP_DEBUG_DISPLAY control.

Last edited 11 months ago by autotutorial (previous) (diff)

#8 @SergeyBiryukov
10 months ago

#49148 was marked as a duplicate.

#9 @SergeyBiryukov
10 months ago

  • Milestone changed from Awaiting Review to 5.3.3
  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

For reference, this was previously fixed in [37448] / #36708, but the fix was reverted in [45611/trunk/src/wp-includes/load.php] per the coding standards.

Since this still appears to cause warnings, let's either add a function_exists() check or restore the error suppression here for now with an exception for PHPCS.

@AkSDvP
9 months ago

Added a condition check to check whether if "ini_set" core function is exists, then only include "ini_set" statement. Or else suppress the errors.

#10 @AkSDvP
9 months ago

  • Keywords needs-testing added; needs-patch removed

#11 @AkSDvP
9 months ago

  • Keywords has-patch added

#12 @autotutorial
9 months ago

No need to be collected in error_log (since the suppression operator @ temporarily sets error_reporting to 0).

<?php
error_reporting(0);
ini_set( 'display_errors', 1);
if( ( $var = ini_set( 'display_errors', 1) ) {
//ini_set work
} elseif( null !== ini_get( 'display_errors') ) {
$var = ini_get( 'display_errors' );
$bool = true;
}
if( isset( $bool ) ) {
//return false value from ini_get in this case it is wrong,
//because only the true value is valid
} else {
//else value from ini_set, any value (boolean, string etc.)
}
if( $var && empty( $bool ) ) {
//ini_set is usable.
}
if( isset( $bool ) ) {
    if( ! $var ) {
        //ini_get work with false value, without ini_set
    } else {
        //ini_get work with true value, without ini_set
    }
}
error_reporting( E_ALL );

1) This code initially means set to true and then check the return value for the second ini_set, 2) otherwise if it is different from null it returns the value of ini_get which means ini_set disabled, (because initially the value is set to true), 3) or otherwise returns the value from ini_set null, false means ini_set disabled (because the first value is set to true)

Currently my hosting does not allow ini_set but the function exists, modifying the php source code is common practice to use screen errors except for ini_set, since php does not offer this support.
So the example pseudo code is always a good encoding (check the second value of ini_set if it is true, ini_set is usable).
Add Silence all errors, error_reporting start from E_ALL.

<?php
error_reporting( 0 );
        if ( WP_DEBUG ) {

                if ( WP_DEBUG_DISPLAY ) {
                        ini_set( 'display_errors', 1 );
                } elseif ( null !== WP_DEBUG_DISPLAY ) {
                        ini_set( 'display_errors', 0 );
                }

                if ( in_array( strtolower( (string) WP_DEBUG_LOG ), array( 'true', '1' ), true ) ) {
                        $log_path = WP_CONTENT_DIR . '/debug.log';
                } elseif ( is_string( WP_DEBUG_LOG ) ) {
                        $log_path = WP_DEBUG_LOG;
                } else {
                        $log_path = false;
                }

                if ( $log_path ) {
                        ini_set( 'log_errors', 1 );
                        ini_set( 'error_log', $log_path );
                }
                error_reporting( E_ALL );
        } else {
                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 );
        }
$old_value = error_reporting();
error_reporting( 0 );
        if ( defined( 'XMLRPC_REQUEST' ) || defined( 'REST_REQUEST' ) || ( defined( 'WP_INSTALLING' ) && WP_INSTALLING ) || wp_doing_ajax() || wp_is_json_request() ) {
                ini_set( 'display_errors', 0 );
        }
error_reporting( $old_value );
}
Last edited 9 months ago by autotutorial (previous) (diff)

#13 @audrasjb
9 months ago

  • Milestone changed from 5.3.3 to 5.4

Moving all unfixed tickets from 5.3.3 to milestone 5.4, as there is no plan for a 5.3.3 maintenance release for now.

#14 @ayeshrajans
9 months ago

I don't think that we should gracefully handle the lack of ini_set and ini_get in the runtime.

This can have severe impact in security, because throughout the WordPress core and plugins, I highly doubt every ini_set call is checked. In the site health report, WordPress reports that ini_get function is not available, and I believe this as far as WordPress should be flexible.

For example, if we cannot set display_errors=Off, we should at least trigger a warning in site owners logs instead of silently not setting this configuration value, which might result in the whole stack trace to be printed on screen.

Last edited 9 months ago by ayeshrajans (previous) (diff)

#15 @audrasjb
8 months ago

  • Milestone changed from 5.4 to Future Release

Hi,

With 5.4 Beta 3 approaching and the Beta period reserved for bugs introduced during the cycle, this is being moved to Future Release. If any maintainer or committer feels this should be included or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

Note: See TracTickets for help on using tickets.