Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add encoding to TransportBuilder #24906

Conversation

elvinristi
Copy link
Contributor

@elvinristi elvinristi commented Oct 7, 2019

Description (*)

While having Magento 2.3.3 and when customer registers account while having in name non-ASCII chars then after form submit User doesn't receive email and log file var/log/exception.log included Exception with message Invalid header value detected

[2019-10-07 11:33:57] report.CRITICAL: Invalid header value detected {"exception":"[object] (Magento\\Framework\\Exception\\MailException(code: 0): Invalid header value detected at vendor/magento/module-email/Model/Transport.php:104, Zend\\Mail\\Header\\Exception\\RuntimeException(code: 0): Invalid header value detected at vendor/zendframework/zend-mail/src/Header/HeaderValue.php:112)"}

More details in #24902

Fixed Issues (if relevant)

#24902 M2.3.3 breaks email sending when name contains non-ASCII char

Manual testing scenarios (*)

  1. Open registration page
  2. Fill fields and include with name fields unicode chars, i.e. õöü

Questions or comments

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds are green)

@m2-assistant
Copy link

m2-assistant bot commented Oct 7, 2019

Hi @elvinristi. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento give me test instance - deploy test instance based on PR changes
  • @magento give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

@magento-engcom-team magento-engcom-team added Release Line: 2.3 Partner: Vaimo Pull Request is created by partner Vaimo partners-contribution Pull Request is created by Magento Partner labels Oct 7, 2019
@elvinristi
Copy link
Contributor Author

@magento give me test instance

@magento-engcom-team
Copy link
Contributor

Hi @elvinristi. Thank you for your request. I'm working on Magento instance for you

@magento-engcom-team
Copy link
Contributor

Hi @elvinristi, here is your new Magento instance.
Admin access: https://pr-24906.instances.magento-community.engineering/admin
Login: admin Password: 123123q

@elvinristi
Copy link
Contributor Author

Verified in test instance that when I including non-ASCII chars in Sender Name-fields under Store Email Addresses

and trying to subscribe to Newsletter from the footer then message Thank you for your subscription. was shown to frontend. Previously it failed in #24902 (comment)

Sender Name settings

@Karlasa Karlasa closed this Oct 7, 2019
@m2-assistant
Copy link

m2-assistant bot commented Oct 7, 2019

Hi @elvinristi, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@Karlasa Karlasa reopened this Oct 7, 2019
@VladimirZaets VladimirZaets added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Oct 7, 2019
@magento-engcom-team
Copy link
Contributor

Hi @VladimirZaets, thank you for the review.
ENGCOM-6034 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

@elvinristi thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@engcom-Alfa
Copy link
Contributor

✔️ QA Passed

@elfeffe
Copy link
Contributor

elfeffe commented Nov 4, 2019

I use latest Magento, 2.3.3, with the email patch EmailMessageInterface_2.3.3_backward_compatibility_module-email.patch
I also use PHP 7.2
If I test an email using an accent like é in the address, I still get invalid headers
My prepareMessage function looks like

protected function prepareMessage()
   {
       $template = $this->getTemplate();
       $content = $template->processTemplate();
       switch ($template->getType()) {
           case TemplateTypesInterface::TYPE_TEXT:
               $part['type'] = MimeInterface::TYPE_TEXT;
               break;

           case TemplateTypesInterface::TYPE_HTML:
               $part['type'] = MimeInterface::TYPE_HTML;
               break;

           default:
               throw new LocalizedException(
                   new Phrase('Unknown template type')
               );
       }
       $mimePart = $this->mimePartInterfaceFactory->create(['content' => $content]);

       $this->messageData['encoding'] = $mimePart->getCharset();
       $this->messageData['body'] = $this->mimeMessageInterfaceFactory->create(
           ['parts' => [$mimePart]]
       );

       $this->messageData['subject'] = html_entity_decode(
           (string)$template->getSubject(),
           ENT_QUOTES
       );

       $this->message = $this->emailMessageInterfaceFactory->create($this->messageData);

       return $this;
   }

