Make WordPress Core

Opened 7 months ago

Last modified 2 weeks ago

#53152 new enhancement

wp_set_object_terms ampersand in term name

Reported by: kapacity Owned by:
Milestone: 6.0 Priority: normal
Severity: normal Version: 5.7.1
Component: Taxonomy Keywords: has-patch has-unit-tests dev-feedback
Focuses: administration Cc:

Description (last modified by hellofromTonya)

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 4 months ago.
Before/After patch

Change History (11)

#1 @kapacity
7 months ago

Any update on this?

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

4 months 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

4 months ago

Before/After patch

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

4 months ago

#4 @costdev
4 months ago

  • Keywords has-unit-tests added

#5 @costdev
4 months 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 4 months ago by costdev (previous) (diff)

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

3 months ago

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

2 months ago

#8 @SergeyBiryukov
2 months ago

  • Component changed from Administration to Taxonomy
  • Focuses administration added
  • Milestone changed from Awaiting Review to 5.9

#9 @hellofromTonya
2 weeks ago

  • Description modified (diff)

#10 @hellofromTonya
2 weeks ago

  • Milestone changed from 5.9 to 6.0
  • Type changed from defect (bug) to enhancement

A single term slug, single term ID, or array of either term slugs or IDs.

The wp_set_object_terms() function accepts a slug or ID, but not a name. This ticket is not a bug, but rather an enhancement request to allow the function to also accept a name.

5.9 is in feature freeze with Beta 1 tomorrow. It's too late for this ticket to land in 5.9.

Moving it to 6.0 and leaving dev-feedback as it needs discussion and consensus about expanding it to accept a term name.

Note: See TracTickets for help on using tickets.