[#11353] - Email Cloaking plugin corrupts HTML
- Closed
- 4 Aug 2016
- Medium
- Build: staging
- # 11353
- Diff
- demis-palma:email-cloak
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success JTracker/HumanTestResults Human Test Results: 4 Successful 0 Failed. Details
User tests: Successful: Unsuccessful:
The plugin “Content - Email Cloaking”, under some circumstances, corrupts the HTML of the processed content.
The use of email addresses within attributes of HTML tags is legitimate in the HTML code of a Joomla article.
Chances are high this actually happens when users include any kind of modules within articles, using the “Content - Load Modules” plugin (now used more than ever, due to the new editor button “Module”).
An article containing this HTML code
<img src="/envelope.png" title="email@address.com">
Should appear like that:
But the cloak plugin corrupts the output
<img src="/envelope.png" title="<span id="cloak84493">This email address is being protected ...</span><script type='text/javascript'>...</script>
Which produce that result:
The same goes for an input element
<input type="text" value="" placeholder="email@address.com">
Which should appear like that:
but it becomes like that instead:
<input type="text" value="" placeholder="<span id="cloak77416">This email address is being protected ...</span><script type='text/javascript'>...</script>
The cause
HTML attributes (like title=”...”) don't allow any further nested HTML tags, nor JavaScripts inside them.
Summary of Changes
- The negative lookahead '(?![^<]*>)' added to the regular expression is used to exclude occurrences within HTML tags.
- Added exhaustive comments to explain the reason behind the lookahead.
Testing Instructions
I have tested the new regular expression with a lot of sample text, and it behaves good to me, but everyone interested is encouraged to test further (we know how tricky regular expressions can be).
I have tested this item
Confirmed issue and works as described.
Did some tests and all worked fine.
But again...
(we know how tricky regular expressions can be)
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11353.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11353.
| Status | New | ⇒ | Ready to Commit |
RTC on testing.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11353.
RTC
@joomla-cms-bot please add the RTC label!
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11353.
Now lets try to make the bot work
it seems that the bot doesn't work :)
RTC please
Adding it manually seems to have worked
| Category | ⇒ | Plugins |
| Labels | |||
I have tested this item
Installed 3.6.1 - added this html to article
<img title="phil@phil-taylor.com" src="/images/powered_by.png" alt="phil@phil-taylor.com">
It was broken when rendered on frontend.
applied PR #11353 - then the rendering was fine, and rendered the html as above
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11353.
It was originally scheduled for 3.6.2, but it has been moved to 3.6.3.
As far as i know, no new features will be added to the new 3.6.2.
It's not a new feature it's fixing a bug :)
its a bug fix - not a new feature
I have tested this item
I applied the patch but as long as the Email Cloaking Plugin is enable, my display is not correct as it removes my CSS class:
If I have for example this: <a href="mailto:toto@toto.com?subject=Subject" class="nameofmyclass">email</a>
It becomes in the source code: <span id="cloak9b08867df87471a610f686815c73ade8"><a href="mailto:toto@toto.com?subject=subject">email</a></span>
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11353.
@Sandra97 to be clear - your input to the test is the HTML
<a href="mailto:toto@toto.com?subject=Subject" class="nameofmyclass">email</a>
right?
With that input I get this as an output:
<span id="cloakf157cce915dfb5ec51f818d7e55aee9e"><a href="mailto:toto@toto.com?subject=Subject">email</a></span>
which is like you say _missing_ the CSS class...
This is exactly the type of bug that would benefit from 100 unit tests on it!!!
There you are Phil. Something for you to do over the long summer nights.
lol - I dont know where you think I get so much free time from... tomorrow I have to sit for 2 hours to watch 3-9year olds sing Chitty Chitty Bang Bang in a amateur play... give me unit testing anytime over that!!! #sendhelp
The same place as everyone else
@Sandra97 Thanks for testing, and I confirm your result: the class is removed, while it shouldn't.
However, this is not caused or fixed by this PR. It is simply a different bug which was present before and is still present after this patch. You'll find that your current version of Joomla is affected as well.
If you open a separate issue, I can fix it.
But consider that the current implementation of Email Cloak is based on regular expressions.
Regular expressions are definitely not good for parse HTML.
As much as I can fix something, someone will be always able to find another rare code sequence that cause problems.
But as I said, I would open a different issue, because this is a different bug, it was already present before this PR, and this PR is not intended to fix it.
As much as I can fix something, someone will be always able to find another rare code sequence that cause problems.
Which is why a set of reproducible unit tests on this code would be invaluable!
@PhilETaylor I agree. However, you know, parsing HTML using regexp is fighting against windmills.
@demis-palma, thanks. I'm gonna open a new issue
@demis-palma Please see #11456.
Thank you.
Removed RTC as it has been merged
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11353.
@demis-palma Please see CS PR demis-palma#1