The bundle labels provided for menus in xmlsitemap_menu_entity_info_alter() have the word "menu" appended to the end of each.

That's inconsistent with the way most bundles work (e.g. nodes and taxonomy), plus for menus that already have the word "menu" in their name, it leads to some awkward wording on the XML Sitemap settings screen... e.g., "Main menu menu" :)

There also seems to be a double-check-plain bug here, since entity bundle labels aren't normally sanitized as far as I know; they get sanitized on output only.

So the attached patch switches to using just the menu name itself for the label.

CommentFileSizeAuthor
xmlsitemap-double-menu-in-entity-name.patch607 bytesDavid_Rothstein

Comments

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Needs review » Needs work

We need to probably double check then that this is actually check_plain()'d on output then? Because I don't think it currently is.

David_Rothstein’s picture

David_Rothstein commented
Status: Needs work » Needs review

It is check_plain()'d in the places I've checked. For example, in xmlsitemap_add_form_entity_summary() the bundle name is output using l(), so it's sanitized there (and I explicitly tried that one out).

A couple other places in the code I looked at seemed fine too.

If there's somewhere where the bundle labels aren't being sanitized on output, then it seems like that would be an independent problem? Because for other entity types (nodes and taxonomy) the bundle names aren't run through check_plain() by hook_entity_info() either; e.g., see http://api.drupal.org/api/drupal/modules--node--node.module/function/nod...

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

Yeah I agree sanitation on output is fine (although we appear to contradict since we provide escaped, translated in other 'label' keys. However, there's a condition in xmlsitemap_add_form_entity_summary() if the user is not able to change the bundle, then it outputs the bundle label directly - and I think that is currently un-sanitized.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Needs review » Fixed

Committed with escaping the bundle labels.
http://drupalcode.org/project/xmlsitemap.git/commit/69b3663

David_Rothstein’s picture

David_Rothstein commented

Ah, good catch. I'm not sure how I managed to miss that when it was two lines of code below the one I was already looking at :) Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.