Skip to content

[Mailer] No default sender possible with sf messenger+ sf serializer #37414

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.

Sign up for GitHub

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

Jump to bottom
Closed
j-schumann opened this issue Jun 25, 2020 · 6 comments · Fixed by #37847
Closed

[Mailer] No default sender possible with sf messenger+ sf serializer #37414

j-schumann opened this issue Jun 25, 2020 · 6 comments · Fixed by #37847
Labels
Bug Mailer Status: Needs Review

Comments

@j-schumann
Copy link

j-schumann commented Jun 25, 2020 •
edited
Loading

Symfony version(s) affected: 5.1, 5.2

Description
A MessageListener cannot be used to add a FROM header when the symfony/mailer is used together with symfony/messenger and symfony/serializer, a LogicException is thrown: An email must have a "From" or a "Sender" header.

How to reproduce
I'm sorry, at the moment I have no reproduce code.

  • create symfony project with messenger, mailer and serializer
  • create a custom event listener for the MessageEvent like the documentation suggests: "Instead of calling ->from() every time you create a new email, you can create an event subscriber and listen to the MessageEvent event to set the same From email to all messages." 
framework:
    mailer:
        dsn: '%env(MAILER_DSN)%'
        headers:
            from: 'mail@domain.tld'
  • configure the messenger to use the the symfony/serializer:
        serializer:
            default_serializer: 'messenger.transport.symfony_serializer'
  • try to send an email without a sender address, relying on the listener to add it.

Result: exception with 'An email must have a "From" or a "Sender" header.'

Possible Solution
Do not clone $message for the event in https://github.com/symfony/mailer/blob/master/Mailer.php#L48 to allow listeners to modify the message before serialization, they can check isQueued to prevent duplicate modification.

Or, probably not that easy: Don't trigger ensureValidity() when the serializer calls getBody() on Symfony\Component\Mime\Email

Additional context
relevant part of the trace:

#0  Symfony\Component\Mime\Message->ensureValidity() called at [/var/www/html/vendor/symfony/mime/Email.php:408]
#1  Symfony\Component\Mime\Email->ensureValidity() called at [/var/www/html/vendor/symfony/mime/Email.php:433]
#2  Symfony\Component\Mime\Email->generateBody() called at [/var/www/html/vendor/symfony/mime/Email.php:399]
#3  Symfony\Component\Mime\Email->getBody() called at [/var/www/html/vendor/symfony/property-access/PropertyAccessor.php:405]
#4  Symfony\Component\PropertyAccess\PropertyAccessor->readProperty() called at [/var/www/html/vendor/symfony/property-access/PropertyAccessor.php:100]
#5  Symfony\Component\PropertyAccess\PropertyAccessor->getValue() called at [/var/www/html/vendor/symfony/serializer/Normalizer/ObjectNormalizer.php:142]
#6  Symfony\Component\Serializer\Normalizer\ObjectNormalizer->getAttributeValue() called at [/var/www/html/vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:176]
#7  Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->normalize() called at [/var/www/html/vendor/symfony/serializer/Serializer.php:154]
#8  Symfony\Component\Serializer\Serializer->normalize() called at [/var/www/html/vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:201]
#9  Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->normalize() called at [/var/www/html/vendor/symfony/serializer/Serializer.php:154]
#10 Symfony\Component\Serializer\Serializer->normalize() called at [/var/www/html/vendor/symfony/serializer/Serializer.php:127]
#11 Symfony\Component\Serializer\Serializer->serialize() called at [/var/www/html/vendor/symfony/messenger/Transport/Serialization/Serializer.php:105]
#12 Symfony\Component\Messenger\Transport\Serialization\Serializer->encode() called at [/var/www/html/vendor/symfony/amqp-messenger/Transport/AmqpSender.php:42]
#13 Symfony\Component\Messenger\Bridge\Amqp\Transport\AmqpSender->send() called at [/var/www/html/vendor/symfony/amqp-messenger/Transport/AmqpTransport.php:66]
#14 Symfony\Component\Messenger\Bridge\Amqp\Transport\AmqpTransport->send() called at [/var/www/html/vendor/symfony/messenger/Middleware/SendMessageMiddleware.php:69]
#15 Symfony\Component\Messenger\Middleware\SendMessageMiddleware->handle() called at [/var/www/html/vendor/symfony/messenger/Middleware/FailedMessageProcessingMiddleware.php:34]
#16 Symfony\Component\Messenger\Middleware\FailedMessageProcessingMiddleware->handle() called at [/var/www/html/vendor/symfony/messenger/Middleware/DispatchAfterCurrentBusMiddleware.php:67]
#17 Symfony\Component\Messenger\Middleware\DispatchAfterCurrentBusMiddleware->handle() called at [/var/www/html/vendor/symfony/messenger/Middleware/RejectRedeliveredMessageMiddleware.php:48]
#18 Symfony\Component\Messenger\Middleware\RejectRedeliveredMessageMiddleware->handle() called at [/var/www/html/vendor/symfony/messenger/Middleware/AddBusNameStampMiddleware.php:37]
#19 Symfony\Component\Messenger\Middleware\AddBusNameStampMiddleware->handle() called at [/var/www/html/vendor/symfony/messenger/Middleware/TraceableMiddleware.php:43]
#20 Symfony\Component\Messenger\Middleware\TraceableMiddleware->handle() called at [/var/www/html/vendor/symfony/messenger/MessageBus.php:80]
#21 Symfony\Component\Messenger\MessageBus->dispatch() called at [/var/www/html/vendor/symfony/mailer/Mailer.php:54]

