Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Hi there, if you add a mailto: link with a subject that contains certain characters the output will be broken (see issue 1422462 which touches on this)
These characters include '()* and probably quite a few more. To quickly get it working for me I have amended line 121 of spamspan.module to:
"(?:\?[A-Za-z0-9_= %\.\-\~\_\&;\!\'\*\(\)]*)?)" . # an optional ? followed
but perhaps the whole regexp needs to be a bit less prescriptive:
Additionally line breaks were being stripped out of the link due to line 55 of spamspan.js which has a decodeURIComponent() in it.
Not entirely why the decode is in here as if you disable javascript the source href attr is not decoded. Removing decodeURIComponent() from around the _mailto lets the subject work as intended. (Maybe a security thing, or maybe other email clients need it?)
Here is a sample Message Body which will cause the problem:
"This is a mess'age body! < > "?
!"£$%^&*():@~;'#<>?,./ [] {} -= _+
Isn't it fantastic!Happy to work up a patch if required - interested to get any thoughts on the decodeURIComponent thing first though,
Thanks
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | spamspan-some_chars_break_mailto-2451115-6.patch | 10.46 KB | vitalie |
| #5 | spamspan-some_chars_break_mailto-2451115-5.patch | 3.19 KB | earlofsandwich |
Comments
Comment #1
earlofsandwich CreditAttribution: earlofsandwich commentedComment #2
vitalie CreditAttribution: vitalie commentedthx @earlofsandwich
I think it is expected that subject & body be uri encoded, which should solve the issue of broken output, since the odd characters would get encoded. Patches are of course welcome.
Comment #3
earlofsandwich CreditAttribution: earlofsandwich commentedOkay, here (at last) is a patch that should allow most body and subject text to work correctly. Of course nothing is as simple as it first seems and I had to do extra replacing on the ' character and allow checking on & and # as well to let through the htmlentity encoded quote.
Comment #4
vitalie CreditAttribution: vitalie commented@earlofsandwich thanks a lot! Looks good. Would it be easy for you to add a few tests which cover your changes? They are in spamspan.test.
Comment #5
earlofsandwich CreditAttribution: earlofsandwich commentedSorry it's taken so long, but finally here is a patch with a couple of additional tests. I do one test with a large selection of different characters, and one with the ' char in it (which is dealt with differently)
Hope that helps!
Comment #6
vitalie CreditAttribution: vitalie commentedthank you @earlofsandwich. I noticed now that the changes to the js introduced by the patch bring back the issue of #1540732: %40 instead of @ in link href. Here a patch based on yours which also cleans up a bit the php code that deals with mailto 'headers', but more importantly, urlencodes them in php. This actually would ensure the correct parsing by the js (e.g. js splits headers at ', '). Could you give it a test? What do you think?
Comment #7
earlofsandwich CreditAttribution: earlofsandwich commentedGood find @vitalie, and that's quite an update you've done! I've tested against against a variety of different characters (quite a nasty list and some Chinese ones for good measure) as well as more simple tests without any extra headers and it appears to be working a treat. Newlines work and copying works. Given it a whirl on Android an iPhone as well and working there too.
Comment #8
vitalie CreditAttribution: vitalie commented@earlofsandwich, great, thanks a lot. The changes in js files are to do mostly with moved-around lines. The changes in the lines related to the regular expression are only some extra spacing before comments come in. The update in itself is not so big :), as I said mostly making sure strings are urlencoded already by php.
So should this be set to Reviewed and tested by Community?
Comment #9
earlofsandwich CreditAttribution: earlofsandwich commentedEek. Pressure! Well, it's been tested by me - and certainly seems to be working well for my use cases. I guess it could go into dev and hopefully get a more thorough test there. (I'll let you press the buttons. I'll go hide in a corner somewhere ;)
Comment #11
vitalie CreditAttribution: vitalie commentedthx @earlofsandwich!