WordPress.org

Make WordPress Core

Opened 3 weeks ago

Last modified 3 weeks ago

#48653 new defect (bug)

PHP Notices when requesting a post with an unregistered status

Reported by: roytanck Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: trunk
Component: Posts, Post Types Keywords: has-patch needs-unit-tests
Focuses: Cc:
PR Number:

Description

Plugins can register new post statuses using register_post_status. When such a plugin is then disabled, it is possible that posts remain in the database with a post status that is no longer registered. In such cases, get_post_status_object will return null. This return value is not handled correctly in map_meta_cap and the WP_Query class, causing PHP notices.

Step to reproduce:

  • Set WP_DEBUG to true in wp-config.php.
  • Create posts with post_status that is not native to WordPress (I use "bogus"). The quickest way to fake this is to edit the database directly.
  • Request the post using the REST API, or by calling its guid/permalink in the browser.
  • PHP Notices will be shown, saying something like "Notice: Trying to get property 'public' of non-object".

This can be fixed by adding some defensive programming in the map_meta_cap function, and the WP_Query class. I'll attach a patch that suggests a way to do this.

Attachments (1)

48653.diff (2.9 KB) - added by roytanck 3 weeks ago.
Adds defensive programming to deal with get_post_status_object() returning null.

Download all attachments as: .zip

Change History (7)

@roytanck
3 weeks ago

Adds defensive programming to deal with get_post_status_object() returning null.

#1 @roytanck
3 weeks ago

  • Keywords has-patch added

#2 @SergeyBiryukov
3 weeks ago

  • Component changed from General to Posts, Post Types

Related: [34091], #16956.

#3 follow-up: @Rarst
3 weeks ago

I am not sure I agree with "doing_it_wrong" handling here, since there is nothing wrong with attempting the check. The problem is with invalid state of the system, not with developer making incorrect call. Sounds like a straight error case to me?

No opinion on implementation/behavior.

#4 in reply to: ↑ 3 @roytanck
3 weeks ago

Replying to Rarst:

I am not sure I agree with "doing_it_wrong" handling here, since there is nothing wrong with attempting the check. The problem is with invalid state of the system, not with developer making incorrect call. Sounds like a straight error case to me?

No opinion on implementation/behavior.

Thank you. Using doing_it_wrong here was simply "ego-less programming", since the rest of the function uses this in similar situations. The point is to catch the invalid state and provide meaningful feedback. Chances are the developer doing the check is unaware of previously registered post statuses still lingering in the database.

#5 follow-up: @peterwilsoncc
3 weeks ago

  • Keywords needs-unit-tests added

I agree a notice would be preferable but better to stick with _doing_it_wrong() for the sake of consistency with the post_type check even though it's not quite right. (Don't get me started on the name of the function ;)

@roytanck Are you able to add some unit tests for the WP_Query and capability changes? It's no problem if you don't have time or aren't clear on the process though.

#6 in reply to: ↑ 5 @roytanck
3 weeks ago

Replying to peterwilsoncc:

I agree a notice would be preferable but better to stick with _doing_it_wrong() for the sake of consistency with the post_type check even though it's not quite right. (Don't get me started on the name of the function ;)

@roytanck Are you able to add some unit tests for the WP_Query and capability changes? It's no problem if you don't have time or aren't clear on the process though.

I'd love to dive into (WP) unit testing if I had more time, but somehow, WP_Query does not sound like the best place for first baby steps. Also, time unfortunately is an issue. If someone else could jump in, that would help keep this ticket moving.

Note: See TracTickets for help on using tickets.