Bad handling of ampersand in post titles

[Commeuneimage]

Titles with ampersand (&) are not correctly handled:

1/ the_title_attribute() doesn't transform & to & causing XHTML validation errors

2/ titles with & followed by ; generate truncated post name (slug)
e.g. "Test this & believe ; what ?" => "test-this-what"

[mdawaffe]

I cannot confirm 1. the_title_attribute() does correctly turn ampersands into HTML entities for me.

I can confirm 2.

It comes from the following line in sanitize_title_with_dashes().

$title = preg_replace('/&.+?;/', '', $title); // kill entities

Fixing this could generate more bugs since the output is used in permalinks.

[dd32]

Could that be changed to :

$title = preg_replace('/&[a-z0-9]+?;/', '', $title); // kill entities

perhaps? That'd still catch all valid entities...

[williamsba1]

Tested out dd32's code and it fixes the issue. Like mdawaffe said this could cause other issues so wanted to patch so others could test.

[pufuwozu]

Sorry to be pedantic but that patch introduces a slight regression for cases such as × (the permalinks would include "215").

Maybe we should include the hash character in the regex, like so:

$title = preg_replace('/&[#a-z0-9]+?;/', '', $title); // kill entities

After that, I can't think of any other cases.

[dd32]

Sorry to be pedantic but that patch introduces a slight regression for cases such as × (the permalinks would include "215").

Very true, That thought came to me after posting, I thought I'd added another reply to that effect..

[solarissmoke]

Patch addresses item (2) - I'm not sure if this would break anything else. In any case this seems quite edge.

[nacin]

I imagine we have a regex somewhere that deals with killing entities.

Of note I also see this regex used here:

./wp-includes/formatting.php:758:	$username = preg_replace( '/&.+?;/', '', $username ); // Kill entities

[solarissmoke]

Use \w instead of [^\s]

[SergeyBiryukov]

Related: #10797

[SergeyBiryukov]

Related: #10792

[azaozz]

Replying to pufuwozu:

$title = preg_replace('/&[#a-z0-9]+?;/', '', $title); // kill entities

That looks good except the "?" is not needed. Also we can specify the length of that string:

$title = preg_replace('/&[#a-z0-9]{2,6};/', '', $title);

[westi]

Too late in the cycle to be tweaking this.

We should also add some unit tests around this change.

[dd32]

dd32 * ​http://unit-tests.trac.wordpress.org/changeset/489 /wp-testcase/test_includes_formatting.php: Cover more entity striping cases for sanitize_user() and sanitize_title_with_dashes(); See ​http://core.trac.wordpress.org/ticket/10823

Testing for that was done with/passed with the following regex:

'/&#?\w+;/'

I'm sure the above regex's would probably pass with this still.

[MikeHansenMe]

Tested this patch, it still seems to work with 3.6-alpha. Maybe we should try to get this in 3.6 early?

[wonderboymusic]

Related #17648

[boonebgorges]

See #30284.

[MikeHansenMe]

Refreshed

[MikeHansenMe]

Patch still applies with some offset

[SergeyBiryukov]

#36282 was marked as a duplicate.

[MikeHansenMe]

Patch needs to be refreshed.

[lukecavanagh]

Patch Refreshed