Skip to content
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.

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

[Mailer] Do not ping the SMTP server before sending every message #35633

Merged
merged 1 commit into from Feb 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -12,8 +12,12 @@
namespace Symfony\Component\Mailer\Tests\Transport\Smtp;

use PHPUnit\Framework\TestCase;
use Symfony\Component\Mailer\Envelope;
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\RawMessage;

class SmtpTransportTest extends TestCase
{
Expand All @@ -25,4 +29,95 @@ public function testToString()
$t = new SmtpTransport((new SocketStream())->setHost('127.0.0.1')->setPort(2525)->disableTls());
$this->assertEquals('smtp://127.0.0.1:2525', (string) $t);
}

public function testSendDoesNotPingBelowThreshold(): void
{
$stream = new DummyStream();
$envelope = new Envelope(new Address('sender@example.org'), [new Address('recipient@example.org')]);

$transport = new SmtpTransport($stream);
$transport->send(new RawMessage('Message 1'), $envelope);
$transport->send(new RawMessage('Message 2'), $envelope);
$transport->send(new RawMessage('Message 3'), $envelope);

$this->assertNotContains("NOOP\r\n", $stream->getCommands());
}

public function testSendDoesPingAboveThreshold(): void
{
$stream = new DummyStream();
$envelope = new Envelope(new Address('sender@example.org'), [new Address('recipient@example.org')]);

$transport = new SmtpTransport($stream);
$transport->setPingThreshold(1);

$transport->send(new RawMessage('Message 1'), $envelope);
$transport->send(new RawMessage('Message 2'), $envelope);

$this->assertNotContains("NOOP\r\n", $stream->getCommands());

$stream->clearCommands();
sleep(1);

$transport->send(new RawMessage('Message 3'), $envelope);
$this->assertContains("NOOP\r\n", $stream->getCommands());
}
}

class DummyStream extends AbstractStream
{
/**
* @var string
*/
private $nextResponse;

/**
* @var string[]
*/
private $commands;

public function initialize(): void
{
$this->nextResponse = '220 localhost';
}

public function write(string $bytes, $debug = true): void
{
$this->commands[] = $bytes;

if (0 === strpos($bytes, 'DATA')) {
$this->nextResponse = '354 Enter message, ending with "." on a line by itself';
} elseif (0 === strpos($bytes, 'QUIT')) {
$this->nextResponse = '221 Goodbye';
} else {
$this->nextResponse = '250 OK';
}
}

public function readLine(): string
{
return $this->nextResponse;
}

public function flush(): void
{
}

/**
* @return string[]
*/
public function getCommands(): array
{
return $this->commands;
}

public function clearCommands(): void
{
$this->commands = [];
}

protected function getReadConnectionDescription(): string
{
return 'null';
}
}
32 changes: 31 additions & 1 deletion src/Symfony/Component/Mailer/Transport/Smtp/SmtpTransport.php
Expand Up @@ -35,6 +35,8 @@ class SmtpTransport extends AbstractTransport
private $restartThreshold = 100;
private $restartThresholdSleep = 0;
private $restartCounter;
private $pingThreshold = 100;
private $lastMessageTime = 0;
private $stream;
private $domain = '[127.0.0.1]';

Expand Down Expand Up @@ -66,6 +68,28 @@ public function setRestartThreshold(int $threshold, int $sleep = 0): self
return $this;
}

/**
* Sets the minimum number of seconds required between two messages, before the server is pinged.
* If the transport wants to send a message and the time since the last message exceeds the specified threshold,
* the transport will ping the server first (NOOP command) to check if the connection is still alive.
* Otherwise the message will be sent without pinging the server first.
*
* Do not set the threshold too low, as the SMTP server may drop the connection if there are too many
* non-mail commands (like pinging the server with NOOP).
*
* By default, the threshold is set to 100 seconds.
*
* @param int $seconds The minimum number of seconds between two messages required to ping the server
*
* @return $this
*/
public function setPingThreshold(int $seconds): self
{
$this->pingThreshold = $seconds;

return $this;
}

/**
* Sets the name of the local domain that will be used in HELO.
*
Expand Down Expand Up @@ -160,7 +184,10 @@ public function executeCommand(string $command, array $codes): string

protected function doSend(SentMessage $message): void
{
$this->ping();
if (microtime(true) - $this->lastMessageTime > $this->pingThreshold) {
$this->ping();
}

if (!$this->started) {
$this->start();
}
Expand All @@ -183,6 +210,8 @@ protected function doSend(SentMessage $message): void
$e->appendDebug($this->stream->getDebug());

throw $e;
} finally {
$this->lastMessageTime = microtime(true);
}
}

Expand Down Expand Up @@ -213,6 +242,7 @@ private function start(): void
$this->assertResponseCode($this->getFullResponse(), [220]);
$this->doHeloCommand();
$this->started = true;
$this->lastMessageTime = 0;

$this->getLogger()->debug(sprintf('Email transport "%s" started', __CLASS__));
}
Expand Down