Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 864df6f

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
bug #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
2 parents 0a7825b + 684e7af commit 864df6f
Copy full SHA for 864df6f

File tree

4 files changed

+38
-2
lines changed
Filter options

4 files changed

+38
-2
lines changed

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

Copy file name to clipboard
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#!/usr/bin/env php
2+
<?php
3+
print "Sending failed";
4+
exit(42);

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Mailer/Tests/Transport/SendmailTransportTest.php
+25Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,15 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\Mailer\DelayedEnvelope;
16+
use Symfony\Component\Mailer\Exception\TransportException;
1617
use Symfony\Component\Mailer\Transport\SendmailTransport;
1718
use Symfony\Component\Mime\Address;
1819
use Symfony\Component\Mime\Email;
1920

2021
class SendmailTransportTest extends TestCase
2122
{
2223
private const FAKE_SENDMAIL = __DIR__.'/Fixtures/fake-sendmail.php -t';
24+
private const FAKE_FAILING_SENDMAIL = __DIR__.'/Fixtures/fake-failing-sendmail.php -t';
2325

2426
/**
2527
* @var string
@@ -89,4 +91,27 @@ public function testRecipientsAreUsedWhenSet()
8991

9092
$this->assertStringEqualsFile($this->argsPath, __DIR__.'/Fixtures/fake-sendmail.php -ffrom@mail.com recipient@mail.com');
9193
}
94+
95+
public function testThrowsTransportExceptionOnFailure()
96+
{
97+
if ('\\' === \DIRECTORY_SEPARATOR) {
98+
$this->markTestSkipped('Windows does not support shebangs nor non-blocking standard streams');
99+
}
100+
101+
$mail = new Email();
102+
$mail
103+
->from('from@mail.com')
104+
->to('to@mail.com')
105+
->subject('Subject')
106+
->text('Some text')
107+
;
108+
109+
$envelope = new DelayedEnvelope($mail);
110+
$envelope->setRecipients([new Address('recipient@mail.com')]);
111+
112+
$sendmailTransport = new SendmailTransport(self::FAKE_FAILING_SENDMAIL);
113+
$this->expectException(TransportException::class);
114+
$this->expectExceptionMessage('Process failed with exit code 42: Sending failed');
115+
$sendmailTransport->send($mail, $envelope);
116+
}
92117
}

‎src/Symfony/Component/Mailer/Transport/Smtp/Stream/AbstractStream.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Mailer/Transport/Smtp/Stream/AbstractStream.php
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ abstract class AbstractStream
2727
protected $stream;
2828
protected $in;
2929
protected $out;
30+
protected $err;
3031

3132
private $debug = '';
3233

@@ -65,7 +66,7 @@ abstract public function initialize(): void;
6566

6667
public function terminate(): void
6768
{
68-
$this->stream = $this->out = $this->in = null;
69+
$this->stream = $this->err = $this->out = $this->in = null;
6970
}
7071

7172
public function readLine(): string

‎src/Symfony/Component/Mailer/Transport/Smtp/Stream/ProcessStream.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Mailer/Transport/Smtp/Stream/ProcessStream.php
+7-1Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,20 @@ public function initialize(): void
4545
}
4646
$this->in = &$pipes[0];
4747
$this->out = &$pipes[1];
48+
$this->err = &$pipes[2];
4849
}
4950

5051
public function terminate(): void
5152
{
5253
if (null !== $this->stream) {
5354
fclose($this->in);
55+
$out = stream_get_contents($this->out);
5456
fclose($this->out);
55-
proc_close($this->stream);
57+
$err = stream_get_contents($this->err);
58+
fclose($this->err);
59+
if (0 !== $exitCode = proc_close($this->stream)) {
60+
throw new TransportException('Process failed with exit code '.$exitCode.': '.$out.$err);
61+
}
5662
}
5763

5864
parent::terminate();

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.