bug symfony#46863 [Mime] Fix invalid DKIM signature with multiple parts (BrokenSourceCode)

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

Discussion
----------

[Mime] Fix invalid DKIM signature with multiple parts

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix symfony#42407, symfony#40131, symfony#39354
| License       | MIT

After many, many hours of investigations, I finally isolated the issue. When sending an email with a DKIM signature, **2** boundaries are generated because **2** instances of [`AbstractMultipartPart`](https://github.com/symfony/symfony/blob/04ae33a1f92bd76e17c7f0e7bbaacb5248cde79e/src/Symfony/Component/Mime/Part/AbstractMultipartPart.php) ([`AlternativePart`](https://github.com/symfony/symfony/blob/04ae33a1f92bd76e17c7f0e7bbaacb5248cde79e/src/Symfony/Component/Mime/Part/Multipart/AlternativePart.php) in my case) are created for **1** email.

Backtrace of the first boundary:

```
Symfony\Component\Mime\Part\AbstractMultipartPart->getBoundary()
Symfony\Component\Mime\Part\AbstractMultipartPart->bodyToIterable()
Symfony\Component\Mime\Crypto\DkimSigner->hashBody()
Symfony\Component\Mime\Crypto\DkimSigner->sign()
```

Backtrace of the second boundary:

```
Symfony\Component\Mime\Part\AbstractMultipartPart->getBoundary()
Symfony\Component\Mime\Part\AbstractMultipartPart->getPreparedHeaders()
Symfony\Component\Mime\Part\AbstractPart->toIterable()
Symfony\Component\Mime\Message->toIterable()
Symfony\Component\Mime\RawMessage->toIterable()
Symfony\Component\Mailer\Transport\Smtp\Stream\AbstractStream::replace()
Symfony\Component\Mailer\Transport\Smtp\SmtpTransport->doSend()
Symfony\Component\Mailer\Transport\AbstractTransport->send()
Symfony\Component\Mailer\Transport\Smtp\SmtpTransport->send()
Symfony\Component\Mailer\Mailer->send()
```

The fix is to use a single instance of [`AbstractMultipartPart`](https://github.com/symfony/symfony/blob/04ae33a1f92bd76e17c7f0e7bbaacb5248cde79e/src/Symfony/Component/Mime/Part/AbstractMultipartPart.php). I suggest to use a cache system for the method [`Email->getBody()`](https://github.com/symfony/symfony/blob/04ae33a1f92bd76e17c7f0e7bbaacb5248cde79e/src/Symfony/Component/Mime/Email.php#L409-L416) with a property named `$cachedBody`. In this way, only one instance will be created. The property `$cachedBody` will be reset whenever necessary.

Thanks to @calebsolano for putting me on the right way with his comment symfony#39354 (comment).

> My instincts tell me it has something to do with the multipart boundary, in particular with it being random...but I can't determine where in the code it instantiates a second one that would have a different value between creating the body hash and the actual body

I hope this solution will suit you, I couldn't think of an easier one.

Commits
-------

f09f960 [Mime] Fix invalid DKIM signature with multiple parts

Author: fabpot

src/Symfony/Component/Mime/Email.php

@@ -43,6 +43,7 @@ class Email extends Message
     private $html;
     private $htmlCharset;
     private $attachments = [];
+    private ?AbstractPart $cachedBody = null; // Used to avoid wrong body hash in DKIM signatures with multiple parts (e.g. HTML + TEXT) due to multiple boundaries.
 
     /**
      * @return $this
@@ -282,6 +283,7 @@ public function text($body, string $charset = 'utf-8')
             throw new \TypeError(sprintf('The body must be a string, a resource or null (got "%s").', get_debug_type($body)));
         }
 
+        $this->cachedBody = null;
         $this->text = $body;
         $this->textCharset = $charset;
 
@@ -312,6 +314,7 @@ public function html($body, string $charset = 'utf-8')
             throw new \TypeError(sprintf('The body must be a string, a resource or null (got "%s").', get_debug_type($body)));
         }
 
+        $this->cachedBody = null;
         $this->html = $body;
         $this->htmlCharset = $charset;
 
@@ -342,6 +345,7 @@ public function attach($body, string $name = null, string $contentType = null)
             throw new \TypeError(sprintf('The body must be a string or a resource (got "%s").', get_debug_type($body)));
         }
 
+        $this->cachedBody = null;
         $this->attachments[] = ['body' => $body, 'name' => $name, 'content-type' => $contentType, 'inline' => false];
 
         return $this;
@@ -352,6 +356,7 @@ public function attach($body, string $name = null, string $contentType = null)
      */
     public function attachFromPath(string $path, string $name = null, string $contentType = null)
     {
+        $this->cachedBody = null;
         $this->attachments[] = ['path' => $path, 'name' => $name, 'content-type' => $contentType, 'inline' => false];
 
         return $this;
@@ -368,6 +373,7 @@ public function embed($body, string $name = null, string $contentType = null)
             throw new \TypeError(sprintf('The body must be a string or a resource (got "%s").', get_debug_type($body)));
         }
 
+        $this->cachedBody = null;
         $this->attachments[] = ['body' => $body, 'name' => $name, 'content-type' => $contentType, 'inline' => true];
 
         return $this;
@@ -378,6 +384,7 @@ public function embed($body, string $name = null, string $contentType = null)
      */
     public function embedFromPath(string $path, string $name = null, string $contentType = null)
     {
+        $this->cachedBody = null;
         $this->attachments[] = ['path' => $path, 'name' => $name, 'content-type' => $contentType, 'inline' => true];
 
         return $this;
@@ -388,6 +395,7 @@ public function embedFromPath(string $path, string $name = null, string $content
      */
     public function attachPart(DataPart $part)
     {
+        $this->cachedBody = null;
         $this->attachments[] = ['part' => $part];
 
         return $this;
@@ -446,6 +454,10 @@ public function ensureValidity()
      */
     private function generateBody(): AbstractPart
     {
+        if (null !== $this->cachedBody) {
+            return $this->cachedBody;
+        }
+
         $this->ensureValidity();
 
         [$htmlPart, $attachmentParts, $inlineParts] = $this->prepareParts();
@@ -471,6 +483,8 @@ private function generateBody(): AbstractPart
             }
         }
 
+        $this->cachedBody = $part;
+
         return $part;
     }
 

src/Symfony/Component/Mime/Tests/EmailTest.php

@@ -458,4 +458,24 @@ public function testTextBodyAcceptedTypes()
         $email->text($contents);
         $this->assertSame($contents, $email->getTextBody());
     }
+
+    public function testBodyCache()
+    {
+        $email = new Email();
+        $email->from('[email protected]');
+        $email->to('[email protected]');
+        $email->text('foo');
+        $body1 = $email->getBody();
+        $body2 = $email->getBody();
+        $this->assertSame($body1, $body2, 'The two bodies must reference the same object, so the body cache ensures that the hash for the DKIM signature is unique.');
+
+        $email = new Email();
+        $email->from('[email protected]');
+        $email->to('[email protected]');
+        $email->text('foo');
+        $body1 = $email->getBody();
+        $email->html('<b>bar</b>'); // We change a part to reset the body cache.
+        $body2 = $email->getBody();
+        $this->assertNotSame($body1, $body2, 'The two bodies must not reference the same object, so the body cache does not ensure that the hash for the DKIM signature is unique.');
+    }
 }