Skip to content

[Mailer] Check email validity before opening an SMTP connection #33471

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 6, 2019
Merged

[Mailer] Check email validity before opening an SMTP connection #33471

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/Symfony/Component/Mailer/Bridge/Sendgrid/Tests/Transport/SendgridApiTransportTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);
}
}
2 changes: 2 additions & 0 deletions src/Symfony/Component/Mailer/SentMessage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
20 changes: 11 additions & 9 deletions src/Symfony/Component/Mailer/Transport/Smtp/SmtpTransport.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Mailer/Transport/Smtp/Stream/SocketStream.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];
Expand Down
14 changes: 11 additions & 3 deletions src/Symfony/Component/Mime/Email.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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) {
Expand Down
9 changes: 9 additions & 0 deletions src/Symfony/Component/Mime/Message.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, '@');
Expand Down
9 changes: 9 additions & 0 deletions src/Symfony/Component/Mime/RawMessage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace Symfony\Component\Mime;

use Symfony\Component\Mime\Exception\LogicException;

/**
* @author Fabien Potencier <fabien@symfony.com>
*/
Expand Down Expand Up @@ -51,6 +53,13 @@ public function toIterable(): iterable
$this->message = $message;
}

/**
* @throws LogicException if the message is not valid
*/
public function ensureValidity()
{
}

/**
* @internal
*/
Expand Down
27 changes: 13 additions & 14 deletions src/Symfony/Component/Mime/Tests/EmailTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,76 +251,75 @@ public function testGenerateBody()
$att = new DataPart($file = fopen(__DIR__.'/Fixtures/mimetypes/test', 'r'));
$img = new DataPart($image = fopen(__DIR__.'/Fixtures/mimetypes/test.gif', 'r'), 'test.gif');

$e = new Email();
$e = (new Email())->from('me@example.com');
$e->text('text content');
$this->assertEquals($text, $e->getBody());
$this->assertEquals('text content', $e->getTextBody());

$e = new Email();
$e = (new Email())->from('me@example.com');
$e->html('html content');
$this->assertEquals($html, $e->getBody());
$this->assertEquals('html content', $e->getHtmlBody());

$e = new Email();
$e = (new Email())->from('me@example.com');
$e->html('html content');
$e->text('text content');
$this->assertEquals(new AlternativePart($text, $html), $e->getBody());

$e = new Email();
$e = (new Email())->from('me@example.com');
$e->html('html content', 'iso-8859-1');
$e->text('text content', 'iso-8859-1');
$this->assertEquals('iso-8859-1', $e->getTextCharset());
$this->assertEquals('iso-8859-1', $e->getHtmlCharset());
$this->assertEquals(new AlternativePart(new TextPart('text content', 'iso-8859-1'), new TextPart('html content', 'iso-8859-1', 'html')), $e->getBody());

$e = new Email();
$e = (new Email())->from('me@example.com');
$e->attach($file);
$e->text('text content');
$this->assertEquals(new MixedPart($text, $att), $e->getBody());

$e = new Email();
$e = (new Email())->from('me@example.com');
$e->attach($file);
$e->html('html content');
$this->assertEquals(new MixedPart($html, $att), $e->getBody());

$e = new Email();
$e = (new Email())->from('me@example.com');
$e->attach($file);
$this->assertEquals(new MixedPart($att), $e->getBody());

$e = new Email();
$e = (new Email())->from('me@example.com');
$e->html('html content');
$e->text('text content');
$e->attach($file);
$this->assertEquals(new MixedPart(new AlternativePart($text, $html), $att), $e->getBody());

$e = new Email();
$e = (new Email())->from('me@example.com');
$e->html('html content');
$e->text('text content');
$e->attach($file);
$e->attach($image, 'test.gif');
$this->assertEquals(new MixedPart(new AlternativePart($text, $html), $att, $img), $e->getBody());

$e = new Email();
$e = (new Email())->from('me@example.com');
$e->text('text content');
$e->attach($file);
$e->attach($image, 'test.gif');
$this->assertEquals(new MixedPart($text, $att, $img), $e->getBody());

$e = new Email();
$e = (new Email())->from('me@example.com');
$e->html($content = 'html content <img src="test.gif">');
$e->text('text content');
$e->attach($file);
$e->attach($image, 'test.gif');
$fullhtml = new TextPart($content, 'utf-8', 'html');
$this->assertEquals(new MixedPart(new AlternativePart($text, $fullhtml), $att, $img), $e->getBody());

$e = new Email();
$e = (new Email())->from('me@example.com');
$e->html($content = 'html content <img src="cid:test.gif">');
$e->text('text content');
$e->attach($file);
$e->attach($image, 'test.gif');
$fullhtml = new TextPart($content, 'utf-8', 'html');
$inlinedimg = (new DataPart($image, 'test.gif'))->asInline();
$body = $e->getBody();
$this->assertInstanceOf(MixedPart::class, $body);
$this->assertCount(2, $related = $body->getParts());
Expand All @@ -336,7 +335,7 @@ public function testGenerateBody()
fwrite($r, $content);
rewind($r);

$e = new Email();
$e = (new Email())->from('me@example.com');
$e->html($r);
// embedding the same image twice results in one image only in the email
$e->embed($image, 'test.gif');
Expand Down