Hi,

I speak in french and then my website is translate in french.

I use user mail workflow to send email to my user (Creation account, ...). With swiftmailer plugin I have a problem because you define in "massageMessageBody" that the text need to be an instance of MarkupInterface to avoid going through the escape function.

User mail define by drupal is simply a text plain and then you pass by HTML::escape. It's a problem because you have single quote encoding. And then when you recieve an email after user creation, I have single quote replace by &...;

I have define plain text on my configuration. Then I think it's not a right choice to use escape function on a body that don't need to do it.

I have try do create a patch. I had test to define if body (not safe) contains html or not. Because if the text is send as plain text and don't have html in his body. You don't need to escape the body part.

PS: Sry for my english

CommentFileSizeAuthor
#7 2937293-7.patch988 byteswebflo
#4 swiftmailer-text_plain_without_escape-2937293-4-8.x-1.x.patch672 bytesid.rem.dev
#4 interdiff.txt609 bytesid.rem.dev
#2 swiftmailer-text_plain_without_escape-2937293-2-8.x-1.x.patch670 bytesisaacrc
Text_plain_without_escape.patch924 bytesnoita
mail_02.png25.07 KBnoita
mail_01.png25.78 KBnoita
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

noita created an issue. See original summary.

isaacrc’s picture

isaacrc CreditAttribution: isaacrc commented
FileSize
swiftmailer-text_plain_without_escape-2937293-2-8.x-1.x.patch670 bytes

Update for the latest version.

noita’s picture

noita CreditAttribution: noita as a volunteer commented
Priority: Major » Critical

Someone ?

id.rem.dev’s picture

id.rem.dev CreditAttribution: id.rem.dev at Internetdevels, Drupal Ukraine Community commented
Status: Active » Needs review
FileSize
interdiff.txt609 bytes
swiftmailer-text_plain_without_escape-2937293-4-8.x-1.x.patch672 bytes

Improved patch a little bit.
See http://php.net/manual/en/language.operators.comparison.php#example-104
Please check it.

noita’s picture

noita CreditAttribution: noita as a volunteer commented

Not necessary but if you prefer...

benoit.condaminet’s picture

benoit.condaminet CreditAttribution: benoit.condaminet at Niji commented

Hello,

I think if format is not html, we should not HTML::escape message body at all (even if <> are found). Because in dev for exemple, we could have twig template debug markup added to the mail. Plain text version of mail should not be rendered by mail client (or it's a big issue for them).

webflo’s picture

webflo CreditAttribution: webflo at UEBERBIT GmbH commented
FileSize
2937293-7.patch988 bytes

Hi, could you try this patch? It uses MailFormatHelper::htmlToText. This is same them logic as in Drupals default mail plugin.

TwoD’s picture

TwoD
Primary language Swedish
Location Sweden
CreditAttribution: TwoD at Websystem for PwC commented
Status: Needs review » Reviewed & tested by the community

This looks good and helped us fix issues both with Core's plaintext emails and our custom emails sent as plain text.

The last submitted patch, 2: swiftmailer-text_plain_without_escape-2937293-2-8.x-1.x.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

joep.hendrix’s picture

joep.hendrix CreditAttribution: joep.hendrix at CompuBase Internet Solutions commented

#7 works for me.

Thank you.

  • webflo committed d21ad59 on 8.x-1.x 
    Issue #2937293 by noita, Vladimirrem, webflo, isaacrc: Fix encoding of...
webflo’s picture

webflo CreditAttribution: webflo at UEBERBIT GmbH commented
Status: Reviewed & tested by the community » Fixed

Thanks for testing this patch.

  • webflo committed 4d63854 on 8.x-2.x 
    Issue #2937293 by noita, Vladimirrem, webflo, isaacrc: Fix encoding of...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

AdamPS’s picture

AdamPS CreditAttribution: AdamPS at AlbanyWeb commented
Status: Closed (fixed) » Needs work
Issue tags: -mail encode

I think this patch might cause a significant problem. It added a call to MailFormatHelper::htmlToText() that runs even if the target format is HTML. I believe this will particularly affect people who have put HTML markup into their user account messages - a scenario that worked prior to this patch.

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.

AdamPS’s picture

AdamPS CreditAttribution: AdamPS at AlbanyWeb commented
Status: Needs work » Fixed

Sorry that wasn't the correct description of the problem.

Code before this patch Html::escape() is correct if output format is HTML.
Code after this patch MailFormatHelper::htmlToText()() is correct if output format is plain text.

What we need is an if statement that chooses between the two. I'll just close this again and we can fix it in #3122389: Bugs with format conversion.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.