@elvinristi Any idea about how to fix it?
I have applied the changes here 95878b7
But I still don't send an email with an é in the name

@grll
Copy link

grll commented Nov 15, 2019

@elfeffe got the same issue did you find a fix ?

@elfeffe
Copy link
Contributor

elfeffe commented Nov 16, 2019

@grll no, my Magento store is not sending emails when the customs has an accent on his name, this is terrible.
I have tried all the patches, nothing works.

@wilhelmpa
Copy link

same here! What to do? Magento ist not usable at the moment

@Robertxc
Copy link

Any solution? Same issue here

@voytech-net
Copy link

Hello guys,

after all the patches and investigations, I have managed to fix the mailing issue for M2.3.3.
The issue is in the \Magento\Sales\Model\Order\Email\SenderBuilder where headers are being created with email and customer name. If customer name contains diacritics, it fails and the email is not sent.

I have overriden the class with di.xml to just extend the send() method and removed the customer name from the headers which solved the issue for me. I was not able to find out whether the name is needed or whether it enhances the email in another way. But for me the main reason for it is to get the emails working.

Here is the class in case some one wonders what is going on:

class SenderBuilder extends \Magento\Sales\Model\Order\Email\SenderBuilder
{
    /**
     * @throws \Magento\Framework\Exception\LocalizedException
     * @throws \Magento\Framework\Exception\MailException
     */
    public function send()
    {
        $this->configureEmailTemplate();
        //Removed Customer name because of diacritics
        $this->transportBuilder->addTo(
            $this->identityContainer->getCustomerEmail()
        );

        $copyTo = $this->identityContainer->getEmailCopyTo();
        if (!empty($copyTo) && $this->identityContainer->getCopyMethod() == 'bcc') {
            foreach ($copyTo as $email) {
                $this->transportBuilder->addBcc($email);
            }
        }

        $transport = $this->transportBuilder->getTransport();
        $transport->sendMessage();
    }
}

The $this->transportBuilder->addTo can take second argument (customer name) which by default is an empty string. I removed it and the mails started working again.

@lfehr
Copy link

lfehr commented Dec 4, 2019

Hi,
a probably better approach could be to change the following lines in vendor/magento/framework/Mail/EmailMessage.php (using a patch or overwrite it with di.xml)

    /**
     * Converts MailAddress array to AddressList
     *
     * @param Address[] $arrayList
     * @return AddressList
     */
    private function convertAddressArrayToAddressList(array $arrayList): AddressList
    {
        $zendAddressList = new AddressList();
        foreach ($arrayList as $address) {

            // use iconv for valid name
            $name = iconv('UTF-8', 'ASCII//TRANSLIT', $address->getName());
            $zendAddressList->add($address->getEmail(), $name);

        }

        return $zendAddressList;
    }

This also sends emails, if a user with diacritics registers to your shop or any other action that sends emails from magento.

Every name will be translated from utf-8 to ascii-charset: Jürgen Müller -> Juergen Mueller
You can also manipulate the name in any way you like at this place (i.e. delete it / set it to an empty string).

Hope this helps.

@un3us
Copy link

un3us commented Dec 6, 2019

Hi,
following plugin also solves the issue by encoding e-mail headers by preparation of public functions:

use Magento\Framework\Mail\Address;
use Magento\Framework\Mail\MimePart;
use Magento\Framework\Mail\Template\TransportBuilder as MageTransportBuilder;
use Zend\Mail\Header\HeaderWrap;

class TransportBuilder
{

    /**
     * @param MageTransportBuilder $subject
     * @param $address
     * @param string $name
     * @return array
     * @see \Magento\Framework\Mail\Template\TransportBuilder::addTo()
     */
    public function beforeAddTo(MageTransportBuilder $subject,$address, $name = '')
    {
        if (is_array($address)) {
            $this->encodeAdressArray($address);
        }
        return [$address,$this->encodeString($name)];
    }

