fix: Display the images in notification emails correctly

Probesys · Dec 6, 2024 · 1bcf9c9 · 1bcf9c9
1 parent 0ebb324
commit 1bcf9c9
Show file tree

Hide file tree

Showing 4 changed files with 170 additions and 50 deletions.
diff --git a/src/MessageHandler/CreateTicketsFromMailboxEmailsHandler.php b/src/MessageHandler/CreateTicketsFromMailboxEmailsHandler.php
@@ -295,32 +295,10 @@ private function storeAttachments(MailboxEmail $mailboxEmail): array
      */
     private function replaceAttachmentsUrls(string $content, array $messageDocuments): string
     {
-        if (!$content) {
-            return '';
-        }
-
-        $contentDom = new \DOMDocument();
-
-        // DOMDocument::loadHTML considers the source string to be encoded in
-        // ISO-8859-1 by default. In order to not ending with weird characters,
-        // we encode the non-ASCII chars (i.e. all chars above >0x80) to HTML
-        // entities.
-        $content = mb_encode_numericentity(
-            $content,
-            [0x80, 0x10FFFF, 0, -1],
-            'UTF-8'
-        );
-
-        $contentDom->loadHTML($content, \LIBXML_NOERROR);
-        $contentDomXPath = new \DomXPath($contentDom);
+        $mapping = [];
 
         foreach ($messageDocuments as $attachmentId => $messageDocument) {
-            $imageNodes = $contentDomXPath->query("//img[@src='cid:{$attachmentId}']");
-
-            if ($imageNodes === false || $imageNodes->length === 0) {
-                // no corresponding node, the document was probably not inlined
-                continue;
-            }
+            $initialUrl = 'cid:' . $attachmentId;
 
             $messageDocumentUrl = $this->urlGenerator->generate(
                 'message document',
@@ -331,20 +309,10 @@ private function replaceAttachmentsUrls(string $content, array $messageDocuments
                 UrlGeneratorInterface::ABSOLUTE_URL
             );
 
-            foreach ($imageNodes as $imageNode) {
-                if ($imageNode instanceof \DOMElement) {
-                    $imageNode->setAttribute('src', $messageDocumentUrl);
-                }
-            }
-        }
-
-        $result = $contentDom->saveHTML();
-
-        if ($result === false) {
-            return $content;
+            $mapping[$initialUrl] = $messageDocumentUrl;
         }
 
-        return $result;
+        return Utils\DomHelper::replaceImagesUrls($content, $mapping);
     }
 
     private function markError(MailboxEmail $mailboxEmail, string $error): void

diff --git a/src/MessageHandler/SendMessageEmailHandler.php b/src/MessageHandler/SendMessageEmailHandler.php
@@ -6,28 +6,31 @@
 
 namespace App\MessageHandler;
 
-use App\Repository\MessageRepository;
-use App\Message\SendMessageEmail;
-use App\Service\MessageDocumentStorage;
+use App\Repository;
+use App\Message;
+use App\Service;
+use App\Utils;
 use Psr\Log\LoggerInterface;
 use Symfony\Bridge\Twig\Mime\TemplatedEmail;
 use Symfony\Component\Mailer\Transport\TransportInterface;
 use Symfony\Component\Messenger\Attribute\AsMessageHandler;
 use Symfony\Component\Mime\Part\DataPart;
 use Symfony\Component\Mime\Part\File;
+use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
 
 #[AsMessageHandler]
 class SendMessageEmailHandler
 {
     public function __construct(
-        private MessageRepository $messageRepository,
-        private MessageDocumentStorage $messageDocumentStorage,
+        private Repository\MessageRepository $messageRepository,
+        private Service\MessageDocumentStorage $messageDocumentStorage,
         private TransportInterface $transportInterface,
         private LoggerInterface $logger,
+        private UrlGeneratorInterface $urlGenerator,
     ) {
     }
 
-    public function __invoke(SendMessageEmail $data): void
+    public function __invoke(Message\SendMessageEmail $data): void
     {
         $messageId = $data->getMessageId();
         $message = $this->messageRepository->find($messageId);
@@ -89,15 +92,10 @@ public function __invoke(SendMessageEmail $data): void
         $email->bcc(...$recipients);
         $email->subject($subject);
         $email->locale($locale);
-        $content = $message->getContent();
-        $email->context([
-            'subject' => $subject,
-            'ticket' => $ticket,
-            'content' => $content,
-        ]);
-        $email->htmlTemplate('emails/message.html.twig');
-        $email->textTemplate('emails/message.txt.twig');
 
+        $replacingMapping = [];
+
+        // Attach the message documents as attachments to the email
         foreach ($message->getMessageDocuments() as $messageDocument) {
             $filepath = $this->messageDocumentStorage->getPathname($messageDocument);
             $file = new File($filepath);
@@ -107,8 +105,31 @@ public function __invoke(SendMessageEmail $data): void
                 $messageDocument->getMimetype()
             );
             $email->addPart($dataPart);
+
+            $initialUrl = $this->urlGenerator->generate(
+                'message document',
+                [
+                    'uid' => $messageDocument->getUid(),
+                    'extension' => $messageDocument->getExtension(),
+                ],
+                UrlGeneratorInterface::ABSOLUTE_URL
+            );
+
+            $replacingMapping[$initialUrl] = 'cid:' . $dataPart->getContentId();
         }
 
+        // Change the images URLs in the email in order to point to the "cid" references.
+        $content = $message->getContent();
+        $content = Utils\DomHelper::replaceImagesUrls($content, $replacingMapping);
+
+        $email->context([
+            'subject' => $subject,
+            'ticket' => $ticket,
+            'content' => $content,
+        ]);
+        $email->htmlTemplate('emails/message.html.twig');
+        $email->textTemplate('emails/message.txt.twig');
+
         // Set correct references headers so email clients can add the email to
         // the conversation thread.
         $emailReferences = [];

diff --git a/src/Utils/DomHelper.php b/src/Utils/DomHelper.php
@@ -0,0 +1,66 @@
+<?php
+
+// This file is part of Bileto.
+// Copyright 2022-2024 Probesys
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+namespace App\Utils;
+
+class DomHelper
+{
+    /**
+     * Replace the images URLs in a DOM string.
+     *
+     * The content string must be a valid DOM element.
+     *
+     * The mapping variable contains a list of URLs to replace, where the keys
+     * are the URLs in the DOM and the values are the new URLs.
+     *
+     * @param array<string, string> $mapping
+     */
+    public static function replaceImagesUrls(string $content, array $mapping): string
+    {
+        if (!$content) {
+            return '';
+        }
+
+        $contentDom = new \DOMDocument();
+
+        // DOMDocument::loadHTML considers the source string to be encoded in
+        // ISO-8859-1 by default. In order to not ending with weird characters,
+        // we encode the non-ASCII chars (i.e. all chars above >0x80) to HTML
+        // entities.
+        $content = mb_encode_numericentity(
+            $content,
+            [0x80, 0x10FFFF, 0, -1],
+            'UTF-8'
+        );
+
+        $libxmlOptions = \LIBXML_NOERROR | \LIBXML_HTML_NOIMPLIED | \LIBXML_HTML_NODEFDTD;
+        $contentDom->loadHTML($content, $libxmlOptions);
+        $contentDomXPath = new \DomXPath($contentDom);
+
+        foreach ($mapping as $initialUrl => $newUrl) {
+            $imageNodes = $contentDomXPath->query("//img[@src='{$initialUrl}']");
+
+            if ($imageNodes === false || $imageNodes->length === 0) {
+                // no corresponding node, the URL doesn't appear in the content
+                continue;
+            }
+
+            foreach ($imageNodes as $imageNode) {
+                if ($imageNode instanceof \DOMElement) {
+                    $imageNode->setAttribute('src', $newUrl);
+                }
+            }
+        }
+
+        $result = $contentDom->saveHTML();
+
+        if ($result === false) {
+            return $content;
+        }
+
+        return $result;
+    }
+}
diff --git a/tests/Utils/DomHelperTest.php b/tests/Utils/DomHelperTest.php
@@ -0,0 +1,65 @@
+<?php
+
+// This file is part of Bileto.
+// Copyright 2022-2024 Probesys
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+namespace App\Tests\Utils;
+
+use App\Utils\DomHelper;
+use PHPUnit\Framework\TestCase;
+
+class DomHelperTest extends TestCase
+{
+    public function testReplaceImagesUrls(): void
+    {
+        $content = '<img alt="" src="https://example.coop/image.jpg">';
+        $mapping = [
+            'https://example.coop/image.jpg' => 'cid:image.jpg',
+        ];
+
+        $newContent = DomHelper::replaceImagesUrls($content, $mapping);
+
+        $newContent = trim($newContent);
+        $this->assertSame('<img alt="" src="cid:image.jpg">', $newContent);
+    }
+
+    public function testReplaceImagesUrlsWithUnmatchingUrl(): void
+    {
+        $content = '<img alt="" src="/image.jpg">';
+        $mapping = [
+            'https://example.coop/image.jpg' => 'cid:image.jpg',
+        ];
+
+        $newContent = DomHelper::replaceImagesUrls($content, $mapping);
+
+        $newContent = trim($newContent);
+        $this->assertSame($content, $newContent);
+    }
+
+    public function testReplaceImagesUrlsWithNoImage(): void
+    {
+        $content = '<p>My content</p>';
+        $mapping = [
+            'https://example.coop/image.jpg' => 'cid:image.jpg',
+        ];
+
+        $newContent = DomHelper::replaceImagesUrls($content, $mapping);
+
+        $newContent = trim($newContent);
+        $this->assertSame($content, $newContent);
+    }
+
+    public function testReplaceImagesUrlsWithEmptyContent(): void
+    {
+        $content = '';
+        $mapping = [
+            'https://example.coop/image.jpg' => 'cid:image.jpg',
+        ];
+
+        $newContent = DomHelper::replaceImagesUrls($content, $mapping);
+
+        $newContent = trim($newContent);
+        $this->assertSame('', $newContent);
+    }
+}