[Mailer] Allow configuring port and tls options when using the Amazon SES bridge #62029
Conversation
| } | ||
|
|
||
| parent::__construct($host, 465, true, $dispatcher, $logger); | ||
| parent::__construct($host, $port, true, $dispatcher, $logger); |
There was a problem hiding this comment.
The $tls argument must now depend on the port I guess so that it is false when the given port mandates using STARTTLS.
That said should we also limit the accepted ports to the ones listed in the linked issue?
There was a problem hiding this comment.
@xabbuh That makes sense.
Changes made:
If the user provides wether or not to use tls, it will be respected.
If the user does not define tls in the DSN, it will use AWS's mapping to make sense of it:
465 or 2465 = TLS
25, 587, or 2587 = STARTTLS
However, we should not lock them to accepted ports as specified by AWS, because the client's can use port mapping to forward 25 -> 465 because 25 was taken.
Edge case? Sure, but giving the user's flexbility to choose wether to use tls or which port to use seem like the correct choice here. With a fallback working the way you explained it ( if no port or tls configurations are made ).
fb27adf to
6afe2a4
Compare
c34c3ab to
536decd
Compare
536decd to
914d3ad
Compare
914d3ad to
65d3bcb
Compare
xabbuh
left a comment
There was a problem hiding this comment.
I have some suggestions to make it easier for consumers to not shoot themselves in the foot.
src/Symfony/Component/Mailer/Bridge/Amazon/Transport/SesSmtpTransport.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Mailer/Bridge/Amazon/Transport/SesSmtpTransport.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Mailer/Bridge/Amazon/Transport/SesSmtpTransport.php
Show resolved
Hide resolved
…ansport.php Co-authored-by: Christian Flothmann <christian.flothmann@gmail.com>
…ansport.php Co-authored-by: Christian Flothmann <christian.flothmann@gmail.com>
…ansport.php Co-authored-by: Christian Flothmann <christian.flothmann@gmail.com>
| { | ||
| if (!\in_array($port, [], true)) { | ||
| throw new InvalidArgumentException(\sprintf('The port must be one of [25, 587, 2587, 465, 2465] (%d given).', $port); |
There was a problem hiding this comment.
| throw new InvalidArgumentException(\sprintf('The port must be one of [25, 587, 2587, 465, 2465] (%d given).', $port); | |
| throw new InvalidArgumentException(\sprintf('The port must be one of [25, 587, 2587, 465, 2465] (%d given).', $port)); |
This feature addresses feature request on #62024
User's now have ability to select a different port when using ses+smtp
It takes the port from DSN and backward compatiblity is maintained by passing 465 as the default parameter.