Make WordPress Core

Opened 17 months ago

Closed 13 months ago

Last modified 13 months ago

#57728 closed defect (bug) (fixed)

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

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

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 16 months ago.

Download all attachments as: .zip

Change History (15)

#1 @SergeyBiryukov
17 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 6.3

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


17 months 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
17 months ago

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

#4 @hrdelwar
17 months ago

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

#5 @hasanmisbah
17 months ago

hi @dhrupo Your PR. was fantastic

#6 @SergeyBiryukov
16 months 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.

Version 0, edited 16 months ago by SergeyBiryukov (next)

#7 @oglekler
13 months 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.


13 months 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
13 months 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.


13 months ago

#11 @mukesh27
13 months ago

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

#12 @SergeyBiryukov
13 months ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to 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:


13 months ago
#13

Thanks for the PR! Merged in r56132.

@SergeyBiryukov commented on PR #4776:


13 months ago
#14

Thanks for the PR! Merged in r56132.

Note: See TracTickets for help on using tickets.