HTML in Drupal core emails

Hi @jwilson3

The problem is when formatting the emails from Swiftmailer module:

  /**
   * Massages the message body into the format expected for rendering.
   *
   * @param array $message
   *   The message.
   *
   * @return array
   */
  public function massageMessageBody(array $message) {
    // Get default mail line endings and merge all lines in the e-mail body
    // separated by the mail line endings. Keep Markup objects and escape others
    // and then treat the result as safe markup.
    $line_endings = Settings::get('mail_line_endings', PHP_EOL);
    $message['body'] = Markup::create(implode($line_endings, array_map(function ($body) {
      // If $item is not marked safe then it will be escaped.
      return $body instanceof MarkupInterface ? $body : Html::escape($body);
    }, $message['body'])));
    return $message;
  }

This line:

 return $body instanceof MarkupInterface ? $body : Html::escape($body);

By default if you add HTML for example to reset_password email from the UI, the $body will be a string and that's the problem. IMHO makes sense to escape but we cannot add html to the body for the recovery password emails and others.

Would it be out of scope for this module to add some handling to core emails (eg, attach a format dropdown to the form, or similar)? I'm not remembering offhand how this was previously handled in D7...

Hi,

for me, I have partial HTML in the new user welcome email translations,

Ciao <b>[user:display-name]</b>,<br/>
<p>
Ti ringraziamo per la registrazione su [site:name]. Puoi accedere al sito cliccando il link seguente o copiandolo ed incollandolo nella barra degli indirizzi del tuo browser:
<a href="[user:one-time-login-url]">[user:one-time-login-url]</a>
</p>
<p>
Questo link può essere utilizzato una sola volta e ti dirigerà ad una pagina dove potrai impostare la tua password.<br />
Dopo aver impostato la tua password potrai accedere al sito all'indirizzo [site:login-url] usando:<br />
nome utente: <b>[user:display-name]</b><br />
password: la tua password<br />
</p>

I develop the following change to check also partial html (not instanceof MarkupInterface) and now email is correctly html formatted in function

\Drupal\swiftmailer\Plugin\Mail\SwiftMailer::massageMessageBody

      // If $item is not marked safe then it will be escaped.
      return $this->isHTML($body) ? $body : Html::escape($body);
    }, $message['body'])));
    return $message;
  }

  /**
   * Function to check also partial HTML
   *
   * @param string $text to check if is HTML
   *
   * @return bool TRUE if text is HTML
   *
   * @author Maurizio Pianfetti<maurizio.pianfetti@occhioinformatico.it>
   */
    function isHTML($text)
    {
      if($text instanceOf MarkupInterface)
      {
        return true;
      }
      if($text instanceOf MarkupInterface)
      {
        return true;
      }
      return (strlen(strip_tags($text)) !== strlen($text));
    }

Certainly I think there is a best way of mine to do this work.
What do you think ?
I'm not be able to do the overrite of Plugin instead to change your code, could you help me, please ?

Thank you,
Best Regards.
Bye Boymix81.

Hi,

after your last release of module (version 8.x-1.0-beta1 2017-Feb-16) the update overrides my modify and I had new wrong email.

Now I find like to override your plugin but for me your module non work well if I have a translation of email in HTML, like tell in my last post.

I use the following code.

namespace Drupal\mynick_custommod\Plugin\Mail;

use Drupal\Component\Render\MarkupInterface;
use Drupal\Component\Utility\Html;
use Drupal\Core\Render\Markup;
use Drupal\Core\Site\Settings;
use Drupal\swiftmailer\Plugin\Mail\SwiftMailer;

/**
 * Provides a 'MyNickName Mailer' plugin to send emails.
 *
 * @Mail(
 *   id = "mynick_custommod",
 *   label = @Translation("MyNickName Mailer"),
 *   description = @Translation("MyNickName Mailer Plugin.")
 * )
 */

