Make WordPress Core

Opened 5 months ago

Last modified 2 weeks ago

#53152 new defect (bug)

wp_set_object_terms ampersand in term name

Reported by: kapacity Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.7.1
Component: Administration Keywords: has-patch has-unit-tests dev-feedback
Focuses: Cc:


This is a follow-up to #32248.
I believe we are running into this same core issue when using the wp_set_object_terms wp function. Here are the repro steps:

For a given post and taxonomy call:

wp_set_object_terms($post_id, foo & bar?, 'my_taxonomy', false);
When the term does not yet exist it will be created with slug 'foo-bar' and name 'foo & bar in db.

Rename term to "this & that" in admin, slug remains foo-bar and name is 'this & that' in db.

Now call

wp_set_object_terms($post_id, this & that?, 'my_taxonomy', false);
You get error 'A term with the name provided already exists in this taxonomy.'
This is because the name matching logic in term_exists does not find the term by name since it is comparing the non escaped name 'this & that' to the escaped name in the db 'this & that' and determines it does not exist.

I can work around this problem by calling the lower level wp functions myself, but this seems to be a bug.

Attachments (1)

53152 - Before and After patch.gif (4.0 MB) - added by costdev 5 weeks ago.
Before/After patch

Change History (7)

#1 @kapacity
4 months ago

Any update on this?

This ticket was mentioned in PR #1586 on WordPress/wordpress-develop by costdev.

5 weeks ago

  • Keywords has-patch added

Sanitize the $term argument being supplied to wp_set_object_terms() using sanitize_term_field( 'name' ... ).

Includes prior check that sanitize_title( $term ) !== $term to avoid sanitizing if the $term argument provided is the term's slug.
N.B. sanitize_title() is specifically what wp_insert_term() does to create a term slug.

Before/After video will be posted in the Trac ticket below.

Trac ticket: https://core.trac.wordpress.org/ticket/53152

5 weeks ago

Before/After patch

This ticket was mentioned in Slack in #core by costdev. View the logs.

5 weeks ago

#4 @costdev
5 weeks ago

  • Keywords has-unit-tests added

#5 @costdev
5 weeks ago

  • Keywords dev-feedback added

The Docs don't include the term name as a valid value for the $terms argument.

However, passing the term name as the value will either match an existing term, or create a new term if term_exists() is unable to find a match.

The patch resolves this ticket's issue with the existing functionality, but I'm not sure if:

  • nothing should be done
  • the issue should be closed due to incorrect usage of the function (although, we can be sure this isn't the only time that the term name has been/will be passed)
  • the docs should be updated to mention the term name as a valid value
  • or wp_set_object_terms() should be locked down a bit more - possibly with an error if is_string( $term ) && sanitize_title( $term ) !== $term to rule out non-slug string values - a breaking change for existing sites that pass the term name

If it's the latter, wp_remove_object_terms() may need to be updated too. If this is the best direction, I can look into this and open another ticket if the issue exists there too.

It would be great if a dev could give some guidance on how to proceed with this.

Last edited 5 weeks ago by costdev (previous) (diff)

This ticket was mentioned in Slack in #core by costdev. View the logs.

2 weeks ago

Note: See TracTickets for help on using tickets.