Skip to content

Commit 165c59c

Browse files
committed
After review 0
1 parent 4a9e895 commit 165c59c

12 files changed

Lines changed: 65 additions & 66 deletions

File tree

config/PhpCodeSniffer/ruleset.xml

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,6 @@
5454
<!-- Functions -->
5555
<rule ref="Squiz.Functions.FunctionDuplicateArgument"/>
5656
<rule ref="Squiz.Functions.GlobalFunction"/>
57-
<rule ref="Squiz.WhiteSpace.OperatorSpacing">
58-
<properties>
59-
<property name="ignoreNewlines" value="true"/>
60-
</properties>
61-
</rule>
6257

6358
<!-- Metrics -->
6459
<rule ref="Generic.Metrics.CyclomaticComplexity"/>
@@ -108,6 +103,10 @@
108103
<rule ref="PEAR.WhiteSpace.ScopeClosingBrace"/>
109104
<rule ref="Squiz.WhiteSpace.CastSpacing"/>
110105
<rule ref="Squiz.WhiteSpace.LogicalOperatorSpacing"/>
111-
<rule ref="Squiz.WhiteSpace.OperatorSpacing"/>
106+
<rule ref="Squiz.WhiteSpace.OperatorSpacing">
107+
<properties>
108+
<property name="ignoreNewlines" value="true"/>
109+
</properties>
110+
</rule>
112111
<rule ref="Squiz.WhiteSpace.SemicolonSpacing"/>
113112
</ruleset>

resources/translations/messages.en.xlf

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,10 @@ Thank you.</target>
746746
<source>phplist has started sending the campaign with subject %s</source>
747747
<target>__phplist has started sending the campaign with subject %s</target>
748748
</trans-unit>
749+
<trans-unit id="jE9yPKp" resname="phplist has started sending the campaign with subject %subject%">
750+
<source>phplist has started sending the campaign with subject %subject%</source>
751+
<target>__phplist has started sending the campaign with subject %subject%</target>
752+
</trans-unit>
749753
</body>
750754
</file>
751755
</xliff>

src/Domain/Common/ExternalImageService.php

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -202,27 +202,19 @@ private function isValidCacheFile(string $cacheFileName): bool
202202

