Skip to content

[Mailer] Fix sendmail transport failure handling and interactive mode #54572

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.

Sign up for GitHub

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
May 30, 2024
Merged

[Mailer] Fix sendmail transport failure handling and interactive mode #54572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions src/Symfony/Component/Mailer/Tests/Transport/Fixtures/fake-failing-sendmail.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
#!/usr/bin/env php
<?php
$argsPath = sys_get_temp_dir().\DIRECTORY_SEPARATOR.'sendmail_args';

file_put_contents($argsPath, implode(' ', $argv));

print "Sending failed";
exit(42);
109 changes: 89 additions & 20 deletions src/Symfony/Component/Mailer/Tests/Transport/SendmailTransportTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,21 @@

use PHPUnit\Framework\TestCase;
use Symfony\Component\Mailer\DelayedEnvelope;
use Symfony\Component\Mailer\Envelope;
use Symfony\Component\Mailer\Exception\TransportException;
use Symfony\Component\Mailer\SentMessage;
use Symfony\Component\Mailer\Transport\SendmailTransport;
use Symfony\Component\Mailer\Transport\Smtp\Stream\ProcessStream;
use Symfony\Component\Mailer\Transport\TransportInterface;
use Symfony\Component\Mime\Address;
use Symfony\Component\Mime\Email;
use Symfony\Component\Mime\RawMessage;

