Quotes in comment name prevent "awaiting moderation" display

[thomasperi]

If a double or single quote mark (not the "curly" or "smart" version of either) is entered in the Name field of the comment form, the comment does not appear to the user who posted it until it has been approved.

Reproduced in r9508

For some reason this diff isn't showing up properly, oh well.

I fixed this by running $comment_author through htmlspecialchars_decode, but only for the purposes of that prepare() call... I wasn't sure if $comment_author itself was used anywhere else, so to be safe I didn't modify it's value directly.

BTW, this is happening because it grabs the comment author's name from the comment_author_COOKIEHASH cookie, where it is urlencoded, and then attempts to stick it directly into a query... At some point (not entirely sure where), quotes in the string are turned into htmlspecialchars. This is great for displaying this on a page but doesn't work so well in a SQL query.

Has anyone taken a look at this?

Finally got the diff cleared up! :)

(In [9773]) Show pending comments to users with quotes in their name. props regulatethis. fixes #6992

(In [9825]) Revert [9773]. htmlspecialchars_decode() requires PHP 5.1. see #6992

htmlspecialchars_decode

There's a Compat version of the function here:  http://pear.php.net/reference/PHP_Compat-1.6.0a1/__filesource/fsource_PHP_Compat__PHP_Compat-1.6.0a1CompatFunctionhtmlspecialchars_decode.php.html#a18

PHP4 compatible fix

I just lifted the code and cleaned up some formatting. It's licensed under the LGPL. Can this go in as is?

Actually, 'ENT_COMPAT' does NOT translated single-quotes. PHP says "ENT_COMPAT, Will convert double-quotes and leave single-quotes alone."

Also I wouldn't set $quote_style = null) instead use $quote_style = ENT_COMPAT) which makes it clear, ENT_COMPAT is the default option.

PHP4 compatible fix, made to behave more like PHP5's htmlspecialchars_decode (thanks Hirvine)

For 2.7 I'd rather use htmlspecialchars_decode() only when it exists if we do do this at all. We're too close to RC to introduce a new compat function.

So do we push this back to another release or should I start looking for another solution? The only thing I can think of is to go back to where this value is encoded in the first place and change that from happening, but it seems like its encoded for a good reason so I don't know if that's the best idea.

Let's postpone to 2.8 so we have time to do a full fix.

Does a "full fix" imply the usage of the compat function? It seems like that value _should_ be encoded normally. I'm definitely open to working on another angle, I just don't know what it could be :)

Since this seems to be awaiting a fuller solution, it's not really has-patch any more.

So again, what's a "fuller" solution? Anyone have any input here? I'd be glad to work on this, but I'd need some suggestions, obviously my previous fix was not used.

New patch, using html_entity_decode in the wp_get_current_commenter()

Actually, this is bad because the values are obviously displayed on the page at some point. Doh. So that last patch is garbage, I'll see if I can figure out something else. I'm starting to think that a better solution would be to scrap the SELECTs as noted for comments_template() in comments-template.php.

Punting due to feature freeze. Reconsider with next release.

the real issue here is sanitize_comment_cookies(), which is a bit too overzealous on the comment author name. we should store its original value (which is correctly set in the cookie) for use in the database query.

using the cookie's original value

Replying to Denis-de-Bernardy:

the real issue here is sanitize_comment_cookies(), which is a bit too overzealous on the comment author name. we should store its original value (which is correctly set in the cookie) for use in the database query.

We can just reverse the work in the one place we need the un-sanitised version.

(In [11220]) Decode the commenters name why looking up un-moderated comments. Fixes #6992.

Replying to westi:

Replying to Denis-de-Bernardy:

the real issue here is sanitize_comment_cookies(), which is a bit too overzealous on the comment author name. we should store its original value (which is correctly set in the cookie) for use in the database query.

We can just reverse the work in the one place we need the un-sanitised version.

Well, unless you know a means to reverse the work done by filters, you're in for big surprises with the patch you just committed.

So can someone explain the actual problem here? We have a value stored in a cookie and we need to decode html entities in it when using it in a query. Seems straightforward. It's so basic in fact, that it is essentially the exact same fix that I provided *6 months ago*, except the function used is named differently.

Why is this such a big issue?

Replying to Denis-de-Bernardy:

Replying to westi:

Replying to Denis-de-Bernardy:

the real issue here is sanitize_comment_cookies(), which is a bit too overzealous on the comment author name. we should store its original value (which is correctly set in the cookie) for use in the database query.

We can just reverse the work in the one place we need the un-sanitised version.

Well, unless you know a means to reverse the work done by filters, you're in for big surprises with the patch you just committed.

Nope.

We are not reversing the work done by the filters.

The same work is done on the comment author on the way into the db so those filters have already been applied to the value before it was stored in the cookie so they should have no effect.

We are reversing the esc_attr call in sanitize_comment_cookies function and that is all.

esc_attr() call, which, incidentally, has a filter hiding in the background...

leaving this closed, but see also #9934