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

Magento 2.3.3 notifications converted to attachments in MS Exchange - Disposition Header #25076

Closed
jaminion opened this issue Oct 15, 2019 · 55 comments · Fixed by #32720 · May be fixed by zendframework/zend-mail#251
Closed
Assignees
Labels
Component: Email Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: ready for confirmation Priority: P1 Once P0 defects have been fixed, a defect having this priority is the next candidate for fixing. Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: done Severity: S1 Affects critical data or functionality and forces users to employ a workaround. Triage: Ready for Triage Issue is ready to me triaged with Product Manager

Comments

@jaminion
Copy link

Preconditions (*)

  1. Magento version 2.3.3 (worked fine previously on 2.3.2)
  2. Outlook 2010 behind MS Exchange 2010 (don't have later version accessible for immediate testing with this project), or Outlook Web Access, have also confirmed with Mail clients on Mac, iPhone and Android that pull mail from this Exchange server
  3. Issue does not appear to affect rendering from Gmail/etc - seems to only be clients that are connecting to MS Exchange 2010 to retrieve the message

Steps to reproduce (*)

  1. Submit an order, send order notification or send shipment notification
  2. Receive email using a client/server combination similar to above noted configuration (Outlook/Exchange)

Expected result (*)

  1. Receive an email with the body of the message containing the rendered template for the order notification email

Actual result (*)

  1. Receive an email with a blank body, and the content included as an ATT*-labeled attachment to the email
  2. EmailContentAsAttachment

Notes

I understand that this Outlook/Exchange combination is fairly aged, but it's what I'm working with for at least the next couple of months.

I believe this is associated with the changes from #23643, putting the quoted-printable [back] into the emails.

From what I can determine, when a header is rendered, it's processed by a class implementing Zend\Mail\Header\HeaderInterface. There is no class registered for the header "Content-Disposition", so it ends up getting rendered through the Zend\Mail\Header\GenericHeader class. The getFieldValue() method looks like this:

    public function getFieldValue($format = HeaderInterface::FORMAT_RAW)
    {
        if (HeaderInterface::FORMAT_ENCODED === $format) {
            return HeaderWrap::wrap($this->fieldValue, $this);
        }

        return $this->fieldValue;
    }

When commenting out the if() block above, the email looks fine when it's received. What's happening is that the Content-Disposition is getting set to "inline", however when rendered it is being translated as quoted-printable and ends up with a value of =?utf-8?Q?inline?=, instead of inline. The MS Exchange 2010 involved appears to reject or not understand this disposition type and treats it as a generic attachment.

I don't believe that it's necessary/good to have the Content-Disposition header encoded like this. Maybe it is. I'm not completely familiar with the relevant RFCs and it's completely possible that this Exchange server is not handling it correctly. I will be attempting to find a workaround I can implement as a local plugin for now, but wanted to get this out in front of more experienced Magento developers in case the impact is greater than an old and just about out-of-support version of MS Exchange.

Thank you!

The 2.3.3 rollout has been working pretty well for us so far outside of this issue - all the hard work is much appreciated, you guys rock!

@m2-assistant
Copy link

m2-assistant bot commented Oct 15, 2019

Hi @jaminion. Thank you for your report.
To help us process this issue please make sure that you provided the following information:

  • Summary of the issue
  • Information on your environment
  • Steps to reproduce
  • Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.3-develop instance - upcoming 2.3.x release

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

@jaminion do you confirm that you were able to reproduce the issue on vanilla Magento instance following steps to reproduce?

  • yes
  • no

@magento-engcom-team magento-engcom-team added the Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed label Oct 15, 2019
@jaminion
Copy link
Author

@magento give me 2.3-develop instance

@magento-engcom-team
Copy link
Contributor

Hi @jaminion. Thank you for your request. I'm working on Magento 2.3-develop instance for you

@magento-engcom-team
Copy link
Contributor

Hi @jaminion, here is your Magento instance.
Admin access: https://i-25076-2-3-develop.instances.magento-community.engineering/admin
Login: admin Password: 123123q
Instance will be terminated in up to 3 hours.

@jaminion
Copy link
Author

I'm sorry - I set up quick on this develop instance, but it doesn't appear like email's going anywhere from it, I'm not receiving anything.

That is probably by design. It would be a spammer's delight if they could just keep asking for instances from you guys and send mail all day. I understand. I'm thinking through setting up a third party smtp somewhere - but this ends up being public credentials/access temporarily and someone could hijack that as well. So, I'm kind of stuck on the reproduction, as it's not something that you can generate directly from the Magento site. It's like trying to see if the dog ate your diamond bracelet - you can't inspect the dog so easily for that, you gotta wait for and inspect the poop instead.

If there happens to be some standard way to approach the notification-from-a-develop-instance problem, please let me know, I'm all ears. Otherwise, I'll leave this hanging out here, and tag it if I come up with something that might be more useful.

Thank you!

@davidverholen davidverholen self-assigned this Oct 16, 2019
@m2-assistant
Copy link

m2-assistant bot commented Oct 16, 2019

Hi @davidverholen. Thank you for working on this issue.
In order to make sure that issue has enough information and ready for development, please read and check the following instruction: 👇

  • 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).

    DetailsIf the issue has a valid description, the label Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: Format is valid appears.

  • 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add Issue: Clear Description label to the issue by yourself.

  • 3. Add Component: XXXXX label(s) to the ticket, indicating the components it may be related to.

  • 4. Verify that the issue is reproducible on 2.3-develop branch

    Details- Add the comment @magento give me 2.3-develop instance to deploy test instance on Magento infrastructure.
    - If the issue is reproducible on 2.3-develop branch, please, add the label Reproduced on 2.3.x.
    - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!

  • 5. Add label Issue: Confirmed once verification is complete.

  • 6. Make sure that automatic system confirms that report has been added to the backlog.

@davidverholen davidverholen added Component: Email Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Oct 16, 2019
@magento-engcom-team magento-engcom-team added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Oct 16, 2019
@magento-engcom-team
Copy link
Contributor

✅ Confirmed by @davidverholen
Thank you for verifying the issue. Based on the provided information internal tickets MC-21868 were created

Issue Available: @davidverholen, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

@jaminion
Copy link
Author

My for-this-project-immediately solution to this was to add a preference over the Magento\Email\Model\Transport class. The difficulty is that the Zend\Mail\Message instance is the one that's processing the headers, and it is generated using a static method from its class inside the sendMessage method, and used there - it's never accessible at a scope where it's targetable with DI, at least how I'm familiar. It's not so useful to use an around() plugin on that sendMessage() method because the object you'd need to work with, the MessageInterface, is private to that class instance and inaccessible.

I added a DispositionHeader class inheriting from the GenericHeader it would otherwise use and tweaked the getFieldValue() for that, and then adjusted the sendMessage() to pull a reference to the class loader with $zendMessage->getHeaders()->getPluginClassLoader(), and used the registerPlugin() method to add an entry for the disposition to the table. Just a crutch - my immediate use case doesn't handle attachments and filenames, and that's where things get fun - and more susceptible to user generated content.

zendframework/zendframework#4917 is interesting, was closed because nobody was fighting for it when they did a cleanup, but suggests this is kind of a lingering thing with ZF2.

Thank you again, very much, for taking an interest in this problem.

@Timothy-V
Copy link

I can confirm this issue as well for Outlook and Outlook Web App behind MS Exchange 2013!

@AndyJAllen
Copy link

AndyJAllen commented Oct 21, 2019

Also experiencing this after 2.3.3 upgrade.

I tested what jaminion suggested about commenting out the if statement and it worked for me. Are there any side effects of doing this?

@david-kominek
Copy link

Also experiencing, this is blocking our upgrade.

@verhaeghep
Copy link

Also experiencing since 2.3.3 upgrade. Not only in Outlook but also in Thunderbird with IMAP server.

@AndyJAllen
Copy link

AndyJAllen commented Oct 22, 2019

