bug #39597 [Mailer] Handle failure when sending DATA (jderusse)

This PR was merged into the 4.4 branch.

Discussion
----------

[Mailer] Handle failure when sending DATA

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

When an exception is thrown while sending an email via SMTP (ie. A attachment is not readable) the SMTP connection is left opened with a partial message sent.

This PR closes the connection (we can't abort after sending the `DATA` command) in such situation.

/cc @fabpot

Commits
-------

849211a780 Handle failure when sending DATA
This commit is contained in:
Jérémy Derussé 2020-12-24 10:07:56 +01:00
commit 457c8b119d
No known key found for this signature in database
GPG Key ID: 2083FA5758C473D2
2 changed files with 44 additions and 4 deletions

View File

@ -18,6 +18,8 @@ use Symfony\Component\Mailer\Transport\Smtp\SmtpTransport;
use Symfony\Component\Mailer\Transport\Smtp\Stream\AbstractStream;
use Symfony\Component\Mailer\Transport\Smtp\Stream\SocketStream;
use Symfony\Component\Mime\Address;
use Symfony\Component\Mime\Email;
use Symfony\Component\Mime\Exception\InvalidArgumentException;
use Symfony\Component\Mime\RawMessage;
class SmtpTransportTest extends TestCase
@ -86,6 +88,29 @@ class SmtpTransportTest extends TestCase
$transport->send(new RawMessage('Message 3'), $envelope);
$this->assertContains("NOOP\r\n", $stream->getCommands());
}
public function testSendInvalidMessage()
{
$stream = new DummyStream();
$transport = new SmtpTransport($stream);
$transport->setPingThreshold(1);
$message = new Email();
$message->to('recipient@example.org');
$message->from('sender@example.org');
$message->attachFromPath('/does_not_exists');
try {
$transport->send($message);
$this->fail('Expected Symfony\Component\Mime\Exception\InvalidArgumentException to be thrown');
} catch (InvalidArgumentException $e) {
$this->assertMatchesRegularExpression('{Path "/does_not_exists"}i', $e->getMessage());
}
$this->assertNotContains("\r\n.\r\n", $stream->getCommands());
$this->assertTrue($stream->isClosed());
}
}
class DummyStream extends AbstractStream
@ -164,4 +189,10 @@ class DummyStream extends AbstractStream
{
return $this->closed;
}
public function terminate(): void
{
parent::terminate();
$this->closed = true;
}
}

View File

@ -159,7 +159,7 @@ class SmtpTransport extends AbstractTransport
return $name;
}
return sprintf('smtp://sendmail');
return 'smtp://sendmail';
}
/**
@ -200,10 +200,19 @@ class SmtpTransport extends AbstractTransport
}
$this->executeCommand("DATA\r\n", [354]);
foreach (AbstractStream::replace("\r\n.", "\r\n..", $message->toIterable()) as $chunk) {
$this->stream->write($chunk, false);
try {
foreach (AbstractStream::replace("\r\n.", "\r\n..", $message->toIterable()) as $chunk) {
$this->stream->write($chunk, false);
}
$this->stream->flush();
} catch (TransportExceptionInterface $e) {
throw $e;
} catch (\Exception $e) {
$this->stream->terminate();
$this->started = false;
$this->getLogger()->debug(sprintf('Email transport "%s" stopped', __CLASS__));
throw $e;
}
$this->stream->flush();
$this->executeCommand("\r\n.\r\n", [250]);
$message->appendDebug($this->stream->getDebug());
$this->lastMessageTime = microtime(true);