class SendmailTransportTest extends TestCase
{
private const FAKE_SENDMAIL = __DIR__.'/Fixtures/fake-sendmail.php -t';
private const FAKE_FAILING_SENDMAIL = __DIR__.'/Fixtures/fake-failing-sendmail.php -t';
private const FAKE_INTERACTIVE_SENDMAIL = __DIR__.'/Fixtures/fake-failing-sendmail.php -bs';

/**
* @var string
Expand Down Expand Up @@ -49,9 +55,7 @@ public function testToString()

public function testToIsUsedWhenRecipientsAreNotSet()
{
if ('\\' === \DIRECTORY_SEPARATOR) {
$this->markTestSkipped('Windows does not support shebangs nor non-blocking standard streams');
}
$this->skipOnWindows();

$mail = new Email();
$mail
Expand All @@ -71,20 +75,9 @@ public function testToIsUsedWhenRecipientsAreNotSet()

public function testRecipientsAreUsedWhenSet()
{
if ('\\' === \DIRECTORY_SEPARATOR) {
$this->markTestSkipped('Windows does not support shebangs nor non-blocking standard streams');
}
$this->skipOnWindows();

$mail = new Email();
$mail
->from('from@mail.com')
->to('to@mail.com')
->subject('Subject')
->text('Some text')
;

$envelope = new DelayedEnvelope($mail);
$envelope->setRecipients([new Address('recipient@mail.com')]);
[$mail, $envelope] = $this->defaultMailAndEnvelope();

$sendmailTransport = new SendmailTransport(self::FAKE_SENDMAIL);
$sendmailTransport->send($mail, $envelope);
Expand All @@ -93,11 +86,90 @@ public function testRecipientsAreUsedWhenSet()
}

public function testThrowsTransportExceptionOnFailure()
{
$this->skipOnWindows();

[$mail, $envelope] = $this->defaultMailAndEnvelope();

$sendmailTransport = new SendmailTransport(self::FAKE_FAILING_SENDMAIL);
$this->expectException(TransportException::class);
$this->expectExceptionMessage('Process failed with exit code 42: Sending failed');
$sendmailTransport->send($mail, $envelope);

$streamProperty = new \ReflectionProperty(SendmailTransport::class, 'stream');
$streamProperty->setAccessible(true);
$stream = $streamProperty->getValue($sendmailTransport);
$this->assertNull($stream->stream);
}

public function testStreamIsClearedOnFailure()
{
$this->skipOnWindows();

[$mail, $envelope] = $this->defaultMailAndEnvelope();

$sendmailTransport = new SendmailTransport(self::FAKE_FAILING_SENDMAIL);
try {
$sendmailTransport->send($mail, $envelope);
} catch (TransportException $e) {
}

$streamProperty = new \ReflectionProperty(SendmailTransport::class, 'stream');
$streamProperty->setAccessible(true);
$stream = $streamProperty->getValue($sendmailTransport);
$innerStreamProperty = new \ReflectionProperty(ProcessStream::class, 'stream');
$innerStreamProperty->setAccessible(true);
$this->assertNull($innerStreamProperty->getValue($stream));
}

public function testDoesNotThrowWhenInteractive()
{
$this->skipOnWindows();

[$mail, $envelope] = $this->defaultMailAndEnvelope();

$sendmailTransport = new SendmailTransport(self::FAKE_INTERACTIVE_SENDMAIL);
$transportProperty = new \ReflectionProperty(SendmailTransport::class, 'transport');
$transportProperty->setAccessible(true);

// Replace the transport with an anonymous consumer that trigger the stream methods
$transportProperty->setValue($sendmailTransport, new class($transportProperty->getValue($sendmailTransport)->getStream()) implements TransportInterface {
private $stream;

public function __construct(ProcessStream $stream)
{
$this->stream = $stream;
}

public function send(RawMessage $message, ?Envelope $envelope = null): ?SentMessage
{
$this->stream->initialize();
$this->stream->write('SMTP');
$this->stream->terminate();

return new SentMessage($message, $envelope);
}

public function __toString(): string
{
return 'Interactive mode test';
}
});

$sendmailTransport->send($mail, $envelope);

$this->assertStringEqualsFile($this->argsPath, __DIR__.'/Fixtures/fake-failing-sendmail.php -bs');
}

private function skipOnWindows()
{
if ('\\' === \DIRECTORY_SEPARATOR) {
$this->markTestSkipped('Windows does not support shebangs nor non-blocking standard streams');
}
}

private function defaultMailAndEnvelope(): array
{
$mail = new Email();
$mail
->from('from@mail.com')
Expand All @@ -109,9 +181,6 @@ public function testThrowsTransportExceptionOnFailure()
$envelope = new DelayedEnvelope($mail);
$envelope->setRecipients([new Address('recipient@mail.com')]);

$sendmailTransport = new SendmailTransport(self::FAKE_FAILING_SENDMAIL);
$this->expectException(TransportException::class);
$this->expectExceptionMessage('Process failed with exit code 42: Sending failed');
$sendmailTransport->send($mail, $envelope);
return [$mail, $envelope];
}
}
1 change: 1 addition & 0 deletions src/Symfony/Component/Mailer/Transport/SendmailTransport.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ public function __construct(?string $command = null, ?EventDispatcherInterface $
$this->stream = new ProcessStream();
if (str_contains($this->command, ' -bs')) {
$this->stream->setCommand($this->command);
$this->stream->setInteractive(true);
$this->transport = new SmtpTransport($this->stream, $dispatcher, $logger);
}
}
Expand Down
13 changes: 12 additions & 1 deletion src/Symfony/Component/Mailer/Transport/Smtp/Stream/ProcessStream.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,18 @@ final class ProcessStream extends AbstractStream
{
private $command;

private $interactive = false;

public function setCommand(string $command)
{
$this->command = $command;
}

public function setInteractive(bool $interactive)
{
$this->interactive = $interactive;
}

public function initialize(): void
{
$descriptorSpec = [
Expand Down Expand Up @@ -57,11 +64,15 @@ public function terminate(): void
$err = stream_get_contents($this->err);
fclose($this->err);
if (0 !== $exitCode = proc_close($this->stream)) {
throw new TransportException('Process failed with exit code '.$exitCode.': '.$out.$err);
$errorMessage = 'Process failed with exit code '.$exitCode.': '.$out.$err;
}
}

parent::terminate();

if (!$this->interactive && isset($errorMessage)) {
throw new TransportException($errorMessage);
}
}

protected function getReadConnectionDescription(): string
Expand Down
Loading