203203
private function writeCacheFile(string $cacheFileName, $content): void
204204
{
205-
try {
206-
$handle = fopen($cacheFileName, 'wb');
207-
208-
if ($handle === false) {
209-
$this->logger->error('Cannot open cache file', [
210-
'file' => $cacheFileName,
211-
]);
212-
return;
213-
}
205+
$bytes = @file_put_contents($cacheFileName, $content, LOCK_EX);
214206

215-
if (flock($handle, LOCK_EX)) {
216-
fwrite($handle, $content);
217-
fflush($handle);
218-
flock($handle, LOCK_UN);
219-
}
207+
if ($bytes === false) {
208+
$this->logger->error('Cache file write failed', ['file' => $cacheFileName]);
209+
return;
210+
}
220211

221-
fclose($handle);
222-
} catch (Throwable $e) {
223-
$this->logger->error('Cache file write failed', [
212+
$expected = strlen($content);
213+
if ($bytes !== $expected) {
214+
$this->logger->error('Cache file partial write', [
224215
'file' => $cacheFileName,
225-
'error' => $e->getMessage(),
216+
'expected' => $expected,
217+
'written' => $bytes,
226218
]);
227219
}
228220
}

src/Domain/Common/RemotePageFetcher.php

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public function __invoke(string $url, array $userData): string
6767
$this->entityManager->remove($cache);
6868
}
6969
$urlCache = (new UrlCache())->setUrl($url)->setContent($content)->setLastModified($lastModified);
70-
$this->entityManager->persist($urlCache);
70+
$this->urlCacheRepository->persist($urlCache);
7171

7272
$this->cache->set($cacheKey, [
7373
'fetched' => time(),
@@ -115,15 +115,8 @@ private function prepareUrl(string $url, array $userData): string
115115

116116
foreach ($userData as $key => $val) {
117117
if ($key !== 'password') {
118-
$url = mb_convert_encoding(
119-
str_ireplace(
120-
'[' . $key . ']',
121-
urlencode($val),
122-
mb_convert_encoding($url, 'ISO-8859-1', 'UTF-8')
123-
),
124-
'ISO-8859-1',
125-
'UTF-8'
126-
);
118+
$url = str_replace('[' . $key . ']', rawurlencode($val), $url);
119+
$url = mb_convert_encoding($url, 'ISO-8859-1', 'UTF-8');
127120
}
128121
}
129122

src/Domain/Common/TextParser.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public function __invoke(string $text): string
2525
//<a href="javascript:window.open('http://hacker.com?cookie='+document.cookie)">
2626
$url = preg_replace('/:/', '', $url);
2727
}
28-
$link[$index] = '<a href=" ' .$url . '" ' . $rest . '>' . $matches[4] . '</a>';
28+
$link[$index] = '<a href="' . $url . '" ' . $rest . '>' . $matches[4] . '</a>';
2929
$text = $matches[1] . '%%' . $index . '%%' . $matches[5];
3030
++$index;
3131
}

src/Domain/Messaging/MessageHandler/CampaignProcessorMessageHandler.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ public function __invoke(CampaignProcessorMessage|SyncCampaignProcessorMessage $
110110
// }
111111
// $userSelection = $loadedMessageData['userselection'];
112112

113-
$cacheKey = sprintf('messaging.message.base.%d', $campaign->getId());
113+
$cacheKey = sprintf('messaging.message.base.%d.%d', $campaign->getId(), 0);
114114
if (!$this->precacheService->precacheMessage($campaign, $loadedMessageData)) {
115115
$this->updateMessageStatus($campaign, MessageStatus::Suspended);
116116

@@ -298,12 +298,16 @@ private function handleAdminNotifications(Message $campaign, array $loadedMessag
298298
to: $notification,
299299
subject: $this->translator->trans('Campaign started'),
300300
message: $this->translator->trans(
301-
'phplist has started sending the campaign with subject %s',
302-
$loadedMessageData['subject']
301+
'phplist has started sending the campaign with subject %subject%',
302+
['%subject%' => $loadedMessageData['subject']]
303303
),
304304
inBlast: false,
305305
);
306306

307+
if (!$email) {
308+
continue;
309+
}
310+
307311
// todo: check if from name should be from config
308312
$envelope = new Envelope(
309313
sender: new Address($this->messageEnvelope, 'PHPList'),

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,24 +8,24 @@
88
class MessagePrecacheDto
99
{
1010
public string $replyToEmail = '';
11-
public string $replyToName;
12-
public string $fromName;
13-
public string $fromEmail;
14-
public string $to;
15-
public string $subject;
16-
public string $content;
11+
public ?string $replyToName = null;
12+
public ?string $fromName = null;
13+
public ?string $fromEmail = null;
14+
public ?string $to = null;
15+
public ?string $subject = null;
16+
public ?string $content = null;
1717
public string $textContent = '';
18-
public string $footer;
19-
public string $textFooter;
18+
public ?string $footer = null;
19+
public ?string $textFooter = null;
2020
public string $htmlFooter = '';
21-
public bool $htmlFormatted;
22-
public string $sendFormat;
21+
public bool $htmlFormatted = false;
22+
public ?string $sendFormat = null;
2323
public ?string $template = null;
2424
public ?string $templateText = null;
2525
public ?int $templateId = null;
2626
// public string $htmlCharset= 'UTF-8';
2727
// public string $textCharset= 'UTF-8';
28-
public bool $userSpecificUrl;
29-
public string $googleTrack;
28+
public bool $userSpecificUrl = false;
29+
public ?string $googleTrack = null;
3030
public array $adminAttributes = [];
3131
}

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -77,23 +77,23 @@ public function buildPhplistEmail(
7777

7878
private function validateRecipientAndSubject(?string $to, ?string $subject): bool
7979
{
80-
if (preg_match("/\n/", $to)) {
81-
$this->eventLogManager->log('', 'Error: invalid recipient, containing newlines, email blocked');
80+
if (!$to) {
81+
$this->eventLogManager->log('', sprintf('Error: empty To: in message with subject %s to send', $subject));
8282

8383
return false;
8484
}
85-
if (preg_match("/\n/", $subject)) {
86-
$this->eventLogManager->log('', 'Error: invalid subject, containing newlines, email blocked');
85+
if (!$subject) {
86+
$this->eventLogManager->log('', sprintf('Error: empty Subject: in message to send to %s', $to));
8787

8888
return false;
8989
}
90-
if (!$to) {
91-
$this->eventLogManager->log('', sprintf('Error: empty To: in message with subject %s to send', $subject));
90+
if (preg_match("/\n/", $to)) {
91+
$this->eventLogManager->log('', 'Error: invalid recipient, containing newlines, email blocked');
9292

9393
return false;
9494
}
95-
if (!$subject) {
96-
$this->eventLogManager->log('', sprintf('Error: empty Subject: in message to send to %s', $to));
95+
if (preg_match("/\n/", $subject)) {
96+
$this->eventLogManager->log('', 'Error: invalid subject, containing newlines, email blocked');
9797

9898
return false;
9999
}

src/Domain/Messaging/Service/Manager/TemplateImageManager.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,11 +167,9 @@ private function decodeLogoImageData(?string $logoData): ?string
167167
return $imageContent;
168168
}
169169

170-
$fallback = base64_decode('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABAQMAAAAl21bKAAAABGdBTUEAALGPC/'
170+
$fallbackContent = base64_decode('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABAQMAAAAl21bKAAAABGdBTUEAALGPC/'
171171
. 'xhBQAAAAZQTFRF////AAAAVcLTfgAAAAF0Uk5TAEDm2GYAAAABYktHRACIBR1IAAAACXBIWXMAAAsSAAALEgHS3X78'
172-
. 'AAAAB3RJTUUH0gQCEx05cqKA8gAAAApJREFUeJxjYAAAAAIAAUivpHEAAAAASUVORK5CYII=');
173-
174-
$fallbackContent = base64_decode($fallback, true);
172+
. 'AAAAB3RJTUUH0gQCEx05cqKA8gAAAApJREFUeJxjYAAAAAIAAUivpHEAAAAASUVORK5CYII=', true);
175173

176174
return $fallbackContent !== false ? $fallbackContent : null;
177175
}

src/Domain/Messaging/Service/MessageDataLoader.php

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use PhpList\Core\Domain\Messaging\Model\Message;
1010
use PhpList\Core\Domain\Messaging\Repository\MessageDataRepository;
1111
use PhpList\Core\Domain\Messaging\Repository\MessageRepository;
12+
use Psr\Log\LoggerInterface;
1213

1314
class MessageDataLoader
1415
{
@@ -20,6 +21,7 @@ public function __construct(
2021
private readonly ConfigProvider $configProvider,
2122
private readonly MessageDataRepository $messageDataRepository,
2223
private readonly MessageRepository $messageRepository,
24+
private readonly LoggerInterface $logger,
2325
private readonly int $defaultMessageAge,
2426
) {
2527
$this->messageFromAddress = $configProvider->getValue(ConfigOption::MessageFromAddress);
@@ -45,6 +47,8 @@ public function __invoke(Message $message): array
4547

4648
private function getDateArray(?int $timestamp = null): array
4749
{
50+
$timestamp = $timestamp ?? time();
51+
4852
return [
4953
'year' => date('Y', $timestamp),
5054
'month' => date('m', $timestamp),
@@ -113,6 +117,10 @@ private function mergeStoredMessageData(array &$messageData, Message $message):
113117
foreach ($storedMessageData as $storedMessageDatum) {
114118
if (str_starts_with($storedMessageDatum->getData(), 'SER:')) {
115119
$unserialized = unserialize(substr($storedMessageDatum->getData(), 4), ['allowed_classes' => false]);
120+
if (!is_array($unserialized)) {
121+
$this->logger->warning('Invalid serialized data for message ID: ' . $message->getId());
122+
continue;
123+
}
116124
array_walk_recursive($unserialized, function (&$val) {
117125
$val = stripslashes($val);
118126
});
@@ -190,7 +198,7 @@ private function normaliseFromField(array &$messageData, ?string $defaultFrom):
190198
$messageData['fromname'] = trim($messageData['fromname']);
191199

192200
// erase double spacing
193-
while (strpos($messageData['fromname'], ' ')) {
201+
while (str_contains($messageData['fromname'], ' ')) {
194202
$messageData['fromname'] = str_replace(' ', ' ', $messageData['fromname']);
195203
}
196204

0 commit comments

Comments
 (0)