Skip to content

Commit 493baf6

Browse files
fabpotfabpot
committed
Merge branch '7.1' into 7.2
* 7.1: [DoctrineBridge] Revert deprecating by-{id} mapping of entities Add missing UPGRADE notes for 7.1 [Mailer] Fix sendmail transport failure handling and interactive mode [Security] reviewed Romanian translation of key 20
2 parents cb08cb2 + 1528f3f commit 493baf6

File tree

4 files changed

+105
-21
lines changed

4 files changed

+105
-21
lines changed

‎Tests/Transport/Fixtures/fake-failing-sendmail.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
#!/usr/bin/env php
22
<?php
3+
$argsPath = sys_get_temp_dir().\DIRECTORY_SEPARATOR.'sendmail_args';
4+
5+
file_put_contents($argsPath, implode(' ', $argv));
6+
37
print "Sending failed";
48
exit(42);

‎Tests/Transport/SendmailTransportTest.php‎

Lines changed: 89 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,21 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\Mailer\DelayedEnvelope;
16+
use Symfony\Component\Mailer\Envelope;
1617
use Symfony\Component\Mailer\Exception\TransportException;
18+
use Symfony\Component\Mailer\SentMessage;
1719
use Symfony\Component\Mailer\Transport\SendmailTransport;
20+
use Symfony\Component\Mailer\Transport\Smtp\Stream\ProcessStream;
21+
use Symfony\Component\Mailer\Transport\TransportInterface;
1822
use Symfony\Component\Mime\Address;
1923
use Symfony\Component\Mime\Email;
24+
use Symfony\Component\Mime\RawMessage;
2025

