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.

[simonwheatley]

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

[nacin]

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

[scribu]

A good idea indeed.

[dd32]

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

[dd32]

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

[dd32]

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

[dd32]

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.

[scribu]

Leveraging sanitize_term_field() makes sense to me.

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

[dd32]

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

[jkudish]

removed needs-patch and added has-patch

[simonwheatley]

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

[simonwheatley]

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

[kurtpayne]

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' ) ) );

[kovshenin]

#23894 was marked as a duplicate.

[kovshenin]

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

[kovshenin]

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

[DrewAPicture]

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

[nacin]

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.

[Mamaduka]

WP_Query also returns stings representing post IDs, when fields is set to ids.

[ericmann]

#17652 was marked as a duplicate.

[wonderboymusic]

these are all marked 3.7-early

[nacin]

I'm holding this until early 3.8, day one.

[wonderboymusic]

In 26010:

Cast proper fields to int when returning from wp_get_object_terms(). Add term_taxonomy_id and object_id to the list in sanitize_term() and sanitize_term_field().

Fixes #17646. Adds unit tests.
Props simonwheatley, dd32, kovshenin.

[SergeyBiryukov]

Related: #25852