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

[kitchin]

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.

[SergeyBiryukov]

Hi there, thanks for the ticket!

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

[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.

[kitchin]

First patch, minimal

[kitchin]

Alternate patch, not tested.

[devmuhib]

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.

[kitchin]

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

[rajinsharwar]

Hi @kitchin, thanks for the test instructions. Your 2nd patch does seem to solve the issue and address this behavior, on the single-site installation. I didn't get a chance to test this on the multi-site today, but if someone can share if it addresses the issue, we can get this merged sooner.

[devmuhib]

I downloaded the latest version with wget ​https://wordpress.org/nightly-builds/wordpress-latest.zip and my version is 6.6-alpha-58025. But I could not reproduce the error. I also checked wp-admin/error_log. but not found anything error code.

wp-admin/error_log.

[hellofromTonya]

Reproduction Report

Environment

• OS: macOS 14.5
• Web Server: Nginx
• PHP: 8.3
• WordPress: 6.6-beta2-58411
• Browser: Firefox 126.0.1
• Theme: TT4
• Active Plugins: None

Testing Steps

1. Ran wp db reset --yes to wipe the database.
2. Walked through the install, while checking "Discourage search engines" option.
3. Logged in.
4. Navigated to Settings / Writing.
5. Clicked "Save Changes" button.
6. Then checked the logs.

Actual Results

• ✅ Able to reproduce after clicking the "Save Changes" button.

[14-Jun-2024 15:38:51 UTC] PHP Deprecated:  explode(): Passing null to parameter #2 ($string) of type string is deprecated in ./wp-includes/formatting.php on line 4952

[hellofromTonya]

To evaluate the different patches / approaches in this ticket, I needed to first understand the relationship between 'blog_public' and 'ping_sites'.

Relationship between 'blog_public' and 'ping_sites'

Source: ​https://developer.wordpress.org/apis/options/#writing

ping_sites: When you publish a new post, WordPress automatically notifies the following site update services. For more about this, see Update Services. Separate multiple service URLs with line breaks. Requires blog_public to have a value of 1.
Default: ‘​http://rpc.pingomatic.com/’
Data type:String (possibly multi-line)

This relationship shows itself in the options-writing.php code (shown here), as the textarea#ping_sites only renders when '1' === get_option( 'blog_public' ).

Commit history for context:

• options.php 'ping_sites' added to the options array[] via [12825].
• options-writing.php: textarea#ping_sites added in [949].
• options-writing.php: conditionally rendering the ping_sites based on blog_public added in [4326].

Evaluating the patches / approaches

The root cause is as @kitchin noted. kitchin59818.diff​ makes sense to me given the relationship between these 2 options and the purpose of the list of allowed 'writing' options.

While defensive guarding and consistency from 59818.diff​ also makes sense, the expected data type for the 'ping_sites' option is string (not nullable string).

So if the ping_sites option is not in the post back, then it shouldn't be added to the options list to sanitize in sanitize_option().

As I note, both patches resolve the issue, while IMO kitchin59818.diff​ resolves the root cause that is driving the error.

For 6.6, I'm in favor of moving forward with kitchin59818.diff​.

@SergeyBiryukov any objections? What do you think?

Marking kitchin59818.diff​ for commit, but will wait until before 6.6 Beta 3 to give Sergey time to review.

[SergeyBiryukov]

Replying to hellofromTonya:

For 6.6, I'm in favor of moving forward with kitchin59818.diff​.

@SergeyBiryukov any objections? What do you think?

I agree with your assessment, thanks!

[hellofromTonya]

Replying to SergeyBiryukov:

Replying to hellofromTonya:

For 6.6, I'm in favor of moving forward with kitchin59818.diff​.

@SergeyBiryukov any objections? What do you think?

I agree with your assessment, thanks!

Awesome :) Thanks @SergeyBiryukov.

I'm preparing the commit now.

Commit confidence check to ensure `kitchin59818.diff` passes all CI jobs before committing.

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

[hellofromTonya]

In 58425:

Code Modernization: Fix 'ping_sites' non-nullable deprecation.

When saving options from the Settings page, include the 'ping_sites' option in the allowed "writing" options list only when the 'blog_public' option is '1'.

Fixes a PHP 8.1 and above "null to non-nullable" deprecation notice in sanitize_option() (which happens when here as part of [22255]):

Deprecated: explode(): Passing null to parameter #2 ($string) of type string is deprecated in .../wp-includes/formatting.php

Explanation

​Per the documentation, the ping_sites option requires the 'blog_public' option to have a value of '1' and must be a string data type. null is not valid for this option.

The relationship between the 2 options shows itself in the options-writing.php code (shown here and in [4326]), as the textarea#ping_sites only renders when '1' === get_option( 'blog_public' ).

What happens if 'blog_public' is not '1'?

The 'ping_sites' option will not be a field on the page. Upon saving:

• HTTP POST ($_POST) does not include 'ping_sites'.
• Before this commit:
  • The option's value was set to null before being passed to update_option().
  • update_option() invokes sanitize_option().
  • A null value for the 'ping_sites' case was passed to explode(), which threw a deprecation notice on PHP 8.1 and above.
• With this commit, the 'ping_sites' option is no longer included in the allow list and thus will not be passed to update_options() > sanitize_option() > explode().

Follow-up to [22255], [12825], [4326], [949].

Props kitchin, SergeyBiryukov, swissspidy, devmuhib, rajinsharwar, hellofromTonya.
Fixes #59818.

Committed via https://core.trac.wordpress.org/changeset/58425.