MessageDataLoaderTest

tatevikg1 · tatevikg1 · commit 2b0c256c98a8 · 2025-12-22T13:36:25.000+04:00
diff --git a/config/parameters.yml.dist b/config/parameters.yml.dist
@@ -100,7 +100,7 @@ parameters:
     messaging.use_manual_text_part: '%%env(USE_MANUAL_TEXT_PART)%%'
     env(USE_MANUAL_TEXT_PART): 0
     messaging.blacklist_grace_time: '%%env(MESSAGING_BLACKLIST_GRACE_TIME)%%'
-    env(MESSAGING_BLACKLIST_GRACE_TIME):
+    env(MESSAGING_BLACKLIST_GRACE_TIME): '600'
     messaging.google_sender_id: '%%env(GOOGLE_SENDERID)%%'
     env(GOOGLE_SENDERID): ''
     messaging.use_amazon_ses: '%%env(USE_AMAZONSES)%%'
@@ -114,11 +114,11 @@ parameters:
     messaging.external_image_timeout: '%%env(EXTERNALIMAGE_TIMEOUT)%%'
     env(EXTERNALIMAGE_TIMEOUT): 30
     messaging.external_image_max_size: '%%env(EXTERNALIMAGE_MAXSIZE)%%'
-    env(EXTERNALIMAGE_MAXSIZE): 2048
+    env(EXTERNALIMAGE_MAXSIZE): 204800
 
     phplist.upload_images_dir: '%%env(PHPLIST_UPLOADIMAGES_DIR)%%'
     env(PHPLIST_UPLOADIMAGES_DIR): 'images'
     phplist.editor_images_dir: '%%env(FCKIMAGES_DIR)%%'
     env(FCKIMAGES_DIR): 'uploadimages'
     phplist.public_schema: '%%env(PUBLIC_SCHEMA)%%'
-    env(PUBLIC_SCHEMA): 'http'
+    env(PUBLIC_SCHEMA): 'https'
diff --git a/src/Domain/Common/RemotePageFetcher.php b/src/Domain/Common/RemotePageFetcher.php
@@ -35,7 +35,15 @@ public function __invoke(string $url, array $userData): string
 
         foreach ($userData as $key => $val) {
             if ($key !== 'password') {
-                $url = utf8_encode(str_ireplace("[$key]", urlencode($val), utf8_decode($url)));
+                $url = mb_convert_encoding(
+                    str_ireplace(
+                        "[$key]",
+                        urlencode($val),
+                        mb_convert_encoding($url, 'ISO-8859-1', 'UTF-8')
+                    ),
+                    'ISO-8859-1',
+                    'UTF-8'
+                );
             }
         }
 
diff --git a/src/Domain/Messaging/Exception/MessageCacheMissingException.php b/src/Domain/Messaging/Exception/MessageCacheMissingException.php
@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpList\Core\Domain\Messaging\Exception;
+
+use RuntimeException;
+
+class MessageCacheMissingException extends RuntimeException
+{
+    public function __construct()
+    {
+        parent::__construct('Message cache is missing or expired');
+    }
+}
diff --git a/src/Domain/Messaging/MessageHandler/CampaignProcessorMessageHandler.php b/src/Domain/Messaging/MessageHandler/CampaignProcessorMessageHandler.php
@@ -9,6 +9,7 @@
 use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
 use Doctrine\ORM\EntityManagerInterface;
 use PhpList\Core\Domain\Configuration\Service\UserPersonalizer;
+use PhpList\Core\Domain\Messaging\Exception\MessageCacheMissingException;
 use PhpList\Core\Domain\Messaging\Exception\MessageSizeLimitExceededException;
 use PhpList\Core\Domain\Messaging\Message\CampaignProcessorMessage;
 use PhpList\Core\Domain\Messaging\Message\SyncCampaignProcessorMessage;
@@ -200,6 +201,9 @@ public function __invoke(CampaignProcessorMessage|SyncCampaignProcessorMessage $
             }
 
             $messagePrecacheDto = $this->cache->get($cacheKey);
+            if ($messagePrecacheDto === null) {
+                throw new MessageCacheMissingException();
+            }
             $this->handleEmailSending($campaign, $subscriber, $userMessage, $messagePrecacheDto);
         }
 
diff --git a/src/Domain/Messaging/Service/Builder/EmailBuilder.php b/src/Domain/Messaging/Service/Builder/EmailBuilder.php
@@ -12,6 +12,7 @@
 use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository;
 use PhpList\Core\Domain\Subscription\Repository\UserBlacklistRepository;
 use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberHistoryManager;
