Skip to content

Commit 4e6a8b2

Browse files
fabpotfabpot
committed
[Mailer] simplified the way TLS/SSL/StartTls work
1 parent f8f74b7 commit 4e6a8b2

File tree

10 files changed

+113
-39
lines changed

10 files changed

+113
-39
lines changed

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ CHANGELOG
44
4.4.0
55
-----
66

7+
* STARTTLS cannot be enabled anymore (it is used automatically if TLS is disabled and the server supports STARTTLS)
8+
* [BC BREAK] Removed the `encryption` DSN option (use `smtps` instead)
9+
* Added support for the `smtps` protocol (does the same as using `smtp` and port `465`)
710
* Added PHPUnit constraints
811
* Added `MessageDataCollector`
912
* Added `MessageEvents` and `MessageLoggerListener` to allow collecting sent emails

‎Test/TransportFactoryTestCase.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public function testCreate(Dsn $dsn, TransportInterface $transport): void
6969
$factory = $this->getFactory();
7070

7171
$this->assertEquals($transport, $factory->create($dsn));
72-
if ('smtp' !== $dsn->getScheme()) {
72+
if ('smtp' !== $dsn->getScheme() && 'smtps' !== $dsn->getScheme()) {
7373
$this->assertStringMatchesFormat($dsn->getScheme().'://%S'.$dsn->getHost().'%S', $transport->getName());
7474
}
7575
}

‎Tests/Transport/Smtp/EsmtpTransportFactoryTest.php‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ public function supportsProvider(): iterable
2222
true,
2323
];
2424

25+
yield [
26+
new Dsn('smtps', 'example.com'),
27+
true,
28+
];
29+
2530
yield [
2631
new Dsn('api', 'example.com'),
2732
false,
@@ -33,19 +38,33 @@ public function createProvider(): iterable
3338
$eventDispatcher = $this->getDispatcher();
3439
$logger = $this->getLogger();
3540

36-
$transport = new EsmtpTransport('example.com', 25, null, null, $eventDispatcher, $logger);
41+
$transport = new EsmtpTransport('localhost', 25, false, null, $eventDispatcher, $logger);
3742

3843
yield [
39-
new Dsn('smtp', 'example.com'),
44+
new Dsn('smtp', 'localhost'),
4045
$transport,
4146
];
4247

43-
$transport = new EsmtpTransport('example.com', 99, 'ssl', 'login', $eventDispatcher, $logger);
48+
$transport = new EsmtpTransport('example.com', 99, true, 'login', $eventDispatcher, $logger);
4449
$transport->setUsername(self::USER);
4550
$transport->setPassword(self::PASSWORD);
4651

4752
yield [
48-
new Dsn('smtp', 'example.com', self::USER, self::PASSWORD, 99, ['encryption' => 'ssl', 'auth_mode' => 'login']),
53+
new Dsn('smtps', 'example.com', self::USER, self::PASSWORD, 99, ['auth_mode' => 'login']),
54+
$transport,
55+
];
56+
57+
$transport = new EsmtpTransport('example.com', 465, true, null, $eventDispatcher, $logger);
58+
59+
yield [
60+
new Dsn('smtps', 'example.com'),
61+
$transport,
62+
];
63+
64+
$transport = new EsmtpTransport('example.com', 465, true, null, $eventDispatcher, $logger);
65+
66+
yield [
67+
new Dsn('smtp', 'example.com', '', '', 465),
4968
$transport,
5069
];
5170
}

‎Tests/Transport/Smtp/EsmtpTransportTest.php‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Mailer\Tests\Transport\Smtp;
13+
14+
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\Mailer\Transport\Smtp\EsmtpTransport;
16+
17+
class EsmtpTransportTest extends TestCase
18+
{
19+
public function testName()
20+
{
21+
$t = new EsmtpTransport();
22+
$this->assertEquals('smtp://localhost', $t->getName());
23+
24+
$t = new EsmtpTransport('example.com');
25+
if (\defined('OPENSSL_VERSION_NUMBER')) {
26+
$this->assertEquals('smtps://example.com', $t->getName());
27+
} else {
28+
$this->assertEquals('smtp://example.com', $t->getName());
29+
}
30+
31+
$t = new EsmtpTransport('example.com', 2525);
32+
$this->assertEquals('smtp://example.com:2525', $t->getName());
33+
34+
$t = new EsmtpTransport('example.com', 0, true);
35+
$this->assertEquals('smtps://example.com', $t->getName());
36+
37+
$t = new EsmtpTransport('example.com', 0, false);
38+
$this->assertEquals('smtp://example.com', $t->getName());
39+
40+
$t = new EsmtpTransport('example.com', 466, true);
41+
$this->assertEquals('smtps://example.com:466', $t->getName());
42+
}
43+
}

‎Tests/Transport/Smtp/SmtpTransportTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ class SmtpTransportTest extends TestCase
2020
public function testName()
2121
{
2222
$t = new SmtpTransport();
23-
$this->assertEquals('smtp://localhost:25', $t->getName());
23+
$this->assertEquals('smtps://localhost', $t->getName());
2424

25-
$t = new SmtpTransport((new SocketStream())->setHost('127.0.0.1')->setPort(2525));
25+
$t = new SmtpTransport((new SocketStream())->setHost('127.0.0.1')->setPort(2525)->disableTls());
2626
$this->assertEquals('smtp://127.0.0.1:2525', $t->getName());
2727
}
2828
}

‎Tests/Transport/Smtp/Stream/SocketStreamTest.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ public function testSocketErrorBeforeConnectError()
3737
'cafile' => __FILE__,
3838
],
3939
]);
40-
$s->setEncryption('ssl');
4140
$s->setHost('smtp.gmail.com');
4241
$s->setPort(465);
4342
$s->initialize();

‎Transport/Smtp/EsmtpTransport.php‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class EsmtpTransport extends SmtpTransport
3131
private $password = '';
3232
private $authMode;
3333

34-
public function __construct(string $host = 'localhost', int $port = 25, string $encryption = null, string $authMode = null, EventDispatcherInterface $dispatcher = null, LoggerInterface $logger = null)
34+
public function __construct(string $host = 'localhost', int $port = 0, bool $tls = null, string $authMode = null, EventDispatcherInterface $dispatcher = null, LoggerInterface $logger = null)
3535
{
3636
parent::__construct(null, $dispatcher, $logger);
3737

@@ -44,11 +44,23 @@ public function __construct(string $host = 'localhost', int $port = 25, string $
4444

4545
/** @var SocketStream $stream */
4646
$stream = $this->getStream();
47+
48+
if (null === $tls) {
49+
if (465 === $port) {
50+
$tls = true;
51+
} else {
52+
$tls = \defined('OPENSSL_VERSION_NUMBER') && 0 === $port && 'localhost' !== $host;
53+
}
54+
}
55+
if (!$tls) {
56+
$stream->disableTls();
57+
}
58+
if (0 === $port) {
59+
$port = $tls ? 465 : 25;
60+
}
61+
4762
$stream->setHost($host);
4863
$stream->setPort($port);
49-
if (null !== $encryption) {
50-
$stream->setEncryption($encryption);
51-
}
5264
if (null !== $authMode) {
5365
$this->setAuthMode($authMode);
5466
}
@@ -105,13 +117,15 @@ protected function doHeloCommand(): void
105117
return;
106118
}
107119

120+
$capabilities = $this->getCapabilities($response);
121+
108122
/** @var SocketStream $stream */
109123
$stream = $this->getStream();
110-
if ($stream->isTLS()) {
124+
if (!$stream->isTLS() && \defined('OPENSSL_VERSION_NUMBER') && \array_key_exists('STARTTLS', $capabilities)) {
111125
$this->executeCommand("STARTTLS\r\n", [220]);
112126

113127
if (!$stream->startTLS()) {
114-
throw new TransportException('Unable to connect with TLS encryption.');
128+
throw new TransportException('Unable to connect with STARTTLS.');
115129
}
116130

117131
try {
@@ -123,7 +137,6 @@ protected function doHeloCommand(): void
123137
}
124138
}
125139

126-
$capabilities = $this->getCapabilities($response);
127140
if (\array_key_exists('AUTH', $capabilities)) {
128141
$this->handleAuth($capabilities['AUTH']);
129142
}

‎Transport/Smtp/EsmtpTransportFactory.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,12 @@ final class EsmtpTransportFactory extends AbstractTransportFactory
2222
{
2323
public function create(Dsn $dsn): TransportInterface
2424
{
25-
$encryption = $dsn->getOption('encryption');
25+
$tls = 'smtps' === $dsn->getScheme() ? true : null;
2626
$authMode = $dsn->getOption('auth_mode');
27-
$port = $dsn->getPort(25);
27+
$port = $dsn->getPort(0);
2828
$host = $dsn->getHost();
2929

30-
$transport = new EsmtpTransport($host, $port, $encryption, $authMode, $this->dispatcher, $this->logger);
30+
$transport = new EsmtpTransport($host, $port, $tls, $authMode, $this->dispatcher, $this->logger);
3131

3232
if ($user = $dsn->getUser()) {
3333
$transport->setUsername($user);
@@ -42,6 +42,6 @@ public function create(Dsn $dsn): TransportInterface
4242

4343
public function supports(Dsn $dsn): bool
4444
{
45-
return 'smtp' === $dsn->getScheme();
45+
return 'smtp' === $dsn->getScheme() || 'smtps' === $dsn->getScheme();
4646
}
4747
}

‎Transport/Smtp/SmtpTransport.php‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,13 @@ public function send(RawMessage $message, SmtpEnvelope $envelope = null): ?SentM
129129
public function getName(): string
130130
{
131131
if ($this->stream instanceof SocketStream) {
132-
return sprintf('smtp://%s:%d', $this->stream->getHost(), $this->stream->getPort());
132+
$name = sprintf('smtp%s://%s', ($tls = $this->stream->isTLS()) ? 's' : '', $this->stream->getHost());
133+
$port = $this->stream->getPort();
134+
if (!(25 === $port || ($tls && 465 === $port))) {
135+
$name .= ':'.$port;
136+
}
137+
138+
return $name;
133139
}
134140

135141
return sprintf('smtp://sendmail');

‎Transport/Smtp/Stream/SocketStream.php‎

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,9 @@ final class SocketStream extends AbstractStream
2525
{
2626
private $url;
2727
private $host = 'localhost';
28-
private $protocol = 'tcp';
29-
private $port = 25;
28+
private $port = 465;
3029
private $timeout = 15;
31-
private $tls = false;
30+
private $tls = true;
3231
private $sourceIp;
3332
private $streamContextOptions = [];
3433

@@ -72,18 +71,11 @@ public function getPort(): int
7271
}
7372

7473
/**
75-
* Sets the encryption type (tls or ssl).
74+
* Sets the TLS/SSL on the socket (disables STARTTLS).
7675
*/
77-
public function setEncryption(string $encryption): self
78-
{
79-
$encryption = strtolower($encryption);
80-
if ('tls' === $encryption) {
81-
$this->protocol = 'tcp';
82-
$this->tls = true;
83-
} else {
84-
$this->protocol = $encryption;
85-
$this->tls = false;
86-
}
76+
public function disableTls(): self
77+
{
78+
$this->tls = false;
8779

8880
return $this;
8981
}
@@ -128,8 +120,8 @@ public function getSourceIp(): ?string
128120
public function initialize(): void
129121
{
130122
$this->url = $this->host.':'.$this->port;
131-
if ($this->protocol) {
132-
$this->url = $this->protocol.'://'.$this->url;
123+
if ($this->tls) {
124+
$this->url = 'ssl://'.$this->url;
133125
}
134126
$options = [];
135127
if ($this->sourceIp) {
@@ -138,9 +130,8 @@ public function initialize(): void
138130
if ($this->streamContextOptions) {
139131
$options = array_merge($options, $this->streamContextOptions);
140132
}
141-
if ($this->isTLS()) {
142-
$options['ssl']['crypto_method'] = $options['ssl']['crypto_method'] ?? STREAM_CRYPTO_METHOD_TLS_CLIENT | STREAM_CRYPTO_METHOD_TLSv1_2_CLIENT | STREAM_CRYPTO_METHOD_TLSv1_1_CLIENT;
143-
}
133+
// do it unconditionnally as it will be used by STARTTLS as well if supported
134+
$options['ssl']['crypto_method'] = $options['ssl']['crypto_method'] ?? STREAM_CRYPTO_METHOD_TLS_CLIENT | STREAM_CRYPTO_METHOD_TLSv1_2_CLIENT | STREAM_CRYPTO_METHOD_TLSv1_1_CLIENT;
144135
$streamContext = stream_context_create($options);
145136

146137
set_error_handler(function ($type, $msg) {

0 commit comments

Comments
 (0)