Skip to content

Commit 8b3da02

Browse files
fabpotfabpot
committed
feature #33471 [Mailer] Check email validity before opening an SMTP connection (fabpot)
This PR was merged into the 4.4 branch. Discussion ---------- [Mailer] Check email validity before opening an SMTP connection | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes-ish | New feature? | yes | BC breaks? | no-ish | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | n/a | License | MIT | Doc PR | n/a When using an SMTP server to send emails, the connection to the SMTP server happens before being sure that the email to sent is valid. That does not happen with HTTP transports. This pull request implements a new method to be sure that we don't connect to the SMTP server if the email is not valid. Commits ------- dc376f52a5 [Mailer] Check email validity before opening an SMTP connection
2 parents 89b462c + 051c479 commit 8b3da02

File tree

10 files changed

+49
-37
lines changed

10 files changed

+49
-37
lines changed

‎Sendgrid/Tests/Transport/SendgridApiTransportTest.php‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ public function testSend()
5050
$email = new Email();
5151
$email->from('foo@example.com')
5252
->to('bar@example.com')
53-
->bcc('baz@example.com');
53+
->bcc('baz@example.com')
54+
->text('content');
5455

5556
$response = $this->createMock(ResponseInterface::class);
5657

@@ -74,14 +75,15 @@ public function testSend()
7475
],
7576
],
7677
'from' => ['email' => 'foo@example.com'],
77-
'content' => [],
78+
'content' => [
79+
['type' => 'text/plain', 'value' => 'content'],
80+
],
7881
],
7982
'auth_bearer' => 'foo',
8083
])
8184
->willReturn($response);
8285

8386
$mailer = new SendgridApiTransport('foo', $httpClient);
84-
8587
$mailer->send($email);
8688
}
8789
}

‎SendgridApiTransportTest.php‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ public function testSend()
5050
$email = new Email();
5151
$email->from('foo@example.com')
5252
->to('bar@example.com')
53-
->bcc('baz@example.com');
53+
->bcc('baz@example.com')
54+
->text('content');
5455

5556
$response = $this->createMock(ResponseInterface::class);
5657

@@ -74,14 +75,15 @@ public function testSend()
7475
],
7576
],
7677
'from' => ['email' => 'foo@example.com'],
77-
'content' => [],
78+
'content' => [
79+
['type' => 'text/plain', 'value' => 'content'],
80+
],
7881
],
7982
'auth_bearer' => 'foo',
8083
])
8184
->willReturn($response);
8285

8386
$mailer = new SendgridApiTransport('foo', $httpClient);
84-
8587
$mailer->send($email);
8688
}
8789
}

