improve error handling of is_object_in_term in taxonomy.php

[rpayne7264]

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.

[boonebgorges]

Thanks for the report, @rpayne7264. Previously:

• [34812] first started populating the cache here. So 4.4 would have introduced a bug (not specifically noted here, but related) whereby a WP_Error object might be incorrectly cached. See #32044.
• [37573] changed the taxonomy relationship cache structure so that only item IDs are stored. This is when the 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.

[boonebgorges]

In 38277:

In is_object_in_term(), return error object rather than caching it.

This change prevents an error object from being stored in the cache,
and prevents notices from being thrown when plucking term IDs to put
into the relationship cache.

See #32044, #36814.

Props rpayne7264.
Fixes #37721.

[boonebgorges]

Reopening for 4.6.1 and review.

[sc0ttkclark]

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

[jeremyfelt]

Looks good. +1 for merge to the 4.6 branch.

[rpayne7264]

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;
}

[boonebgorges]

@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.

[boonebgorges]

In 38346:

In is_object_in_term(), return error object rather than caching it.

This change prevents an error object from being stored in the cache,
and prevents notices from being thrown when plucking term IDs to put
into the relationship cache.

See #32044, #36814.

Merges [38277] to the 4.6 branch.

Props rpayne7264.
Fixes #37721.