map_meta_cap() causing PHP notice on term meta permissions

[caercam]

Mentioned in #40889: when checking for permission to add/edit/delete term meta, map_meta_cap() only performs a simple boolean test to make sure the term exists: https://core.trac.wordpress.org/browser/tags/4.7/src/wp-includes/capabilities.php#L277

If get_term() returns a WP_Error the test fails and $term->taxonomy causes an "Undefined property" notice. There should be an additional check using is_wp_error(), a simple isset( $term->taxonomy ) or maybe a simpler term_exists() to make sure the term actually exists.

[boonebgorges]

Thanks for the ticket and for the patch, @caercam !

isset( $term->taxonomy ) feels like a roundabout way of checking whether we actually have a term. Maybe better to check this more directly: $term instanceof WP_Term. What do you think?

Are you able to write a unit test that demonstrates the error, so we can avoid future regressions?

[caercam]

Replying to boonebgorges:
$term instanceof WP_Term makes sense! Updated patch + added unit test. First ever released unit test, please bare with me and point any mistake I may have made.

[boonebgorges]

Thank you, @caercam! The test is a good start, but (a) as written, it fails after the patch is applied (because the expected notice is no longer thrown), and (b) it doesn't actually test anything. I've written a more targeted test that checks for the actual return value of user_can() when passing a bad term ID, which fails with an error before the patch. Can you double check to see that the logic is correct? 40891.3.diff​

[caercam]

Logic confirmed. Thanks for putting up with me!

[boonebgorges]

In 40999:

Avoid PHP notices when checking termmeta capabilities against a non-existent term.

Previously, checks like current_user_can( 'edit_term_meta', $term_id )
returned the proper value, but generated a PHP notice due to the fact
that get_term( $term_id ) could, in certain instances, return
WP_Error objects.

Props caercam.
Fixes #40891.