    /**
     * @param MageTransportBuilder $subject
     * @param $address
     * @param string $name
     * @return array
     * @see \Magento\Framework\Mail\Template\TransportBuilder::addCc()
     */
    public function beforeAddCc(MageTransportBuilder $subject,$address, $name = '')
    {
        if (is_array($address)) {
            $this->encodeAdressArray($address);
        }
        return [$address,$this->encodeString($name)];
    }

    /**
     * @param MageTransportBuilder $subject
     * @param $email
     * @param null $name
     * @return array
     * @see \Magento\Framework\Mail\Template\TransportBuilder::setReplyTo()
     */
    public function beforeSetReplyTo(MageTransportBuilder $subject,$email, $name = null)
    {
        //\Magento\Framework\Mail\Template\TransportBuilder::setReplyTo
        if (is_array($email)) {
            $this->encodeAdressArray($email);
        }
        return [$email,$name !== null ? $this->encodeString($name) : $name];
    }

    /**
     * @param MageTransportBuilder $subject
     * @param $address
     * @return array
     * @see \Magento\Framework\Mail\Template\TransportBuilder::addBcc()
     */
    public function beforeAddBcc(MageTransportBuilder $subject,$address)
    {
        if (is_array($address)) {
            $this->encodeAdressArray($address);
        }
        return [$address];
    }

    /**
     * @param string $input
     * @return string
     */
    protected function encodeString(string $input)
    {
        return HeaderWrap::canBeEncoded($input) ? HeaderWrap::mimeEncodeValue($input,MimePart::CHARSET_UTF8) : $input;
    }

    /**
     * @param array $addresses
     */
    protected function encodeAdressArray(array $addresses)
    {
        foreach ($addresses as $address) {
            if ($address instanceof Address) {
                $name = $this->encodeString($address->getName());
                $mail = $address->getEmail();
                $address->__construct($mail,$name);
            }
        }
    }
}

Hope this helps!

@ivanweiler
Copy link

From what we experienced, it seems this patch solves encoding of utf8 symbols, let's say emails are sent if sender name is "Čšćžđ", but if sender name contains @<> or similar symbols, our server refuses to send email because of "Mail failure - malformed recipient address"

I'm not sure if this is a problem in Zend or Magento, but 2.3.3. completely broke email sending for us which is terrible.

So, as already mentioned by others, we also tried all official patches (EmailMessageInterface_2.3.3_backward_compatibility.patch + this PR) and it didn't help for 2.3.3.

@Vishrootways
Copy link

Hello,

@elvinristi I am also getting same issue with Magento 2.3.3 version. Contact us and order emails not getting send to customer as well admin. We have not added any non-ASCII chars to email sender and/or receiver detail.

NOTE: We have migrated data from Magento 1.9 to Magento 2.3

It would be good if you can suggest us what can be the issue into this case.

@IDonated
Copy link

Hi,
following plugin also solves the issue by encoding e-mail headers by preparation of public functions:

use Magento\Framework\Mail\Address;
use Magento\Framework\Mail\MimePart;
use Magento\Framework\Mail\Template\TransportBuilder as MageTransportBuilder;
use Zend\Mail\Header\HeaderWrap;

class TransportBuilder
{

    /**
     * @param MageTransportBuilder $subject
     * @param $address
     * @param string $name
     * @return array
     * @see \Magento\Framework\Mail\Template\TransportBuilder::addTo()
     */
    public function beforeAddTo(MageTransportBuilder $subject,$address, $name = '')
    {
        if (is_array($address)) {
            $this->encodeAdressArray($address);
        }
        return [$address,$this->encodeString($name)];
    }

