Skip to content

[Mailer] Too strict syntax validation for In-Reply-To and References headers #37361

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
ThomasLandauer opened this issue Jun 19, 2020 · 6 comments · Fixed by #44732
Closed

[Mailer] Too strict syntax validation for In-Reply-To and References headers #37361

ThomasLandauer opened this issue Jun 19, 2020 · 6 comments · Fixed by #44732

Comments

@ThomasLandauer
Copy link
Contributor

Symfony version(s) affected: 4.4.10, but also present in master

Description
In-Reply-To and References headers are validated as ID-Headers: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Mime/Header/Headers.php#L37
...which leads to validating them as Address: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Mime/Header/IdentificationHeader.php#L86

Yes, this is exactly what RFC 2822 is saying.

However, I'm suggesting to loosen the validation.

Why? When replying to an email, the original email's Message-ID goes into In-Reply-To or References. This means: If the value doesn't adhere to the msg-id spec, I can't do anything about it! The only choice I'm having is to include it as-is, or omit the header entirely. And IMO including it is the by far better choice, since it gives the recipient's MUA the possibility to correctly append it to an existing thread.

@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@ThomasLandauer
Copy link
Contributor Author

@carsonbot Yes, still relevant. No workaround.

@carsonbot carsonbot removed the Stalled label Feb 18, 2021
@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@carsonbot
Copy link

Friendly reminder that this issue exists. If I don't hear anything I'll close this.

@stof
Copy link
Member

stof commented Sep 2, 2021

@fabpot what do you think about this ?

@carsonbot carsonbot removed the Stalled label Sep 2, 2021
@nicolas-grekas
Copy link
Member

Relaxing this would work for me. Please send a PR.

@fabpot fabpot closed this as completed Dec 21, 2021
fabpot added a commit that referenced this issue Dec 21, 2021
…uer)

This PR was submitted for the 6.1 branch but it was squashed and merged into the 4.4 branch instead.

Discussion
----------

[Mime] Relaxing in-reply-to header validation

| Q             | A
| ------------- | ---
| Branch?       | 6.1
| Bug fix?      | no
| New feature?  | ?
| Deprecations? | no
| Tickets       | Fix #37361
| License       | MIT
| Doc PR        | not necessary

@nicolas-grekas
1. Is it OK to just use `UnstructuredHeader`?
2. Some tests at `IdentificationHeaderTest` are irrelevant now (but still pass) - should I remove them? And create some new test cases in `UnstructuredHeaderTest`? Or rely on every aspect being tested with other headers there, and don't add anything?

Commits
-------

04ddc12 [Mime] Relaxing in-reply-to header validation
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.

6 participants