Make WordPress Core

Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#57728 closed defect (bug) (fixed)

PHP 8.1 deprecation notice for gmt_offset on wp-admin/options.php

Reported by: SergeyBiryukov Owned by: SergeyBiryukov
Priority: normal Milestone: 6.3
Component: Date/Time Version:
Severity: normal Keywords: php81 has-patch
Cc: Focuses:

Description

  1. In General Settings, select a named timezone (not a UTC+- one).
  2. When saving the settings, there is a PHP 8.1 deprecation notice:
    Deprecated: preg_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in wp-includes/formatting.php on line 4862
    

The notice comes from sanitize_option():

case 'gmt_offset':
	$value = preg_replace( '/[^0-9:.-]/', '', $value ); // Strips slashes.
	break;

Appears to be caused by this fragment in wp-admin/options.php where the default value for any options not passed in the current $_POST request (which happens to be gmt_offset here) is set to null:

foreach ( $options as $option ) {

	...

	$option = trim( $option );
	$value  = null;
	if ( isset( $_POST[ $option ] ) ) {
		$value = $_POST[ $option ];
		if ( ! is_array( $value ) ) {
			$value = trim( $value );
		}
		$value = wp_unslash( $value );
	}
	update_option( $option, $value );
}

Attachments (1)

57728.diff​ (492 bytes ) - added by SergeyBiryukov 3 years ago.

Download all attachments as: .zip

Change History (15)

#1 @SergeyBiryukov
4 years ago

  • Keywords needs-patch added
  • Milestone Awaiting Review → 6.3

This ticket was mentioned in ​PR #4082 on ​WordPress/wordpress-develop by ​@dhrupo.


4 years ago
#2

  • Keywords has-patch added; needs-patch removed

Updated the options table if the value exists in the POST request

Trac ticket: https://core.trac.wordpress.org/ticket/57728

#3 @dhrupo
4 years ago

Hi @SergeyBiryukov,
Please check the code and let me know :)

#4 @hrdelwar
4 years ago

hi @dhrupo, just checked the PR and it works perfectly.

#5 @hasanmisbah
4 years ago

hi @dhrupo Your PR. was fantastic

@SergeyBiryukov
3 years ago

#6 @SergeyBiryukov
3 years ago

Thanks for the PR!

Looks like it might have unintended consequences though, as some options, e.g. blog_public, actually rely on null being passed to update_option() if the option was not sent to options.php, see [21849] and [21851] / #16416.

I believe we can specifically check if gmt_offset is a string in sanitize_option(), see 57728.diff​.

Last edited 3 years ago by SergeyBiryukov (previous) (diff)

#7 @oglekler
3 years ago

This can be a quick fix as needed with patch Sergey as suggested but I believe that it needs further discussion.

Is it ok to have null as gmt_offset in the first place? I wonder if it needs to be checked from where it's coming from. But somehow with PHP8.1 (by online testing tool) I am getting a fatal error, and it needs to be addressed anyway. If this is the case, others placed when preg_replace() is used, need to be checked to avoid receiving null as subject.

This ticket was mentioned in ​PR #4776 on ​WordPress/wordpress-develop by ​@adi3890.


3 years ago
#8

Adding a check to verify data type
if $value is_string returns $value, or else returns empty string.
This prevents deprecation notice and also saves data as empty string as saved in prior php versions

Trac ticket: https://core.trac.wordpress.org/ticket/57728

#9 @adi3890
3 years ago

@oglekler @SergeyBiryukov Just modified the logic of is_string check with a default value as empty string. This way data will be saved as empty string as it is saved now, and this validation prevents php deprecation notice.

This ticket was mentioned in ​Slack in #core by mukeshpanchal27. ​View the logs.


3 years ago

#11 @mukesh27
3 years ago

@SergeyBiryukov do you have capacity to review the latest PR?

#12 @SergeyBiryukov
3 years ago

  • Owner set to SergeyBiryukov
  • Resolution → fixed
  • Status new → closed

In 56132:

Options, Meta APIs: Check if the gmt_offset value is numeric in sanitize_option().

When saving the settings via the admin UI, the default value for any options not passed in the current $_POST request is set to null in wp-admin/options.php. Some options, e.g. blog_public, then rely on null being passed to update_option() to determine whether the value was changed or not.

This commit resolves a PHP 8.1 deprecation notice when saving the gmt_offset option without any changes:

Deprecated: preg_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated

Includes a similar fix for the blog_charset option.

Follow-up to [4112], [4329], [5541], [21849].

Props adi3890, dhrupo, hrdelwar, hasanmisbah, oglekler, mukesh27, SergeyBiryukov.
Fixes #57728.

​@SergeyBiryukov commented on ​PR #4082:


3 years ago
#13

Thanks for the PR! Merged in r56132.

​@SergeyBiryukov commented on ​PR #4776:


3 years ago
#14

Thanks for the PR! Merged in r56132.

Note: See TracTickets for help on using tickets.