    /**
     * @param MageTransportBuilder $subject
     * @param $address
     * @param string $name
     * @return array
     * @see \Magento\Framework\Mail\Template\TransportBuilder::addCc()
     */
    public function beforeAddCc(MageTransportBuilder $subject,$address, $name = '')
    {
        if (is_array($address)) {
            $this->encodeAdressArray($address);
        }
        return [$address,$this->encodeString($name)];
    }

    /**
     * @param MageTransportBuilder $subject
     * @param $email
     * @param null $name
     * @return array
     * @see \Magento\Framework\Mail\Template\TransportBuilder::setReplyTo()
     */
    public function beforeSetReplyTo(MageTransportBuilder $subject,$email, $name = null)
    {
        //\Magento\Framework\Mail\Template\TransportBuilder::setReplyTo
        if (is_array($email)) {
            $this->encodeAdressArray($email);
        }
        return [$email,$name !== null ? $this->encodeString($name) : $name];
    }

    /**
     * @param MageTransportBuilder $subject
     * @param $address
     * @return array
     * @see \Magento\Framework\Mail\Template\TransportBuilder::addBcc()
     */
    public function beforeAddBcc(MageTransportBuilder $subject,$address)
    {
        if (is_array($address)) {
            $this->encodeAdressArray($address);
        }
        return [$address];
    }

    /**
     * @param string $input
     * @return string
     */
    protected function encodeString(string $input)
    {
        return HeaderWrap::canBeEncoded($input) ? HeaderWrap::mimeEncodeValue($input,MimePart::CHARSET_UTF8) : $input;
    }

    /**
     * @param array $addresses
     */
    protected function encodeAdressArray(array $addresses)
    {
        foreach ($addresses as $address) {
            if ($address instanceof Address) {
                $name = $this->encodeString($address->getName());
                $mail = $address->getEmail();
                $address->__construct($mail,$name);
            }
        }
    }
}

Hope this helps!

would you mind telling me where to put this plugin?

@Vishrootways
Copy link

Vishrootways commented Dec 16, 2019

Issue found by me and it has been solved. The issue was, the template of contact us form was not there in Magento 2 as we have migrated data from Magento 1. Set contact form template to default and all emails are start sending.

@kunzi
Copy link

kunzi commented Dec 17, 2019

This did not work for us:

a probably better approach could be to change the following lines in vendor/magento/framework/Mail/EmailMessage.php (using a patch or overwrite it with di.xml)

    /**
     * Converts MailAddress array to AddressList
     *
     * @param Address[] $arrayList
     * @return AddressList
     */
    private function convertAddressArrayToAddressList(array $arrayList): AddressList
    {
        $zendAddressList = new AddressList();
        foreach ($arrayList as $address) {

            // use iconv for valid name
            $name = iconv('UTF-8', 'ASCII//TRANSLIT', $address->getName());
            $zendAddressList->add($address->getEmail(), $name);

        }

        return $zendAddressList;
    }

That only resulter these two errors to debug.log:

main.CRITICAL: Type Error occurred when creating object: Magento\Framework\Mail\EmailMessage, iconv() expects parameter 3 to be string, null given [] []
main.ERROR: Type Error occurred when creating object: Magento\Framework\Mail\EmailMessage [] []

But the following modification did work and order confirmations seem to be sending again even with special characters:

The issue is in the \Magento\Sales\Model\Order\Email\SenderBuilder where headers are being created with email and customer name. If customer name contains diacritics, it fails and the email is not sent.

I have overriden the class with di.xml to just extend the send() method and removed the customer name from the headers which solved the issue for me. I was not able to find out whether the name is needed or whether it enhances the email in another way. But for me the main reason for it is to get the emails working.

Here is the class in case some one wonders what is going on:

