craftcms / cms

Build bespoke content experiences with Craft.
https://craftcms.com
Other
3.2k stars 617 forks source link

Link not visible in system emails #5155

Closed FreekVR closed 3 months ago

FreekVR commented 4 years ago

Description

Related to #3495

System emails are sent with invalid HTML, resulting in (for example) the activation link not being visible to end users.

Output:

<body>
    <div style=3D"max-width: 500px; font-size: 13px; line-height: 18px; fon=
t-family: HelveticaNeue, sans-serif;">
        <p>Hey xxx,</p>
<p>To reset your xxx CMS password, click on this link:</p>
<p><xxx/admin/actions/users/set=
-password?code=3DFvO5Gxb5M-auoqEZ85rZUM6DTviFytwN&id=3Df20b2348-779d-41e6-8=
056-721efe0b568e></p>
<p>If you were not expecting this email, just ignore it.</p>

    </div>
</body>
</html>

Steps to reproduce

  1. Install craft
  2. Configure mail as follows (excerpt from our project.yaml config)
    email:
    fromEmail: $SMTP_MAILFROM
    fromName: 'xxx CMS'
    template: ''
    transportType: craft\mail\transportadapters\Smtp
    transportSettings:
    host: $SMTP_HOST
    port: $SMTP_PORT
    useAuthentication: ''
    username: ''
    password: ''
    encryptionMethod: ''
    timeout: '10'
  3. Enable craft headless mode (not sure if that affects this issue) -- for reference, our full general.php config:
    
    <?php
    /**
    * General Configuration
    *
    * All of your system's general configuration settings go in here. You can see a
    * list of the available settings in vendor/craftcms/cms/src/config/GeneralConfig.php.
    */

return [ // Global settings '*' => [ // Default Week Start Day (0 = Sunday, 1 = Monday...) 'defaultWeekStartDay' => 1,

    // Enable CSRF Protection (recommended, will be enabled by default in Craft 3)
    'enableCsrfProtection' => true,

    // Whether "index.php" should be visible in URLs
    'omitScriptNameInUrls' => true,

    // Control Panel trigger word
    'cpTrigger' => 'admin',

    // The secure key Craft will use for hashing and encrypting data
    'securityKey' => getenv('SECURITY_KEY'),

    'useProjectConfigFile' => true,
    'allowAdminChanges' => false,
    'allowUpdates' => false,
    'preventUserEnumeration' => true,
    'runQueueAutomatically' => false,
    'sendPoweredByHeader' => false,
    'useFileLocks' => true,
    'useSecureCookies' => true,
    'useSslOnTokenizedUrls' => true,
    'headlessMode' => true
],

// Dev environment settings
'dev' => [
    // Dev Mode (see https://craftcms.com/support/dev-mode)
    'devMode' => true,
    'allowAdminChanges' => true,
    'allowUpdates' => true,
    'runQueueAutomatically' => true
],

// Staging environment settings
'staging' => [
    // Base site URL
    'siteUrl' => null,
],

// Production environment settings
'production' => [
    // Base site URL
    'siteUrl' => null,
],

];

5. Configure `.env` variables to establish a working connection with SMTP
6. Request a password reset

### Additional info

- Craft version: 3.3.10
- PHP version: 7.2
- Database driver & version: MySQL 5.7
- Plugins & versions: 

bugsnag: edition: standard enabled: true schemaVersion: 2.0.0 schema: edition: standard enabled: true schemaVersion: 1.0.0 redactor: schemaVersion: 2.3.0 enabled: true super-table: edition: standard enabled: true schemaVersion: 2.2.0 seo: edition: standard enabled: true schemaVersion: 3.1.0

brandonkelly commented 4 years ago

The default email template does contain an <html> tag:

https://github.com/craftcms/cms/blob/6bcc6538dcabfa2601b851a461b055efc779015c/src/templates/_special/email.html#L1

Is that getting stripped out, or did you just forget to copy it?

What email client/service are you using?

FreekVR commented 4 years ago

Oh yea, I just omitted it from my quick copy paste :) It is there in the output in the email itself.

I've only tested this in gmail and apple mail. The problem appears to be that <https://somelink.com> is valid markdown but not a valid HTML element.

I would assume that normally this text would be converted to https://somelink.com or https://somelink.com.

Could it be that headless mode affects the rendering of the mail templates somehow? I did notice that it changes the escaping mode in Twig.

Here's the full plain text + HTML output as reported by gmail.

--_=_swift_1571823845_0b32110b62ea2712831b05522ac24d4a_=_
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

Hey admin,

To reset your xxx CMS password, click on this link:

<https://cms.xxx.nl/admin/actions/users/set-pa=
ssword?code=3Dscgs7bFQM9Aya6XUNXE6QZn2RbE3wBmA&id=3Dbd74d610-db1c-43a1-92ae=
-eb0cdc49fd92>

If you were not expecting this email, just ignore it.

--_=_swift_1571823845_0b32110b62ea2712831b05522ac24d4a_=_
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: quoted-printable

<html>
<body>
    <div style=3D"max-width: 500px; font-size: 13px; line-height: 18px; fon=