On a related note I am also experiencing an issue sending tracking emails from admin.
image
Click "Send Tracking Information" and it just displays the following:
image

Anyone else not able to send tracking emails? Edit: created a new thread for this: #25221

This update has been a disaster for me so far. It also broke my "Multiple Custom Forms" module.

@jaminion
Copy link
Author

@AndyJAllen we are not having that particular problem - did you by chance apply the "EmailMessageInterface backward compatibility issue patch"? If you haven't seen that, it might be worthwhile to review, although I have not applied that patch as I haven't found any affected code. A link to it:

https://magento.com/tech-resources/download#download2333

They changed the function signature (interface classes mainly) for the Transport class with this update, and didn't appear to anticipate the issues that would cause with third party code using it. Otherwise, in your case it really seems like that particular transactional email template has been wiped and is only displaying the rendered tracking information blocks, the Magento_Sales/templates/email/shipment/track.phtml template.

@AndyJAllen
Copy link

AndyJAllen commented Oct 22, 2019

@AndyJAllen we are not having that particular problem - did you by chance apply the "EmailMessageInterface backward compatibility issue patch"? If you haven't seen that, it might be worthwhile to review, although I have not applied that patch as I haven't found any affected code. A link to it:

https://magento.com/tech-resources/download#download2333

They changed the function signature (interface classes mainly) for the Transport class with this update, and didn't appear to anticipate the issues that would cause with third party code using it. Otherwise, in your case it really seems like that particular transactional email template has been wiped and is only displaying the rendered tracking information blocks, the Magento_Sales/templates/email/shipment/track.phtml template.

I had tried doing just the patch (which seemed a lot more confusing and complicated than it needs to be). I had put the patch in my root directory and ran the patch command, but it kept failing with errors. I gave up on it.

I then grabbed all the files I could find that were recently updated on github and applied them to my installation; which fixed my custom emailing module...however I still experience this problem where I can't use the "Send Tracking Information" button.... I use the Fedex module.

Thanks for the reply.

Edit: I think you put me on the right path. When I comment out the following line of code in track.phtml it works (albeit, without the tracking number).

<a href="<?= $block->escapeUrl($block->getTrackingUrl()->getUrl($_item)) ?>" target="_blank"> <?= $block->escapeHtml($_item->getNumber()) ?> </a>

@elvinristi
Copy link
Contributor

elvinristi commented Oct 28, 2019

Confirming also issue when mail server is Kerio version 9.2.5 patch 3 (3336) and Outlook 2016 as email client.

