#48653 closed defect (bug) (fixed)
PHP Notices when requesting a post with an unregistered status
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.4 | Priority: | normal |
Severity: | normal | Version: | 3.0 |
Component: | Posts, Post Types | Keywords: | has-patch needs-unit-tests |
Focuses: | Cc: |
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)
Change History (16)
#3
follow-up:
↓ 4
@
5 years 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
@
5 years 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:
↓ 6
@
5 years 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
@
5 years 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 thepost_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.
#8
@
5 years ago
- Milestone changed from Awaiting Review to 5.4
- Owner set to SergeyBiryukov
- Status changed from new to reviewing
#15
in reply to:
↑ 14
@
5 years ago
Replying to audrasjb:
Post query is broken after [47178]. Looks like it would need some changes :-)
Thanks for catching that! As noted in comment:4:ticket:49368, it should be fixed in [47181]. The nightly build has indeed ended up with the wrong commit, but is now refreshed.
Adds defensive programming to deal with get_post_status_object() returning null.