‎SentMessage.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ class SentMessage
2929
*/
3030
public function __construct(RawMessage $message, SmtpEnvelope $envelope)
3131
{
32+
$message->ensureValidity();
33+
3234
$this->raw = $message instanceof Message ? new RawMessage($message->toIterable()) : $message;
3335
$this->original = $message;
3436
$this->envelope = $envelope;

‎Smtp/SmtpTransport.php‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,18 +104,15 @@ public function getLocalDomain(): string
104104

105105
public function send(RawMessage $message, SmtpEnvelope $envelope = null): ?SentMessage
106106
{
107-
$this->ping();
108-
if (!$this->started) {
109-
$this->start();
110-
}
111-
112107
try {
113108
$message = parent::send($message, $envelope);
114109
} catch (TransportExceptionInterface $e) {
115-
try {
116-
$this->executeCommand("RSET\r\n", [250]);
117-
} catch (TransportExceptionInterface $_) {
118-
// ignore this exception as it probably means that the server error was final
110+
if ($this->started) {
111+
try {
112+
$this->executeCommand("RSET\r\n", [250]);
113+
} catch (TransportExceptionInterface $_) {
114+
// ignore this exception as it probably means that the server error was final
115+
}
119116
}
120117

121118
throw $e;
@@ -163,6 +160,11 @@ public function executeCommand(string $command, array $codes): string
163160

164161
protected function doSend(SentMessage $message): void
165162
{
163+
$this->ping();
164+
if (!$this->started) {
165+
$this->start();
166+
}
167+
166168
try {
167169
$envelope = $message->getEnvelope();
168170
$this->doMailFromCommand($envelope->getSender()->getAddress());

‎Smtp/Stream/SocketStream.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ final class SocketStream extends AbstractStream
2626
private $url;
2727
private $host = 'localhost';
2828
private $port = 465;
29-
private $timeout = 15;
29+
private $timeout = 5;
3030
private $tls = true;
3131
private $sourceIp;
3232
private $streamContextOptions = [];

‎SmtpTransport.php‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,18 +104,15 @@ public function getLocalDomain(): string
104104

105105
public function send(RawMessage $message, SmtpEnvelope $envelope = null): ?SentMessage
106106
{
107-
$this->ping();
108-
if (!$this->started) {
109-
$this->start();
110-
}
111-
112107
try {
113108
$message = parent::send($message, $envelope);
114109
} catch (TransportExceptionInterface $e) {
115-
try {
116-
$this->executeCommand("RSET\r\n", [250]);
117-
} catch (TransportExceptionInterface $_) {
118-
// ignore this exception as it probably means that the server error was final
110+
if ($this->started) {
111+
try {
112+
$this->executeCommand("RSET\r\n", [250]);
113+
} catch (TransportExceptionInterface $_) {
114+
// ignore this exception as it probably means that the server error was final
115+
}
119116
}
120117

121118
throw $e;
@@ -163,6 +160,11 @@ public function executeCommand(string $command, array $codes): string
163160

164161
protected function doSend(SentMessage $message): void
165162
{
163+
$this->ping();
164+
if (!$this->started) {
165+
$this->start();
166+
}
167+
166168
try {
167169
$envelope = $message->getEnvelope();
168170
$this->doMailFromCommand($envelope->getSender()->getAddress());

‎SocketStream.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ final class SocketStream extends AbstractStream
2626
private $url;
2727
private $host = 'localhost';
2828
private $port = 465;
29-
private $timeout = 15;
29+
private $timeout = 5;
3030
private $tls = true;
3131
private $sourceIp;
3232
private $streamContextOptions = [];

‎Stream/SocketStream.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ final class SocketStream extends AbstractStream
2626
private $url;
2727
private $host = 'localhost';
2828
private $port = 465;
29-
private $timeout = 15;
29+
private $timeout = 5;
3030
private $tls = true;
3131
private $sourceIp;
3232
private $streamContextOptions = [];

‎Transport/Smtp/SmtpTransport.php‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,18 +104,15 @@ public function getLocalDomain(): string
104104

105105
public function send(RawMessage $message, SmtpEnvelope $envelope = null): ?SentMessage
106106
{
107-
$this->ping();
108-
if (!$this->started) {
109-
$this->start();
110-
}
111-
112107
try {
113108
$message = parent::send($message, $envelope);
114109
} catch (TransportExceptionInterface $e) {
115-
try {
116-
$this->executeCommand("RSET\r\n", [250]);
117-
} catch (TransportExceptionInterface $_) {
118-
// ignore this exception as it probably means that the server error was final
110+
if ($this->started) {
111+
try {
112+
$this->executeCommand("RSET\r\n", [250]);
113+
} catch (TransportExceptionInterface $_) {
114+
// ignore this exception as it probably means that the server error was final
115+
}
119116
}
120117

121118
throw $e;
@@ -163,6 +160,11 @@ public function executeCommand(string $command, array $codes): string
163160

164161
protected function doSend(SentMessage $message): void
165162
{
163+
$this->ping();
164+
if (!$this->started) {
165+
$this->start();
166+
}
167+
166168
try {
167169
$envelope = $message->getEnvelope();
168170
$this->doMailFromCommand($envelope->getSender()->getAddress());

‎Transport/Smtp/Stream/SocketStream.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ final class SocketStream extends AbstractStream
2626
private $url;
2727
private $host = 'localhost';
2828
private $port = 465;
29-
private $timeout = 15;
29+
private $timeout = 5;
3030
private $tls = true;
3131
private $sourceIp;
3232
private $streamContextOptions = [];

0 commit comments

Comments
 (0)