Some guys quick-fixed this by removing Content-Disposition header (which probably isn't the overall solution to this):

cc @gwharton (ref: #23650)

@elvinristi
Copy link
Contributor

elvinristi commented Oct 28, 2019

@jaminion did you do something similar to this?

     {
         try {
             $zendMessage = Message::fromString($this->message->getRawMessage())->setEncoding('utf-8');
+            $zendPluginLoader = $zendMessage->getHeaders()->getPluginClassLoader();
+            $zendPluginLoader->registerPlugin('contentdisposition', MyCustomContentDisposition::class);
+
             if (2 === $this->isSetReturnPath && $this->returnPathValue) {
                 $zendMessage->setSender($this->returnPathValue);
             } elseif (1 === $this->isSetReturnPath && $zendMessage->getFrom()->count()) {

@jaminion
Copy link
Author

@elvinristi yes, that's exactly what I did - it's unfortunate that a full override of that file was necessary due to the use private members for all of the important bits.

Following the example from some of the other plugins registered for the ZF2 mail header loader, I also registered the MyCustomContentDisposition for "contentdisposition", "content_disposition" and "content-disposition".

In the MyCustomContentDisposition, I only did this:

<?php
class DispositionHeader extends \Zend\Mail\Header\GenericHeader {

    // No wrapping
    public function getFieldValue($format = HeaderInterface::FORMAT_RAW) {
        return $this->fieldValue;
    }

    // specifically just this header
    public function getFieldName() {
        return 'Content-Disposition';
    }

}

I'm sorry I'm not too much more use with this - the M2 codebase can be a bit overwhelming at times, and I always feel that there's a better way to do things.

@RaphaelBronsveld
Copy link

Is there any update on this? I'd like to apply a composer patch, or is a complete overwrite via a preference the the only way to fix this?

Thanks in advance.

@elvinristi
Copy link
Contributor

elvinristi commented Oct 29, 2019

my current hotfix patch was to patch vendor/zendframework/zend-mail/src/Headers.php

Fix: Emails are converted to attachments in MS Exchange due encoded value in Content-Disposition header.

@issue https://github.com/magento/magento2/issues/25076

@package zendframework/zend-mail
@version >=2.10.0

--- src/Headers.php.org	2019-10-28 12:00:52.998605927 +0100
+++ src/Headers.php	2019-10-28 12:06:44.672407149 +0100
@@ -512,7 +512,10 @@
         }
 
         $current = $headers;
-        $current->setEncoding($encoding);
+        // PATCH
+        //$current->setEncoding($encoding);
+        $current->setEncoding($key === 'contentdisposition' && $current->getFieldValue(false) === 'inline' ? 'ASCII' : $encoding);
+        // ~PATCH
         $this->headers[$index] = $current;
         return $current;
     }

Haven't got yet confirmation from customer if it solved but at least I see header difference.
[EDIT] Got confirmation that now emails are not shown as attachments anymore after the fix.

Header before:
Content-Disposition: =?utf-8?Q?inline?=

Header after:
Content-Disposition: inline

@melnikovi (cc @buskamuza), any suggestions on this what would be proper approach? Should we include plugin for zend mail in Magento email component?

@ghost ghost added Issue: ready for confirmation and removed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed labels Oct 20, 2020
@befrednguyen
Copy link

befrednguyen commented Nov 11, 2020

The same issue on 2.2.11

@stale
Copy link

stale bot commented Jan 26, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 14 days if no further activity occurs. Is this issue still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? Thank you for your contributions!

@stale stale bot added the stale issue label Jan 26, 2021
@cundd
Copy link

cundd commented Jan 26, 2021

Has this been fixed in Magento 2.4?

@stale stale bot removed the stale issue label Jan 26, 2021
@gwharton
Copy link
Contributor

No, I tried to get it moving with this issue raised on 2.4. It was reconfirmed 6 days ago, so maybe its about to gain some traction.

#29258

@stale
Copy link

stale bot commented Apr 12, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 14 days if no further activity occurs. Is this issue still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? Thank you for your contributions!

@stale stale bot added the stale issue label Apr 12, 2021
@cundd
Copy link

cundd commented Apr 12, 2021

Isn't there anybody that could take care of this?

@stale stale bot removed the stale issue label Apr 12, 2021
@ihor-sviziev ihor-sviziev added the Severity: S1 Affects critical data or functionality and forces users to employ a workaround. label Apr 13, 2021
@ihor-sviziev
Copy link
Contributor

@sidolov @sivaschenko @gabrieldagama, I think this issue should have p1 priority as Magento's emails are not rendering correctly.

@cundd
Copy link

cundd commented Apr 13, 2021

Thank you guys for giving this another look!

@m2-community-project m2-community-project bot added the Priority: P1 Once P0 defects have been fixed, a defect having this priority is the next candidate for fixing. label Apr 13, 2021
@christianbookpwood
Copy link

I can confirm this is happening for us in Magento 2.3.4.

@m2-community-project m2-community-project bot added the Priority: P2 A defect with this priority could have functionality issues which are not to expectations. label Jul 15, 2021
@m2-community-project m2-community-project bot assigned fredden and unassigned fredden Oct 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Email Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: ready for confirmation Priority: P1 Once P0 defects have been fixed, a defect having this priority is the next candidate for fixing. Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: done Severity: S1 Affects critical data or functionality and forces users to employ a workaround. Triage: Ready for Triage Issue is ready to me triaged with Product Manager
Projects
Archived in project