+use Psr\Log\LoggerInterface;
 use Symfony\Component\Mime\Address;
 use Symfony\Component\Mime\Email;
 
@@ -25,6 +26,7 @@ public function __construct(
         private readonly SubscriberRepository $subscriberRepository,
         private readonly SystemMailConstructor $systemMailConstructor,
         private readonly TemplateImageEmbedder $templateImageEmbedder,
+        private readonly LoggerInterface $logger,
         private readonly string $googleSenderId,
         private readonly bool $useAmazonSes,
         private readonly bool $usePrecedenceHeader,
@@ -64,6 +66,11 @@ public function buildPhplistEmail(
         if (!$skipBlacklistCheck && $this->blacklistRepository->isEmailBlacklisted($to)) {
             $this->eventLogManager->log('', sprintf('Error, %s is blacklisted, not sending', $to));
             $subscriber = $this->subscriberRepository->findOneByEmail($to);
+            if (!$subscriber) {
+                $this->logger->error('Error: subscriber not found', ['email' => $to]);
+
+                return null;
+            }
             $subscriber->setBlacklisted(true);
 
             $this->subscriberHistoryManager->addHistory(
diff --git a/src/Domain/Messaging/Service/MessageDataLoader.php b/src/Domain/Messaging/Service/MessageDataLoader.php
@@ -76,7 +76,7 @@ public function __invoke(Message $message): array
         $storedMessageData = $this->messageDataRepository->getForMessage($message->getId());
         foreach ($storedMessageData as $storedMessageDatum) {
             if (str_starts_with($storedMessageDatum->getData(), 'SER:')) {
-                $unserialized = unserialize(substr($storedMessageDatum->getData(), 4));
+                $unserialized = unserialize(substr($storedMessageDatum->getData(), 4), ['allowed_classes' => false]);
                 array_walk_recursive($unserialized, function (&$val) {
                     $val = stripslashes($val);
                 });
@@ -123,7 +123,7 @@ public function __invoke(Message $message): array
             $messageData['fromemail'] = str_replace('>', '', $messageData['fromemail']);
             // make sure there are no quotes around the name
             $messageData['fromname'] = str_replace('"', '', ltrim(rtrim($messageData['fromname'])));
-        } elseif (strpos($messageData['fromfield'], ' ')) {
+        } elseif (str_contains($messageData['fromfield'], ' ')) {
             // if there is a space, we need to add the email
             $messageData['fromname'] = $messageData['fromfield'];
             $messageData['fromemail'] = $defaultFrom;
diff --git a/src/Domain/Messaging/Service/MessageProcessingPreparator.php b/src/Domain/Messaging/Service/MessageProcessingPreparator.php
@@ -77,8 +77,8 @@ public function processMessageLinks(
             $cachedMessageDto->content = $this->replaceLinks($savedLinks, $cachedMessageDto->content);
         }
 
-        if ($cachedMessageDto->footer) {
-            $cachedMessageDto->footer = $this->replaceLinks($savedLinks, $cachedMessageDto->footer);
+        if ($cachedMessageDto->htmlFooter) {
+            $cachedMessageDto->htmlFooter = $this->replaceLinks($savedLinks, $cachedMessageDto->htmlFooter);
         }
 
         return $cachedMessageDto;
diff --git a/src/Domain/Messaging/Service/RateLimitedCampaignMailer.php b/src/Domain/Messaging/Service/RateLimitedCampaignMailer.php
@@ -40,7 +40,7 @@ public function composeEmail(
             ->subject($processedContent->subject)
             ->text($processedContent->textContent)
             // todo: check htmlFooterit should be html of textContent
-            ->html($processedContent->htmlFooter);
+            ->html($processedContent->content);
     }
 
     /**
diff --git a/src/Domain/Messaging/Service/SystemMailConstructor.php b/src/Domain/Messaging/Service/SystemMailConstructor.php
@@ -68,7 +68,7 @@ public function __invoke($message, $subject = ''): array
                 } else {
                     $phpListPowered = $this->configProvider->getValue(ConfigOption::PoweredByText);
                 }
-                if (strpos($htmlContent, '[SIGNATURE]')) {
+                if (str_contains($htmlContent, '[SIGNATURE]')) {
                     $htmlContent = str_replace('[SIGNATURE]', $phpListPowered, $htmlContent);
                 } elseif (strpos($htmlContent, '</body>')) {
                     $htmlContent = str_replace('</body>', $phpListPowered.'</body>', $htmlContent);
@@ -80,7 +80,7 @@ public function __invoke($message, $subject = ''): array
                 $textContent = str_replace('[SUBJECT]', $subject, $textContent);
                 $textContent = str_replace('[FOOTER]', '', $textContent);
                 $phpListPowered = trim(($this->html2Text)($this->configProvider->getValue(ConfigOption::PoweredByText)));
-                if (strpos($textContent, '[SIGNATURE]')) {
+                if (str_contains($textContent, '[SIGNATURE]')) {
                     $textContent = str_replace('[SIGNATURE]', $phpListPowered, $textContent);
                 } else {
                     $textContent .= "\n\n" . $phpListPowered;
diff --git a/src/Domain/Messaging/Service/TemplateImageEmbedder.php b/src/Domain/Messaging/Service/TemplateImageEmbedder.php
@@ -112,7 +112,9 @@ public function __invoke(string $html, int $messageId): string
             sort($htmlImages);
             for ($i = 0; $i < count($htmlImages); ++$i) {
                 if ($image = $this->getTemplateImage($templateId, $htmlImages[$i])) {
-                    $content_type = $this->mimeMap[strtolower(substr($htmlImages[$i],  strrpos($htmlImages[$i], '.') + 1))];
+                    $content_type = $this->mimeMap[strtolower(
+                        substr($htmlImages[$i], strrpos($htmlImages[$i], '.') + 1)
+                    )];
                     $cid = $this->addHtmlImage($image->getData(), basename($htmlImages[$i]), $content_type);
                     if (!empty($cid)) {
                         $html = str_replace(basename($htmlImages[$i]), "cid:$cid", $html);
@@ -146,6 +148,7 @@ private function getFilesystemImage(string $filename): string
     {
         //# get the image contents
         $localFile = basename(urldecode($filename));
+        $pageRoot = $this->configProvider->getValue(ConfigOption::PageRoot);
         if ($this->uploadImagesDir) {
             $imageRoot = $this->configProvider->getValue(ConfigOption::UploadImageRoot);
             if (is_file($imageRoot.$localFile)) {
@@ -161,15 +164,17 @@ private function getFilesystemImage(string $filename): string
                     );
 
                     return base64_encode(file_get_contents($this->documentRoot.$localFile));
-                } elseif (is_file($this->documentRoot.'/'.$this->uploadImagesDir.'/image/'.$localFile)) {
+                } elseif (is_file($this->documentRoot . '/' . $this->uploadImagesDir . '/image/' . $localFile)) {
                     $this->configManager->create(
                         ConfigOption::UploadImageRoot->value,
-                        $this->documentRoot.'/'.$this->uploadImagesDir.'/image/',
+                        $this->documentRoot . '/' . $this->uploadImagesDir . '/image/',
                         false,
                         'string',
                     );
 
-                    return base64_encode(file_get_contents($this->documentRoot.'/'.$this->uploadImagesDir.'/image/'.$localFile));
+                    return base64_encode(
+                        file_get_contents($this->documentRoot . '/' . $this->uploadImagesDir . '/image/' . $localFile)
+                    );
                 } elseif (is_file($this->documentRoot.'/'.$this->uploadImagesDir.'/'.$localFile)) {
                     $this->configManager->create(
                         ConfigOption::UploadImageRoot->value,
@@ -178,20 +183,26 @@ private function getFilesystemImage(string $filename): string
                         'string',
                     );
 
-                    return base64_encode(file_get_contents($this->documentRoot.'/'.$this->uploadImagesDir.'/'.$localFile));
+                    return base64_encode(
+                        file_get_contents($this->documentRoot . '/' . $this->uploadImagesDir . '/' . $localFile)
+                    );
                 }
             }
-        } elseif (is_file($this->documentRoot.$this->configProvider->getValue(ConfigOption::PageRoot).'/'.$this->editorImagesDir.'/'.$localFile)) {
+        } elseif (is_file($this->documentRoot . $pageRoot . '/' . $this->editorImagesDir . '/' . $localFile)) {
             $elements = parse_url($filename);
             $localFile = basename($elements['path']);
 
-            return base64_encode(file_get_contents($this->documentRoot.$this->configProvider->getValue(ConfigOption::PageRoot).'/'.$this->editorImagesDir.'/'.$localFile));
-        } elseif (is_file($this->documentRoot.$this->configProvider->getValue(ConfigOption::PageRoot).'/'.$this->editorImagesDir.'/image/'.$localFile)) {
-            return base64_encode(file_get_contents($this->documentRoot.$this->configProvider->getValue(ConfigOption::PageRoot).'/'.$this->editorImagesDir.'/image/'.$localFile));
-        } elseif (is_file('../'.$this->editorImagesDir.'/'.$localFile)) {
-            return base64_encode(file_get_contents('../'.$this->editorImagesDir.'/'.$localFile));
-        } elseif (is_file('../'.$this->editorImagesDir.'/image/'.$localFile)) {
-            return base64_encode(file_get_contents('../'.$this->editorImagesDir.'/image/'.$localFile));
+            return base64_encode(
+                file_get_contents($this->documentRoot . $pageRoot . '/' . $this->editorImagesDir . '/' . $localFile)
+            );
+        } elseif (is_file($this->documentRoot . $pageRoot . '/' . $this->editorImagesDir . '/image/' . $localFile)) {
+            return base64_encode(
+                file_get_contents($this->documentRoot . $pageRoot . '/' . $this->editorImagesDir . '/image/' . $localFile)
+            );
+        } elseif (is_file('../' . $this->editorImagesDir . '/' . $localFile)) {
+            return base64_encode(file_get_contents('../' . $this->editorImagesDir. '/' . $localFile));
+        } elseif (is_file('../' . $this->editorImagesDir . '/image/' . $localFile)) {
+            return base64_encode(file_get_contents('../' . $this->editorImagesDir . '/image/' . $localFile));
         }
 
         return '';
@@ -241,6 +252,7 @@ private function filesystemImageExists($filename): bool
     {
         //#  find the image referenced and see if it's on the server
         $imageRoot = $this->configProvider->getValue(ConfigOption::UploadImageRoot);
+        $pageRoot = $this->configProvider->getValue(ConfigOption::UploadImageRoot);
 
         $elements = parse_url($filename);
         $localFile = basename($elements['path']);
@@ -254,11 +266,11 @@ private function filesystemImageExists($filename): bool
                 || is_file($imageRoot.'/'.$localFile);
         } else {
             return
-                is_file($this->documentRoot.$this->configProvider->getValue(ConfigOption::PageRoot).'/'.$this->editorImagesDir.'/image/'.$localFile)
-                || is_file($this->documentRoot.$this->configProvider->getValue(ConfigOption::PageRoot).'/'.$this->editorImagesDir.'/'.$localFile)
+                is_file($this->documentRoot . $pageRoot . '/' . $this->editorImagesDir . '/image/' . $localFile)
+                || is_file($pageRoot . '/' . $this->editorImagesDir . '/' . $localFile)
                 //# commandline
-                || is_file('../'.$this->editorImagesDir.'/image/'.$localFile)
-                || is_file('../'.$this->editorImagesDir.'/'.$localFile);
+                || is_file('../' . $this->editorImagesDir . '/image/' . $localFile)
+                || is_file('../' . $this->editorImagesDir . '/' . $localFile);
         }
     }
 
diff --git a/tests/Unit/Domain/Messaging/Service/MessageDataLoaderTest.php b/tests/Unit/Domain/Messaging/Service/MessageDataLoaderTest.php
@@ -0,0 +1,131 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpList\Core\Tests\Unit\Domain\Messaging\Service;
+
+use Doctrine\Common\Collections\ArrayCollection;
+use PhpList\Core\Domain\Configuration\Model\ConfigOption;
+use PhpList\Core\Domain\Configuration\Service\Provider\ConfigProvider;
+use PhpList\Core\Domain\Messaging\Model\Message;
+use PhpList\Core\Domain\Messaging\Model\MessageData;
+use PhpList\Core\Domain\Messaging\Repository\MessageDataRepository;
+use PhpList\Core\Domain\Messaging\Repository\MessageRepository;
+use PhpList\Core\Domain\Messaging\Service\MessageDataLoader;
+use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\TestCase;
+
+class MessageDataLoaderTest extends TestCase
+{
+    private ConfigProvider&MockObject $config;
+    private MessageDataRepository&MockObject $messageDataRepository;
+    private MessageRepository&MockObject $messageRepository;
+
+    protected function setUp(): void
+    {
+        $this->config = $this->createMock(ConfigProvider::class);
+        $this->messageDataRepository = $this->createMock(MessageDataRepository::class);
+        $this->messageRepository = $this->createMock(MessageRepository::class);
+    }
+
+    public function testLoadsMessageDataMergesAndParses(): void
+    {
+        $defaultMessageAge = 3600;
+
+        $this->config->method('getValue')->willReturnMap([
+            [ConfigOption::MessageFromAddress, 'from@example.com'],
+            [ConfigOption::AdminAddress, 'admin@example.com'],
+            [ConfigOption::DefaultMessageTemplate, '123'],
+            [ConfigOption::MessageFooter, 'footer'],
+            [ConfigOption::ForwardFooter, 'ffooter'],
+            [ConfigOption::NotifyStartDefault, 'start@example.com'],
+            [ConfigOption::NotifyEndDefault, 'end@example.com'],
+            [ConfigOption::AlwaysAddGoogleTracking, '1'],
+        ]);
+
+        $messageId = 10;
+
+        // Non-empty fields from MessageRepository
+        $this->messageRepository
+            ->method('getNonEmptyFields')
+            ->with($messageId)
+            ->willReturn([
+                'subject' => '(no title)',
+                'message' => 'Hello [URL:https://example.org/p]',
+                'fromfield' => '',
+            ]);
+
+        // Stored message data rows (repository)
+        $md1 = (new MessageData())->setId($messageId)->setName('ashtml')->setData('1');
+        $md2 = (new MessageData())->setId($messageId)->setName('criteria_match')->setData('any');
+        $md3 = (new MessageData())->setId($messageId)->setName('embargo')->setData('string');
+
+        $this->messageDataRepository
+            ->method('getForMessage')
+            ->with($messageId)
+            ->willReturn([$md1, $md2, $md3]);
+
+        // Use a Message mock instead of an anonymous stub
+        $message = $this->createMock(Message::class);
+        $message->method('getId')->willReturn($messageId);
+        $message->method('getListMessages')->willReturn(
+            new ArrayCollection([
+                new class {
+                    public function getListId(): int { return 42; }
+                },
+            ])
+        );
+
+        $loader = new MessageDataLoader(
+            configProvider: $this->config,
+            messageDataRepository: $this->messageDataRepository,
+            messageRepository: $this->messageRepository,
+            defaultMessageAge: $defaultMessageAge,
+        );
+
+        $before = time();
+        $result = ($loader)($message);
+        $after = time();
+
+        // Core expectations
+        $this->assertSame('123', $result['template']);
+        $this->assertTrue($result['google_track']);
+
+        // subject mapping
+        $this->assertSame('(no subject)', $result['subject']);
+
+        // stored data merged (and AS_FORMAT_FIELDS ignored)
+        $this->assertSame('any', $result['criteria_match']);
+        $this->assertArrayNotHasKey('ashtml', $result, 'ashtml should not overwrite values');
+
+        // schedule fields normalized to arrays when not arrays
+        $this->assertIsArray($result['embargo']);
+        $this->assertIsArray($result['repeatuntil']);
+        $this->assertIsArray($result['requeueuntil']);
+
+        // target list from message listMessages
+        $this->assertArrayHasKey(42, $result['targetlist']);
+        $this->assertSame(1, $result['targetlist'][42]);
+
+        // sendurl inferred from message body
+        $this->assertSame('https://example.org/p', $result['sendurl']);
+        $this->assertSame('inputhere', $result['sendmethod']);
+
+        // From parsing defaults
+        $this->assertSame('from@example.com', $result['fromemail']);
+        $this->assertSame('from@example.com', $result['fromname']);
+
+        // finishsending should be now + defaultMessageAge (allow small drift)
+        $fs = $result['finishsending'];
+        $this->assertIsArray($fs);
+        $fsTimestamp = strtotime(sprintf(
+            '%s-%s-%s %s:%s:00',
+            $fs['year'], $fs['month'], $fs['day'], $fs['hour'], $fs['minute']
+        ));
+
+        $expectedMin = $before + $defaultMessageAge - 120;
+        $expectedMax = $after + $defaultMessageAge + 120;
+        $this->assertGreaterThanOrEqual($expectedMin, $fsTimestamp);
+        $this->assertLessThanOrEqual($expectedMax, $fsTimestamp);
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -77,8 +77,8 @@ public function processMessageLinks(`
`77`	`77`	`$cachedMessageDto->content = $this->replaceLinks($savedLinks, $cachedMessageDto->content);`
`78`	`78`	`}`
`79`	`79`
`80`		`- if ($cachedMessageDto->footer) {`
`81`		`- $cachedMessageDto->footer = $this->replaceLinks($savedLinks, $cachedMessageDto->footer);`
	`80`	`+ if ($cachedMessageDto->htmlFooter) {`
	`81`	`+ $cachedMessageDto->htmlFooter = $this->replaceLinks($savedLinks, $cachedMessageDto->htmlFooter);`
`82`	`82`	`}`
`83`	`83`
`84`	`84`	`return $cachedMessageDto;`
Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ public function composeEmail(`
`40`	`40`	`->subject($processedContent->subject)`
`41`	`41`	`->text($processedContent->textContent)`
`42`	`42`	`// todo: check htmlFooterit should be html of textContent`
`43`		`- ->html($processedContent->htmlFooter);`
	`43`	`+ ->html($processedContent->content);`
`44`	`44`	`}`
`45`	`45`
`46`	`46`	`/**`