When _taxonomy_menu_save() generates the title attribute for the links, doesn't encode possible html special characters originating from the corresponding vocabulary term. This leads in validation failures of type

character "&" is the first character of a delimiter but occurred as data

. The following patch adds some check_plain()s to prevent this.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Tri’s picture

Attaching patch

indytechcook’s picture

Status: Needs review » Fixed
tomsm’s picture

Status: Fixed » Needs work

I think that this patch generates the following errors in my logs, like:

Details
Type error
Date Friday, 11 June, 2010 - 16:50
User admin
Location http://localhost/drupal/en/admin/content/taxonomy/edit/vocabulary/1
Referrer http://localhost/drupal/en/admin/content/taxonomy/edit/vocabulary/1
Message Menu and taxonomy name mismatch: Fysioband & Tubing != Fysioband & Tubing
Severity notice
Hostname 127.0.0.1

I have these errors since I installed 6.x.-2.8 and rebuilt the menu. If I install the previous version and rebuilt the menu, the errors are gone.

When I remove this patch (check_plain), the errors are gone.

indytechcook’s picture

Version: 6.x-2.7 » 6.x-2.8

Anybody else confirm this?

jefftrnr’s picture

I think I've confirmed this.
If I have an ampersand in my term name, it displays as & in the menu item when I save the term-edit form.

jefftrnr’s picture

As a temporary fix, I added a str_replace for each of the 2 elements in the $link array that were changed by the patch. They simply replace & with & again.

It seems fixed now, but there's probably a more elegant solution.

  $link = array(
    'link_title' => check_plain($item['name']),
    'menu_name' => $item['menu_name'],
    'plid' => $plid,
    'options' => array('attributes' => array('title' => trim($item['description'])
      ? check_plain($item['description']) : check_plain($item['name']))),
    'weight' => $item['weight'],
    'module' => 'taxonomy_menu',
    'expanded' => variable_get('taxonomy_menu_expanded_'. $item['vid'], TRUE),
    'link_path' => $path,
  );

  // next 2 links are a bugfix for & problem -- see http://drupal.org/node/799428
  $link['link_title'] = str_replace('&','&',$link['link_title']);
  $link['options']['attributes']['title'] = str_replace('&','&',$link['options']['attributes']['title']);

queenbeenz’s picture

I have a similar issue triggered by an apostrophe in the menu.

rogerpfaff’s picture

I got the issue with an ampersand too

CardinalFang’s picture

Same issue here with apostrophes and ampersands. The problem didn't start until I fooled around with Content Taxonomy and Taxonomy Super Select. Once I disabled and uninstalled those, my problems began.

indytechcook’s picture

Status: Needs work » Needs review

The issue occurs on line 1009 of taxonomy_menu.module when the term->name is being compared with the menu item's title. Since the menu item's title hit check_plain() and the term->name didn't it's finding a mismatch. This in no way has any adverse effect on your the menu itself. It's just a warning.

Change line 1009 from

