Opened 8 years ago
Closed 8 years ago
#37721 closed defect (bug) (fixed)
improve error handling of is_object_in_term in taxonomy.php
Reported by: | rpayne7264 | Owned by: | boonebgorges |
---|---|---|---|
Milestone: | 4.6.1 | Priority: | normal |
Severity: | normal | Version: | 4.6 |
Component: | Taxonomy | Keywords: | commit |
Focuses: | Cc: |
Description
Line 4276 in taxonomy.php can return a WP_Error object, which is not checked before calling wp_list_pluck().
Result is a fatal error:
Fatal error: Cannot use object of type WP_Error as array in /wp-includes/functions.php on line 3511
The code should pass the WP_Error object to the calling function.
Change History (9)
#3
@
8 years ago
- Keywords dev-feedback added
- Milestone changed from Awaiting Review to 4.6.1
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening for 4.6.1 and review.
#4
@
8 years ago
Starting to run into this on live sites too, where code was calling has_term() for a previously non-existing taxonomy. Of course that's bad practice, running code like that, but leftover code from long ago started fatal erroring a whole site can be scary for folks with less dev skills to track down what was going on :)
Definitely need this in 4.6.1
This ticket was mentioned in Slack in #core by jeremyfelt. View the logs.
8 years ago
#6
@
8 years ago
- Keywords commit added; dev-feedback removed
Looks good. +1 for merge to the 4.6 branch.
#7
@
8 years ago
As an additional precaution, you may want to add a check at the entrance of wp_list_pluck() to ensure that the $list param is, indeed, an array.
This is how I would code it:
function wp_list_pluck( $list, $field, $index_key = null ) { $newlist = array(); if(! is_array( $list ))return $newlist; if ( ! $index_key ) { /* * This is simple. Could at some point wrap array_column() * if we knew we had an array of arrays. */ foreach ( $list as $key => $value ) { if ( is_object( $value ) ) { $list[ $key ] = $value->$field; } else { $list[ $key ] = $value[ $field ]; } } return $list; } /* * When index_key is not set for a particular item, push the value * to the end of the stack. This is how array_column() behaves. */ foreach ( $list as $value ) { if ( is_object( $value ) ) { if ( isset( $value->$index_key ) ) { $newlist[ $value->$index_key ] = $value->$field; } else { $newlist[] = $value->$field; } } else { if ( isset( $value[ $index_key ] ) ) { $newlist[ $value[ $index_key ] ] = $value[ $field ]; } else { $newlist[] = $value[ $field ]; } } } return $newlist; }
#8
@
8 years ago
@jeremyfelt Thanks for the review.
@rpayne7264 Thanks for the suggestion. There has been discussion in the past about this kind of check in wp_list_pluck()
, but the general sense is that the PHP notice is a good indicator for developers that they've done something wrong, and should not be silenced. See #35087.
Thanks for the report, @rpayne7264. Previously:
WP_Error
object might be incorrectly cached. See #32044.wp_list_pluck()
call was introduced. See #36814.So the notice is a 4.6 regression, but there's a related bug going back to 4.4.