WordPress.org
Get WordPress

Make WordPress Core

Changeset 54368


Ignore:
Timestamp:
10/03/2022 03:20:49 PM (2 years ago)
Author:
SergeyBiryukov
Message:

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.

Location:
trunk
Files:
2 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/wp-includes/comment.php

    r53944 r54368  
    34283428function wp_handle_comment_submission( $comment_data ) {
    34293429    $comment_post_id      = 0;
    3430     $comment_author       = null;
    3431     $comment_author_email = null;
    3432     $comment_author_url   = null;
    3433     $comment_content      = null;
     3430    $comment_author       = '';
     3431    $comment_author_email = '';
     3432    $comment_author_url   = '';
     3433    $comment_content      = '';
    34343434    $comment_parent       = 0;
    34353435    $user_id              = 0;

  • trunk/tests/phpunit/tests/comment-submission.php

    r53863 r54368  
    224224     */
    225225    public function test_submitting_comment_to_password_protected_post_succeeds() {
    226         if ( PHP_VERSION_ID >= 80100 ) {
    227             /*
    228              * For the time being, ignoring PHP 8.1 "null to non-nullable" deprecations coming in
    229              * via hooked in filter functions until a more structural solution to the
    230              * "missing input validation" conundrum has been architected and implemented.
    231              */
    232             $this->expectDeprecation();
    233             $this->expectDeprecationMessageMatches( '`Passing null to parameter \#[0-9]+ \(\$[^\)]+\) of type [^ ]+ is deprecated`' );
    234         }
    235 
    236226        $password = 'password';
    237227        $hasher   = new PasswordHash( 8, true );
    … …  
    322312     * @covers ::wp_handle_comment_submission
    323313     */
    324     public function test_submitting_comment_handles_slashes_correctly_handles_slashes() {
    325         if ( PHP_VERSION_ID >= 80100 ) {
    326             /*
    327              * For the time being, ignoring PHP 8.1 "null to non-nullable" deprecations coming in
    328              * via hooked in filter functions until a more structural solution to the
    329              * "missing input validation" conundrum has been architected and implemented.
    330              */
    331             $this->expectDeprecation();
    332             $this->expectDeprecationMessageMatches( '`Passing null to parameter \#[0-9]+ \(\$[^\)]+\) of type [^ ]+ is deprecated`' );
    333         }
    334 
     314    public function test_submitting_comment_handles_slashes_correctly() {
    335315        $data    = array(
    336316            'comment_post_ID' => self::$post->ID,
    … …  
    497477     */
    498478    public function test_anonymous_user_cannot_comment_unfiltered_html() {
    499         if ( PHP_VERSION_ID >= 80100 ) {
    500             /*
    501              * For the time being, ignoring PHP 8.1 "null to non-nullable" deprecations coming in
    502              * via hooked in filter functions until a more structural solution to the
    503              * "missing input validation" conundrum has been architected and implemented.
    504              */
    505             $this->expectDeprecation();
    506             $this->expectDeprecationMessageMatches( '`Passing null to parameter \#[0-9]+ \(\$[^\)]+\) of type [^ ]+ is deprecated`' );
    507         }
    508 
    509479        $data    = array(
    510480            'comment_post_ID' => self::$post->ID,
    … …  
    833803     */
    834804    public function test_submitting_comment_with_empty_type_results_in_correct_type() {
    835         if ( PHP_VERSION_ID >= 80100 ) {
    836             /*
    837              * For the time being, ignoring PHP 8.1 "null to non-nullable" deprecations coming in
    838              * via hooked in filter functions until a more structural solution to the
    839              * "missing input validation" conundrum has been architected and implemented.
    840              */
    841             $this->expectDeprecation();
    842             $this->expectDeprecationMessageMatches( '`Passing null to parameter \#[0-9]+ \(\$[^\)]+\) of type [^ ]+ is deprecated`' );
    843         }
    844 
    845805        $data    = array(
    846806            'comment_post_ID' => self::$post->ID,
    … …  
    898858
    899859        $comment = wp_handle_comment_submission( $data );
    900 
    901         remove_filter( 'preprocess_comment', array( $this, 'filter_preprocess_comment' ) );
    902860
    903861        $this->assertNotWPError( $comment );
    … …  
    921879    }
    922880
     881    /**
     882     * @ticket 56712
     883     *
     884     * @covers ::wp_handle_comment_submission
     885     */
     886    public function test_submitting_comment_without_optional_parameters_sets_them_to_empty_strings() {
     887        $data = array(
     888            'comment_post_ID' => self::$post->ID,
     889        );
     890
     891        add_filter( 'pre_option_require_name_email', '__return_zero' );
     892        add_filter( 'allow_empty_comment', '__return_true' );
     893
     894        add_filter( 'preprocess_comment', array( $this, 'filter_preprocess_comment' ) );
     895
     896        $comment = wp_handle_comment_submission( $data );
     897
     898        $this->assertNotWPError( $comment );
     899        $this->assertInstanceOf( 'WP_Comment', $comment );
     900
     901        $commentdata = $this->preprocess_comment_data;
     902
     903        $this->assertSame( '', $commentdata['comment_author'], 'Comment author should default to an empty string.' );
     904        $this->assertSame( '', $commentdata['comment_author_email'], 'Comment author email should default to an empty string.' );
     905        $this->assertSame( '', $commentdata['comment_author_url'], 'Comment author URL should default to an empty string.' );
     906        $this->assertSame( '', $commentdata['comment_content'], 'Comment content should default to an empty string.' );
     907    }
     908
    923909    public function filter_preprocess_comment( $commentdata ) {
    924910        $this->preprocess_comment_data = $commentdata;
    … …  
    932918     */
    933919    public function test_submitting_duplicate_comments() {
    934         if ( PHP_VERSION_ID >= 80100 ) {
    935             /*
    936              * For the time being, ignoring PHP 8.1 "null to non-nullable" deprecations coming in
    937              * via hooked in filter functions until a more structural solution to the
    938              * "missing input validation" conundrum has been architected and implemented.
    939              */
    940             $this->expectDeprecation();
    941             $this->expectDeprecationMessageMatches( '`Passing null to parameter \#[0-9]+ \(\$[^\)]+\) of type [^ ]+ is deprecated`' );
    942         }
    943 
    944920        $data           = array(
    945921            'comment_post_ID' => self::$post->ID,
    … …  
    960936     */
    961937    public function test_comments_flood() {
    962         if ( PHP_VERSION_ID >= 80100 ) {
    963             /*
    964              * For the time being, ignoring PHP 8.1 "null to non-nullable" deprecations coming in
    965              * via hooked in filter functions until a more structural solution to the
    966              * "missing input validation" conundrum has been architected and implemented.
    967              */
    968             $this->expectDeprecation();
    969             $this->expectDeprecationMessageMatches( '`Passing null to parameter \#[0-9]+ \(\$[^\)]+\) of type [^ ]+ is deprecated`' );
    970         }
    971 
    972938        $data          = array(
    973939            'comment_post_ID' => self::$post->ID,
Note: See TracChangeset for help on using the changeset viewer.