Correct default values in wp_handle_comment_submission()

[SergeyBiryukov]

Background: #34059, #56681.

This came up when looking into the last remaining test failure on PHP 8.1:

1) WP_Test_REST_Posts_Controller::test_get_post_draft_edit_context
Failed asserting that '<ul class="wp-block-latest-posts__list wp-block-latest-posts"><li><a class="wp-block-latest-posts__post-title" href="http://example.org/?p=3701">Protected: Hola</a><div class="wp-block-latest-posts__post-excerpt">Hello World!</div></li>\n
<li><a class="wp-block-latest-posts__post-title" href="http://example.org/?p=3601">Post 28</a><div class="wp-block-latest-posts__post-excerpt">Post excerpt 0005016</div></li>\n
<li><a class="wp-block-latest-posts__post-title" href="http://example.org/?p=3600">Post 27</a><div class="wp-block-latest-posts__post-excerpt">Post excerpt 0005015</div></li>\n
<li><a class="wp-block-latest-posts__post-title" href="http://example.org/?p=3599">Post 26</a><div class="wp-block-latest-posts__post-excerpt">Post excerpt 0005014</div></li>\n
<li><a class="wp-block-latest-posts__post-title" href="http://example.org/?p=3598">Post 25</a><div class="wp-block-latest-posts__post-excerpt">Post excerpt 0005013</div></li>\n
</ul> <ul class="wp-block-latest-posts__list wp-block-latest-posts"><li><a class="wp-block-latest-posts__post-title" href="http://example.org/?p=3701">Protected: Hola</a><div class="wp-block-latest-posts__post-full-content">Hello World!</div></li>\n
<li><a class="wp-block-latest-posts__post-title" href="http://example.org/?p=3601">Post 28</a><div class="wp-block-latest-posts__post-full-content">Post content 0005016</div></li>\n
<li><a class="wp-block-latest-posts__post-title" href="http://example.org/?p=3600">Post 27</a><div class="wp-block-latest-posts__post-full-content">Post content 0005015</div></li>\n
<li><a class="wp-block-latest-posts__post-title" href="http://example.org/?p=3599">Post 26</a><div class="wp-block-latest-posts__post-full-content">Post content 0005014</div></li>\n
<li><a class="wp-block-latest-posts__post-title" href="http://example.org/?p=3598">Post 25</a><div class="wp-block-latest-posts__post-full-content">Post content 0005013</div></li>\n
</ul>' does not contain "Hello World!".

/var/www/tests/phpunit/tests/rest-api/rest-posts-controller.php:1980
/var/www/vendor/bin/phpunit:123

The test was added in [50717] and has to do with allowing authenticated users to read the contents of password protected posts if they have the edit_post meta capability for the post. While the test could use some improvement, as noted in comment:3:ticket:56681, it fails when running the full test suite on PHP 8.1 but passes when run in isolation, which indicates that it could be affected by some other test.

After some investigation, the culprit was found: test_submitting_comment_to_password_protected_post_succeeds()​, which temporarily assigns the $_COOKIE[ 'wp-postpass_' . COOKIEHASH ] value. Due to how WordPress handles password protected posts, once that value is set, it affects all posts protected with the same password, see #16483. Both tests in question happen to use password as the password value.

While the latter test unsets the $_COOKIE[ 'wp-postpass_' . COOKIEHASH ] value afterwards, the problem is that the test itself also triggers a PHP 8.1 "null to non-nullable" deprecation notice that was temporarily silenced in [51968]. After some debugging it became apparent that once that notice is triggered, the test stops any further execution, unintentionally leaving the $_COOKIE[ 'wp-postpass_' . COOKIEHASH ] in place, which then affects the other test.

Looking into the source of that deprecation notice, it is caused by the defaults in wp_handle_comment_submission():

$comment_author       = null;
$comment_author_email = null;
$comment_author_url   = null;
$comment_content      = null;

These values are all documented to be a string in various related filters, and core also expects them to be a string, so there is no reason for these defaults to be set to null. Setting them to an empty string instead resolves the issues:

• The latter test can proceed as expected and unset the cookie value so that it does not affect the former test.
• It allows us to remove six stopgap conditionals added in [51968] to silence the deprecation notices.

The patch includes:

• Setting the defaults in wp_handle_comment_submission() to an empty string.
• Adding a dedicated unit test to verify the type of these default values.
• Removing the deprecation notice silencing as no longer needed.

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

[mukesh27]

Thanks @SergeyBiryukov

​PR #3387 look good.

[SergeyBiryukov]

In 54368:

Code Modernization: Correct default values in wp_handle_comment_submission().

This affects the following parameters subsequently passed to wp_new_comment():

• comment_author
• comment_author_email
• comment_author_url
• comment_content

The default values for these parameters were previously set to null, causing PHP 8.1 "null to non-nullable" deprecation notices when running sanitization filters on them via wp_filter_comment().

While the deprecation notices were temporarily silenced in the unit test suite, that caused an unexpected issue in a test for submitting a comment to a password protected post​, where the $_COOKIE[ 'wp-postpass_' . COOKIEHASH ] value was no longer unset, as the test stopped any further execution once the deprecation notice was triggered.

Due to how WordPress handles password protected posts, once that value is set, it affects all posts protected with the same password, so this resulted in unintentionally affecting another test​ which happened to use the same password.

These values are all documented to be a string in various related filters, and core also expects them to be a string, so there is no reason for these defaults to be set to null. Setting them to an empty string instead resolves the issues.

This commit includes:

• Setting the defaults in wp_handle_comment_submission() to an empty string.
• Adding a dedicated unit test to verify the type of these default values.
• Removing the deprecation notice silencing as no longer needed.

Follow-up to [34799], [34801], [51968].

Props jrf, desrosj, mukesh27, SergeyBiryukov.
Fixes #56712. See #56681, #55656.

Merged in r54368.