From b8ff9a7ce05a532523ac1535ed128250f18f57d0 Mon Sep 17 00:00:00 2001
From: Arnout Boks <arnoutboks@gmail.com>
Date: Mon, 11 Mar 2024 15:45:10 +0100
Subject: [PATCH] [Mailer] Fix sendmail transport not handling failure

---
 .../Fixtures/fake-failing-sendmail.php        |  4 +++
 Tests/Transport/SendmailTransportTest.php     | 25 +++++++++++++++++++
 Transport/Smtp/Stream/AbstractStream.php      |  3 ++-
 Transport/Smtp/Stream/ProcessStream.php       |  8 +++++-
 4 files changed, 38 insertions(+), 2 deletions(-)
 create mode 100755 Tests/Transport/Fixtures/fake-failing-sendmail.php

diff --git a/Tests/Transport/Fixtures/fake-failing-sendmail.php b/Tests/Transport/Fixtures/fake-failing-sendmail.php
new file mode 100755
index 00000000..920b980e
--- /dev/null
+++ b/Tests/Transport/Fixtures/fake-failing-sendmail.php
@@ -0,0 +1,4 @@
+#!/usr/bin/env php
+<?php
+print "Sending failed";
+exit(42);
diff --git a/Tests/Transport/SendmailTransportTest.php b/Tests/Transport/SendmailTransportTest.php
index d0c6f186..e452934c 100644
--- a/Tests/Transport/SendmailTransportTest.php
+++ b/Tests/Transport/SendmailTransportTest.php
@@ -13,6 +13,7 @@
 
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Mailer\DelayedEnvelope;
+use Symfony\Component\Mailer\Exception\TransportException;
 use Symfony\Component\Mailer\Transport\SendmailTransport;
 use Symfony\Component\Mime\Address;
 use Symfony\Component\Mime\Email;
@@ -20,6 +21,7 @@
 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';
 
     /**
      * @var string
@@ -89,4 +91,27 @@ public function testRecipientsAreUsedWhenSet()
 
         $this->assertStringEqualsFile($this->argsPath, __DIR__.'/Fixtures/fake-sendmail.php -ffrom@mail.com recipient@mail.com');
     }
+
+    public function testThrowsTransportExceptionOnFailure()
+    {
+        if ('\\' === \DIRECTORY_SEPARATOR) {
+            $this->markTestSkipped('Windows does not support shebangs nor non-blocking standard streams');
+        }
+
+        $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')]);
+
+        $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);
+    }
 }
diff --git a/Transport/Smtp/Stream/AbstractStream.php b/Transport/Smtp/Stream/AbstractStream.php
index c53f8b54..2be8fce9 100644
--- a/Transport/Smtp/Stream/AbstractStream.php
+++ b/Transport/Smtp/Stream/AbstractStream.php
@@ -27,6 +27,7 @@ abstract class AbstractStream
     protected $stream;
     protected $in;
     protected $out;
+    protected $err;
 
     private $debug = '';
 
@@ -65,7 +66,7 @@ abstract public function initialize(): void;
 
     public function terminate(): void
     {
-        $this->stream = $this->out = $this->in = null;
+        $this->stream = $this->err = $this->out = $this->in = null;
     }
 
     public function readLine(): string
diff --git a/Transport/Smtp/Stream/ProcessStream.php b/Transport/Smtp/Stream/ProcessStream.php
index bc721ad0..808d9eb5 100644
--- a/Transport/Smtp/Stream/ProcessStream.php
+++ b/Transport/Smtp/Stream/ProcessStream.php
@@ -45,14 +45,20 @@ public function initialize(): void
         }
         $this->in = &$pipes[0];
         $this->out = &$pipes[1];
+        $this->err = &$pipes[2];
     }
 
     public function terminate(): void
     {
         if (null !== $this->stream) {
             fclose($this->in);
+            $out = stream_get_contents($this->out);
             fclose($this->out);
-            proc_close($this->stream);
+            $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);
+            }
         }
 
         parent::terminate();