if ($item['title'] != ($term->name . $display_num)) {

To

if ($item['title'] != (check_plain($term->name . $display_num))) {

I'm not familiar enough with the tt function to know it should also be wrapped to check_plain. I tend to think it should as those lines of code overwrite the original check_plain.

I'll make a proper commit later today. Please test and let me know if this works.

indytechcook’s picture

Version: 6.x-2.8 » 6.x-2.x-dev
FileSize
1.31 KB

Here is a patch against the latest from the DRUPAL-6--2 branch. Since my i18n usage is nill, i'd love it if someone would test this.

indytechcook’s picture

Added comment from #841440: Check plain transforms HTML tags instead of removing them by doitDave:

check_plain (for menu title and description) is of course right and important, but ignores that it is possible to use HTML in a vocabulary/term description (which I do intensively to make result pages look even nice without using "views").

Please excuse that since this is my first report ever and I am currently heavily involved in a huge project, I am yet only "leeching" but not ready to learn the CVS instructions, so I simply post the necessary changes here, hopefully someone would patch it. It is really simply done:

in function _taxonomy_menu_save:

replace

'options' => array('attributes' => array('title' => trim($item['description'])
  ? check_plain($item['description']) : check_plain($item['name']))),

with

'options' => array('attributes' => array('title' => trim($item['description'])
  ? check_plain(strip_tags($item['description'])) : check_plain(strip_tags($item['name'])))),

Thanks for your great work anyway!

This will be committed with the above patch when the patch has been tested.

sreese’s picture

I have tried the patches posted in this thread and rebuilt my taxonomy menus but to no avail. I still see '& amp;' (had to add a space there) instead of '&' in the menu. Is there some trick to getting this to work that I am unaware of?

happysnowmantech’s picture

I am seeing the problem with ampersands not being displayed properly as well, e.g. if I have a term named 'A & B', the menu displays 'A & B'

FYI, theme_menu_item_link() also runs the menu item title through check_plain(), via its call to l(). If the title is run through check_plain() twice, once in _taxonomy_menu_save() and then again in theme_menu_item_link(), the & problem will result.

For now, I'm using a patch similar to #6 as a temporary workaround:

  // next 4 lines are a bugfix for & and apostrophe problem -- see http://drupal.org/node/799428
  $link['link_title'] = str_replace('&','&',$link['link_title']);
  $link['options']['attributes']['title'] = str_replace('&','&',$link['options']['attributes']['title']);
  $link['link_title'] = str_replace(''',"'",$link['link_title']);
  $link['options']['attributes']['title'] = str_replace(''',"'",$link['options']['attributes']['title']);
sreese’s picture

The temporary workaround posted in comment #6 also worked for me. Had a bit of a hard time figuring out where to put it but ended up adding the 3 lines starting at line 554 in the -dev version of taxonomy_menu.module which is towards the end of the _taxonomy_menu_save($item) function.

giorgio79’s picture

+1

& in term names turned to &

fengtan’s picture

FileSize
714 bytes

Here is another patch to get rid of the & but also of any html entities.

I don't think performing a check_plain() is needed on $item['name'] since $item is to be saved with menu_link_save($item), which does not expect $item['name'] to be sanitized.

indytechcook’s picture

Here is the commit: http://drupal.org/cvs?commit=391318

Please test this when the new dev release is rolled in the next 12 hours.

nadavoid’s picture

I'd like to test, but upon enabling the latest dev, I get this error:

PHP Fatal error:  Can't use function return value in write context in /home/clients/websites/w_deepc/public_html/deepc/sites/deepcreek1.dev3.webenabled.net/modules/taxonomy_menu/taxonomy_menu.module on line 574
tomsm’s picture

@ nadavoid

An issue has been opened about this error:
http://drupal.org/node/853438

nadavoid’s picture

@tomsms I tried the latest dev which looked like it had #853438: Fatal Error - Can's use function return value in write context applied. Thanks for pointing that out. It works, and I am able to test further now.

Now, when entering new terms, the description of the term (which is used as the title attribute of the menu item) displays html encoded ampersands. I have a wysiwyg installed. When a user enters an ampersand, it is html encoded by the wysiwyg. That is presented exactly as entered by the menu system. However, this is not what anyone would want. So I'm submitting a patch that replaces the html encoded ampersand with a single ampersand character.

Attaching a patch which replaces this:

check_plain(strip_tags($item['description'])) :

with this:

strip_tags(html_entity_decode($item['description'])) :

The check_plain is not needed because the menu item rendering is run through check_plain. With check_plain here, we get double encoded html entities rendered in the title.

The html_entity_decode is just a way to decode any html-encoded characters. I currently have some encoded ampersands because I have a wysiwyg enabled for term descriptions.

I'd appreciate any feedback. Thanks!

nadavoid’s picture

Updated patch. Same update applying to term title if term description is empty.

hanoii’s picture

subscribe

indytechcook’s picture

Ok. Let's hope this is the last change because I want to do a new release.

http://drupal.org/cvs?commit=396554

PS. I really need to spend time to make some tests.

nadavoid’s picture

Status: Needs review » Reviewed & tested by the community

I just tested the latest dev release which has this patch applied, and it works as expected for me. Marking RTBC. Thanks for committing it indytechcook!

tomsm’s picture

I also tested latest dev. All ok.

indytechcook’s picture

Status: Reviewed & tested by the community » Fixed

Setting to fixed with the new release: http://drupal.org/node/872158.

Status: Fixed » Closed (fixed)

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

Magur’s picture

Status: Closed (fixed) » Needs review
FileSize
913 bytes

If I have special characters in the description of the term like "Jahresangabe für Berichte" then i get a database entry in the table menu-links which has the following content in the options-field:

a:1:{s:10:"attributes";a:1:{s:5:"title";s:25:"Jahresangabe f

So everything after (and including) the special character is missing.

I think the reason for this is that html_entity_decode decodes a character to ISO-8859-1 by default. Setting it to UTF-8 fixes this issue for me.

indytechcook’s picture

Need someone else to test this.

wuh’s picture

Status: Needs review » Needs work

The patch doesn't work for me - I still have menu titles with unencoded html special chars - I use the i18n module - could this be causing issues?

organicwire’s picture

I had the same problem as I had a taxonomy term having an ampersand in it's name. The problem was resolved updating to the new module version (6.x-2.9) AND opening+saving the taxonomy term afterwards.

abx’s picture

Just tried version 2.9 and I can't get menu list of the taxonomy name with "slash" in it. :(
plaque de cuisson

rkodrupal’s picture

FileSize
37.74 KB

Sorry if this isn't the correct discussion thread, but I'm still getting my ampersands showing up as unencoded for taxonomy terms ... see attachment.

I get this behavior for both new terms and for old terms that I've updated.

addendum: I am at 6x-2.9 on Taxonomy Menu
addendum#2: firebug shows the "&" as "&" ... and I'm using bb2html (http://drupal.org/node/28537, which doesn't seem to be getting invoked)

rkodrupal’s picture

update on #34 ... bb2html does indeed get invoked. I can place [amp] in the taxonomy item and it shows up okay as an ampersand (due to the magic of bb2html) ... but bb2html doesn't work everywhere and [amp] just shows up as [amp] ...

rkodrupal’s picture

I found a fix for me and am sharing it so it may help others:

replaced $title with strip_tags(html_entity_decode($title)) line 205 in page.tpl.php

... this inspired me to also do the following:

replaced $snippet with strip_tags(html_entity_decode($snippet)) in search-results.tpl.php

I use the zincout theme, so you may need to find the equivalent tpl's for your theme.

Agileware’s picture

I have been having problems when my term description includes  
The same as those described in #29.

The patch in #29 fixes the problem for me.
I am not using i18n.

The problem was causing this error:

PHP Fatal error: Unsupported operand types in /includes/common.inc on line 1592

because when drupal unserializes the array

a:1:{s:10:"attributes";a:1:{s:5:"title";s:25:"Jahresangabe f

You get FALSE.

Agileware’s picture

@abx re #33:
It is not a good idea to change old posts because it is hard for people to see what has changed.
If you have just made the post it's fine but not if it is months old because people expect the comments to be in chronological order and you will find anything that gets edited back into an old post is likely to be ignored because people don't see it.

abx’s picture

@Agileware

That was strange but that's not me who changed it. I didn't come back to this topic since I replied on that one. (And it's not Jan 30, 2011 for sure. I think I replied it sometime in the last quarter of 2010) Still, wondering if it's problem on drupal.org or my own computer.

Could it be some security problem of Drupal.org? I'm quite sure I was not the one that put up the link "plaque de cuisson" on it. I believe it's done by some spam robot.

soulfroys’s picture

#29 fix it for me! (6.29)

dstol’s picture

Issue summary: View changes
Status: Needs work » Needs review

The last submitted patch, 11: taxonomy_menu-799428-11.patch, failed testing.

The last submitted patch, 17: taxonomy_menu-799428-17.patch, failed testing.

The last submitted patch, 21: taxonomy_menu_799428_ampersands.patch, failed testing.

The last submitted patch, 22: taxonomy_menu_799428_ampersands.patch, failed testing.

The last submitted patch, 29: taxonomy_menu_799428_encoding.patch, failed testing.

The last submitted patch, 1: taxonomy_menu.module-799428.patch, failed testing.

dstol’s picture

Status: Needs review » Closed (outdated)

The Drupal 6 version is no longer supported