class SwiftMailerWithPartialHTML extends SwiftMailer
{
    /**
   * Massages the message body into the format expected for rendering.
   *
   * @param array $message
   *   The message.
   *
   * @return array
   */
  public function massageMessageBody(array $message) {
    // Get default mail line endings and merge all lines in the e-mail body
    // separated by the mail line endings. Keep Markup objects and escape others
    // and then treat the result as safe markup.
    $line_endings = Settings::get('mail_line_endings', PHP_EOL);
    $message['body'] = Markup::create(implode($line_endings, array_map(function ($body) {
      // If $item is not marked safe then it will be escaped.
      return $this->isHTML($body) ? $body : Html::escape($body);
    }, $message['body'])));
    return $message;
  }
	
  /**
   * Function to check also partial HTML
   * 
   * @param string $text to check if is HTML
   * 
   * @return bool TRUE if text is HTML
   * 
   * @author Maurizio Pianfetti<maurizio.pianfetti@occhioinformatico.it>
   */
    function isHTML($text)
    {
      if($text instanceOf MarkupInterface)
      {
        return true;
      }  
      return (strlen(strip_tags($text)) !== strlen($text));
    }
}

and I select my plugin in the Mail System configuration.
I attach the email example.

I hope you fix this problem so I not must to do an override Plugin.

Best Regards.
Boymix81.

@Boymix81, thanks for sharing. Your code works great with HTML format selected, otherwise it wasn't working. Cheers for sharing! :)

I'm using this to force HTML output on whatever was inputted as body for the mail: (works great for user mails)

namespace Drupal\yourModule\Plugin\Mail;

use Drupal\Core\Render\Markup;
use Drupal\Core\Site\Settings;
use Drupal\swiftmailer\Plugin\Mail\SwiftMailer;

/**
 * Provides a 'Forced HTML SwiftMailer' plugin to send emails.
 *
 * @Mail(
 *   id = "swiftmailer_forced_html",
 *   label = @Translation("SwiftMailer Forced HTML"),
 *   description = @Translation("Forces the given body text to be interpreted as HTML.")
 * )
 */

class SwiftMailerForcedHTML extends SwiftMailer {

  /**
   * Massages the message body into the format expected for rendering.
   *
   * @param array $message
   *   The message.
   *
   * @return array
   */
  public function massageMessageBody(array $message) {
    // @see: SwiftMailer::massageMessageBody()
    $line_endings = Settings::get('mail_line_endings', PHP_EOL);
    $message['body'] = Markup::create(implode($line_endings, array_map(function ($body) {
      // If the field contains no html tags we can assume newlines will need be converted to <br>
      if (strlen(strip_tags($body)) === strlen($body)) {
        $body = str_replace("\r", '', $body);
        $body = str_replace("\n", '<br>', $body);
      }
      return check_markup($body, 'full_html');
    }, $message['body'])));
    return $message;
  }

}

Bumping this to "major" because it basically breaks core email handling.

#15 works perfectly for me.

  $line_endings = \Drupal\Core\Site\Settings::get('mail_line_endings', PHP_EOL);
  $message['body'] = array_map(function ($body) {
    if (strlen(strip_tags($body)) === strlen($body)) {
      $body = str_replace("\n", '<br>', $body);
    }
    return $body instanceof MarkupInterface ? $body : Markup::create($body);
  }, $message['body']);
  $message['headers']['Content-Type'] = SWIFTMAILER_FORMAT_HTML;

on a hook_mail_alter() will also help

In swiftmailer.html.twig, I used:
{{ body|nl2br }}

It does the job without the need of a hook or a plugin class.

The attached approach uses the fallback_filter for non-HTML. That filter escapes the markup as long someone hasn't intentionally mucked up their site's fallback filter. The added benefit, is it also converts new lines to brs and converts urls to links. All things I'd want to see in emails.

