Make WordPress Core

Opened 4 months ago

Last modified 3 days ago

#59818 accepted defect (bug)

[PHP 8.1] deprecated null in `explode()` for 'ping_sites' in `wp-admin/options-writing.php` with "Discourage search engines" on

Reported by: kitchin's profile kitchin Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.5 Priority: normal
Severity: normal Version:
Component: Administration Keywords: php81 has-patch has-testing-info
Focuses: php-compatibility Cc:

Description

When "Discourage search engines" is on, any update to Dashboard / Writing causes:
Deprecated: explode(): Passing null to parameter #2 ($string) of type string is deprecated in .../wp-includes/formatting.php

In this case, the 'ping_sites' option is not displayed, so the $_POST value is null. The deprecation warning is triggered in wp-includes/formatting.php, in sanitize_option(), "case 'ping_sites':".

Tested on today's nightly, WordPress 6.5-alpha-57063. Also affects latest release, WP 6.3.2.

Attachments (3)

59818.diff (1.2 KB) - added by SergeyBiryukov 4 months ago.
kitchin59818.diff (585 bytes) - added by kitchin 4 months ago.
First patch, minimal
kitchin59818.alt.diff (4.3 KB) - added by kitchin 4 months ago.
Alternate patch, not tested.

Download all attachments as: .zip

Change History (10)

#1 @swissspidy
4 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 6.5
  • Version trunk deleted

#2 @SergeyBiryukov
4 months ago

  • Keywords has-patch added; needs-patch removed
  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

Hi there, thanks for the ticket!

59818.diff follows the approach from [56132] / #57728.

#3 @SergeyBiryukov
4 months ago

  • Keywords php81 added

#4 follow-up: @kitchin
4 months ago

Thanks for the quick action! I have a different approach that addresses the underling issue.

The controller wp-admin/options.php uses update_option() unconditionally on all relevant elements of $allowed_options, even if they don't appear in $_POST. Luckily in the current codebase, or as part of its design, there is a filter on get_option( 'ping_sites' ), called privacy_ping_filter(), which checks:

if ( '1' === get_option( 'blog_public' ) ) {
	return $sites;
} else {
	return '';
}

So when update_option() uses get_option() to compare $value to $old_value, it sees '' === '', where explode( "\n", $value ) has cast NULL to ''. Thus no data loss.

My first patch mods $allowed_options. It does it directly, rather than using the filter, since the variable is very specific to the file it is in. Might as well make it easy to read. This patch is tested as far as updating options goes.

My alternate patch also reconciles the strictness and specificity of the evaluation of 'blog_public' across the codebase, except multisite. In other words, '1' === vs. '0' != , etc. It also makes the filter signature in wp-includes/functions.php correct. Multisite seems to have its own way of doing things. This patch is not tested.

@kitchin
4 months ago

First patch, minimal

@kitchin
4 months ago

Alternate patch, not tested.

#5 in reply to: ↑ 4 @devmuhib
6 weeks ago

Can you add the testing instruction please?

Replying to kitchin:

Thanks for the quick action! I have a different approach that addresses the underling issue.

The controller wp-admin/options.php uses update_option() unconditionally on all relevant elements of $allowed_options, even if they don't appear in $_POST. Luckily in the current codebase, or as part of its design, there is a filter on get_option( 'ping_sites' ), called privacy_ping_filter(), which checks:

if ( '1' === get_option( 'blog_public' ) ) {
	return $sites;
} else {
	return '';
}

So when update_option() uses get_option() to compare $value to $old_value, it sees '' === '', where explode( "\n", $value ) has cast NULL to ''. Thus no data loss.

My first patch mods $allowed_options. It does it directly, rather than using the filter, since the variable is very specific to the file it is in. Might as well make it easy to read. This patch is tested as far as updating options goes.

My alternate patch also reconciles the strictness and specificity of the evaluation of 'blog_public' across the codebase, except multisite. In other words, '1' === vs. '0' != , etc. It also makes the filter signature in wp-includes/functions.php correct. Multisite seems to have its own way of doing things. This patch is not tested.

#6 @kitchin
5 weeks ago

STR:

  1. Drop all tables. Delete all files in public_html.
  2. wget https://wordpress.org/nightly-builds/wordpress-latest.zip
  3. unzip wordpress-latest.zip
  4. Move files and upload wp-config.php, with define('WP_DEBUG', true );
  5. Visit /wp-admin to install. Check "Discourage search engines" and install.
  6. Login and go to Settings / Writing.
  7. Click Save Changes.

Depending on your PHP settings, you will see the error on the screen and/or in wp-admin/error_log.

WordPress 6.5-alpha-57320
Linux 4.18... x86_64
PHP 8.2...

#7 @swissspidy
3 days ago

  • Keywords has-testing-info added
Note: See TracTickets for help on using tickets.