Make WordPress Core

Opened 8 years ago

Last modified 6 years ago

#38243 new defect (bug)

Attempting to create a term with invalid UTF8 characters creates a blank term

Reported by: dd32's profile dd32 Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Taxonomy Keywords: has-patch needs-unit-tests
Focuses: Cc:

Description

Attempting to insert a term which contains invalid UTF8 characters will incorrectly create a term in the database with a blank name & slug. This happens as we check that the term name & slug is provided, but fail to check after sanitizing the term.

In the scenario that I've run into, something similar to this happens:

$term_name = urldecode( "360%BF" ); // Invalid UTF8 character
wp_insert_term( $term_name, 'my_taxonomy' );

What this causes is

  • the checks on $name to pass
  • it then hits sanitize_term() and after passing through sanitize_text_field() and then wp_check_invalid_utf8() the name field of the term is set to an empty string.
  • wp_insert_term() then takes this empty name and creates an equally empty slug from it.
  • wp_insert_term() then calls get_terms( array( 'name' => '' ) ) and needlessly & badly loads up all 60,000 terms into memory of the custom taxonomy
  • wp_insert_term() then see's an empty slug and ultimates settles on a setting the slug to the numeric ID of the term somehow
  • wp_insert_term() finally inserts a term with a numeric slug and empty name field

I think at a minimum, we should verify that the term name is still valid after term sanitisation. See patch for that.

Attachments (1)

38243.diff (1.2 KB) - added by dd32 8 years ago.

Download all attachments as: .zip

Change History (3)

@dd32
8 years ago

#1 @dd32
8 years ago

There also exists a validation on the parent field there, but the worst thing you could do there is really to change the parent parameter to an invalid term_id.. and even then, I can't see how that would break things badly.

#2 @desrosj
6 years ago

  • Keywords needs-unit-tests added
  • Milestone changed from Awaiting Review to Future Release

This makes sense, but should get a unit test.

Note: See TracTickets for help on using tickets.