Skip to content

[Mailer] Improve extensibility of EsmtpTransport #44446

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

Merged
merged 1 commit into from
Apr 1, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Symfony/Component/Mailer/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ CHANGELOG
---

* Make `start()` and `stop()` methods public on `SmtpTransport`
* Improve extensibility of `EsmtpTransport`

6.0
---
Expand Down
110 changes: 110 additions & 0 deletions src/Symfony/Component/Mailer/Tests/Transport/Smtp/DummyStream.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Mailer\Tests\Transport\Smtp;

use Symfony\Component\Mailer\Exception\TransportException;
use Symfony\Component\Mailer\Transport\Smtp\Stream\AbstractStream;

class DummyStream extends AbstractStream
{
private string $nextResponse;
private array $commands = [];
private bool $closed = true;

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

public function disableTls(): static
{
return $this;
}

public function isTLS(): bool
{
return false;
}

public function setHost(string $host): static
{
return $this;
}

public function setPort(int $port): static
{
return $this;
}

public function write(string $bytes, $debug = true): void
{
if ($this->closed) {
throw new TransportException('Unable to write bytes on the wire.');
}

$this->commands[] = $bytes;

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

public function readLine(): string
{
return $this->nextResponse."\r\n";
}

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';
}

public function close(): void
{
$this->closed = true;
}

public function isClosed(): bool
{
return $this->closed;
}

public function terminate(): void
{
parent::terminate();
$this->closed = true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use PHPUnit\Framework\TestCase;
use Symfony\Component\Mailer\Transport\Smtp\EsmtpTransport;
use Symfony\Component\Mime\Email;

class EsmtpTransportTest extends TestCase
{
Expand Down Expand Up @@ -40,4 +41,40 @@ public function testToString()
$t = new EsmtpTransport('example.com', 466, true);
$this->assertEquals('smtps://example.com:466', (string) $t);
}

public function testExtensibility()
{
$stream = new DummyStream();
$transport = new CustomEsmtpTransport(stream: $stream);

$message = new Email();
$message->from('sender@example.org');
$message->addTo('recipient@example.org');
$message->text('.');

$transport->send($message);

$this->assertContains("MAIL FROM:<sender@example.org> RET=HDRS\r\n", $stream->getCommands());
$this->assertContains("RCPT TO:<recipient@example.org> NOTIFY=FAILURE\r\n", $stream->getCommands());
}
}

class CustomEsmtpTransport extends EsmtpTransport
{
public function executeCommand(string $command, array $codes): string
{
$command = match (true) {
str_starts_with($command, 'MAIL FROM:') && isset($this->getCapabilities()['DSN']) => substr_replace($command, ' RET=HDRS', -2, 0),
str_starts_with($command, 'RCPT TO:') && isset($this->getCapabilities()['DSN']) => substr_replace($command, ' NOTIFY=FAILURE', -2, 0),
default => $command,
};

$response = parent::executeCommand($command, $codes);

if (str_starts_with($command, 'EHLO ')) {
$response .= "250 DSN\r\n";
}

return $response;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use Symfony\Component\Mailer\Envelope;
use Symfony\Component\Mailer\Exception\TransportException;
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;
Expand Down Expand Up @@ -147,87 +146,3 @@ public function testStop()
$this->assertTrue($stream->isClosed());
}
}

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

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

/**
* @var bool
*/
private $closed = true;

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

public function write(string $bytes, $debug = true): void
{
if ($this->closed) {
throw new TransportException('Unable to write bytes on the wire.');
}

$this->commands[] = $bytes;

if (str_starts_with($bytes, 'DATA')) {
$this->nextResponse = '354 Enter message, ending with "." on a line by itself';
} elseif (str_starts_with($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';
}

public function close(): void
{
$this->closed = true;
}

public function isClosed(): bool
{
return $this->closed;
}

public function terminate(): void
{
parent::terminate();
$this->closed = true;
}
}
40 changes: 25 additions & 15 deletions src/Symfony/Component/Mailer/Transport/Smtp/EsmtpTransport.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Symfony\Component\Mailer\Exception\TransportException;
use Symfony\Component\Mailer\Exception\TransportExceptionInterface;
use Symfony\Component\Mailer\Transport\Smtp\Auth\AuthenticatorInterface;
use Symfony\Component\Mailer\Transport\Smtp\Stream\AbstractStream;
use Symfony\Component\Mailer\Transport\Smtp\Stream\SocketStream;

/**
Expand All @@ -29,10 +30,11 @@ class EsmtpTransport extends SmtpTransport
private array $authenticators = [];
private string $username = '';
private string $password = '';
private array $capabilities;

public function __construct(string $host = 'localhost', int $port = 0, bool $tls = null, EventDispatcherInterface $dispatcher = null, LoggerInterface $logger = null)
public function __construct(string $host = 'localhost', int $port = 0, bool $tls = null, EventDispatcherInterface $dispatcher = null, LoggerInterface $logger = null, AbstractStream $stream = null)
{
parent::__construct(null, $dispatcher, $logger);
parent::__construct($stream, $dispatcher, $logger);

// order is important here (roughly most secure and popular first)
$this->authenticators = [
Expand Down Expand Up @@ -98,24 +100,32 @@ public function addAuthenticator(AuthenticatorInterface $authenticator): void
$this->authenticators[] = $authenticator;
}

protected function doHeloCommand(): void
public function executeCommand(string $command, array $codes): string
{
return [250] === $codes && str_starts_with($command, 'HELO ') ? $this->doEhloCommand() : parent::executeCommand($command, $codes);
}

final protected function getCapabilities(): array
{
return $this->capabilities;
}

private function doEhloCommand(): string
{
try {
$response = $this->executeCommand(sprintf("EHLO %s\r\n", $this->getLocalDomain()), [250]);
} catch (TransportExceptionInterface $e) {
parent::doHeloCommand();

return;
return parent::executeCommand(sprintf("HELO %s\r\n", $this->getLocalDomain()), [250]);
}

$capabilities = $this->getCapabilities($response);
$this->capabilities = $this->parseCapabilities($response);

/** @var SocketStream $stream */
$stream = $this->getStream();
// WARNING: !$stream->isTLS() is right, 100% sure :)
// if you think that the ! should be removed, read the code again
// if doing so "fixes" your issue then it probably means your SMTP server behaves incorrectly or is wrongly configured
if (!$stream->isTLS() && \defined('OPENSSL_VERSION_NUMBER') && \array_key_exists('STARTTLS', $capabilities)) {
if (!$stream->isTLS() && \defined('OPENSSL_VERSION_NUMBER') && \array_key_exists('STARTTLS', $this->capabilities)) {
$this->executeCommand("STARTTLS\r\n", [220]);

if (!$stream->startTLS()) {
Expand All @@ -124,20 +134,20 @@ protected function doHeloCommand(): void

try {
$response = $this->executeCommand(sprintf("EHLO %s\r\n", $this->getLocalDomain()), [250]);
$capabilities = $this->getCapabilities($response);
$this->capabilities = $this->parseCapabilities($response);
} catch (TransportExceptionInterface $e) {
parent::doHeloCommand();

return;
return parent::executeCommand(sprintf("HELO %s\r\n", $this->getLocalDomain()), [250]);
}
}

if (\array_key_exists('AUTH', $capabilities)) {
$this->handleAuth($capabilities['AUTH']);
if (\array_key_exists('AUTH', $this->capabilities)) {
$this->handleAuth($this->capabilities['AUTH']);
}

return $response;
}

private function getCapabilities(string $ehloResponse): array
private function parseCapabilities(string $ehloResponse): array
{
$capabilities = [];
$lines = explode("\r\n", trim($ehloResponse));
Expand Down
6 changes: 4 additions & 2 deletions src/Symfony/Component/Mailer/Transport/Smtp/SmtpTransport.php
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,6 @@ public function __toString(): string
* @param int[] $codes
*
* @throws TransportException when an invalid response if received
*
* @internal
*/
public function executeCommand(string $command, array $codes): string
{
Expand Down Expand Up @@ -225,6 +223,10 @@ protected function doSend(SentMessage $message): void
}
}

/**
* @internal since version 6.1, to be made private in 7.0
* @final since version 6.1, to be made private in 7.0
*/
protected function doHeloCommand(): void
{
$this->executeCommand(sprintf("HELO %s\r\n", $this->domain), [250]);
Expand Down