When a site title contains an & it is rendered as &
The page processing uses filter_xss_admin() to protect the site title from XSS attacks. This ensures that any bare &s are converted to &. The protected string is then used with l() to form a link to the home page. As the 'html' option flag is not set to TRUE the string is passed through check_plain(), which uses htmlentities(). That blindly converts all &s to & with the result that the string now contains & instead of & (or &).
The basic solution is to set the 'html' flag. (See: basic.diff)
filter_xss_admin() is actually over-permissive in this case as you shouldn't (IMHO) be using complex markup in the title, although I can see why you might want to include some elements to help with styling. I've included an alternative patch that also restricts the allowed markup to inline elements. (See: extended.diff)
| Comment | File | Size | Author |
|---|---|---|---|
| extended.diff | 670 bytes | slite | |
| basic.diff | 317 bytes | slite |
Comments
Comment #1
slite commentedDon't you just love the consistency. The issue system is over-zealous about converting & back into bare &. Note to self: Always remember to preview 8-)
8<----8<----
When a site title contains an & it is rendered as &
The page processing uses filter_xss_admin() to protect the site title from XSS attacks. This ensures that any bare &s are converted to &. The protected string is then used with l() to form a link to the home page. As the 'html' option flag is not set to TRUE the string is passed through check_plain(), which uses htmlentities(). That blindly converts all &s to & with the result that the string now contains &amp; instead of & (or &).
8<----8<----
Comment #2
Jeff Burnz commentedMaybe it makes more sense to use check_markup(variable_get('site_name'), 1) (assuming id #1 is filtered html, which I havent actually checked :) This shit always confuses the crap out of me, so I probably am being over zealous with security.
Powered by Dreditor.
Comment #3
heine commentedJust like an individual's name, a sitename is plaintext and should be displayed as is.
To do so, it needs to be check_plained, which is what l() already does.
Comment #4
slite commentedThe real issue for me isn't that check_plain() is being used, but that it is being used as part of a combination which breaks perfectly reasonable site titles. Feeding the site title directly into l() would be a perfectly good solution (as far as I'm concerned 8-) but the patch was on the principle of least breakage.
Personally I don't care whether tags are allowed in the site name or not, although if they *aren't* allowed then IMHO this should be enforced by Drupal core. (This isn't the only theme where problems occur because of the confusion/inconsistent usage of this.) I accounted for them simply because they *are* currently allowed.
Comment #5
jmlane commentedI spent a bit of time digging around the Drupal 7 theme API trying to get a sense of where the $site_name variable in page.tpl.php is being set and how it is potentially being filtered. It seems that
variable_get(site_name, "Drupal")is considered to be HTML content and thus the call tofilter_xss_admin()which strips all non-whitelisted tags from the output. From this brief analysis, I'd say it has something to with thel()call wrapping$sitenamethat is then assigned to$variables['site_name']. I'll have to debug it a bit when I have some more time.This seems to be the main issue for the #461938: Core should consistently filter_xss_admin() on $site_slogan and check_plain $site_name issue that is still unresolved for both Drupal 7 and 8 HEAD.
Comment #6
Jeff Burnz commentedFixed in 7.x-3.x, need to check if I changed this in older versions, now we just run with Heine's original view to just use l()
Comment #7
Jeff Burnz commented