Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#54159 closed defect (bug) (fixed)

comments_open() & pings_open() trigger PHP Notice when called on a page without a $post

Reported by: dd32's profile dd32 Owned by: hellofromtonya's profile hellofromTonya
Milestone: 5.9 Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch commit has-unit-tests
Focuses: Cc:

Description

Both comments_open() and pings_open() contain some slightly odd logic, that accounts for the $post not existing on one line, and assuming it does exist on the next. Causing a PHP Notice under some specific circumstances.

Calling comments_open( $post_id_that_does_not_exist ) would also trigger it.

See attached.

Attachments (2)

54159.diff (2.1 KB) - added by dd32 3 years ago.
54159.1.diff (841 bytes) - added by audrasjb 3 years ago.
patch refreshed against trunk

Download all attachments as: .zip

Change History (11)

@dd32
3 years ago

#1 @SergeyBiryukov
3 years ago

  • Milestone changed from Awaiting Review to 5.9

#2 @audrasjb
3 years ago

  • Keywords needs-refresh added

The patch doesn't apply anymore against trunk

@audrasjb
3 years ago

patch refreshed against trunk

#3 @audrasjb
3 years ago

  • Keywords needs-refresh removed

#4 @hellofromTonya
3 years ago

  • Owner set to hellofromTonya
  • Status changed from new to accepted

Though today is 5.9 Beta 1, assigning to me to check test coverage and get this one committed.

#5 @costdev
3 years ago

Test Report

Env

  • WordPress 5.9-alpha-20211117.140912
  • Chrome 95.0.4638.69
  • Windows 10
  • Theme: Twenty Twenty Two
  • Gutenberg Editor
  • Plugin: None activated

Steps to test

  1. Add comments_open( 999999 ); to your theme's functions.php file.
  2. Load any page. You should see a PHP notice.
  3. Apply the patch.
  4. Reload the page. You should not see a PHP notice.

Results

Without the patch: A PHP notice displays for Warning: Attempt to read property "comment_status" on null.
With the patch: The PHP notice no longer displays.

#6 @hellofromTonya
3 years ago

  • Keywords commit added

Marking for commit.

This ticket was mentioned in PR #1921 on WordPress/wordpress-develop by hellofromtonya.


3 years ago
#7

  • Keywords has-unit-tests added

Adds unit tests for both functions. Confirms PHP notice.
Applies patch 54159.1.diff. Confirms notice is fixed.

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

#8 @hellofromTonya
3 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 52223:

Comments: Fix PHP Notice "trying to get property of non-object" in comments_open() and pings_open().

The post for the comments or pings is retrieved by get_post(). If the post exists, get_post() returns an instance of WP_Post; else, it returns null.

In both comments_open() and pings_open(), the returned value from get_post() is used without checking if the object returned, if the post exists. When the post does not exist, the following notices occur:

PHP Notice:  Trying to get property 'comment_status' of non-object in .../src/wp-includes/comment-template.php on line 1244

and

PHP Notice:  Trying to get property 'pings_open' of non-object in ../src/wp-includes/comment-template.php on line 1274

This commit fixes these notices by checking if the post has a non-falsey value before using it as an object to set the $open state. As the return from get_post() will only be an object or null, the truthy check is appropriate and slightly more performant.

Tests added to validate the fix.

Follow-up to [1964], [40666].

Props dd32, audrasjb, costdev, hellofromTonya, sergeybiryukov.
Fixes #54159.

Note: See TracTickets for help on using tickets.