From 1c394713747b3ecb2433d9cac9b78d123051b2a3 Mon Sep 17 00:00:00 2001 From: Bob van de Vijver Date: Thu, 11 Apr 2024 15:38:30 +0200 Subject: [PATCH] [Mailer] Fix sendmail transport failure handling and interactive mode --- .../Fixtures/fake-failing-sendmail.php | 4 + Tests/Transport/SendmailTransportTest.php | 109 ++++++++++++++---- Transport/SendmailTransport.php | 1 + Transport/Smtp/Stream/ProcessStream.php | 13 ++- 4 files changed, 106 insertions(+), 21 deletions(-) diff --git a/Tests/Transport/Fixtures/fake-failing-sendmail.php b/Tests/Transport/Fixtures/fake-failing-sendmail.php index 920b980..1ce9872 100755 --- a/Tests/Transport/Fixtures/fake-failing-sendmail.php +++ b/Tests/Transport/Fixtures/fake-failing-sendmail.php @@ -1,4 +1,8 @@ #!/usr/bin/env php markTestSkipped('Windows does not support shebangs nor non-blocking standard streams'); - } + $this->skipOnWindows(); $mail = new Email(); $mail @@ -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); @@ -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') @@ -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]; } } diff --git a/Transport/SendmailTransport.php b/Transport/SendmailTransport.php index 22aea4e..712016b 100644 --- a/Transport/SendmailTransport.php +++ b/Transport/SendmailTransport.php @@ -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); } } diff --git a/Transport/Smtp/Stream/ProcessStream.php b/Transport/Smtp/Stream/ProcessStream.php index 808d9eb..d717055 100644 --- a/Transport/Smtp/Stream/ProcessStream.php +++ b/Transport/Smtp/Stream/ProcessStream.php @@ -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 = [ @@ -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