From 0a29d9703e70887db40dbc62fc6f439ce96fba33 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Wed, 31 Aug 2022 08:44:02 +0200 Subject: [PATCH] [Mime] Simplify adding Parts to an Email --- .../Twig/Tests/Mime/TemplatedEmailTest.php | 10 ++- src/Symfony/Bridge/Twig/composer.json | 2 +- src/Symfony/Component/Mailer/composer.json | 2 +- src/Symfony/Component/Mime/Email.php | 90 ++++--------------- .../Component/Mime/MessageConverter.php | 5 +- src/Symfony/Component/Mime/Part/BodyFile.php | 42 +++++++++ src/Symfony/Component/Mime/Part/DataPart.php | 46 ++-------- src/Symfony/Component/Mime/Part/TextPart.php | 26 +++++- .../Component/Mime/Tests/EmailTest.php | 18 ++-- .../Component/Mime/Tests/Fixtures/content.txt | 1 + .../Component/Mime/Tests/MessageTest.php | 5 -- .../Mime/Tests/Part/TextPartTest.php | 9 ++ src/Symfony/Component/Mime/composer.json | 2 +- .../Normalizer/MimeMessageNormalizer.php | 1 + 14 files changed, 122 insertions(+), 137 deletions(-) create mode 100644 src/Symfony/Component/Mime/Part/BodyFile.php create mode 100644 src/Symfony/Component/Mime/Tests/Fixtures/content.txt diff --git a/src/Symfony/Bridge/Twig/Tests/Mime/TemplatedEmailTest.php b/src/Symfony/Bridge/Twig/Tests/Mime/TemplatedEmailTest.php index 77548fb119626..cb2786d8ea75d 100644 --- a/src/Symfony/Bridge/Twig/Tests/Mime/TemplatedEmailTest.php +++ b/src/Symfony/Bridge/Twig/Tests/Mime/TemplatedEmailTest.php @@ -74,10 +74,16 @@ public function testSymfonySerialize() "htmlCharset": null, "attachments": [ { + "filename": "test.txt", + "mediaType": "application", "body": "Some Text file", + "charset": null, + "subtype": "octet-stream", + "disposition": "attachment", "name": "test.txt", - "content-type": null, - "inline": false + "encoding": "base64", + "headers": [], + "class": "Symfony\\\Component\\\Mime\\\Part\\\DataPart" } ], "headers": { diff --git a/src/Symfony/Bridge/Twig/composer.json b/src/Symfony/Bridge/Twig/composer.json index be7c2d2ab31ac..5a4f501ebce42 100644 --- a/src/Symfony/Bridge/Twig/composer.json +++ b/src/Symfony/Bridge/Twig/composer.json @@ -43,7 +43,7 @@ "symfony/security-core": "^5.4|^6.0", "symfony/security-csrf": "^5.4|^6.0", "symfony/security-http": "^5.4|^6.0", - "symfony/serializer": "^5.4|^6.0", + "symfony/serializer": "^6.2", "symfony/stopwatch": "^5.4|^6.0", "symfony/console": "^5.4|^6.0", "symfony/expression-language": "^5.4|^6.0", diff --git a/src/Symfony/Component/Mailer/composer.json b/src/Symfony/Component/Mailer/composer.json index 6cf280c4cf822..d77448de3d468 100644 --- a/src/Symfony/Component/Mailer/composer.json +++ b/src/Symfony/Component/Mailer/composer.json @@ -21,7 +21,7 @@ "psr/event-dispatcher": "^1", "psr/log": "^1|^2|^3", "symfony/event-dispatcher": "^5.4|^6.0", - "symfony/mime": "^5.4|^6.0", + "symfony/mime": "^6.2", "symfony/service-contracts": "^1.1|^2|^3" }, "require-dev": { diff --git a/src/Symfony/Component/Mime/Email.php b/src/Symfony/Component/Mime/Email.php index c3145295a5f9d..697c71ced8127 100644 --- a/src/Symfony/Component/Mime/Email.php +++ b/src/Symfony/Component/Mime/Email.php @@ -13,6 +13,7 @@ use Symfony\Component\Mime\Exception\LogicException; use Symfony\Component\Mime\Part\AbstractPart; +use Symfony\Component\Mime\Part\BodyFile; use Symfony\Component\Mime\Part\DataPart; use Symfony\Component\Mime\Part\Multipart\AlternativePart; use Symfony\Component\Mime\Part\Multipart\MixedPart; @@ -326,14 +327,7 @@ public function getHtmlCharset(): ?string */ public function attach($body, string $name = null, string $contentType = null): static { - if (!\is_string($body) && !\is_resource($body)) { - 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; + return $this->attachPart(new DataPart($body, $name, $contentType)); } /** @@ -341,10 +335,7 @@ public function attach($body, string $name = null, string $contentType = null): */ public function attachFromPath(string $path, string $name = null, string $contentType = null): static { - $this->cachedBody = null; - $this->attachments[] = ['path' => $path, 'name' => $name, 'content-type' => $contentType, 'inline' => false]; - - return $this; + return $this->attachPart(new DataPart(new BodyFile($path), $name, $contentType)); } /** @@ -354,14 +345,7 @@ public function attachFromPath(string $path, string $name = null, string $conten */ public function embed($body, string $name = null, string $contentType = null): static { - if (!\is_string($body) && !\is_resource($body)) { - 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; + return $this->attachPart((new DataPart($body, $name, $contentType))->asInline()); } /** @@ -369,10 +353,7 @@ public function embed($body, string $name = null, string $contentType = null): s */ public function embedFromPath(string $path, string $name = null, string $contentType = null): static { - $this->cachedBody = null; - $this->attachments[] = ['path' => $path, 'name' => $name, 'content-type' => $contentType, 'inline' => true]; - - return $this; + return $this->attachPart((new DataPart(new BodyFile($path), $name, $contentType))->asInline()); } /** @@ -381,22 +362,17 @@ public function embedFromPath(string $path, string $name = null, string $content public function attachPart(DataPart $part): static { $this->cachedBody = null; - $this->attachments[] = ['part' => $part]; + $this->attachments[] = $part; return $this; } /** - * @return array|DataPart[] + * @return DataPart[] */ public function getAttachments(): array { - $parts = []; - foreach ($this->attachments as $attachment) { - $parts[] = $this->createDataPart($attachment); - } - - return $parts; + return $this->attachments; } public function getBody(): AbstractPart @@ -502,35 +478,23 @@ private function prepareParts(): ?array } $otherParts = $relatedParts = []; - foreach ($this->attachments as $attachment) { - $part = $this->createDataPart($attachment); - if (isset($attachment['part'])) { - $attachment['name'] = $part->getName(); - } - - $related = false; + foreach ($this->attachments as $part) { foreach ($names as $name) { - if ($name !== $attachment['name']) { + if ($name !== $part->getName()) { continue; } if (isset($relatedParts[$name])) { continue 2; } - $part->setDisposition('inline'); + $html = str_replace('cid:'.$name, 'cid:'.$part->getContentId(), $html, $count); - if ($count) { - $related = true; - } - $part->setName($part->getContentId()); + $relatedParts[$name] = $part; + $part->setName($part->getContentId())->asInline(); - break; + continue 2; } - if ($related) { - $relatedParts[$attachment['name']] = $part; - } else { - $otherParts[] = $part; - } + $otherParts[] = $part; } if (null !== $htmlPart) { $htmlPart = new TextPart($html, $this->htmlCharset, 'html'); @@ -539,24 +503,6 @@ private function prepareParts(): ?array return [$htmlPart, $otherParts, array_values($relatedParts)]; } - private function createDataPart(array $attachment): DataPart - { - if (isset($attachment['part'])) { - return $attachment['part']; - } - - if (isset($attachment['body'])) { - $part = new DataPart($attachment['body'], $attachment['name'] ?? null, $attachment['content-type'] ?? null); - } else { - $part = DataPart::fromPath($attachment['path'] ?? '', $attachment['name'] ?? null, $attachment['content-type'] ?? null); - } - if ($attachment['inline']) { - $part->asInline(); - } - - return $part; - } - /** * @return $this */ @@ -606,12 +552,6 @@ public function __serialize(): array $this->html = (new TextPart($this->html))->getBody(); } - foreach ($this->attachments as $i => $attachment) { - if (isset($attachment['body']) && \is_resource($attachment['body'])) { - $this->attachments[$i]['body'] = (new TextPart($attachment['body']))->getBody(); - } - } - return [$this->text, $this->textCharset, $this->html, $this->htmlCharset, $this->attachments, parent::__serialize()]; } diff --git a/src/Symfony/Component/Mime/MessageConverter.php b/src/Symfony/Component/Mime/MessageConverter.php index 0539eac8e5bdc..5436e4a9a663c 100644 --- a/src/Symfony/Component/Mime/MessageConverter.php +++ b/src/Symfony/Component/Mime/MessageConverter.php @@ -114,10 +114,7 @@ private static function attachParts(Email $email, array $parts): Email throw new RuntimeException(sprintf('Unable to create an Email from an instance of "%s" as the body is too complex.', get_debug_type($email))); } - $headers = $part->getPreparedHeaders(); - $method = 'inline' === $headers->getHeaderBody('Content-Disposition') ? 'embed' : 'attach'; - $name = $headers->getHeaderParameter('Content-Disposition', 'filename'); - $email->$method($part->getBody(), $name, $part->getMediaType().'/'.$part->getMediaSubtype()); + $email->attachPart($part); } return $email; diff --git a/src/Symfony/Component/Mime/Part/BodyFile.php b/src/Symfony/Component/Mime/Part/BodyFile.php new file mode 100644 index 0000000000000..979026ee1e36c --- /dev/null +++ b/src/Symfony/Component/Mime/Part/BodyFile.php @@ -0,0 +1,42 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Mime\Part; + +use Symfony\Component\Mime\MimeTypes; + +/** + * @author Fabien Potencier + */ +class BodyFile +{ + private static $mimeTypes; + + public function __construct( + private string $path, + ) { + } + + public function getPath(): string + { + return $this->path; + } + + public function getContentType(): string + { + $ext = strtolower(pathinfo($this->path, \PATHINFO_EXTENSION)); + if (null === self::$mimeTypes) { + self::$mimeTypes = new MimeTypes(); + } + + return self::$mimeTypes->getMimeTypes($ext)[0] ?? 'application/octet-stream'; + } +} diff --git a/src/Symfony/Component/Mime/Part/DataPart.php b/src/Symfony/Component/Mime/Part/DataPart.php index 2d7bf65716a20..076f081d7811e 100644 --- a/src/Symfony/Component/Mime/Part/DataPart.php +++ b/src/Symfony/Component/Mime/Part/DataPart.php @@ -13,7 +13,6 @@ use Symfony\Component\Mime\Exception\InvalidArgumentException; use Symfony\Component\Mime\Header\Headers; -use Symfony\Component\Mime\MimeTypes; /** * @author Fabien Potencier @@ -23,22 +22,23 @@ class DataPart extends TextPart /** @internal */ protected $_parent; - private static $mimeTypes; - private $filename; private $mediaType; private $cid; - private $handle; /** - * @param resource|string $body + * @param resource|string|BodyFile $body Use a BodyFile instance to defer loading the file until rendering */ public function __construct($body, string $filename = null, string $contentType = null, string $encoding = null) { unset($this->_parent); + if ($body instanceof BodyFile && !$filename) { + $filename = basename($body->getPath()); + } + if (null === $contentType) { - $contentType = 'application/octet-stream'; + $contentType = $body instanceof BodyFile ? $body->getContentType() : 'application/octet-stream'; } [$this->mediaType, $subtype] = explode('/', $contentType); @@ -53,32 +53,7 @@ public function __construct($body, string $filename = null, string $contentType public static function fromPath(string $path, string $name = null, string $contentType = null): self { - if (null === $contentType) { - $ext = strtolower(substr($path, strrpos($path, '.') + 1)); - if (null === self::$mimeTypes) { - self::$mimeTypes = new MimeTypes(); - } - $contentType = self::$mimeTypes->getMimeTypes($ext)[0] ?? 'application/octet-stream'; - } - - if ((is_file($path) && !is_readable($path)) || is_dir($path)) { - throw new InvalidArgumentException(sprintf('Path "%s" is not readable.', $path)); - } - - if (false === $handle = @fopen($path, 'r', false)) { - throw new InvalidArgumentException(sprintf('Unable to open path "%s".', $path)); - } - - if (!is_file($path)) { - $cache = fopen('php://temp', 'r+'); - stream_copy_to_stream($handle, $cache); - $handle = $cache; - } - - $p = new self($handle, $name ?: basename($path), $contentType); - $p->handle = $handle; - - return $p; + return new self(new BodyFile($path), $name, $contentType); } /** @@ -158,13 +133,6 @@ private function generateContentId(): string return bin2hex(random_bytes(16)).'@symfony'; } - public function __destruct() - { - if (null !== $this->handle && \is_resource($this->handle)) { - fclose($this->handle); - } - } - public function __sleep(): array { // converts the body to a string diff --git a/src/Symfony/Component/Mime/Part/TextPart.php b/src/Symfony/Component/Mime/Part/TextPart.php index 8219ab52c2125..8188ac3610300 100644 --- a/src/Symfony/Component/Mime/Part/TextPart.php +++ b/src/Symfony/Component/Mime/Part/TextPart.php @@ -40,7 +40,7 @@ class TextPart extends AbstractPart private $seekable; /** - * @param resource|string $body + * @param resource|string|BodyFile $body Use a BodyFile instance to defer loading the file until rendering */ public function __construct($body, ?string $charset = 'utf-8', string $subtype = 'plain', string $encoding = null) { @@ -48,8 +48,15 @@ public function __construct($body, ?string $charset = 'utf-8', string $subtype = parent::__construct(); - if (!\is_string($body) && !\is_resource($body)) { - throw new \TypeError(sprintf('The body of "%s" must be a string or a resource (got "%s").', self::class, get_debug_type($body))); + if (!\is_string($body) && !\is_resource($body) && !$body instanceof BodyFile) { + throw new \TypeError(sprintf('The body of "%s" must be a string, a resource, or an instance of "%s" (got "%s").', self::class, BodyFile::class, get_debug_type($body))); + } + + if ($body instanceof BodyFile) { + $path = $body->getPath(); + if ((is_file($path) && !is_readable($path)) || is_dir($path)) { + throw new InvalidArgumentException(sprintf('Path "%s" is not readable.', $path)); + } } $this->body = $body; @@ -111,6 +118,10 @@ public function getName(): ?string public function getBody(): string { + if ($this->body instanceof BodyFile) { + return file_get_contents($this->body->getPath()); + } + if (null === $this->seekable) { return $this->body; } @@ -129,7 +140,14 @@ public function bodyToString(): string public function bodyToIterable(): iterable { - if (null !== $this->seekable) { + if ($this->body instanceof BodyFile) { + $path = $this->body->getPath(); + if (false === $handle = @fopen($path, 'r', false)) { + throw new InvalidArgumentException(sprintf('Unable to open path "%s".', $path)); + } + + yield from $this->getEncoder()->encodeByteStream($handle); + } elseif (null !== $this->seekable) { if ($this->seekable) { rewind($this->body); } diff --git a/src/Symfony/Component/Mime/Tests/EmailTest.php b/src/Symfony/Component/Mime/Tests/EmailTest.php index b0c59b940b67c..cc89d6fdec9e8 100644 --- a/src/Symfony/Component/Mime/Tests/EmailTest.php +++ b/src/Symfony/Component/Mime/Tests/EmailTest.php @@ -504,7 +504,9 @@ public function testSerialize() $expected = clone $e; $n = unserialize(serialize($e)); $this->assertEquals($expected->getHeaders(), $n->getHeaders()); - $this->assertEquals($e->getBody(), $n->getBody()); + $a = preg_replace(["{boundary=.+\r\n}", "{^\-\-.+\r\n}m"], ['boundary=x', '--x'], $e->getBody()->toString()); + $b = preg_replace(["{boundary=.+\r\n}", "{^\-\-.+\r\n}m"], ['boundary=x', '--x'], $n->getBody()->toString()); + $this->assertSame($a, $b); } public function testSymfonySerialize() @@ -525,10 +527,16 @@ public function testSymfonySerialize() "htmlCharset": "utf-8", "attachments": [ { + "filename": "test.txt", + "mediaType": "application", "body": "Some Text file", + "charset": null, + "subtype": "octet-stream", + "disposition": "attachment", "name": "test.txt", - "content-type": null, - "inline": false + "encoding": "base64", + "headers": [], + "class": "Symfony\\\Component\\\Mime\\\Part\\\DataPart" } ], "headers": { @@ -587,7 +595,7 @@ public function testMissingHeaderDoesNotThrowError() public function testAttachBodyExpectStringOrResource() { $this->expectException(\TypeError::class); - $this->expectExceptionMessage('The body must be a string or a resource (got "bool").'); + $this->expectExceptionMessage('The body of "Symfony\Component\Mime\Part\TextPart" must be a string, a resource, or an instance of "Symfony\Component\Mime\Part\BodyFile" (got "bool").'); (new Email())->attach(false); } @@ -595,7 +603,7 @@ public function testAttachBodyExpectStringOrResource() public function testEmbedBodyExpectStringOrResource() { $this->expectException(\TypeError::class); - $this->expectExceptionMessage('The body must be a string or a resource (got "bool").'); + $this->expectExceptionMessage('The body of "Symfony\Component\Mime\Part\TextPart" must be a string, a resource, or an instance of "Symfony\Component\Mime\Part\BodyFile" (got "bool").'); (new Email())->embed(false); } diff --git a/src/Symfony/Component/Mime/Tests/Fixtures/content.txt b/src/Symfony/Component/Mime/Tests/Fixtures/content.txt new file mode 100644 index 0000000000000..6b584e8ece562 --- /dev/null +++ b/src/Symfony/Component/Mime/Tests/Fixtures/content.txt @@ -0,0 +1 @@ +content \ No newline at end of file diff --git a/src/Symfony/Component/Mime/Tests/MessageTest.php b/src/Symfony/Component/Mime/Tests/MessageTest.php index 6ed5aabdbe680..f35590ce9e174 100644 --- a/src/Symfony/Component/Mime/Tests/MessageTest.php +++ b/src/Symfony/Component/Mime/Tests/MessageTest.php @@ -202,7 +202,6 @@ public function testSymfonySerialize() "disposition": null, "name": null, "encoding": "quoted-printable", - "seekable": null, "headers": [], "class": "Symfony\\\\Component\\\\Mime\\\\Part\\\TextPart" }, @@ -213,7 +212,6 @@ public function testSymfonySerialize() "disposition": null, "name": null, "encoding": "quoted-printable", - "seekable": null, "headers": [], "class": "Symfony\\\\Component\\\\Mime\\\\Part\\\\TextPart" } @@ -224,15 +222,12 @@ public function testSymfonySerialize() { "filename": "text.txt", "mediaType": "application", - "cid": null, - "handle": null, "body": "text data", "charset": null, "subtype": "octet-stream", "disposition": "attachment", "name": "text.txt", "encoding": "base64", - "seekable": null, "headers": [], "class": "Symfony\\\\Component\\\\Mime\\\\Part\\\\DataPart" } diff --git a/src/Symfony/Component/Mime/Tests/Part/TextPartTest.php b/src/Symfony/Component/Mime/Tests/Part/TextPartTest.php index ea14fe29f88af..9381f6cc77677 100644 --- a/src/Symfony/Component/Mime/Tests/Part/TextPartTest.php +++ b/src/Symfony/Component/Mime/Tests/Part/TextPartTest.php @@ -15,6 +15,7 @@ use Symfony\Component\Mime\Header\Headers; use Symfony\Component\Mime\Header\ParameterizedHeader; use Symfony\Component\Mime\Header\UnstructuredHeader; +use Symfony\Component\Mime\Part\BodyFile; use Symfony\Component\Mime\Part\TextPart; class TextPartTest extends TestCase @@ -46,6 +47,14 @@ public function testConstructorWithResource() fclose($f); } + public function testConstructorWithBodyFile() + { + $p = new TextPart(new BodyFile(\dirname(__DIR__).'/Fixtures/content.txt')); + $this->assertSame('content', $p->getBody()); + $this->assertSame('content', $p->bodyToString()); + $this->assertSame('content', implode('', iterator_to_array($p->bodyToIterable()))); + } + public function testConstructorWithNonStringOrResource() { $this->expectException(\TypeError::class); diff --git a/src/Symfony/Component/Mime/composer.json b/src/Symfony/Component/Mime/composer.json index a09c9e274f4c4..84dc3a3bdad6a 100644 --- a/src/Symfony/Component/Mime/composer.json +++ b/src/Symfony/Component/Mime/composer.json @@ -27,7 +27,7 @@ "symfony/dependency-injection": "^5.4|^6.0", "symfony/property-access": "^5.4|^6.0", "symfony/property-info": "^5.4|^6.0", - "symfony/serializer": "^5.4|^6.0" + "symfony/serializer": "^6.2" }, "conflict": { "egulias/email-validator": "~3.0.0", diff --git a/src/Symfony/Component/Serializer/Normalizer/MimeMessageNormalizer.php b/src/Symfony/Component/Serializer/Normalizer/MimeMessageNormalizer.php index f112b3e2cbbb4..5f599ba655b6f 100644 --- a/src/Symfony/Component/Serializer/Normalizer/MimeMessageNormalizer.php +++ b/src/Symfony/Component/Serializer/Normalizer/MimeMessageNormalizer.php @@ -62,6 +62,7 @@ public function normalize(mixed $object, string $format = null, array $context = if ($object instanceof AbstractPart) { $ret = $this->normalizer->normalize($object, $format, $context); $ret['class'] = $object::class; + unset($ret['seekable'], $ret['cid']); return $ret; }