The normal flow in working cases is:

  • no messenger
    • the mailer directly calls the transport which dispatches the MessageEvent, the listener adds the FROM header -> everything working
  • messenger with the default serializer

The problem occurs when the symfony serializer is used, which triggers the ensureValidity() method when the message is sent to the bus (see trace).
Because the listeners only receive a cloned message they cannot modify the real mail message object and thus cannot add a sender address.

This also effects PR #36736: The config options to set a default sender will not work because the mentioned exception is thrown because the included Symfony\Component\Mailer\EventListener\MessageListener will not be able to set headers because of the clone().
@j-schumann j-schumann added the Bug label Jun 25, 2020
@xabbuh xabbuh added the Mailer label Jun 25, 2020
@j-schumann
Copy link
Author

This would be fixed by #37324 I guess.

@fabpot fabpot mentioned this issue Aug 16, 2020
Merged
@fabpot
Copy link
Member

fabpot commented Aug 17, 2020

This will be fixed by #37847

@fabpot fabpot closed this as completed Aug 17, 2020
@j-schumann
Copy link
Author

j-schumann commented Aug 17, 2020 •
edited
Loading

@fabpot So it is intended that the message can no longer be modified in the MessageEvent triggered by the mailer before serialization, e.g. to set the sender or other details of the message in the context of the current request, which is lost when the message is later processed by the messenger?

@fabpot
Copy link
Member

fabpot commented Aug 17, 2020

Yes, it is intended. The MessageEvent should not be triggered twice for a message. It is triggered just before the message is sent.

@j-schumann
Copy link
Author

j-schumann commented Aug 17, 2020 •
edited
Loading

Thanks for the clarification!
Shouldn't the MessageEvent then be removed completely from Symfony\Component\Mailer\Mailer::send as the Transport always triggers it? Currently it is triggered twice and event listeners can not distinguish the both instances with the third $queued argument of the MessageEvent.

@fabpot
Copy link
Member

fabpot commented Aug 17, 2020

The idea is that you always get the event just before sending the email and another one with the queued argument set to true when pushing to the queue. That allows to gather data for tests and the web profiler.

fabpot added a commit that referenced this issue Aug 17, 2020
@fabpot
feature #37847 [Serializer][Mime] Fix Mime message serialization (fab…
779303a 
…pot)

This PR was merged into the 5.2-dev branch.

Discussion
----------

[Serializer][Mime] Fix Mime message serialization

| Q             | A
| ------------- | ---
| Branch?       | master <!-- see below -->
| Bug fix?      | yes
| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #37414, Fix #37324 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT
| Doc PR        | n/a

Symfony serialization is used by Messenger to serialize Emails. By Email messages are data objects with some logic to prepare emails to be sent. Without configuration, the Symfony Serializer serializes Emails with too many data (and triggers some unneeded validation).

This PR aims to fix the above issue and at the same time makes serialized emails as small as possible and as readable as possible.

Commits
-------

9d869b1 Fix Mime message serialization
hultberg pushed a commit to hultberg/symfony that referenced this issue Sep 17, 2021
@fabpot
feature symfony#37847 [Serializer][Mime] Fix Mime message serializati…
a6c5d17 
…on (fabpot)

This PR was merged into the 5.2-dev branch.

Discussion
----------

[Serializer][Mime] Fix Mime message serialization

| Q             | A
| ------------- | ---
| Branch?       | master <!-- see below -->
| Bug fix?      | yes
| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix symfony#37414, Fix symfony#37324 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT
| Doc PR        | n/a

Symfony serialization is used by Messenger to serialize Emails. By Email messages are data objects with some logic to prepare emails to be sent. Without configuration, the Symfony Serializer serializes Emails with too many data (and triggers some unneeded validation).

This PR aims to fix the above issue and at the same time makes serialized emails as small as possible and as readable as possible.

Commits
-------

9d869b1 Fix Mime message serialization
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Mailer Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Serializer][Mime] Fix Mime message serialization fabpot/symfony
4 participants
@fabpot @j-schumann @xabbuh @carsonbot