Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[FrameworkBundle][Mailer] Add a way to configure some email headers from semantic configuration#36736

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:masterfromfabpot:mailer-default-headers
Jun 9, 2020

Conversation

@fabpot
Copy link
Member

@fabpotfabpot commentedMay 7, 2020
edited
Loading

QA
Branch?master
Bug fix?no
New feature?yes
Deprecations?no
Ticketsn/a
LicenseMIT
Doc PRnot yet

The configuration allows to set globalsender andrecipients, but for theenvelope.

If you want to set some global headers, it was not possible (a defaultfrom header for instance, of abcc).

That's implemented in this PR.

Kocal reacted with heart emoji
@fabpotfabpotforce-pushed themailer-default-headers branch fromed6ee44 to636b293CompareMay 8, 2020 12:41
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks :) Here are some things I spotted.

@fabpotfabpotforce-pushed themailer-default-headers branch from438eb32 to8a6c1ddCompareJune 9, 2020 07:55
@fabpotfabpotforce-pushed themailer-default-headers branch 2 times, most recently froma2851d7 to7d4cd2fCompareJune 9, 2020 08:23
@j-schumann
Copy link

I don't know if commenting here or opening a new issue is better, but this seems the place closest to the action:

I implemented a custom EventSubscriber for the MessageEvent before (currently working in 5.1) but this fails and seems will fail with this PR too in combination with the messenger:

Variant A)
When the messenger is not configured/used to send mails asynchronously the Symfony\Component\Mailer\Mailer::send will directly call the transports send() method:https://github.com/symfony/mailer/blob/master/Mailer.php#L41

Then the transport will dispatch the MessageEvent:https://github.com/symfony/mailer/blob/master/Transport/AbstractTransport.php#L61

Everything works as planned.

Variant B)
The Mailer receives a MessageBus instance and thus will dispatch the MessageEvent himself:https://github.com/symfony/mailer/blob/master/Mailer.php#L47
But because the message and its envelope are cloned before (L48/L49) the listenerscannot modify the message.

The email is than wrapped into a SendEmailMessage which in turn is serialized which eventually calls Symfony\Component\Mime\Message::ensureValidity() which will fail because the message has no FROM header, as the listener could not set it before.

--
The only way now to fix this is to set a (valid) sender address before, check $event->isQueued() in the listener and onlyoverwrite the From header if isQueued == false (when the MessageEvent is triggered by the final transport). This is ugly because there is no sound way to determine if this is a custom sender address or a placeholder which should be replaced by the listener.

I think this PR will fail in the same way: The listener cannot set the headers before the message is serialized (because of the mentioned cloning), thus there will be no From header and serialization will fail. Also the Listener cannot replace the previously set placeholder sender address because it uses'from' => self::HEADER_SET_IF_EMPTY, as default rule, so it will simply skip the placeholder.

For me the current behavior in 5.1 would qualify as a bug, as "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." like the documentation states is not working because in combination with the messenger the listener cannot modify the message. I can open a new bug report for this if it helps.

Best regards,
Jakob

@fabpot
Copy link
MemberAuthor

@j-schumann Thanks for the details report. Can you please open a new issue? As the PR here is merged, we won't be able to track your issue properly and it will get lost.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@javiereguiluzjaviereguiluzjaviereguiluz approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

+2 more reviewers

@ro0NLro0NLro0NL left review comments

@KocalKocalKocal left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

5.2

Development

Successfully merging this pull request may close these issues.

7 participants

@fabpot@j-schumann@javiereguiluz@nicolas-grekas@ro0NL@Kocal@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp