bug symfony#54239 [Mailer] Fix sendmail transport not handling failure (aboks)

This PR was squashed before being merged into the 5.4 branch.

Discussion
----------

[Mailer] Fix sendmail transport not handling failure

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  |no
| Deprecations? | no
| Issues        |
| License       | MIT

I found out that the Mailer component `SendmailTransport` does not detect all failures. The `ProcessStream` checks stderr when the process is started, but it does not check the exit code and output afterwards. This could lead to silent failures.

We found this out due to having `sendmail_path = "/usr/bin/msmtp -t --read-envelope-from"` in `php.ini`, but `msmtp` does not like `--read-envelope-from` in combination with the `-f` option added by Symfony Mailer. In this case, `msmtp` fails with exit status 64 and a message on stdout (rather than stderr). The latter is the reason why I also added the stdout output to the error message (along with the stderr output).

Feel free to modify this PR to bring it more in line with existing standards and conventions. The most important part to me is that a failure like this is detected and the mailer does not silently fail.

Commits
-------

684e7af [Mailer] Fix sendmail transport not handling failure

Author: nicolas-grekas

src/Symfony/Component/Mailer/Tests/Transport/Fixtures/fake-failing-sendmail.php

@@ -0,0 +1,4 @@
+#!/usr/bin/env php
+<?php
+print "Sending failed";
+exit(42);

src/Symfony/Component/Mailer/Tests/Transport/SendmailTransportTest.php

@@ -13,13 +13,15 @@
 
 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;
 
 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 [email protected] [email protected]');
     }
+
+    public function testThrowsTransportExceptionOnFailure()
+    {
+        if ('\\' === \DIRECTORY_SEPARATOR) {
+            $this->markTestSkipped('Windows does not support shebangs nor non-blocking standard streams');
+        }
+
+        $mail = new Email();
+        $mail
+            ->from('[email protected]')
+            ->to('[email protected]')
+            ->subject('Subject')
+            ->text('Some text')
+        ;
+
+        $envelope = new DelayedEnvelope($mail);
+        $envelope->setRecipients([new Address('[email protected]')]);
+
+        $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);
+    }
 }

src/Symfony/Component/Mailer/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

src/Symfony/Component/Mailer/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();