-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[TwigBridge][Mime] Use content for images attached to emails #60098
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
Conversation
Relying on the attachment name (i.e. file path) to equal the content ID of the attachment breaks as soon as the content ID differs and is used to refer to an inline attachment.
Use Twig this would normally result in attachment names like '@assets/img/file.png' which might not be a good file name to expose to email clients.
I'm not sure if the change to Postmark mailer was actually a bug or if an image part attached to an email should never have a content ID. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a test to prevent regressions on the bug.
@@ -42,14 +42,16 @@ public function toName(): string | |||
* some Twig namespace for email images (e.g. '@email/images/logo.png'). | |||
* @param string|null $contentType The media type (i.e. MIME type) of the image file (e.g. 'image/png'). | |||
* Some email clients require this to display embedded images. | |||
* @param string|null $name A custom file name that overrides the original name (filepath) of the image |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a new parameter is a new feature, which should go in 7.3 rather than in 6.4. Please separate that from the bugfix.
I'm not even sure whether #60097 is a bug or not. If attachments from Twig should never have a CID then all is well and I was using it incorrectly. If everything should work even if the part gets a CID after attaching the image from Twig then I'd say generating a CID unconditionally might be a good idea. I really need input on this. |
Looking at: symfony/src/Symfony/Component/Mime/Email.php Lines 485 to 504 in 160529b
This is the reason why attachments always get a CID (as the name is set) and a few lines below that it seems that a CID is always generated unconditionally and then used to replace the name. So from my point of view using the CID directly in TemplatedEmailWrapper just anticipates that. Is there a reason why it was implemented like that with replacing of strings in the rendered HTML content? |
I tested this change with local Maildev and a real Postmark API transport.
Instead of referencing a path like
<img src=3D"cid:@assets/img/email-bg.png"
the content ID of the attachment will be used, e.g.,<img src=3D"cid:18ca3e824a76c3d38ac2f1817ad9f6af@symfony"
.