wp_unique_term_slug() doesn't guarantee unique slugs for child terms

[saskak]

Steps to reproduce:

0. Create a fresh site with WP 5.1 and Twenty Nineteen
1. Create a category "Animal" with slug "animal"
2. Create a category "Dog" with slug "dog"
3. Create a child category for "Animal" named "Cat" with slug "dog-animal"
4. Create a top-level category "Giraffe" with slug "giraffe"
5. Edit the "Giraffe" category so that in a single update, the parent changes from "None" to "Animal" and the slug becomes "dog".

What I expected?
The slug of category "Giraffe" should have become "dog-animal-2".

What did I get?
Category "Giraffe" now has identical slug with "Cat", both having the exact same slug "dog-animal".

---

I tracked this down to wp_unique_term_slug(). It seems that if a parent suffix is added to a term, no further checks are made to ensure its uniqueness. The whole section where a number is appended is skipped.

Why those exact steps are needed to reproduce this has to do with differences in wp_insert_term() and wp_update_term() I won't go into detail on that as it's a completely separate issue and seemingly a huge mess...

[dlh]

Hi @saskak, and welcome to WordPress Trac!

It looks to me like this behavior was introduced in [32837] and that the cause is as you describe.

[yashar_hv]

I encountered this bug when I was trying to auto assign custom taxonomy term slugs by using pre_{$taxonomy}_{$field} filter hook.
I found out in the last part of wp_unique_term_slug() in taxonomy.php, when it tries to append sequel numbers in case $slug is not unique yet, it checks for $parent_suffix ('-parent-slug') and if it exists, the function ignores appending a number and returns $slug.$parent_suffix :

if ( apply_filters( 'wp_unique_term_slug_is_bad_slug', $needs_suffix, $slug, $term ) ) {
	if ( $parent_suffix ) {
		$slug .= $parent_suffix;
	} else {
		if ( ! empty( $term->term_id ) ) {
			$query = $wpdb->prepare( "SELECT slug FROM $wpdb->terms WHERE slug = %s AND term_id != %d", $slug, $term->term_id );
		} else {
			$query = $wpdb->prepare( "SELECT slug FROM $wpdb->terms WHERE slug = %s", $slug );
		}

		if ( $wpdb->get_var( $query ) ) {
			$num = 2;
			do {
				$alt_slug = $slug . "-$num";
				$num++;
				$slug_check = $wpdb->get_var( $wpdb->prepare( "SELECT slug FROM $wpdb->terms WHERE slug = %s", $alt_slug ) );
			} while ( $slug_check );
			$slug = $alt_slug;
		}
	}
}

Possible Solution

I modified this code as below and it seems that this solves the problem and now the number appends to both $slug.$parent_suffix and $slug in case there is a existing term with that slug:

if ( apply_filters( 'wp_unique_term_slug_is_bad_slug', $needs_suffix, $slug, $term ) ) {
	
	/*
	* MODIFIED: $parent_suffix conditional if block is closed right after appending $parent_suffix to $slug.
	*           Else block content for this condition moved to the outside of conditional block.
	*/ 

	if ( $parent_suffix ) {
		$slug .= $parent_suffix;
	}

	if ( ! empty( $term->term_id ) ) {
		$query = $wpdb->prepare( "SELECT slug FROM $wpdb->terms WHERE slug = %s AND term_id != %d", $slug, $term->term_id );
	} else {
		$query = $wpdb->prepare( "SELECT slug FROM $wpdb->terms WHERE slug = %s", $slug );
	}

	if ( $wpdb->get_var( $query ) ) {
		$num = 2;
		do {
			$alt_slug = $slug . "-$num";
			$num++;
			$slug_check = $wpdb->get_var( $wpdb->prepare( "SELECT slug FROM $wpdb->terms WHERE slug = %s", $alt_slug ) );
		} while ( $slug_check );
		$slug = $alt_slug;
	}
}

[boonebgorges]

Thanks all for the detailed report.

I've written a test that confirms what @saskak describes above (minus the additional complication of wp_update_term()), and the test confirms that the suggested fix from @yashar_hv works. Could I get another set of eyes on 46431.diff​ ? @dlh since you dropped by recently? :)

[dlh]

46431.diff​ looks good to me, with a couple potential copy-edits:

• Switch the test annotation from @group to @ticket.
• Drop be_ from the test name: test_duplicate_parent_suffixed_slug_should_get_numeric_suffix.

[boonebgorges]

@dlh Thanks as always for your careful eye! Can you tell that I rewrote the name of that test several times?

[boonebgorges]

In 45893:

Taxonomy: Fix unique-slug check for terms with parents.

wp_unique_term_slug() appends numeric suffixes when the requested slug is
already in use by a sibling term. Changes introduced in [32837] inadvertently
caused this suffixing to be skipped in cases where the requested slug is
suffixed with the parent slug, so that it became possible to have two terms
childslug-parentslug underneath to the same parentslug. We fix this
regression by ensuring that the numeric-suffix routine runs in all cases.

Props yashar_hv, saskak, dlh.
Fixes #46431.