Skip to content
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

[Mailer] Document require_tls option #20701

Open
wants to merge 3 commits into
base: 7.3
Choose a base branch
from

Conversation

ssddanbrown
Copy link

For #20644. This PR documents the added SMTP require_tls option, as introduced in symfony/symfony#59479.

I have referenced that this could throw a TransportException (where TLS is not achieved) but not sure if I should instead reference TransportExceptionInterface.

@OskarStark OskarStark changed the title [Mailer] Document require_tls option [Mailer] Document require_tls option Mar 10, 2025
Copy link
Contributor

@mdoutreluingne mdoutreluingne left a comment

Choose a reason for hiding this comment

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

To have the same "naming" as the other versionadded directive when adding an new option

@@ -420,6 +420,29 @@ setting the ``auto_tls`` option to ``false`` in the DSN::

This setting only works when the ``smtp://`` protocol is used.

Require use of TLS
Copy link
Member

Choose a reason for hiding this comment

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

Maybe:

Suggested change
Require use of TLS
Enforce TLS

Copy link
Author

Choose a reason for hiding this comment

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

Happy to change if preferred, but Enforce feels slightly less accurate to me. Enforce sounds like it'd change/force the connection in some manner and/or force TLS upon the whole connection process, whereas this option doesn't change the connection in any way, just ensures it's eventually used.

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand your concern about the term "Enforce" and I agree with you.
What do you think about "Ensure TLS" instead? It might better reflect that the option guarantees the use of TLS without actively altering the connection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants