Special chars in email are escaped using HTML entities
mygumbo - June 1, 2007 - 01:22
| Project: | Notify |
| Version: | 5.x-1.3 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
Description
I've got a drupal site name that has a single quote in it, e.g. Foo's Bar. When notify sends an email, otherwise working great, the subject line is Foo's Bar new content notification for admin.
#1
Argh. It shows up as
Foo' ;s Bar
#2
I had similar behaviour with single quotes within title (nodes or comments).
I did a little hack by replacing a line in notify.module :
"$body = $node_body . $comment_body;"
to
"$body = notify_entities_to_utf8($node_body . $comment_body);"
For your problem, same thing could be done for the variable $subject
I did this very quickly... working for me but not clean (newbie as i am).
#3
Single quotes aren& #039;t the only problem: that& #039;s the apostrophe too. It& #039;s visually ugly to say the least. Since contractions are pretty common, I& #039;d say this is a normal bug, not a & #039;minor& #039; one. I& #039;m sure you see my point.
Sorry, I& #039;ve just seen this in all my notification emails and it& #039;s driving me nuts ...
#4
I also have this problem with double quotes showing up like:
"My Pages"this is also driving me crazy. Has anyone found a solution to these odditites?
#5
I have the same problem. First I thought it has something to do with FCKeditor, but it doesn't. Email notifications for Organic Groups are pretty much unusable, as single quote is quite common character.
#6
Many variables like sitename, title, author, etc are run through htmlspecialchars() which encodes '&', '"', ''', '<', and '>'. This makes sense if you're displaying the variables in HTML which we don't do here. The escaping takes place because of the usage of the '@' decorator for variables in the t()-function which in turn uses check_plain() which contains htmlspecialchars(). The docs say that you should use the '!' decorator for variables supposed to appear in an email (http://api.drupal.org/api/function/t/5). Why does notify use '@'? Also a statement like
<?phpt('@title', array('@title' => $node->title))
?>
doesn't make much senseto me as you translate a varibale only which is the same as doing
<?phpcheck_plain($node->title);
?>
if I understand this correctly. Shouldn't we change all decorators from '@' to '!' in t() and also remove statements like the one above completely (we don't need to check for security in an HTML context at all, do we)?
#7
I've noticed this issue while I've been doing some work on making messages templatable in notify by views.
If someone can provide a patch for notify, I'll apply it & release, but if I have to do it myself, I may not get to it for a little while yet.
#8
Attached is my patch against 5.x-1.x-dev. I replaced all @ decorators with !. Also I removed the empty t() for the item title.
#9
Initial glance at patch look good, so it's been applied to -dev branch.
Please test & report back.
#10
Also applied to 6.x -dev branch.
#11
Please apply it to the 5.x-dev branch, too.
#12
Hmm... trying to figure out what happened to the tagging with this module.
In the meantime, a 5.x release is here: http://drupal.org/node/303336
#13
Automatically closed -- issue fixed for two weeks with no activity.
#14
Please apply to the 5.x-1.x branch and make a new release 5.x-1.2.
#15
Ran across this patch on a random google search.
Knowing how bad some email clients can be about running scipts(not saying names but I think we all know) this kinda scares me. People aren't as good about keeping them up to date either.
Shouldn't you run some sort of filter on some of this information at least? It will look goofy but drupal will let you put
<em>test<script>alert('hello');</script></em>testinginto a title. It just takes one email client flaw and...#16
notify.module uses PHP's
strip_tags()(Version 5) ordrupal_html_to_text()(Version 6) respectively so don't worry. We're not sending HTML here.#17
Looks like it's been committed. Thanks.
#18
Automatically closed -- issue fixed for 2 weeks with no activity.
#19
The patch in comment #8 notify-5-148454-1.patch is flaky because the replacement has been done too thoroughly.
In particular, the format_plural() function assumes the use of @ not ! so all the counts generated by the module now show up as !count. So please change those back again. (3 instances)
e.g. t('Recent content - @count', array('@count' => format_plural(count($nodes), '1 new post', '@count new posts')))
#20
see above
#21
Is this valid for the Drupal 5 version? See http://drupal.org/node/316234#comment-1041160 for Drupal 6.