class SenderBuilder extends \Magento\Sales\Model\Order\Email\SenderBuilder
{
    /**
     * @throws \Magento\Framework\Exception\LocalizedException
     * @throws \Magento\Framework\Exception\MailException
     */
    public function send()
    {
        $this->configureEmailTemplate();
        //Removed Customer name because of diacritics
        $this->transportBuilder->addTo(
            $this->identityContainer->getCustomerEmail()
        );

        $copyTo = $this->identityContainer->getEmailCopyTo();
        if (!empty($copyTo) && $this->identityContainer->getCopyMethod() == 'bcc') {
            foreach ($copyTo as $email) {
                $this->transportBuilder->addBcc($email);
            }
        }

        $transport = $this->transportBuilder->getTransport();
        $transport->sendMessage();
    }
}

The $this->transportBuilder->addTo can take second argument (customer name) which by default is an empty string. I removed it and the mails started working again.

@kovinet
Copy link

kovinet commented Dec 26, 2019

Issue found by me and it has been solved. The issue was, the template of contact us form was not there in Magento 2 as we have migrated data from Magento 1. Set contact form template to default and all emails are start sending.

Could you elaborate little more on this? So this can be solved in admin interface? Does it only solve issue for contact us email or for all transactional emails? In my case all mails don't work if there is special character in it in Magento 2.3.3. (which is like half surnames in my country)

@Vishrootways
Copy link

@kovinet It solved issues with all emails.

@kovinet
Copy link

kovinet commented Dec 27, 2019

@Vishrootways by template you mean the one in Marketing > Email Templates?
I do have this template as it was migrated from Magento 1. Also, I don't see what could be connection with this template and for example new order email... it doesn't make sense?

@Vishrootways
Copy link

No, check under below locations, remove and type email manually for all emails under below locations.

  • Admin > STORES > Configuration > GENERAL > Contacts
  • Admin > STORES > Configuration > GENERAL > Store Email Addresses
  • Admin > STORES > Configuration > SALES > Sales Emails

@kovinet
Copy link

kovinet commented Dec 27, 2019

I see, TY. But in my case this was not not problem, but applying patch EmailMessageInterface backward compatibility issue patch for Magento 2.3.3 resolved the issue.

@oleksii-lisovyi
Copy link

oleksii-lisovyi commented Mar 6, 2020

The issue is reproducible on Magento 2.3.4 with customer name, that contains special characters like ö, á etc.
How to fix:

  1. Use latest version of "magepal/magento2-gmailsmtpapp":"2.7.0".
  2. Apply patch from commit magepal/magento2-gmail-smtp-app@6d75930

I confirm, that this fixes issue on top of Magento 2.3.4.

@elvinristi
Copy link
Contributor Author

@oleksii-lisovyi
This file Model/ZendMailTwo/Smtp.php doesn't exist in Magento. Is it while using with magepal/magento2-gmail-smtp-app or also without it?

When without magepal module then which file you faced issue?

@elvinristi elvinristi deleted the fix/issue-24902--email-headers-encoding branch March 6, 2020 09:49
@oleksii-lisovyi
Copy link

@elvinristi, yes, the issue is actually connected with using of module magepal/magento2-gmailsmtpapp, where the model Model/ZendMailTwo/Smtp.php comes from.

I suppose, that some people can't recognize the difference between vanilla Magento and using 3rd party module, but the definitely are complained about the real issue causes their websites.

@elvinristi
Copy link
Contributor Author

@oleksii-lisovyi , ok, thanks for the heads up!

@gaiterjones
Copy link

Confirm this plugin fixes this issue in 2.3.3. If you have set mail delivery to async all previously failed emails will deliver after the plugin is activated.

Hi,
following plugin also solves the issue by encoding e-mail headers by preparation of public functions:

use Magento\Framework\Mail\Address;
use Magento\Framework\Mail\MimePart;
use Magento\Framework\Mail\Template\TransportBuilder as MageTransportBuilder;
use Zend\Mail\Header\HeaderWrap;

class TransportBuilder
{