2126
class SendmailTransportTest extends TestCase
2227
{
2328
private const FAKE_SENDMAIL = __DIR__.'/Fixtures/fake-sendmail.php -t';
2429
private const FAKE_FAILING_SENDMAIL = __DIR__.'/Fixtures/fake-failing-sendmail.php -t';
30+
private const FAKE_INTERACTIVE_SENDMAIL = __DIR__.'/Fixtures/fake-failing-sendmail.php -bs';
2531

2632
private string $argsPath;
2733

@@ -46,9 +52,7 @@ public function testToString()
4652

4753
public function testToIsUsedWhenRecipientsAreNotSet()
4854
{
49-
if ('\\' === \DIRECTORY_SEPARATOR) {
50-
$this->markTestSkipped('Windows does not support shebangs nor non-blocking standard streams');
51-
}
55+
$this->skipOnWindows();
5256

5357
$mail = new Email();
5458
$mail
@@ -68,20 +72,9 @@ public function testToIsUsedWhenRecipientsAreNotSet()
6872

6973
public function testRecipientsAreUsedWhenSet()
7074
{
71-
if ('\\' === \DIRECTORY_SEPARATOR) {
72-
$this->markTestSkipped('Windows does not support shebangs nor non-blocking standard streams');
73-
}
75+
$this->skipOnWindows();
7476

75-
$mail = new Email();
76-
$mail
77-
->from('from@mail.com')
78-
->to('to@mail.com')
79-
->subject('Subject')
80-
->text('Some text')
81-
;
82-
83-
$envelope = new DelayedEnvelope($mail);
84-
$envelope->setRecipients([new Address('recipient@mail.com')]);
77+
[$mail, $envelope] = $this->defaultMailAndEnvelope();
8578

8679
$sendmailTransport = new SendmailTransport(self::FAKE_SENDMAIL);
8780
$sendmailTransport->send($mail, $envelope);
@@ -90,11 +83,90 @@ public function testRecipientsAreUsedWhenSet()
9083
}
9184

9285
public function testThrowsTransportExceptionOnFailure()
86+
{
87+
$this->skipOnWindows();
88+
89+
[$mail, $envelope] = $this->defaultMailAndEnvelope();
90+
91+
$sendmailTransport = new SendmailTransport(self::FAKE_FAILING_SENDMAIL);
92+
$this->expectException(TransportException::class);
93+
$this->expectExceptionMessage('Process failed with exit code 42: Sending failed');
94+
$sendmailTransport->send($mail, $envelope);
95+
96+
$streamProperty = new \ReflectionProperty(SendmailTransport::class, 'stream');
97+
$streamProperty->setAccessible(true);
98+
$stream = $streamProperty->getValue($sendmailTransport);
99+
$this->assertNull($stream->stream);
100+
}
101+
102+
public function testStreamIsClearedOnFailure()
103+
{
104+
$this->skipOnWindows();
105+
106+
[$mail, $envelope] = $this->defaultMailAndEnvelope();
107+
108+
$sendmailTransport = new SendmailTransport(self::FAKE_FAILING_SENDMAIL);
109+
try {
110+
$sendmailTransport->send($mail, $envelope);
111+
} catch (TransportException $e) {
112+
}
113+
114+
$streamProperty = new \ReflectionProperty(SendmailTransport::class, 'stream');
115+
$streamProperty->setAccessible(true);
116+
$stream = $streamProperty->getValue($sendmailTransport);
117+
$innerStreamProperty = new \ReflectionProperty(ProcessStream::class, 'stream');
118+
$innerStreamProperty->setAccessible(true);
119+
$this->assertNull($innerStreamProperty->getValue($stream));
120+
}
121+
122+
public function testDoesNotThrowWhenInteractive()
123+
{
124+
$this->skipOnWindows();
125+
126+
[$mail, $envelope] = $this->defaultMailAndEnvelope();
127+
128+
$sendmailTransport = new SendmailTransport(self::FAKE_INTERACTIVE_SENDMAIL);
129+
$transportProperty = new \ReflectionProperty(SendmailTransport::class, 'transport');
130+
$transportProperty->setAccessible(true);
131+
132+
// Replace the transport with an anonymous consumer that trigger the stream methods
133+
$transportProperty->setValue($sendmailTransport, new class($transportProperty->getValue($sendmailTransport)->getStream()) implements TransportInterface {
134+
private $stream;
135+
136+
public function __construct(ProcessStream $stream)
137+
{
138+
$this->stream = $stream;
139+
}
140+
141+
public function send(RawMessage $message, ?Envelope $envelope = null): ?SentMessage
142+
{
143+
$this->stream->initialize();
144+
$this->stream->write('SMTP');
145+
$this->stream->terminate();
146+
147+
return new SentMessage($message, $envelope);
148+
}
149+
150+
public function __toString(): string
151+
{
152+
return 'Interactive mode test';
153+
}
154+
});
155+
156+
$sendmailTransport->send($mail, $envelope);
157+
158+
$this->assertStringEqualsFile($this->argsPath, __DIR__.'/Fixtures/fake-failing-sendmail.php -bs');
159+
}
160+
161+
private function skipOnWindows()
93162
{
94163
if ('\\' === \DIRECTORY_SEPARATOR) {
95164
$this->markTestSkipped('Windows does not support shebangs nor non-blocking standard streams');
96165
}
166+
}
97167

168+
private function defaultMailAndEnvelope(): array
169+
{
98170
$mail = new Email();
99171
$mail
100172
->from('from@mail.com')
@@ -106,9 +178,6 @@ public function testThrowsTransportExceptionOnFailure()
106178
$envelope = new DelayedEnvelope($mail);
107179
$envelope->setRecipients([new Address('recipient@mail.com')]);
108180

109-
$sendmailTransport = new SendmailTransport(self::FAKE_FAILING_SENDMAIL);
110-
$this->expectException(TransportException::class);
111-
$this->expectExceptionMessage('Process failed with exit code 42: Sending failed');
112-
$sendmailTransport->send($mail, $envelope);
181+
return [$mail, $envelope];
113182
}
114183
}

‎Transport/SendmailTransport.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ public function __construct(?string $command = null, ?EventDispatcherInterface $
6464
$this->stream = new ProcessStream();
6565
if (str_contains($this->command, ' -bs')) {
6666
$this->stream->setCommand($this->command);
67+
$this->stream->setInteractive(true);
6768
$this->transport = new SmtpTransport($this->stream, $dispatcher, $logger);
6869
}
6970
}

‎Transport/Smtp/Stream/ProcessStream.php‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,18 @@
2424
final class ProcessStream extends AbstractStream
2525
{
2626
private string $command;
27+
private bool $interactive = false;
2728

2829
public function setCommand(string $command): void
2930
{
3031
$this->command = $command;
3132
}
3233

34+
public function setInteractive(bool $interactive)
35+
{
36+
$this->interactive = $interactive;
37+
}
38+
3339
public function initialize(): void
3440
{
3541
$descriptorSpec = [
@@ -57,11 +63,15 @@ public function terminate(): void
5763
$err = stream_get_contents($this->err);
5864
fclose($this->err);
5965
if (0 !== $exitCode = proc_close($this->stream)) {
60-
throw new TransportException('Process failed with exit code '.$exitCode.': '.$out.$err);
66+
$errorMessage = 'Process failed with exit code '.$exitCode.': '.$out.$err;
6167
}
6268
}
6369

6470
parent::terminate();
71+
72+
if (!$this->interactive && isset($errorMessage)) {
73+
throw new TransportException($errorMessage);
74+
}
6575
}
6676

6777
protected function getReadConnectionDescription(): string

0 commit comments

Comments
 (0)