WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#3699 closed defect (bug) (wontfix)

wp_update_comment_count() causes some plugins to delete usermeta values

Reported by: markjaquith Owned by:
Milestone: Priority: high
Severity: major Version: 2.1
Component: General Keywords:
Focuses: Cc:

Description (last modified by markjaquith)

Some users have reported that plugins like Jerome's Keywords and UTW and other postmeta adding plugins have been deleting their postmeta values for a post when a comment is left on that post.

I installed Jerome's Keywords and experienced the behavior. The problem is this:

  1. Jerome's Keywords always assumes that it was able to successfully insert its field into the edit form.
  2. If the edit_post hook is called in a circumstance where the plugin was NOT able to insert its field into the edit form, it assumes that the user wanted to delete all keywords. Thus, the plugin deletes all the keywords.
  3. Commenting in 2.1 calls wp_update_comment_count(), which runs the edit_post hook.

While plugin authors are definitely to blame for assuming that "doesn't exist" is the same as "exists and is empty," we might want to consider using an alternative hook for announcing that the comment count has been updated for a post.

Change History (5)

comment:1 markjaquith7 years ago

  • Description modified (diff)

comment:2 ryan7 years ago

Have these plugins ever worked when editing a post over xmlrpc? edit_post means "this post object has been modified in some way", not "this post was edited via the edit form". I believe edit_post was added in wp_update_comment_count() and other places to help caching plugins invalidate dirty objects when a post is changed. Either way we go we're going to break someone. Since edit_post has always been issued when doing xmlrpc editing (where there is no edit form), I'd rather not change it's meaning now.

comment:3 markjaquith7 years ago

  • Resolution set to wontfix
  • Status changed from new to closed

Also a good point. I don't think you can tag via XMLRPC so people with the plugin probably never use it. It's possible that they've always been broken like this.

One solution is for them to set a hidden form field with a nonce value when including the form element they're using. On the backend they could verify the nonce and use that as the check. Otherwise malicious commenters could modify the comment form to include the element used as a simple check, and use that method to wipe data.

Plugin authors should also be checking current_user_can() in their edit_post-hooked functions.

So:

Set a hidden form field with a nonced value and check it on the back end, along with checking current_user_can(). That gets you capability and intention. I'll write up a post on it.

comment:5 foolswisdom7 years ago

  • Milestone 2.1.1 deleted
Note: See TracTickets for help on using tickets.