    /**
     * @param MageTransportBuilder $subject
     * @param $address
     * @param string $name
     * @return array
     * @see \Magento\Framework\Mail\Template\TransportBuilder::addTo()
     */
    public function beforeAddTo(MageTransportBuilder $subject,$address, $name = '')
    {
        if (is_array($address)) {
            $this->encodeAdressArray($address);
        }
        return [$address,$this->encodeString($name)];
    }

    /**
     * @param MageTransportBuilder $subject
     * @param $address
     * @param string $name
     * @return array
     * @see \Magento\Framework\Mail\Template\TransportBuilder::addCc()
     */
    public function beforeAddCc(MageTransportBuilder $subject,$address, $name = '')
    {
        if (is_array($address)) {
            $this->encodeAdressArray($address);
        }
        return [$address,$this->encodeString($name)];
    }

    /**
     * @param MageTransportBuilder $subject
     * @param $email
     * @param null $name
     * @return array
     * @see \Magento\Framework\Mail\Template\TransportBuilder::setReplyTo()
     */
    public function beforeSetReplyTo(MageTransportBuilder $subject,$email, $name = null)
    {
        //\Magento\Framework\Mail\Template\TransportBuilder::setReplyTo
        if (is_array($email)) {
            $this->encodeAdressArray($email);
        }
        return [$email,$name !== null ? $this->encodeString($name) : $name];
    }

    /**
     * @param MageTransportBuilder $subject
     * @param $address
     * @return array
     * @see \Magento\Framework\Mail\Template\TransportBuilder::addBcc()
     */
    public function beforeAddBcc(MageTransportBuilder $subject,$address)
    {
        if (is_array($address)) {
            $this->encodeAdressArray($address);
        }
        return [$address];
    }

    /**
     * @param string $input
     * @return string
     */
    protected function encodeString(string $input)
    {
        return HeaderWrap::canBeEncoded($input) ? HeaderWrap::mimeEncodeValue($input,MimePart::CHARSET_UTF8) : $input;
    }

    /**
     * @param array $addresses
     */
    protected function encodeAdressArray(array $addresses)
    {
        foreach ($addresses as $address) {
            if ($address instanceof Address) {
                $name = $this->encodeString($address->getName());
                $mail = $address->getEmail();
                $address->__construct($mail,$name);
            }
        }
    }
}

Hope this helps!

@Stefan0x
Copy link

Stefan0x commented Apr 2, 2020

Sorry, but I still don't understand how I can fix this issue in 2.3.3.
A simple umlaut in a customers name prevents the confirmation email to be sent.
I think this is a very critical error.

The patch mentioned by @kovinet does not help and I don't have the gmailsmtpapp module installed.

@elvinristi
Copy link
Contributor Author

elvinristi commented Apr 2, 2020

@euchenhofer this ticket is only one part. Until you haven't applied official patch EmailMessageInterface_2.3.3_backward_compatibility_composer-2019-10-14-03-43-31.patch mentioned in https://community.magento.com/t5/Magento-DevBlog/Backward-incompatible-Changes-in-the-Mail-Library-for-Magento-2/ba-p/144787 you will have issues anyway.

I think this is included also in M2.3.3-patch1 release.

@gaiterjones
Copy link

@elvinristi the patch is only relevant if you are using third party modules to send email.

@euchenhofer the plugin code posted above does fix this problem, I have verified that and it is running in our production store. If you are unable to implement the plugin yourself in a module then perhaps we can create a module for you.

@elvinristi
Copy link
Contributor Author

@gaiterjones if you mean patch EmailMessageInterface_2.3.3_backward_compatibility_composer-2019-10-14-03-43-31.patch then this is relevant for all and not related to 3rd party module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests Award: bug fix Component: Mail Partner: Vaimo Pull Request is created by partner Vaimo partners-contribution Pull Request is created by Magento Partner Progress: accept Release Line: 2.3 Squashtoberfest 2019
Projects
None yet
Development

Successfully merging this pull request may close these issues.