Skip to content

[Mailer][Amazon] SMTP transport does not provide the (final) Message-ID #46323

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

Closed
drzraf opened this issue May 11, 2022 · 1 comment · Fixed by #46326
Closed

[Mailer][Amazon] SMTP transport does not provide the (final) Message-ID #46323

drzraf opened this issue May 11, 2022 · 1 comment · Fixed by #46326

Comments

@drzraf
Copy link

drzraf commented May 11, 2022

Symfony version(s) affected

6.1.0

Description

In #33967 multiple Mailer Transports were adapted to use $sentMessage->setMessageId(<final Message ID>);.
Among them SesApiTransport and SesHttpTransport but SesSmtpTranspor was not: It still provides the original Message-id (set by Symfony itself).

Since symfony/mailer is often used to abstract transport-specific logic it may easily result in errors where the return value is not the expected one. (In my case I need to check for actual deliverability against AWS SNS service what can only work if I stored the AWS-issued Message-ID)

How to reproduce

MAILER_DSN=ses+smtp://foo:bar@default?region=eu-west-1

$mailer->send($message);
echo $mailer->getMessageId();

Possible Solution

As per https://docs.aws.amazon.com/ses/latest/dg/troubleshoot-smtp.html
The Message-Id is returned as part of the 250 Ok response to the DATA SMTP command.

@drzraf drzraf added the Bug label May 11, 2022
@drzraf drzraf changed the title [Mailer][Amazon] SMTP transport does not provide the Message-ID [Mailer][Amazon] SMTP transport does not provide the (final) Message-ID May 11, 2022
@nicolas-grekas
Copy link
Member

Can you please send a PR to implement this?

@fabpot fabpot closed this as completed Jul 20, 2022
fabpot added a commit that referenced this issue Jul 20, 2022
…ailable (Raphaël Droz)

This PR was squashed before being merged into the 6.2 branch.

Discussion
----------

SMTP Transport to provide the (final) Message-ID if available

bug #46323 [Mailer] SMTP transport does not provide the (final) Message-ID

| Q             | A
| ------------- | ---
| Branch?       | 6.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #46323
| License       | MIT
| Doc PR        | symfony/symfony-docs#

Questions / Answers:
- **Q**: Why storing 250 inside **`$mtaResult`** instead of using the already available `$message->getDebug()`?
- A: Because the later may be huge (especially in case of attachment) and running a regexp over it would be less than safe. Instead `$mtaResult` only contains one line
- **Q**: Why a new **`public`** method?
- A: Because child SMTP Transport may make other assumptions about the return `250 Ok` value and provide a custom logic / regexp.
- **Q**: Why **regexp**?
- A: Because there is no official standard for this line (other than the `250 Ok` prefix)

Additionally (see https://symfony.com/releases):
- [ ] Always add tests and ensure they pass. _I can't. Don't have the time. Left to maintainers / other contributors_
- [x] Bug fixes must be submitted against the lowest maintained branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too.)
- [x] Features and deprecations must be submitted against the latest branch.
- [x] Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
- [x] Never break backward compatibility (see https://symfony.com/bc).

Commits
-------

441c9b0 SMTP Transport to provide the (final) Message-ID if available
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants