wp_update_post() can modify post tag

[kaggdesign]

Problem

Update of post by means of wp_update_post() can modify tags assigned to the post.

Steps to reproduce:

Fresh WP install.

Create two tags with the same name (let say wp_update_post_tag) and different slugs (let say wp_update_post_tag_1} and wp_update_post_tag_2, in this sequence).

Create a post with tag wp_update_post_tag_2. Check that it has tag wp_update_post_tag_2.

Use wp_update_post( $post ). Check that now it has tag wp_update_post_tag_1.

This sequence is demonstrated by the plugin ​https://github.com/kagg-design/update-post-bug.

---

Reason

Problem is caused by the following lines in wp_insert_post():

	if ( isset( $postarr['tags_input'] ) && is_object_in_taxonomy( $post_type, 'post_tag' ) ) {
		wp_set_post_tags( $post_ID, $postarr['tags_input'] );
	}

At this point, $postarr['tags_input'] already is array( 'wp_update_post_tag' ), containing tag_name, not ID. This is because wp_update_post() executes

	// First, get all of the original fields.
	$post = get_post($postarr['ID'], ARRAY_A);

Here $post gets tags_input as array( 'wp_update_post_tag' ), containing tag_name, not ID.

[SergeyBiryukov]

Related: #30615

[kaggdesign]

Replying to SergeyBiryukov:

Related: #30615

Yes, it is related, with the only difference that issue is properly fixed in /wp-admin/includes, but not in /wp-includes. I will try to propose similar commit with relevant tests.

[kaggdesign]

If several tags with same name exist, and post has one of such tags, then wp_update_post() will modify tag assigned to post.

This is caused by $post = get_post($postarr['ID'], ARRAY_A); in wp_update_post(), which returns 'tags_input' as strings, not ids.

We fix the issue by adding conversion of strings in 'tags_input' to ids. )

[kaggdesign]

Improved test, to check situation when 3 tags with the same name exist, and 2 of them are assigned to the post.

[pento]

As WP 5.2 RC1 is being released today, I'm moving this ticket to 5.3.

[davidbaumwald]

This ticket was discussed during today's bug scrub. The most recent patch needs a refresh, and the tests should be in an all-inclusive patch.

[kaggdesign]

@davidbaumwald @SergeyBiryukov Please find attached all-inclusive patch wp_update_post.diff with fix by itself and test. Code in the patch is refreshed and applicable to WordPress 5.3-beta1-46286.

Thank you.

[kaggdesign]

All-inclusive patch for the ticket. Fix by itself and test.

[desrosj]

With 5.3 RC 1 in a few hours and this needing more work, I'm going to punt this.

If someone has the time before RC to properly review and commit, it can be moved back.

[SergeyBiryukov]

In 47311:

Tests: Remove an irrelevant tags_input property assertion from test_get_page_template_property().

This appears to be a copy/paste from the test_get_tags_input_property() test above.

See #45121.

[SergeyBiryukov]

wp_update_post.diff​ does solve the reported problem, but it also prevents any modification of post tags via the tags_input parameter, even if it's intentional.

Something along the lines of [31359] seemed like it might work, however in practice it doesn't, since the tags_input value returned by WP_Post::__get()​ contains tag names, and if they are the same, there's no way to disambiguate them at that point.

I think the correct solution here would be to disregard the tags_input parameter if it's the same as existing post tags. That way tags are only modified if tags_input was explicitly provided, and is different from the existing tags.

See 45121.2.diff​.

[SergeyBiryukov]

In 47317:

Posts, Post Types: Discard tags_input parameter in wp_update_post() if it's the same as existing post tags.

This ensures that wp_update_post() does not unintentionally modify post tags if the post has several tags with the same name but different slugs.

Tags should only be modified if tags_input parameter was explicitly provided, and is different from the existing tags.

Props kaggdesign, SergeyBiryukov.
Fixes #45121.