WordPress.org

Make WordPress Core

Opened 2 months ago

Last modified 2 months ago

#54379 new defect (bug)

`get_comment_ID()` can cause a notice within the `comment_text` filter

Reported by: dd32 Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch
Focuses: Cc:

Description (last modified by dd32)

The comment_text filter is primarily run with the context of a comment, however, during comment submission check_comment() calls the filter without any comment context, and without a global $comment object.

The result of this is that if any plugin is filtering on comment_text calls get_comment_ID() then it'll trigger the following notice:

E_NOTICE: Trying to get property 'comment_ID' of non-object in wp-includes/comment-template.php:677

Most of the comment template functions are written with the expectation that get_comment() will always return a non-false value, but get_comment_ID() seems to be the worst offender, at least among the plugins being used on the WordPress.org site.

The simplest way to trigger this is using wp-cli shell:

wp> error_reporting( E_ALL );
int(32767)

wp> ini_set( 'display_errors', true );
string(0) ""

wp> add_filter( 'comment_text', function( $text ) { return get_comment_ID(); } );
bool(true)

wp> check_comment( "author", "email", "url", "comment", "ip", "agent", "type" );

Notice: Trying to get property 'comment_ID' of non-object in wp-includes/comment-template.php on line 677

Call Stack:
.......
   25.5958   67872048  15. eval('return check_comment( "author", "email", "url", "comment", "ip", "agent", "type" );') phar://bin/wp/vendor/wp-cli/shell-command/src/WP_CLI/Shell/REPL.php:46
   25.5958   67872048  16. check_comment($author = 'author', $email = 'email', $url = 'url', $comment = 'comment', $user_ip = 'ip', $user_agent = 'agent', $comment_type = 'type') phar://bin/wp/vendor/wp-cli/shell-command/src/WP_CLI/Shell/REPL.php(46) : eval()'d code:1
   25.5960   67872048  17. apply_filters($hook_name = 'comment_text', $value = 'comment', NULL, []) wp-includes/comment.php:48
   25.5961   67872456  18. WP_Hook->apply_filters($value = 'comment', $args = [0 => 'comment', 1 => NULL, 2 => []]) wp-includes/plugin.php:189
   25.5969   67882792  19. WP_CLI\Shell\REPL->{closure:phar://bin/wp/vendor/wp-cli/shell-command/src/WP_CLI/Shell/REPL.php(46) : eval()'d code:1-1}($text = 'comment') wp-includes/class-wp-hook.php:305
   25.5969   67882792  20. get_comment_ID() phar://bin/wp/vendor/wp-cli/shell-command/src/WP_CLI/Shell/REPL.php(46) : eval()'d code:1

bool(false)

Attached is a targeted patch for get_comment_ID().

Attachments (4)

54379.diff (1.6 KB) - added by dd32 2 months ago.
54379.get_comment_author.diff (2.2 KB) - added by dd32 2 months ago.
54379.comments_open.diff (2.1 KB) - added by dd32 2 months ago.
54379.2.diff (1.6 KB) - added by dd32 2 months ago.
54379.diff + hook documentation fix. [30092] added $comment, not $comment_ID.

Download all attachments as: .zip

Change History (7)

@dd32
2 months ago

#1 @dd32
2 months ago

  • Description modified (diff)

#2 @dd32
2 months ago

Most of the comment template functions are written with the expectation that get_comment() will always return a non-false value

Other examples:

E_NOTICE: Trying to get property 'comment_status' of non-object in wp-includes/comment-template.php:1244
E_NOTICE: Trying to get property 'ping_status' of non-object in wp-includes/comment-template.php:1274

In those cases the $post object is expected to exist. See 54379.comments_open.diff. Seems to primarily happen during the jp_sitemap_cron_hook cron event, so I'm guessing it's specifically for comments that relate to post_id = 0.

E_NOTICE: Trying to get property 'user_id' of non-object in wp-includes/comment-template.php:28
E_NOTICE: Trying to get property 'comment_ID' of non-object in wp-includes/comment-template.php:48

Those are when get_comment_author() is called without a valid comment set, similar to the original issue here. See 54379.get_comment_author.diff

#3 @dd32
2 months ago

#54149 is related to the first instance here - although it wasn't included in the PR/patch to start with over there.

The other two patches are unrelated enough though.

@dd32
2 months ago

54379.diff + hook documentation fix. [30092] added $comment, not $comment_ID.

Note: See TracTickets for help on using tickets.