[wp-trac] [WordPress Trac] #44591: PHP notice if optional argument isn't passed to map_meta_cap()
WordPress Trac
noreply at wordpress.org
Thu Jan 3 21:10:21 UTC 2019
#44591: PHP notice if optional argument isn't passed to map_meta_cap()
-------------------------------------------+------------------------------
Reporter: henry.wright | Owner: (none)
Type: defect (bug) | Status: new
Priority: normal | Milestone: Awaiting Review
Component: General | Version:
Severity: normal | Resolution:
Keywords: reporter-feedback 2nd-opinion | Focuses:
-------------------------------------------+------------------------------
Changes (by mattheweppelsheimer):
* keywords: reporter-feedback => reporter-feedback 2nd-opinion
Comment:
`map_meta_cap()` doesn't consistently check whether `$args` keys are set
before attempting to access them. Early cases in its `switch` statement
tend to follow this pattern with `isset()` guards, as in the third line
from this snippet:
{{{#!php
case 'remove_user':
// In multisite the user must be a super admin to remove themselves.
if ( isset( $args[0] ) && $user_id == $args[0] && ! is_super_admin(
$user_id ) ) {
$caps[] = 'do_not_allow';
} else {
$caps[] = 'remove_users';
}
break;
}}}
Later though, there are several cases where it accesses `$args[0]` without
an `isset()` first, as in this case which is involved in the `Notice`
@henry.wright reported (line 136 is the second line in this snippet):
{{{#!php
case 'edit_page':
$post = get_post( $args[0] );
if ( ! $post ) {
$caps[] = 'do_not_allow';
break;
}
// continuation of `case` omitted...
}}}
It is possible that core code never executes any of the `map_meta_cap()`
cases that assume `$args` are set, without setting them. (I think whether
or not it does can probably be determined one way or the other with
certainty, but I haven't looked into that.) If that is so, then `Undefined
offset` Notices would only come from plugins/themes, and one might argue
that this is not Core's problem to solve.
I would argue, however, that this is an area where Core could be made
friendlier to plugin/theme developers by going ahead and adding those
guards.
One might also argue against adding these guards because they might
obscure underlying issues. So I would propose to use `_doing_it_wrong()`
and `WP_Error` where appropriate.
I'm happy to work on a patch for this, but I'd like to get a Core
Committer's thoughts beforehand, in case my analysis is flawed.
Is there any good reason **not** to revise `map_meta_cap()` so that it
always checks if an `$args` index is set before accessing it?
--
Ticket URL: <https://core.trac.wordpress.org/ticket/44591#comment:3>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list