Apostrophe in comment author causes comment to be spammed - esc_html

[tellyworth]

Since [11380] - which added esc_html filtering to many items - comments containing an apostrophe (and possibly other characters) in the author name field are flagged as spam by Wordpress.

The root cause is that esc_html() uses decimal entity encoding, so O'Connor becomes O'Connor. But wp_blacklist_check() regards any comment containing a decimal entity as spam (and worse, does so silently and without any way for the blog administrator to stop it).

Possible solutions:

1. esc_html() should use hex entity encoding, not decimal

2. comment_author_name shouldn't use esc_html()

3. wp_blacklist_check() shouldn't spam comments containing decimal entities

All three are trivial fixes so I haven't included a patch. I'd favour (1) if only because it eliminates the regression and reverts to the old behaviour.

[tellyworth]

Actually there's a fourth option, and I think this ought to be the long-term fix:

Spam filtering really needs to happen on raw POST data, before plugins and sanitizers have the opportunity to screw with it. esc_html()'s behaviour would be fine if it occurred only at display time. But the data passed to spam filters (and, importantly, the data stored in the wp_comments table - which is subsequently used when reporting false positives and missed spam to Akismet and other spam filtering services) need to be as close as possible to the original.

[filosofo]

Replying to tellyworth:

3. wp_blacklist_check() shouldn't spam comments containing decimal entities

This needs to be fixed. Aside from being an un-filterable hack, it's probably needlessly blacklisting trackbacks with such entities.

[Denis-de-Bernardy]

see also: ​http://core.trac.wordpress.org/ticket/6992#comment:22 and follow-ups

[josephscott]

I'd encourage Tellyworth's 4th option, storing escaped/converted data in the database is almost always a bad idea. It's causes problems in other areas where WordPress does that, and it's going to be a real pain to undo.

[ryan]

(In [11460]) Don't use esc_html() for DB bound data. see #9934

[ryan]

That good enough for now?

[filosofo]

Let's leave the ticket open until we can do something about wp_blacklist_check()

[ryan]

Remove entity encoding check? Having it in a blacklist check is unexpected.

[ryan]

See #9965 for the blacklist issue. The regression should be fixed so I'll close this ticket.

[tellyworth]

I just re-tested with r11480 and the problem is still present. Investigating.

[tellyworth]

Confirmed, the same problem is still present even after [11460].

wp_specialchars is used on comment_author prior to comment spam filtering. wp_specialchars() calls _wp_specialchars(), which encodes an apostrophe to its decimal numeric entity (formatting.php around line 273).

Removing the blacklist entity check as per #9965 will fix it but that's just covering up the symptom. The real issue is that WP is futzing with comment data before passing it to spam filters, which hampers their ability to produce accurate results.

[ryan]

That moves wp_allow_comment() before wp_filter_comment(). I don't know if that will bust any plugins though.

[ryan]

wp_specialchars(), when passed only one argument, calls esc_html(). esc_html() defaults to ENT_QUOTES. wp_specialchars() used to default to ENT_NOQUOTES.

Do we need esc_html_db() for these instances. (Yes, I know we should escape as little as possible when sending to the db, but I'm going for the minimal fix for 2.8.)

[Denis-de-Bernardy]

esc_html_raw() (consistent with esc_url_raw()) might be a better name?

[ryan]

(In [11488]) Use _wp_specialchars to get NOQUOTES. see #9934

[ryan]

_wp_specialchars() defaults to ENT_NOQUOTES, which should restore previous behavior. The broader issue of entity encoding on the way to the DB can be dealt with in 2.9.

[Denis-de-Bernardy]

fixed, right?