wp-admin/includes/bookmark.php: Fix potential PHP fatal errors. #709
Conversation
|
Status: Blocked The solution for this PR is currently blocked until we figure out the best way to handle |
src/wp-admin/includes/bookmark.php
Outdated
| if ( is_wp_error( $cats ) ) { | ||
| return array(); | ||
| } | ||
| return array_unique( $cats ); |
There was a problem hiding this comment.
Code review discussion with @jrfnl:
- Use the same strategy as in PR 736 by flipping this around to check that
wp_get_object_termsdoes return an array and then pass that intoarray_unique(); else, return an emptyarray(). - We do not need to return the
WP_Error. Why? This function expects to return only an array.
if ( is_array( $cats ) {
return array_unique( $cats );
}
return array();
Updates `wp_update_link` to handle invalid link ids, it also adds a couple of unit tests.
src/wp-admin/includes/bookmark.php
Outdated
| if ( null === $link ) { | ||
| return new WP_Error( 'invalid_link', __( 'Invalid link ID.' ) ); | ||
| } |
There was a problem hiding this comment.
Code discussion with @jrfnl:
The code assumes it $links will be an array. However, get_bookmark() could also return null which is problematic:
$link_cats = $link['link_category'];and
$linkdata = array_merge( $link, $linkdata );https://3v4l.org/KPmBt
https://3v4l.org/4vURI
We need to wrangle this. It does not need WP_Error.
|
Defer to the reorganize branch here and specifically here in the wp-admin/includes/bookmark.php file https://github.com/jrfnl/wordpress-develop-official/blob/reaarrange/xknown/fix/51423-wp-admin-includes-bookmark.php/src/wp-admin/includes/bookmark.php |
3 participants
Updates
wp_update_linkto handle invalid link ids, it also adds a coupleof unit tests.
Trac ticket: https://core.trac.wordpress.org/ticket/51423
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.