8000 bug #35633 [Mailer] Do not ping the SMTP server before sending every … · symfony/symfony@cb42480 · GitHub
[go: up one dir, main page]
Skip to content

Commit cb42480

Browse files
fabpotfabpot
committed
bug #35633 [Mailer] Do not ping the SMTP server before sending every message (micheh)
This PR was squashed before being merged into the 4.4 branch (closes #35633). Discussion ---------- [Mailer] Do not ping the SMTP server before sending every message | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #35515 | License | MIT This pull request changes the SMTP transport to only ping the server if the last message was sent more than a specified number of seconds ago (instead of pinging the server before every message). By default, it will ping the server if 100 or more seconds since the last message have passed. This should make sending emails with the SMTP transport more robust with many emails, as SMTP servers will often drop the connection if too many non-mail commands are sent (like pinging the server with NOOP commands). Commits ------- 2817810 [Mailer] Do not ping the SMTP server before sending every message
2 parents 05663c3 + 2817810 commit cb42480

File tree

2 files changed

+126
-1
lines changed

2 files changed

+126
-1
lines changed

‎src/Symfony/Component/Mailer/Tests/Transport/Smtp/SmtpTransportTest.php

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,12 @@
1212
namespace Symfony\Component\Mailer\Tests\Transport\Smtp;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\Mailer\Envelope;
1516
use Symfony\Component\Mailer\Transport\Smtp\SmtpTransport;
17+
use Symfony\Component\Mailer\Transport\Smtp\Stream\AbstractStream;
1618
use Symfony\Component\Mailer\Transport\Smtp\Stream\SocketStream;
19+
use Symfony\Component\Mime\Address;
20+
use Symfony\Component\Mime\RawMessage;
1721

1822
class SmtpTransportTest extends TestCase
1923
{
@@ -25,4 +29,95 @@ public function testToString()
2529
$t = new SmtpTransport((new SocketStream())->setHost('127.0.0.1')->setPort(2525)->disableTls());
2630
$this->assertEquals('smtp://127.0.0.1:2525', (string) $t);
2731
}
32+
33+
public function testSendDoesNotPingBelowThreshold(): void
34+
{
35+
$stream = new DummyStream();
36+
$envelope = new Envelope(new Address('sender@example.org'), [new Address('recipient@example.org')]);
37+
38+
$transport = new SmtpTransport($stream);
39+
$transport->send(new RawMessage('Message 1'), $envelope);
40+
$transport->send(new RawMessage('Message 2'), $envelope);
41+
$transport->send(new RawMessage('Message 3'), $envelope);
42+
43+
$this->assertNotContains("NOOP\r\n", $stream->getCommands());
44+
}
45+
46+
public function testSendDoesPingAboveThreshold(): void
47+
{
48+
$stream = new DummyStream();
49+
$envelope = new Envelope(new Address('sender@example.org'), [new Address('recipient@example.org')]);
50+
51+
$transport = new SmtpTransport($stream);
52+
$transport->setPingThreshold(1);
53+
54+
$transport->send(new RawMessage('Message 1'), $envelope);
55+
$transport->send(new RawMessage('Message 2'), $envelope);
56+
57+
$this->assertNotContains("NOOP\r\n", $stream->getCommands());
58+
59+
$stream->clearCommands();
60+
sleep(1);
61+
62+
$transport->send(new RawMessage('Message 3'), $envelope);
63+
$this->assertContains("NOOP\r\n", $stream->getCommands());
64+
}
65+
}
66+
67+
class DummyStream extends AbstractStream
68+
{
69+
/**
70+
* @var string
71+
*/
72+
private $nextResponse;
73+
74+
/**
75+
* @var string[]
76+
*/
77+
private $commands;
78+
79+
public function initialize(): void
80+
{
81+
$this->nextResponse = '220 localhost';
82+
}
83+
84+
public function write(string $bytes, $debug = true): void
85+
{
86+
$this->commands[] = $bytes;
87+
88+
if (0 === strpos($bytes, 'DATA')) {
89+
$this->nextResponse = '354 Enter message, ending with "." on a line by itself';
90+
} elseif (0 === strpos($bytes, 'QUIT')) {
91+
$this->nextResponse = '221 Goodbye';
92+
} else {
93+
$this->nextResponse = '250 OK';
94+
}
95+
}
96+
97+
public function readLine(): string
98+
{
99+
return $this->nextResponse;
100+
}
101+
102+
public function flush(): void
103+
{
104+
}
105+
106+
/**
107+
* @return string[]
108+
*/
109+
public function getCommands(): array
110+
{
111+
return $this->commands;
112+
}
113+
114+
public function clearCommands(): void
115+
{
116+
$this->commands = [];
117+
}
118+
119+
protected function getReadConnectionDescription(): string
120+
{
121+
return 1E0A 'null';
122+
}
28123
}

‎src/Symfony/Component/Mailer/Transport/Smtp/SmtpTransport.php

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ class SmtpTransport extends AbstractTransport
3535
private $restartThreshold = 100;
3636
private $restartThresholdSleep = 0;
3737
private $restartCounter;
38+
private $pingThreshold = 100;
39+
private $lastMessageTime = 0;
3840
private $stream;
3941
private $domain = '[127.0.0.1]';
4042

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

71+
/**
72+
* Sets the minimum number of seconds required between two messages, before the server is pinged.
73+
* If the transport wants to send a message and the time since the last message exceeds the specified threshold,
74+
* the transport will ping the server first (NOOP command) to check if the connection is still alive.
75+
* Otherwise the message will be sent without pinging the server first.
76+
*
77+
* Do not set the threshold too low, as the SMTP server may drop the connection if there are too many
78+
* non-mail commands (like pinging the server with NOOP).
79+
*
80+
* By default, the threshold is set to 100 seconds.
81+
*
82+
* @param int $seconds The minimum number of seconds between two messages required to ping the server
83+
*
84+
* @return $this
85+
*/
86+
public function setPingThreshold(int $seconds): self
87+
{
88+
$this->pingThreshold = $seconds;
89+
90+
return $this;
91+
}
92+
6993
/**
7094
* Sets the name of the local domain that will be used in HELO.
7195
*
@@ -160,7 +184,10 @@ public function executeCommand(string $command, array $codes): string
160184

161185
protected function doSend(SentMessage $message): void
162186
{
163-
$this->ping();
187+
if (microtime(true) - $this->lastMessageTime > $this->pingThreshold) {
188+
$this->ping();
189+
}
190+
164191
if (!$this->started) {
165192
$this->start();
166193
}
@@ -183,6 +210,8 @@ protected function doSend(SentMessage $message): void
183210
$e->appendDebug($this->stream->getDebug());
184211

185212
throw $e;
213+
} finally {
214+
$this->lastMessageTime = microtime(true);
186215
}
187216
}
188217

@@ -213,6 +242,7 @@ private function start(): void
213242
$this->assertResponseCode($this->getFullResponse(), [220]);
214243
$this->doHeloCommand();
215244
$this->started = true;
245+
$this->lastMessageTime = 0;
216246

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

0 commit comments

Comments
 (0)
0