And here are two screenshots of what an HTML message looks like (its using commerce's order template, which is html markup from a twig template). And here's an example email from the core user welcome message.

+++ b/src/Plugin/Mail/SwiftMailer.php
@@ -589,6 +589,17 @@ public function massageMessageBody(array $message) {
+      if (strlen(strip_tags($body)) === strlen($body)) {

I wonder if we need to do a check against getApplicableFormat() and confirm we are supposed to send HTML too?

Doing that check. Now we only convert to BRs if someone asks for HTML formatted emails.

The patch looks great and the result even more. Converting Plain Text E-Mails to HTML is a great feature. I wonder if should make the input format configurable? People could add a dedicated format for processing emails.

re #24: I'm +1 for that. What do you think about leaving it a non-UI configurable thing for advanced use cases?

Yeah, its fine for me. We can expose it later if we need it.

I would like to see some test coverage for this feature, this important to catch bc breaks later.

+++ b/src/Plugin/Mail/SwiftMailer.php
@@ -588,7 +588,21 @@ public function massageMessageBody(array $message) {
+      if ($applicable_format == SWIFTMAILER_FORMAT_HTML && strlen(strip_tags($body)) === strlen($body)) {

Lets use \Drupal\Component\Utility\Unicode::strlen instead of the plain strlen.

What type of test coverage are you looking for? Unit, Kernel, BTB?

Or a test of the update hook?

Quick summary of a chat in Slack.

Add a kernel test (with data provider of html & without html). And only test massageMessageBody.

Here's a test as requested.

I missed #25 and was wondering where the option was in the UI :) I'd support adding a select widget to the message config for this.

I think this is RTBC, but we should lower the update hook number. Swiftmailer is currently in version 1.x. It should be something like 8100?

It should indeed be 8101 according to the documentation.

Thanks to all!

Hi there
Sorry for posting in a closed issue.

I got this error:
"Notice: Undefined index: filter_format in Drupal\swiftmailer\Plugin\Mail\SwiftMailer->massageMessageBody() (line 592 of modules/contrib/swiftmailer/src/Plugin/Mail/SwiftMailer.php)."

After I applied the patch.

So, I am wondering if that line (592) should be rewritten as:

$filter_format = isset($this->config['message']['filter_format']) ? $this->config['message']['filter_format'] : 'plain_text';

??

Thanks

EDIT:
Nevermind, I realized I needed to run 'drush updb'
Sorry for the false alarm.

It seems that this issue has introduced a bug. See #2948607: Notice: Undefined index: filter_format.

Same here, got the error but drush updb resolved it ;)

The config setting added in #25 is causing confusion and security risk - see #3016912: Add a setting to apply a HTML filter and my response in #8. Setting anything other than plain text might be a security bug.

See https://api.drupal.org/api/drupal/core%21modules%21filter%21filter.modul... which says

Note: Because filters can inject JavaScript or execute PHP code, security is vital here. When a user supplies a text format, you should validate it using $format->access() before accepting/using it. This is normally done in the validation stage of the Form API. You should for example never make a preview of content in a disallowed format.

We are not doing any validation, and cannot - it is impossible to know which user created the content that is being sent in this email. Therefore to use any format other than the fallback format is breaking the rule quoted above.

Now I think that the intention of #24-26 was that people would set the config value to another format that is guaranteed safe. However people aren't necessarily going to know that.

In fact this patch is hard-coded to use the plain-text format and a site could have created a different format to be the default one then deleted plain-text. Presumably the correct way to get the default format is to call filter_default_format()?

So perhaps we should remove the config setting and replace it with a call to filter_default_format()? I guess there could be a hook to allow altering it??

I have raised #3122389: Bugs with format conversion

OK I have analysed this carefully and I have a proposal. The code here works for many people most of the time - or to put it another way it is unreliable and causes problems in some cases.

I have an alternative plan that I believe works for everyone all the time. Hence in the new 2.x branch I propose we back this commit out, and I'm setting needs work as the closest status I can see to try to indicate this.

The comments here include some prominent members of the Drupal community so please drop over to #3122389: Bugs with format conversion and tell me what you think.

OK Setting this one back to fixed. Obviously it was checked-in and solved a practical problem.

Moving forward:
#3122389: Bugs with format conversion will fix some edge cases, mostly HTML messages that look like plain text and vice versa
#3125042: [META] New & improved format conversions asks the wider question whether there might be a better way to solve the problem