esc_html() ate my ampersand

[mdgl]

While testing #28816 I noticed that esc_html() effectively "eats" an explicit XML/HTML ampersand entity if this is immediately followed by what looks like another valid XML/HTML entity. For example:

Input	Actual Output	Expected Output	Notes
A & B	A & B	A & B	Lone ampersand "corrected"
A & B	A & B	A & B	Valid HTML passed through
A – B	A – B	A – B	Valid HTML passed through
A – B	A – B	A – B	Wrong as ampersand missing
A &ndash B	A &ndash B	A &ndash B	Malformed entity handled correctly

This happens because of the call to wp_specialchars_decode() within _wp_specialchars(). The logic of this is very hard to fathom. If you remove this call, the escaping appears to work correctly with the exception that some numeric character references are not replaced by their named equivalents which breaks one of the unit tests, even though this could be regarded as dubious behaviour.

[DrewAPicture]

I would consider this expected behavior for esc_html().

– on input would not be equal to – on output simply because the ampersand in this instance would (and should) be interpreted as a literal string &. It's basically double-encoded, and esc_html() is not really meant to decode special characters in this way.

[mdgl]

– on input would not be equal to – on output simply because the ampersand in
this instance would (and should) be interpreted as a literal string &. It's basically
double-encoded, and esc_html() is not really meant to decode special characters in this way.

Indeed, you are just confirming the bug. At the moment, esc_html() converts – to just – as my table shows. Like you, I believe it should remain as –.

[DrewAPicture]

Sounds like both @boonebgorges and I read that wrong. Sorry about that.

[MikeHansenMe]

Unit tests showing problem

[MikeHansenMe]

Added some unit tests to show the problem.

[collinsinternet]

Adds & to wp_specialchars_decode() to allow unit tests to pass.

[miqrogroove]

Duplicate of #17780.

[mdgl]

Surely this "bug" can't be a duplicate of a separate "enhancement" request? I think #17780 would need its category and description updating.

[miqrogroove]

Hi @mdgl, this has been resolved for 4.3 per the older ticket. We normally get multiple reports that only require one patch, so we only keep one ticket as needed. This is not to downplay one or the other, but to simply declutter the thousands of tickets.