[Mailer] Check email validity before opening an SMTP connection

symfony · Sep 6, 2019 · 3a5fe8c · 3a5fe8c
1 parent b7371ea
commit 3a5fe8c
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 16 deletions.
diff --git a/src/Symfony/Component/Mailer/Bridge/Sendgrid/Tests/Transport/SendgridApiTransportTest.php b/src/Symfony/Component/Mailer/Bridge/Sendgrid/Tests/Transport/SendgridApiTransportTest.php
@@ -50,7 +50,8 @@ public function testSend()
         $email = new Email();
         $email->from('foo@example.com')
             ->to('bar@example.com')
-            ->bcc('baz@example.com');
+            ->bcc('baz@example.com')
+            ->text('content');
 
         $response = $this->createMock(ResponseInterface::class);
 
@@ -74,14 +75,15 @@ public function testSend()
                         ],
                     ],
                     'from' => ['email' => 'foo@example.com'],
-                    'content' => [],
+                    'content' => [
+                        ['type' => 'text/plain', 'value' => 'content'],
+                    ],
                 ],
                 'auth_bearer' => 'foo',
             ])
             ->willReturn($response);
 
         $mailer = new SendgridApiTransport('foo', $httpClient);
-
         $mailer->send($email);
     }
 }
diff --git a/src/Symfony/Component/Mailer/SentMessage.php b/src/Symfony/Component/Mailer/SentMessage.php
@@ -29,6 +29,8 @@ class SentMessage
      */
     public function __construct(RawMessage $message, SmtpEnvelope $envelope)
     {
+        $message->ensureValidity();
+
         $this->raw = $message instanceof Message ? new RawMessage($message->toIterable()) : $message;
         $this->original = $message;
         $this->envelope = $envelope;

diff --git a/src/Symfony/Component/Mailer/Transport/Smtp/SmtpTransport.php b/src/Symfony/Component/Mailer/Transport/Smtp/SmtpTransport.php
@@ -104,18 +104,15 @@ public function getLocalDomain(): string
 
     public function send(RawMessage $message, SmtpEnvelope $envelope = null): ?SentMessage
     {
-        $this->ping();
-        if (!$this->started) {
-            $this->start();
-        }
-
         try {
             $message = parent::send($message, $envelope);
         } catch (TransportExceptionInterface $e) {
-            try {
-                $this->executeCommand("RSET\r\n", [250]);
-            } catch (TransportExceptionInterface $_) {
-                // ignore this exception as it probably means that the server error was final
+            if ($this->started) {
+                try {
+                    $this->executeCommand("RSET\r\n", [250]);
+                } catch (TransportExceptionInterface $_) {
+                    // ignore this exception as it probably means that the server error was final
+                }
             }
 
             throw $e;
@@ -163,6 +160,11 @@ public function executeCommand(string $command, array $codes): string
 
     protected function doSend(SentMessage $message): void
     {
+        $this->ping();
+        if (!$this->started) {
+            $this->start();
+        }
+
         try {
             $envelope = $message->getEnvelope();
             $this->doMailFromCommand($envelope->getSender()->getAddress());

diff --git a/src/Symfony/Component/Mailer/Transport/Smtp/Stream/SocketStream.php b/src/Symfony/Component/Mailer/Transport/Smtp/Stream/SocketStream.php
@@ -26,7 +26,7 @@ final class SocketStream extends AbstractStream
     private $url;
     private $host = 'localhost';
     private $port = 465;
-    private $timeout = 15;
+    private $timeout = 5;
     private $tls = true;
     private $sourceIp;
     private $streamContextOptions = [];

diff --git a/src/Symfony/Component/Mime/Email.php b/src/Symfony/Component/Mime/Email.php
@@ -399,6 +399,15 @@ public function getBody(): AbstractPart
         return $this->generateBody();
     }
 
+    public function ensureValidity()
+    {
+        if (null === $this->text && null === $this->html && !$this->attachments) {
+            throw new LogicException('A message must have a text or an HTML part or attachments.');
+        }
+
+        parent::ensureValidity();
+    }
+
     /**
      * Generates an AbstractPart based on the raw body of a message.
      *
@@ -421,10 +430,9 @@ public function getBody(): AbstractPart
      */
     private function generateBody(): AbstractPart
     {
+        $this->ensureValidity();
+
         [$htmlPart, $attachmentParts, $inlineParts] = $this->prepareParts();
-        if (null === $this->text && null === $this->html && !$attachmentParts) {
-            throw new LogicException('A message must have a text or an HTML part or attachments.');
-        }
 
         $part = null === $this->text ? null : new TextPart($this->text, $this->textCharset);
         if (null !== $htmlPart) {

diff --git a/src/Symfony/Component/Mime/Message.php b/src/Symfony/Component/Mime/Message.php
@@ -123,6 +123,15 @@ public function toIterable(): iterable
         yield from $body->toIterable();
     }
 
+    public function ensureValidity()
+    {
+        if (!$this->headers->has('From')) {
+            throw new LogicException('An email must have a "From" header.');
+        }
+
+        parent::ensureValidity();
+    }
+
     private function generateMessageId(string $email): string
     {
         return bin2hex(random_bytes(16)).strstr($email, '@');

diff --git a/src/Symfony/Component/Mime/RawMessage.php b/src/Symfony/Component/Mime/RawMessage.php
@@ -11,6 +11,8 @@
 
 namespace Symfony\Component\Mime;
 
+use Symfony\Component\Mime\Exception\LogicException;
+
 /**
  * @author Fabien Potencier <fabien@symfony.com>
  */
@@ -51,6 +53,13 @@ public function toIterable(): iterable
         $this->message = $message;
     }
 
+    /**
+     * @throws LogicException if the message is not valid
+     */
+    public function ensureValidity()
+    {
+    }
+
     /**
      * @internal
      */