bug symfony#37888 [Mailer] Reorder headers used to determine Sender (cvmiert)

fabpot · fabpot · commit 3c270fbb1f5e · 2020-08-19T19:15:16.000+02:00
This PR was submitted for the master branch but it was merged into the 4.4 branch instead. Discussion ---------- [Mailer] Reorder headers used to determine Sender | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | N/A | License | MIT | Doc PR | N/A Use the `Return-Path` header as the last candidate to determine the Envelope's sender address. The `Return-Path` is usually configured _in addition_ to the `Sender` and/or `From` header: it allows for email bounces and complaints to be sent to a dedicated email address. It should therefor not be used as the first candidate header to determine the sender. Commits ------- 7e60a8f [Mailer] Reorder headers used to determine Sender
diff --git a/src/Symfony/Component/Mailer/DelayedEnvelope.php b/src/Symfony/Component/Mailer/DelayedEnvelope.php
@@ -83,15 +83,15 @@ private static function getRecipientsFromHeaders(Headers $headers): array
 
     private static function getSenderFromHeaders(Headers $headers): Address
     {
-        if ($return = $headers->get('Return-Path')) {
-            return $return->getAddress();
-        }
         if ($sender = $headers->get('Sender')) {
             return $sender->getAddress();
         }
         if ($from = $headers->get('From')) {
             return $from->getAddresses()[0];
         }
+        if ($return = $headers->get('Return-Path')) {
+            return $return->getAddress();
+        }
 
         throw new LogicException('Unable to determine the sender of the message.');
     }
diff --git a/src/Symfony/Component/Mailer/Tests/EnvelopeTest.php b/src/Symfony/Component/Mailer/Tests/EnvelopeTest.php
@@ -81,6 +81,24 @@ public function testSenderFromHeadersWithoutFrom()
         $this->assertEquals($from, $e->getSender());
     }
 
+    public function testSenderFromHeadersWithMulitpleHeaders()
+    {
+        $headers = new Headers();
+        $headers->addMailboxListHeader('From', [$from = new Address('from@symfony.com', 'from'), 'some@symfony.com']);
+        $headers->addPathHeader('Return-Path', $return = new Address('return@symfony.com', 'return'));
+        $headers->addMailboxHeader('Sender', $sender = new Address('sender@symfony.com', 'sender'));
+        $headers->addMailboxListHeader('To', ['to@symfony.com']);
+        $e = Envelope::create(new Message($headers));
+        $this->assertEquals($sender, $e->getSender());
+
+        $headers = new Headers();
+        $headers->addMailboxListHeader('From', [$from = new Address('from@symfony.com', 'from'), 'some@symfony.com']);
+        $headers->addPathHeader('Return-Path', $return = new Address('return@symfony.com', 'return'));
+        $headers->addMailboxListHeader('To', ['to@symfony.com']);
+        $e = Envelope::create(new Message($headers));
+        $this->assertEquals($from, $e->getSender());
+    }
+
     public function testRecipientsFromHeaders()
     {
         $headers = new Headers();

Original file line number	Diff line number	Diff line change
`@@ -83,15 +83,15 @@ private static function getRecipientsFromHeaders(Headers $headers): array`
`83`	`83`
`84`	`84`	`private static function getSenderFromHeaders(Headers $headers): Address`
`85`	`85`	`{`
`86`		`- if ($return = $headers->get('Return-Path')) {`
`87`		`- return $return->getAddress();`
`88`		`- }`
`89`	`86`	`if ($sender = $headers->get('Sender')) {`
`90`	`87`	`return $sender->getAddress();`
`91`	`88`	`}`
`92`	`89`	`if ($from = $headers->get('From')) {`
`93`	`90`	`return $from->getAddresses()[0];`
`94`	`91`	`}`
	`92`	`+ if ($return = $headers->get('Return-Path')) {`
	`93`	`+ return $return->getAddress();`
	`94`	`+ }`
`95`	`95`
`96`	`96`	`throw new LogicException('Unable to determine the sender of the message.');`
`97`	`97`	`}`