t-family: HelveticaNeue, sans-serif;">
        <p>Hey admin,</p>
<p>To reset your xxx CMS password, click on this link:</p>
<p><https://cms.xxx.nl/admin/actions/users/set=
-password?code=3Dscgs7bFQM9Aya6XUNXE6QZn2RbE3wBmA&id=3Dbd74d610-db1c-43a1-9=
2ae-eb0cdc49fd92></p>
<p>If you were not expecting this email, just ignore it.</p>

    </div>
</body>
</html>

--_=_swift_1571823845_0b32110b62ea2712831b05522ac24d4a_=_--
brandonkelly commented 4 years ago

So this is what the plain text body of that email was:

Hey admin,

To reset your xxx CMS password, click on this link:

<https://cms.xxx.nl/admin/actions/users/set-password?code=3Dscgs7bFQM9Aya6XUNXE6QZn2RbE3wBmA&id=3Dbd74d610-db1c-43a1-92ae-eb0cdc49fd92>

If you were not expecting this email, just ignore it.

Which looks good. Then Craft takes that and runs it through [yii\helpers\Markdown::process()](https://www.yiiframework.com/doc/api/2.0/yii-helpers-basemarkdown#process()-detail) to create the body of the HTML email:

https://github.com/craftcms/cms/blob/6bcc6538dcabfa2601b851a461b055efc779015c/src/mail/Mailer.php#L133

When I try that locally, process() returns the following:

<p>Hey admin,</p>
<p>To reset your xxx CMS password, click on this link:</p>
<p><a href="https://cms.xxx.nl/admin/actions/users/set-password?code=3Dscgs7bFQM9Aya6XUNXE6QZn2RbE3wBmA&amp;id=3Dbd74d610-db1c-43a1-92ae-eb0cdc49fd92">https://cms.xxx.nl/admin/actions/users/set-password?code=3Dscgs7bFQM9Aya6XUNXE6QZn2RbE3wBmA&amp;id=3Dbd74d610-db1c-43a1-92ae-eb0cdc49fd92</a></p>
<p>If you were not expecting this email, just ignore it.</p>

As you can see, <https://cms.xxx.nl/...> was correctly converted to an <a> tag.

So the question is why isn’t that working for you. (It’s not anything to do with Headless Mode; I verified emails get sent with properly-converted anchor tags even when Headless Mode is enabled.)

brandonkelly commented 4 years ago

If you’re able to reproduce this behavior every time an email is sent, maybe send some server credentials over to support@craftcms.com and we can help you look into it from there.

FreekVR commented 4 years ago

Debugging this locally, the conversion from markdown to <a href""> fails only when the URL is not valid. Eg. <google.com> is left as-is, while <https://google.com> is converted to a link.

Still, the URLs sent by our staging server, where the issue occurs, should be correct, so I'm still investigating where the difference is occuring.

brandonkelly commented 4 years ago

Are you setting your site URL with an environment variable or alias? In that case normal URL validation wouldn’t apply, so you’d be able to get away with just saving google.com when it should have been https://google.com.

FreekVR commented 4 years ago

I did a little debugging on the server where the issue occurs. So far it's been pretty hard to track down. As far as I can determine it should call the parseLt of the LinkTrait of the cebe/Markdown library, but for some reason, it isn't. I'll look a bit further to see where the issue first occurs.

brandonkelly commented 4 years ago

I think the issue is that somehow the URL getting passed to the email is missing http:// or https://. Some things to check:

FreekVR commented 4 years ago

I verified in the email source that the URL itself should be parsed as far as I can determine:

<https://cms.myclient.acc.ceres.digitalnatives.nl/admin/actions/users/set-password?code=NcgV7exvLFxPmC4PMna2y4e4Yy9QMmQM&id=01c61de7-c1a8-4303-9f65-81eaa10f14ba>

What really baffles me is that this issue is not occurring in my dev environment. So I'm now checking if I can set up some isolated test to actually verify the (cause of the) difference more easily and track that down.

FreekVR commented 4 years ago

Okay, so I've finally reproduced AND found the cause of the issue.

In the cebe/markdown package which Yii (and Craft) use to parse markdown, the following is listed in the README under the install instructions:

Note: If you have configured PHP with opcache you need to enable the opcache.save_comments option because inline element parsing relies on PHPdoc annotations to find declared elements.

Incidentally, opcache.save_comments was set to "0" on our staging environment, while it was enabled on our test environment (I think it's enabled by default, and requires an explicit disable).

As soon as I enabled the save_comments setting the issue went away!

As far as I'm concerned this issue could be closed as it is a pretty big edge-case, but it could be argued that this might be an interesting check for the "System report"-checks that Craft does? Especially since the install instructions for cebe/markdown aren't included in the Craft docs anywhere.

brandonkelly commented 4 years ago

Wow, good job getting to the bottom of it! Yeah, probably worth a check in the system report.

rauwebieten commented 2 years ago

Thank you FreekVR, your fix worked perfectly!

timkelty commented 3 months ago

This has been added to craftcms/server-check: https://github.com/craftcms/server-check/commit/445c8eccde570ede09dc96c39fc1357a5296520f