WordPress.org

Make WordPress Core

Opened 14 months ago

Last modified 2 months ago

#45121 reviewing defect (bug)

wp_update_post() can modify post tag

Reported by: kaggdesign Owned by: SergeyBiryukov
Milestone: 5.4 Priority: normal
Severity: normal Version: 4.9.8
Component: Posts, Post Types Keywords: has-patch has-unit-tests needs-refresh
Focuses: Cc:
PR Number:

Description

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.

Attachments (4)

45121.diff (590 bytes) - added by kaggdesign 11 months ago.
45121-test.diff (1.1 KB) - added by kaggdesign 11 months ago.
45121-improved-test.diff (1.3 KB) - added by kaggdesign 11 months ago.
Improved test, to check situation when 3 tags with the same name exist, and 2 of them are assigned to the post.
wp_update_post.diff (2.0 KB) - added by kaggdesign 3 months ago.
All-inclusive patch for the ticket. Fix by itself and test.

Download all attachments as: .zip

Change History (17)

#1 follow-up: @SergeyBiryukov
14 months ago

Related: #30615

#2 in reply to: ↑ 1 @kaggdesign
14 months ago

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.

#3 @kaggdesign
11 months ago

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
11 months ago

#4 @kaggdesign
11 months ago

  • Keywords has-patch has-unit-tests added

@kaggdesign
11 months ago

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

#5 @SergeyBiryukov
10 months ago

  • Milestone changed from Awaiting Review to 5.2
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

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


8 months ago

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


8 months ago

#8 @pento
8 months ago

  • Milestone changed from 5.2 to 5.3

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

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


3 months ago

#10 @davidbaumwald
3 months ago

  • Keywords needs-refresh added

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.

#11 @kaggdesign
3 months ago

@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.

Last edited 3 months ago by kaggdesign (previous) (diff)

@kaggdesign
3 months ago

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

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


2 months ago

#13 @desrosj
2 months ago

  • Milestone changed from 5.3 to 5.4

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.

Note: See TracTickets for help on using tickets.