joomla / joomla-cms

Home of the Joomla! Content Management System
https://www.joomla.org
GNU General Public License v2.0
4.69k stars 3.63k forks source link

CC address in email link being removed after ampersand #21910

Open brawdon opened 5 years ago

brawdon commented 5 years ago

Steps to reproduce the issue

Create an html email link with a CC address...

<a href="mailto:someone@yoursite.com?cc=someoneelse@theirsite.com, another@thatsite.com, me@mysite.com&bcc=lastperson@theirsite.com&subject=Big%20News">Email Us</a>

Expected result

Email link with recipient and CC recipient.

Actual result

Code saves properly, but JS spambot prevention seems to removes the CC address after the ampersand.

System information (as much as possible)

Joomla 3.8.11

Additional comments

ghost commented 5 years ago

Please ask help on the forums. This repository concerns in first Place Joomla-Core coding. For this Reason closing this Issue, thanks.

joomla-cms-bot commented 5 years ago

Set to "closed" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/21910

brawdon commented 5 years ago

This is not a Joomla bug? It appears to be a core issue to me.

brianteeman commented 5 years ago

Re-opened

mbabker commented 5 years ago

There's a core bug. Add this to the unit test for the email cloaking plugin:

            array(
                '<a href="mailto:toto@toto.com?cc=joe@nowhere13.com&bcc=bobo@toto.com&subject=Big%20News">Click Here</a>',
                "<a href='mailto:toto@toto.com?cc=joe@nowhere13.com&bcc=bobo@toto.com&subject=Big%20News'>Click Here</a>",
                "<span id=\"cloak__HASH__\">JLIB_HTML_CLOAKING</span><script type='text/javascript'>
                document.getElementById('cloak__HASH__').innerHTML = '';
                var prefix = '&#109;a' + 'i&#108;' + '&#116;o';
                var path = 'hr' + 'ef' + '=';
                var addy__HASH__ = 't&#111;t&#111;' + '&#64;';
                addy__HASH__ = addy__HASH__ + 't&#111;t&#111;' + '&#46;' + 'c&#111;m?cc=j&#111;&#101;';
                var addy_text__HASH__ = 'Cl&#105;ck H&#101;r&#101;';document.getElementById('cloak__HASH__').innerHTML += '<a ' + path + '\'' + prefix + ':' + addy__HASH__ + '\'>'+addy_text__HASH__+'<\/a>';
                </script>
                "
            ),

(Note that the expected JS is actually what it's giving right now, not what it's supposed to be, but it is easier to show the failing test result with the HTML string)

The test fails...

1) PlgContentEmailcloakTest::testOnContentPrepareWithRowNoFinder with data set #16 ('<a href="mailto:toto@toto.com...re</a>', '<a href='mailto:toto@toto.com...re</a>', '<span id="cloak__HASH__">JLIB...>\n                            ')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<a href='mailto:toto@toto.com?cc=joe'>Click Here</a>'
+'<a href='mailto:toto@toto.com?cc=joe@nowhere13.com&bcc=bobo@toto.com&subject=Big%20News'>Click Here</a>'

tests/unit/suites/plugins/content/emailcloak/PlgContentEmailcloakTest.php:420

Looking at the test cases, we only cover links with single email addresses in them, there's no coverage of links with multiple email addresses and given this test fails it probably means the cloaking mechanism was never designed to work in the first place with something that has multiple addresses.

brianteeman commented 5 years ago

Also see https://github.com/joomla/joomla-cms/issues/19343

mbabker commented 5 years ago

Similar bug, completely unrelated code paths (the email cloaking plugin doesn't invoke any of the methods mentioned in the other issue).

If I had to take a guess though, this line and this line basically confirm that our API only processes single email addresses in these helpers and that we're going to have to get smarter about processing strings with multiple addresses (in this case, the code is running Joomla\CMS\String\PunycodeHelper::emailToUtf8('<a href='mailto:toto@toto.com?cc=joe@nowhere13.com&bcc=bobo@toto.com&subject=Big%20News'>Click Here</a>') and we see how that's turning out).

brianteeman commented 1 year ago

Please retag as J4 Issue