Skip to content
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
a72d2e9
UserPersonalizer in CampaignProcessorMessageHandler
tatevikg1 Dec 11, 2025
d40dedd
HtmlToText
tatevikg1 Dec 11, 2025
613a196
MessageDataLoader
tatevikg1 Dec 11, 2025
759d8e0
TextParser
tatevikg1 Dec 11, 2025
d94f825
RemotePageFetcher
tatevikg1 Dec 13, 2025
3b9267f
Use repo methods
tatevikg1 Dec 14, 2025
5fc8637
Use MessagePrecacheDto
tatevikg1 Dec 14, 2025
69884a8
Refactor
tatevikg1 Dec 14, 2025
077bc63
Todo
tatevikg1 Dec 15, 2025
492e1d0
SystemMailConstructor
tatevikg1 Dec 15, 2025
109b07a
EmailBuilder
tatevikg1 Dec 16, 2025
65c0030
InjectedByHeaderSubscriber
tatevikg1 Dec 16, 2025
7e9bab2
TemplateImageManager
tatevikg1 Dec 17, 2025
3dcb90a
ExternalImageCacher
tatevikg1 Dec 17, 2025
0c5b4f4
TemplateImageEmbedder
tatevikg1 Dec 18, 2025
25ef84a
Mailer
tatevikg1 Dec 21, 2025
166a66c
RemotePageFetcherTest
tatevikg1 Dec 22, 2025
e7f9eca
TextParserTest
tatevikg1 Dec 22, 2025
cec0eb1
MessageDataLoaderTest
tatevikg1 Dec 22, 2025
2b0c256
MessageDataLoaderTest
tatevikg1 Dec 22, 2025
1643a29
Test fix
tatevikg1 Dec 22, 2025
04e84a1
Fix: phpmd
tatevikg1 Dec 23, 2025
4a9e895
Fix: phpcs
tatevikg1 Dec 24, 2025
b6e2ee2
After review 0
tatevikg1 Dec 25, 2025
ca6dc94
After review 1
tatevikg1 Dec 25, 2025
9fea466
Add tests
tatevikg1 Dec 27, 2025
2323119
EmailBuilderTest
tatevikg1 Dec 29, 2025
823e006
update coderabbit.yaml
tatevikg1 Dec 29, 2025
a140c2b
Add tests
tatevikg1 Dec 29, 2025
7f21c58
MailSizeChecker
tatevikg1 Dec 29, 2025
8342b4a
Feat/email building with attachments (#375)
TatevikGr Jan 22, 2026
39712ed
Feat: email forwarding (#377)
TatevikGr Feb 3, 2026
88e25f5
Cutoff from forward_email_period config
tatevikg1 Feb 3, 2026
6f81e7b
ForwardingResult
tatevikg1 Feb 3, 2026
2adf65c
Remove MessageFormat consts
tatevikg1 Feb 3, 2026
7adf1f6
Testing bundle
tatevikg1 Feb 3, 2026
0a9b9a4
After review 3
tatevikg1 Feb 4, 2026
ef65472
MessageDataLoader types
tatevikg1 Feb 5, 2026
d4261d0
Fix HTMLPurifier_Config
tatevikg1 Feb 5, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,11 @@
"ext-imap": "*",
"tatevikgr/rss-feed": "dev-main",
"ext-pdo": "*",
"ezyang/htmlpurifier": "^4.19"
"ezyang/htmlpurifier": "^4.19",
"ext-libxml": "*",
"ext-gd": "*",
"ext-curl": "*",
"ext-fileinfo": "*"
},
"require-dev": {
"phpunit/phpunit": "^9.5",
Expand Down
4 changes: 2 additions & 2 deletions config/PHPMD/rules.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<exclude-pattern>*/Migrations/*</exclude-pattern>

<!-- rules from the "clean code" rule set -->
<rule ref="rulesets/cleancode.xml/BooleanArgumentFlag"/>
<!-- <rule ref="rulesets/cleancode.xml/BooleanArgumentFlag"/>-->
<rule ref="rulesets/codesize.xml/CyclomaticComplexity"/>
<rule ref="rulesets/codesize.xml/NPathComplexity"/>
<rule ref="rulesets/codesize.xml/ExcessiveMethodLength"/>
Expand Down Expand Up @@ -41,7 +41,7 @@
<!-- rules from the "naming" rule set -->
<rule ref="rulesets/naming.xml/ShortVariable">
<properties>
<property name="exceptions" value="id,ip,cc,io"/>
<property name="exceptions" value="id,ip,cc,io,to"/>
</properties>
</rule>
<rule ref="rulesets/naming.xml/LongVariable">
Expand Down
6 changes: 5 additions & 1 deletion config/PhpCodeSniffer/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@
<rule ref="PEAR.WhiteSpace.ScopeClosingBrace"/>
<rule ref="Squiz.WhiteSpace.CastSpacing"/>
<rule ref="Squiz.WhiteSpace.LogicalOperatorSpacing"/>
<rule ref="Squiz.WhiteSpace.OperatorSpacing"/>
<rule ref="Squiz.WhiteSpace.OperatorSpacing">
<properties>
<property name="ignoreNewlines" value="true"/>
</properties>
</rule>
<rule ref="Squiz.WhiteSpace.SemicolonSpacing"/>
</ruleset>
35 changes: 34 additions & 1 deletion config/parameters.yml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,18 @@ parameters:
env(DATABASE_PREFIX): 'phplist_'
list_table_prefix: '%%env(LIST_TABLE_PREFIX)%%'
env(LIST_TABLE_PREFIX): 'listattr_'
app.dev_version: '%%env(APP_DEV_VERSION)%%'
env(APP_DEV_VERSION): '0'
app.dev_email: '%%env(APP_DEV_EMAIL)%%'
env(APP_DEV_EMAIL): 'dev@dev.com'
Comment thread
TatevikGr marked this conversation as resolved.
app.powered_by_phplist: '%%env(APP_POWERED_BY_PHPLIST)%%'
env(APP_POWERED_BY_PHPLIST): '0'

# Email configuration
app.mailer_from: '%%env(MAILER_FROM)%%'
env(MAILER_FROM): 'noreply@phplist.com'
app.mailer_dsn: '%%env(MAILER_DSN)%%'
env(MAILER_DSN): 'null://null'
env(MAILER_DSN): 'null://null' # set local_domain on transport
app.confirmation_url: '%%env(CONFIRMATION_URL)%%'
env(CONFIRMATION_URL): 'https://example.com/subscriber/confirm/'
app.subscription_confirmation_url: '%%env(SUBSCRIPTION_CONFIRMATION_URL)%%'
Expand Down Expand Up @@ -89,3 +95,30 @@ parameters:
env(MESSAGING_MAX_PROCESS_TIME): '600'
messaging.max_mail_size: '%%env(MAX_MAILSIZE)%%'
env(MAX_MAILSIZE): '209715200'
messaging.default_message_age: '%%env(DEFAULT_MESSAGEAGE)%%'
env(DEFAULT_MESSAGEAGE): '691200'
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): '600'
messaging.google_sender_id: '%%env(GOOGLE_SENDERID)%%'
env(GOOGLE_SENDERID): ''
messaging.use_amazon_ses: '%%env(USE_AMAZONSES)%%'
env(USE_AMAZONSES): '0'
messaging.embed_external_images: '%%env(EMBEDEXTERNALIMAGES)%%'
env(EMBEDEXTERNALIMAGES): '0'
messaging.embed_uploaded_images: '%%env(EMBEDUPLOADIMAGES)%%'
env(EMBEDUPLOADIMAGES): '0'
messaging.external_image_max_age: '%%env(EXTERNALIMAGE_MAXAGE)%%'
env(EXTERNALIMAGE_MAXAGE): '0'
messaging.external_image_timeout: '%%env(EXTERNALIMAGE_TIMEOUT)%%'
env(EXTERNALIMAGE_TIMEOUT): '30'
messaging.external_image_max_size: '%%env(EXTERNALIMAGE_MAXSIZE)%%'
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): 'https'
12 changes: 12 additions & 0 deletions resources/translations/messages.en.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,18 @@ Thank you.</target>
<source>Value must be an AttributeTypeEnum or string.</source>
<target>__Value must be an AttributeTypeEnum or string.</target>
</trans-unit>
<trans-unit id="1cRuI31" resname="Campaign started">
<source>Campaign started</source>
<target>__Campaign started</target>
</trans-unit>
<trans-unit id="Y4qXXak" resname="phplist has started sending the campaign with subject %s">
<source>phplist has started sending the campaign with subject %s</source>
<target>__phplist has started sending the campaign with subject %s</target>
</trans-unit>
Comment thread
TatevikGr marked this conversation as resolved.
<trans-unit id="jE9yPKp" resname="phplist has started sending the campaign with subject %subject%">
<source>phplist has started sending the campaign with subject %subject%</source>
<target>__phplist has started sending the campaign with subject %subject%</target>
</trans-unit>
</body>
</file>
</xliff>
3 changes: 0 additions & 3 deletions src/Bounce/Service/LockService.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ public function __construct(
$this->maxWaitCycles = $maxWaitCycles;
}

/**
* @SuppressWarnings("BooleanArgumentFlag")
*/
public function acquirePageLock(
string $page,
bool $force = false,
Expand Down
11 changes: 5 additions & 6 deletions src/Domain/Analytics/Service/LinkTrackService.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
use PhpList\Core\Domain\Analytics\Exception\MissingMessageIdException;
use PhpList\Core\Domain\Analytics\Model\LinkTrack;
use PhpList\Core\Domain\Analytics\Repository\LinkTrackRepository;
use PhpList\Core\Domain\Messaging\Model\Message;
use PhpList\Core\Domain\Messaging\Model\Message\MessageContent;
use PhpList\Core\Domain\Messaging\Model\Dto\MessagePrecacheDto;

class LinkTrackService
{
Expand Down Expand Up @@ -39,7 +38,7 @@ public function isExtractAndSaveLinksApplicable(): bool
* @return LinkTrack[] The saved LinkTrack entities
* @throws MissingMessageIdException
*/
public function extractAndSaveLinks(MessageContent $content, int $userId, ?int $messageId = null): array
public function extractAndSaveLinks(MessagePrecacheDto $content, int $userId, ?int $messageId = null): array
{
if (!$this->isExtractAndSaveLinksApplicable()) {
return [];
Expand All @@ -49,10 +48,10 @@ public function extractAndSaveLinks(MessageContent $content, int $userId, ?int $
throw new MissingMessageIdException();
}

$links = $this->extractLinksFromHtml($content->getText() ?? '');
$links = $this->extractLinksFromHtml($content->content ?? '');

if ($content->getFooter() !== null) {
$links = array_merge($links, $this->extractLinksFromHtml($content->getFooter()));
if ($content->htmlFooter) {
$links = array_merge($links, $this->extractLinksFromHtml($content->htmlFooter));
}

$links = array_unique($links);
Expand Down
222 changes: 222 additions & 0 deletions src/Domain/Common/ExternalImageService.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
<?php

declare(strict_types=1);

namespace PhpList\Core\Domain\Common;

use PhpList\Core\Domain\Configuration\Model\ConfigOption;
use PhpList\Core\Domain\Configuration\Service\Provider\ConfigProvider;
use Psr\Log\LoggerInterface;
use Throwable;

class ExternalImageService
{
private string $externalCacheDir;

public function __construct(
private readonly ConfigProvider $configProvider,
private readonly LoggerInterface $logger,
private readonly string $tempDir,
private readonly int $externalImageMaxAge,
private readonly int $externalImageMaxSize,
private readonly ?int $externalImageTimeout = 30,
) {
$this->externalCacheDir = $this->tempDir . '/external_cache';
}

public function getFromCache(string $filename, int $messageId): ?string
{
$cacheFile = $this->generateLocalFileName($filename, $messageId);

if (!is_file($cacheFile) || filesize($cacheFile) <= 64) {
return null;
}

$content = file_get_contents($cacheFile);
if ($content === false) {
return null;
}

return base64_encode($content);
}

public function cache($filename, $messageId): bool
{
if (!$this->isCacheableUrl($filename)) {
return false;
}

if (!$this->ensureCacheDirectory()) {
return false;
}

$this->removeOldFilesInCache();

$cacheFileName = $this->generateLocalFileName($filename, $messageId);

if (!file_exists($cacheFileName)) {
$cacheFileContent = null;

if (function_exists('curl_init')) {
$cacheFileContent = $this->downloadUsingCurl($filename);
}

if ($cacheFileContent === null) {
$cacheFileContent = $this->downloadUsingFileGetContent($filename);
}

if ($this->externalImageMaxSize && (strlen($cacheFileContent) > $this->externalImageMaxSize)) {
$cacheFileContent = 'MAX_SIZE';
}
Comment thread
TatevikGr marked this conversation as resolved.

$this->writeCacheFile($cacheFileName, $cacheFileContent);
Comment thread
TatevikGr marked this conversation as resolved.
}

return $this->isValidCacheFile($cacheFileName);
}

private function removeOldFilesInCache(): void
{
// phpcs:ignore Generic.PHP.NoSilencedErrors
$extCacheDirHandle = @opendir($this->externalCacheDir);
if (!$this->externalImageMaxAge || !$extCacheDirHandle) {
return;
}

while (true) {
// phpcs:ignore Generic.PHP.NoSilencedErrors
$cacheFile = @readdir($extCacheDirHandle);

if ($cacheFile === false) {
break;
}
// todo: make sure that this is what we need
if (!str_starts_with($cacheFile, '.')) {
// phpcs:ignore Generic.PHP.NoSilencedErrors
$cfmt = @filemtime($this->externalCacheDir . '/' . $cacheFile);

if (is_numeric($cfmt) && ($cfmt > 0) && ((time() - $cfmt) > $this->externalImageMaxAge)) {
// phpcs:ignore Generic.PHP.NoSilencedErrors
@unlink($this->externalCacheDir . '/' . $cacheFile);
}
}
}
// phpcs:ignore Generic.PHP.NoSilencedErrors
@closedir($extCacheDirHandle);
}

private function generateLocalFileName(string $filename, int $messageId): string
{
return $this->externalCacheDir
. '/'
. $messageId
. '_'
. preg_replace([ '~[\.][\.]+~Ui', '~[^\w\.]~Ui',], ['', '_'], $filename);
}
Comment on lines +109 to +116
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Strengthen filename sanitization against path traversal.

The regex ~[\.][\.]+~Ui only removes consecutive dots (.. becomes empty), but ../ is split by the slash and not fully neutralized. An attacker could craft a URL that, after sanitization, still contains relative path components. Use basename() or a whitelist approach to ensure filenames don't escape the cache directory.

🔎 Proposed fix
     private function generateLocalFileName(string $filename, int $messageId): string
     {
-        return $this->externalCacheDir
-            . '/'
-            . $messageId
-            . '_'
-            . preg_replace([ '~[\.][\.]+~Ui', '~[^\w\.]~Ui',], ['', '_'], $filename);
+        $sanitized = preg_replace('~[^\w\.-]~', '_', basename(parse_url($filename, PHP_URL_PATH)));
+        return $this->externalCacheDir . '/' . $messageId . '_' . $sanitized;
     }
🤖 Prompt for AI Agents
In src/Domain/Common/ExternalImageService.php around lines 102 to 109, the
current preg_replace sanitization can still allow path traversal (e.g., "../"
split by slashes) so replace it with a stronger approach: strip URL-decoding and
null bytes, then call basename() on the incoming filename (or apply a strict
whitelist of allowed characters) to remove any directory components, remove any
remaining disallowed characters, and truncate to a safe length before
concatenating with $this->externalCacheDir and $messageId; ensure no directory
separators remain and consider validating the final path is inside the cache dir
(realpath check) before returning.


private function downloadUsingCurl(string $filename): ?string
{
$cURLHandle = curl_init($filename);

if ($cURLHandle !== false) {
curl_setopt($cURLHandle, CURLOPT_HTTPGET, true);
curl_setopt($cURLHandle, CURLOPT_HEADER, 0);
curl_setopt($cURLHandle, CURLOPT_RETURNTRANSFER, true);
curl_setopt($cURLHandle, CURLOPT_TIMEOUT, $this->externalImageTimeout);
curl_setopt($cURLHandle, CURLOPT_FOLLOWLOCATION, true);
curl_setopt($cURLHandle, CURLOPT_MAXREDIRS, 10);
curl_setopt($cURLHandle, CURLOPT_SSL_VERIFYPEER, false);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
curl_setopt($cURLHandle, CURLOPT_FAILONERROR, true);

$cacheFileContent = curl_exec($cURLHandle);

$cURLErrNo = curl_errno($cURLHandle);
$cURLInfo = curl_getinfo($cURLHandle);

curl_close($cURLHandle);

if ($cURLErrNo != 0) {
$cacheFileContent = 'CURL_ERROR_' . $cURLErrNo;
}
if ($cURLInfo['http_code'] >= 400) {
$cacheFileContent = 'HTTP_CODE_' . $cURLInfo['http_code'];
}
}

return $cacheFileContent ?? null;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

private function downloadUsingFileGetContent(string $filename): string
{
$remoteURLContext = stream_context_create([
'http' => [
'method' => 'GET',
'timeout' => $this->externalImageTimeout,
'max_redirects' => '10',
]
]);

$cacheFileContent = file_get_contents($filename, false, $remoteURLContext);
if ($cacheFileContent === false) {
$cacheFileContent = 'FGC_ERROR';
}

return $cacheFileContent;
}
Comment thread
TatevikGr marked this conversation as resolved.

private function isCacheableUrl($filename): bool
{
if (!(str_starts_with($filename, 'http'))
|| str_contains($filename, '://' . $this->configProvider->getValue(ConfigOption::Website) . '/')
) {
return false;
Comment on lines +168 to +173
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Guard str_contains against null website config.

getValue() can return null; str_contains() will TypeError.

🛠️ Fix
-        if (!(str_starts_with($filename, 'http'))
-            || str_contains($filename, '://' . $this->configProvider->getValue(ConfigOption::Website) . '/')
-        ) {
+        $website = $this->configProvider->getValue(ConfigOption::Website) ?? '';
+        if (!(str_starts_with($filename, 'http'))
+            || ($website !== '' && str_contains($filename, '://' . $website . '/'))
+        ) {
             return false;
         }
🤖 Prompt for AI Agents
In `@src/Domain/Common/ExternalImageService.php` around lines 168 - 173, In
isCacheableUrl, guard the str_contains call against a null website value from
$this->configProvider->getValue(ConfigOption::Website): fetch the website into a
local variable (e.g. $website), verify it is a non-empty string (or coalesce to
an empty string) before calling str_contains(..., '://' . $website . '/'), and
only perform that str_contains check when $website is not null/empty so you
avoid a TypeError from str_contains when getValue returns null.

}

return true;
}

private function ensureCacheDirectory(): bool
{

if (!file_exists($this->externalCacheDir)) {
// phpcs:ignore Generic.PHP.NoSilencedErrors
@mkdir($this->externalCacheDir);
}

if (!file_exists($this->externalCacheDir) || !is_writable($this->externalCacheDir)) {
return false;
}

return true;
}

private function isValidCacheFile(string $cacheFileName): bool
{
// phpcs:ignore Generic.PHP.NoSilencedErrors
if (file_exists($cacheFileName) && (@filesize($cacheFileName) > 64)) {
return true;
}

return false;
}

private function writeCacheFile(string $cacheFileName, $content): void
{
// phpcs:ignore Generic.PHP.NoSilencedErrors
$bytes = @file_put_contents($cacheFileName, $content, LOCK_EX);

if ($bytes === false) {
$this->logger->error('Cache file write failed', ['file' => $cacheFileName]);
return;
}

$expected = strlen($content);
if ($bytes !== $expected) {
$this->logger->error('Cache file partial write', [
'file' => $cacheFileName,
'expected' => $expected,
'written' => $bytes,
]);
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Loading
Loading