WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 22 months ago

#14452 new defect (bug)

Duplicate check for comments: Inappropriate errorhandling for xmlrpc

Reported by: mrutz Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.0
Component: XML-RPC Keywords: needs-patch dev-feedback
Focuses: Cc:

Description

The duplicate check for comments just dies in case a dulicate is detected(see wp-includes/comment.php around line 550). This ends up with a html page delivered in any cases. When accessing through xmlrpc this leads to the problem that this html page is send back to the xmlrpc client instead of a proper xmlrpc formated message.

Change History (6)

comment:1 nacin3 years ago

  • Component changed from Comments to XML-RPC
  • Keywords needs-patch 2nd-opinion added; xmlrpc comment duplicate removed
  • Milestone changed from Awaiting Review to 3.1

We make a DOING_AJAX check there. I would want to add in an XMLRPC check, but that seems lame. Maybe we can drop-in an action there, that XMLRPC can then intercept?

comment:2 mrutz3 years ago

Looking at the code I see that there is already the action 'comment_duplicate_trigger'. I don't know exactly, but I would assume that the XMLRPC code should attach to this and handle the duplicate appropriate. Or am I missing something?

If I'm completely wrong, I still would opt for the action as it would be prepared for future upcoming protocols too.

comment:3 nacin3 years ago

  • Keywords 2nd-opinion removed
  • Milestone changed from 3.1 to Future Release

Indeed, exactly. We'd want to do this for APP requests too.

Of note, there's also comment_flood_trigger that has the same wp_die().

This isn't a regression, so I'm moving this to future release.

comment:4 solarissmoke3 years ago

  • Keywords dev-feedback added

Looking at the code I see that there is already the action 'comment_duplicate_trigger'. I don't know exactly, but I would assume that the XMLRPC code should attach to this and handle the duplicate appropriate.

I might be missing something obvious, but I don't think this is possible. IXR_Server calls the wp_newComment() function when that method is invoked. Now, the only way to get a proper error message sent back to the client is to have wp_newComment return an IXR_Error object. Hooking into comment_duplicate_trigger doesn't help because it's an action and not a filter. So you can't use it to tell wp_newComment() what to return.

It seems to me that we need something like DOING_AJAX to get this to work. Or just replicate the check separately in the XML-RPC class (and APP, etc).

comment:5 dd323 years ago

Hopefully implementing some of #16979 will allow the duplicate comment check to be handled properly for xml-rpc

comment:6 markoheijnen22 months ago

Shouldn't we change the function wp_allow_comment() to not use the function die()/wp_die(). Makes more sense that the function what calles wp_allow_comment() to do that.

Note: See TracTickets for help on using tickets.