bug #51489 [Mime] Fix email (de)serialization issues (X-Coder264)

fabpot · fabpot · commit 6f20a87541c6 · 2023-09-29T08:59:40.000+02:00
This PR was squashed before being merged into the 6.3 branch. Discussion ---------- [Mime] Fix email (de)serialization issues | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #47991 | License | MIT | Doc PR | - #48156 fixed #47991 while introducing a big breaking change (the `File` lazy load feature is broken and that was the whole point of that class when it was introduced in #47462 as that feature existed even prior to that PR) on a minor Symfony version (updating from 6.1 to 6.2 broke our application). More context can be found here: #48156 (comment) This PR aims to revert back the `attachFromPath` behavior to what it was before #48156 while still fixing the deserialization issue reported in #47991 The first commit fixes the serialization logic to work the same way it had worked on both 5.4 and 6.1 (which means we are reverting #48156), while the second commit fixes the deserialization issue reported in #47991. I've also added tests to prevent serialization/deserialization regressions in the future. Commits ------- 32836b9 [Mime] Fix email (de)serialization issues
diff --git a/src/Symfony/Component/Mime/Part/DataPart.php b/src/Symfony/Component/Mime/Part/DataPart.php
@@ -156,7 +156,7 @@ public function __wakeup()
             throw new \BadMethodCallException('Cannot unserialize '.__CLASS__);
         }
         foreach (['body', 'charset', 'subtype', 'disposition', 'name', 'encoding'] as $name) {
-            if (null !== $this->_parent[$name] && !\is_string($this->_parent[$name])) {
+            if (null !== $this->_parent[$name] && !\is_string($this->_parent[$name]) && !$this->_parent[$name] instanceof File) {
                 throw new \BadMethodCallException('Cannot unserialize '.__CLASS__);
             }
             $r = new \ReflectionProperty(TextPart::class, $name);
diff --git a/src/Symfony/Component/Mime/Part/TextPart.php b/src/Symfony/Component/Mime/Part/TextPart.php
@@ -226,7 +226,7 @@ private function chooseEncoding(): string
     public function __sleep(): array
     {
         // convert resources to strings for serialization
-        if (null !== $this->seekable || $this->body instanceof File) {
+        if (null !== $this->seekable) {
             $this->body = $this->getBody();
             $this->seekable = null;
         }
diff --git a/src/Symfony/Component/Mime/Tests/EmailTest.php b/src/Symfony/Component/Mime/Tests/EmailTest.php
@@ -658,4 +658,34 @@ public function testBodyCache()
         $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.');
     }
+
+    public function testAttachmentBodyIsPartOfTheSerializationEmailPayloadWhenUsingAttachMethod()
+    {
+        $email = new Email();
+        $email->attach(file_get_contents(__DIR__.\DIRECTORY_SEPARATOR.'Fixtures'.\DIRECTORY_SEPARATOR.'foo_attachment.txt') ?: '');
+
+        $this->assertTrue(str_contains(serialize($email), 'foo_bar_xyz_123'));
+    }
+
+    public function testAttachmentBodyIsNotPartOfTheSerializationEmailPayloadWhenUsingAttachFromPathMethod()
+    {
+        $email = new Email();
+        $email->attachFromPath(__DIR__.\DIRECTORY_SEPARATOR.'Fixtures'.\DIRECTORY_SEPARATOR.'foo_attachment.txt');
+
+        $this->assertFalse(str_contains(serialize($email), 'foo_bar_xyz_123'));
+    }
+
+    public function testEmailsWithAttachmentsWhichAreAFileInstanceCanBeUnserialized()
+    {
+        $email = new Email();
+        $email->attachFromPath(__DIR__.\DIRECTORY_SEPARATOR.'Fixtures'.\DIRECTORY_SEPARATOR.'foo_attachment.txt');
+
+        $email = unserialize(serialize($email));
+        $this->assertInstanceOf(Email::class, $email);
+
+        $attachments = $email->getAttachments();
+
+        $this->assertCount(1, $attachments);
+        $this->assertStringContainsString('foo_bar_xyz_123', $attachments[0]->getBody());
+    }
 }
diff --git a/src/Symfony/Component/Mime/Tests/Fixtures/foo_attachment.txt b/src/Symfony/Component/Mime/Tests/Fixtures/foo_attachment.txt
@@ -0,0 +1 @@
+foo_bar_xyz_123

Original file line number	Diff line number	Diff line change
`@@ -156,7 +156,7 @@ public function __wakeup()`
`156`	`156`	`throw new \BadMethodCallException('Cannot unserialize '.__CLASS__);`
`157`	`157`	`}`
`158`	`158`	`foreach (['body', 'charset', 'subtype', 'disposition', 'name', 'encoding'] as $name) {`
`159`		`- if (null !== $this->_parent[$name] && !\is_string($this->_parent[$name])) {`
	`159`	`+ if (null !== $this->_parent[$name] && !\is_string($this->_parent[$name]) && !$this->_parent[$name] instanceof File) {`
`160`	`160`	`throw new \BadMethodCallException('Cannot unserialize '.__CLASS__);`
`161`	`161`	`}`
`162`	`162`	`$r = new \ReflectionProperty(TextPart::class, $name);`
Original file line number	Diff line number	Diff line change
`@@ -226,7 +226,7 @@ private function chooseEncoding(): string`
`226`	`226`	`public function __sleep(): array`
`227`	`227`	`{`
`228`	`228`	`// convert resources to strings for serialization`
`229`		`- if (null !== $this->seekable \|\| $this->body instanceof File) {`
	`229`	`+ if (null !== $this->seekable) {`
`230`	`230`	`$this->body = $this->getBody();`
`231`	`231`	`$this->seekable = null;`
`232`	`232`	`}`