strip_tags() breaks category names with left angle brackets

[squirreling]

If you create a category named "<something", the category name doesn't show up on any of the category listings. If you create a category named "some<thing" the category name shows up as "some".

[ryan]

strip_tags() is the culprit

[slushpilejs]

rescued lone less than sign in pre_term_name from strip_tags(), separated out pre_term_name filters

[mattyrob]

Will this patch also work for lone greater than symbols?

Maybe we should HTML encode them both (&lt; and &gt;) in category names.

[slushpilejs]

Yes, wp_specialchars encodes the lone greater than symbol. This merely dodges the broken behaviour of strip_tags.

[MattyRob]

Can't we just encode special symbols when categories are created? Only issue here is how to rectify existing category names.

[chineseleper]

I'm also having a problem with XML comments in titles, see #8981. It would be great if WP could use it's own strip_tags function which doesn't remove xml comments and keeps angle brackets.

[mattyrob]

I've tested the patch I submitted 2 months ago in the spirit of the 24-hour has-patch marathon and it works for me. Maybe get someone else to check it before committing :-)

[Denis-de-Bernardy]

$slug = wp_specialchars($category_nicename);

won't we end up with special chars in the slug?

[mattyrob]

I've checked the database and there are no special characters in the slug in the terms table.

Weirdly though the fix appears somewhat erratic, it works fine for > but not at all for <. Is there some JS in use on the admin Categories page?

[Denis-de-Bernardy]

let's mark as needs patch then.

imo, there is no point to add the call to the slug, since that would get sanitized beforehand.

[MattyRob]

Back Again :-)

I've checked the slug out more and it is already being sanitised. The problem with the initial patch was that the un-encoded variable is still passed further down the script in the patched file. An updated patch is attached.

There is another by sending $name rather than $cat_name to the wp_insert_term() function.

[Denis-de-Bernardy]

say, can you confirm the issue? because, currently, there is a filter on:

pre_term_name

which does what you want.

closing as invalid, since it seems fixed in trunk... please reopen with feedback if it's not.

[MattyRob]

Updated to latest trunk to test and it still breaks:

Categories with 'less than' symbols in the cat name have all text after this symbol removed so 'some<thing' becomes 'some' and '<some' becomes null with the category slug becoming an integer.

The patch I submitted still fixes this issue.

[MattyRob]

Should have said, trunk version was 11111

[Denis-de-Bernardy]

confirmed with foo'bar<foo"

patch is broken.

[MattyRob]

Denis,

Can you check the patch again please. I tested with:

• foo'bar<foo"
• foo<bar>
• '"bar"<foo<bar>'

And all three worked on my patched trunk [11270], screen grab attached

[Denis-de-Bernardy]

my bad sorry. I was trying to apply it to wp-includes/taxonomy.php instead of wp-admin/includes/taxonomy.php

one slight issue I ran into: it allowed to create a category called foo<br^{"\!'bar?#, but it denied the same for tags and link categories. a filter should likely apply to all terms on a pre_term_name hook or something like that.}

[MattyRob]

You may well be right but a patch along those lines is beyond my understanding of the WordPress core files :-(

[MattyRob]

Hold the phone! I may be smarter than I thought!!

I've messed about with default-filters in wp-includes and this patch works for categories, tags and link names. The down side is that embedded HTML is now escaped into the name.

[Denis-de-Bernardy]

yeah, something like that. I'll give it a shot tomorrow.

[Denis-de-Bernardy]

I've just uploaded a slightly modified version of your patch. (removed the strip_tags() call as it becomes useless.)

There is a genuine bug in there before any of it can be committed. If you create * foo'bar<foo" * foo<bar> * '"bar"<foo<bar>' as a tag, that tag can be added in the post editor using the autocomplete, but it then won't display correctly in the tag editor.

a few checks to run with * foo'bar<foo" * foo<bar> * '"bar"<foo<bar>':

• can it be added as a post category/does it display correctly in the editor? in the post?
• can it be added as a post tag with/without autocomplete/does it display correctly in the editor? in the post?
• can it be added as a link category/does it display correctly in the editor? in the links widget?

[MattyRob]

Done some testing and the results are as follows:

• Post Category addition, display in editor and display in actual post (in default theme) all works fine.
• Post tag addition works fine, addition to a post in editor and display in editor is flaky probably down to JS encoding of the tag, display in front side post works well (provided you can tag in editor)
• Addition editing and display as links all works fine

So, the problems all seem to be in the "Post Tags" section of the post editor now.

[hakre]

looks like a simple output problem to me. < must be converted to entity when the output should be preserved wihtin (x)html. patches should reflect that instead of patching here and there on good luck.

[MattyRob]

The remaining issue seems to be down to the jQuery processing of tags added on the post screen. I'm not great with javascript or jQuery so someone else needs to tae a look. I suspect the problem is in the handling of special characters (<">' etc) in either post.js or tags.js contained in wp-admin/js.

[Denis-de-Bernardy]

Punting per discussion during the WP meet-up...

[hakre]

Category names should not be allowed to have < or > chars in it. What about filtering the input and removing those values?

[MattyRob]

Replying to hakre:

Category names should not be allowed to have < or > chars in it. What about filtering the input and removing those values?

After the ticket has been logged and open for 10 months I think it's a little late to imply that the ticket is not valid and that the code should simply not allow angle brackets in category names!

The first question posed would be "Why not?" I think putting such symbols into category names, link names and tags is totally acceptable and should not be blocked or filtered, but should be properly handled and escaped.

[hakre]

I just wanted to add my own opinion, please do not feel offended about that. It was just a suggestion, not the way to go. I should have been written that more clearly.

WordPress is not that strict about escaping and encoding and proper handling of such Issues so it *might* be a good Idea to just reduce the number of allowed chars for things like tags, categories, names, slugs etc. pp.. This is the thought behind what I've been written.

It's totally clear that this has implications on a current usage with those chars in those names.

[MattyRob]

Replying to hakre:

I just wanted to add my own opinion, please do not feel offended about that. It was just a suggestion, not the way to go. I should have been written that more clearly.

Okay, fair point and I apologise if I came across a bit prickly!

WordPress is not that strict about escaping and encoding and proper handling of such Issues so it *might* be a good Idea to just reduce the number of allowed chars for things like tags, categories, names, slugs etc. pp.. This is the thought behind what I've been written.

It's totally clear that this has implications on a current usage with those chars in those names.

While one cannot argue that WordPress is fabulous at providing a rich blogging environment it does lack somewhat in documentation and in direction decisions for small but user important issues (like this one). If these characters are deemed inappropriate for category names etc, then why is the ticket still open after 10 months? If this is agreed to be an issue in the software why no update from the WP meet-up?

[Denis-de-Bernardy]

Replying to MattyRob:

If these characters are deemed inappropriate for category names etc, then why is the ticket still open after 10 months?

One word: needs-patch. Every patch provided either fails to fix the issue, or fixes it in an erroneous way, or ends up introducing problems in the tag picker.

[markjaquith]

needs-patch isn't one word. :-p

Punting to future.

[aaronholbrook]

This no longer appears relevant, the problem as described works as expected.