wp_get_object_terms should return arrays of integers for IDs and tt_IDs

[simonwheatley]

Currently when you use wp_get_object_terms to request an array of IDs or tt_IDs, the array returned contains strings representing the IDs, not integers. This then creates issues if you send the values back into wp_set_object_terms as it creates terms named as per those strings, rather than associating the term_ids as expected.

Code to prove the issue:

$term_ids = wp_get_object_terms( get_the_ID(), 'category', array( 'fields' => 'tt_ids' ) );
var_dump( $term_ids );

Results in:

array
  0 => string '1' (length=1)
  1 => string '5' (length=1)

I believe that the array should have all values cast to integers before they are returned. The attached patch does this by mapping a created function to utilise (int) for the desired result.

Cast the values of the array to int if the fields are ids or tt_ids

Sounds like a very good idea. Also probably a duplicate ticket somewhere. Also, we can use absint rather than an anonymous function (which, also, we're no longer allowing in core).

A good idea indeed.

IMO, sanitize_term_field() should be updated to absint() the term_id field. wp_get_object_terms() should call the sanitization functions. This would also take care of tickets such as #17652

IMO, sanitize_term_field() should be updated to absint() the term_id field.

Completely missed the fact that it already (int)'s the term_id value

attachment 17646.diff added

• Not a clean patch, Just a diff showing my thinking
• Utilises sanitize_term() / sanitize_term_field() for sanitizing values, ie. int'ing fields
• causes get_term(), get_term_by(), wp_get_object_terms() and most other high level taxonomy functions to return int'd values as expected (no point fixing one function and leaving the rest) - There are other tickets for these I believe.

Other functions that don't int the results:

• get_objects_in_term() - object_id
• class WP_Tax_Query - Doesn't int term_ids, or sanitize them in any way, just returns the table results into WP_Query
• term_exists() - term_id, term_taxonomy_id as strings
• wp_set_object_terms() term_taxonomy_ids as strings

Example output:

$res = get_term(1, 'category');
var_dump($res);

object(stdClass)[648]
  public 'term_id' => int 1
  public 'name' => string 'Uncategorized' (length=13)
  public 'slug' => string 'uncategorized' (length=13)
  public 'term_group' => int 0
  public 'term_taxonomy_id' => int 1
  public 'taxonomy' => string 'category' (length=8)
  public 'description' => string '' (length=0)
  public 'parent' => int 0
  public 'count' => int 6
  public 'filter' => string 'raw' (length=3)

$term_ids = wp_get_object_terms( 1, 'category', array( 'fields' => 'ids' ) );
var_dump( $term_ids );

array
  0 => int 1

I should mention, that the patch reverts [6321] (part of #5325) as can be seen by the commented out code, sanitize_term_field was written to short-circuit itself after int'ing resulting in that addition not helping as much as it would've for posts, also resulting in string fields rather than int fields in more places.

Leveraging sanitize_term_field() makes sense to me.

We should cover all the functions in the taxonomy API in the next release.

patch refresh; Exclude cruft from around the changes. Fix typo in previous patch

removed needs-patch and added has-patch

Cleaned up DD32's patch so it works on the current trunk

Tested cleaned patch against trunk, integers are returned as expected for tt_ids and ids.

Patch looks good for the original report. The object_id field is still coming through as a string. Test code:

var_dump( wp_get_object_terms( get_the_ID(), 'category', array( 'fields' => 'all' ) ) );
var_dump( wp_get_object_terms( get_the_ID(), 'category', array( 'fields' => 'all_with_object_id' ) ) );
var_dump( wp_get_object_terms( get_the_ID(), 'category', array( 'fields' => 'ids' ) ) );
var_dump( wp_get_object_terms( get_the_ID(), 'category', array( 'fields' => 'tt_ids' ) ) );

#23894 was marked as a duplicate.

Updated in 17646.3.diff​, used the same $int_fields approach we use in other places, added object_id to the integer fields.

17646.tests.diff​ tests the integer fields for all_with_object_id and ids.

17646.3.diff​ works as expected for me using @kurtpayne's test dumps. Didn't run the unit tests.

I have a feeling that (despite this being a very good idea) this could cause problems along the lines of #22324. I don't think this should necessarily stop us, but now that we have unit tests and a patch, this sounds like something that should be landed on the first day of a cycle, not deep into beta.