#13905 closed defect (bug) (fixed)
No sanity check in map_meta_cap caps throws PHP notices
Reported by: | filosofo | Owned by: | filosofo |
---|---|---|---|
Milestone: | 4.4 | Priority: | normal |
Severity: | normal | Version: | 3.0 |
Component: | Posts, Post Types | Keywords: | needs-patch |
Focuses: | Cc: |
Description
map_meta_cap
assumes that the ID which it's passed when evaluating edit_post
, edit_page
and the like actually belongs to a real post object.
In fact, it's quite possible that the object doesn't yet exist (creating a new object, perhaps) or doesn't exist any more (deleted), or that it has otherwise received a syntactically correct ID value that doesn't map to an existing post object (0, e.g.).
Instead, map_meta_cap
should check that the post object actually exists before attempting to branch on its properties.
Patch also removes some apparent debugging comments.
Attachments (5)
Change History (31)
#5
@
14 years ago
- Keywords needs-refresh added; has-patch removed
Patch needs a refresh based on #14122.
#6
@
14 years ago
- Keywords has-patch added; needs-refresh removed
Patch refreshed. I wish I could remember the real-life scenario that revealed this issue to me, but it's been too long.
#7
follow-up:
↓ 8
@
14 years ago
Patch needs refresh again after more work in #14122.
I'm not sure some sanity checks are needed here, such as whether a post type object exists after we've already established that the post exists. They seem fine, though -- but can't we just break
instead of re-indenting everything for most checks?
#8
in reply to:
↑ 7
@
14 years ago
Replying to nacin:
I'm not sure some sanity checks are needed here, such as whether a post type object exists after we've already established that the post exists.
Have we? Maybe that's in the recent changes that I need to refresh against, but as of two weeks ago we didn't check that the post exists.
They seem fine, though -- but can't we just
break
instead of re-indenting everything for most checks?
Multiple breaks for one case seems like asking for trouble, in that it makes the logic harder to follow, more likely to get overlooked in future patches, etc.
I'll refresh the patch.
#9
@
14 years ago
- Keywords needs-patch added; has-patch removed
Changing to needs-patch for the refresh discussed above.
#10
@
14 years ago
If the post doesn't exist, this patch will return no caps for current_user_can( 'edit_post', $nonexistent_post )..
Should current_user_can( 'edit_post', $nonexistent_post ) still go with the baseline primitive cap, e.g. edit_posts?
#12
follow-up:
↓ 13
@
14 years ago
- Milestone Future Release deleted
- Resolution set to wontfix
- Status changed from accepted to closed
#13
in reply to:
↑ 12
@
14 years ago
Replying to filosofo:
Why did you close this - surely we could address this in future?
#14
@
14 years ago
My bad, I guess. I interpreted nacin's comment in #wordpress-dev about it as saying it's too trivial to worry about.
#15
@
14 years ago
My bad if that's what you interpreted.
I don't think it's trivial, rather, it only happens when a nonexistent post/something is requested. While it should never occur, it's severe when it does.
I also noticed yesterday that I'm not sure if a nonexistent post should mean no caps get mapped. See comment 10. Hence punt made sense with a patch needing refresh.
#16
@
14 years ago
- Milestone set to Future Release
- Resolution wontfix deleted
- Status changed from closed to reopened
It's a little hard to see in the diff, but all I'm doing is wrapping the object-property-dependent logic in a
if ( ! empty( $post->ID ) )
check. And removing that commented-out debugging cruft.