Skip to content

Commit b07f4d1

Browse files
committed
After review 1
1 parent 309f8a7 commit b07f4d1

3 files changed

Lines changed: 13 additions & 8 deletions

File tree

src/Domain/Messaging/Service/AttachmentAdder.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public function add(Email $email, int $campaignId, OutputFormat $format, bool $f
5858

5959
case OutputFormat::Text:
6060
$hash = $forwarded ? Attachment::FORWARD : $email->getTo()[0]->getAddress();
61-
$viewUrl = $this->attachmentDownloadUrl . '/' . $att->getId() . '/?uid=' . $hash;
61+
$viewUrl = $this->attachmentDownloadUrl . '/' . $att->getId() . '/?uid=' . urlencode($hash);
6262

6363
$email->text(
6464
$email->getTextBody()

src/Domain/Messaging/Service/AttachmentDownloadService.php

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
use GuzzleHttp\Psr7\Utils;
88
use PhpList\Core\Domain\Messaging\Exception\AttachmentFileNotFoundException;
9-
use PhpList\Core\Domain\Messaging\Exception\MessageNotReceivedException;
9+
use PhpList\Core\Domain\Messaging\Exception\SubscriberNotFoundException;
1010
use PhpList\Core\Domain\Messaging\Model\Attachment;
1111
use PhpList\Core\Domain\Messaging\Model\Dto\DownloadableAttachment;
1212
use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository;
@@ -27,6 +27,9 @@ public function getDownloadable(Attachment $attachment, string $uid): Downloadab
2727
$this->validateUid($uid);
2828

2929
$original = $attachment->getFilename();
30+
if ($original === null || $original === '') {
31+
throw new AttachmentFileNotFoundException('Attachment has no filename.');
32+
}
3033
$filename = basename($original);
3134
$filePath = $this->validateFilePath($filename, $original);
3235

@@ -50,11 +53,13 @@ public function getDownloadable(Attachment $attachment, string $uid): Downloadab
5053

5154
private function validateUid(string $uid): void
5255
{
53-
if ($uid !== Attachment::FORWARD) {
54-
$subscriber = $this->subscriberRepository->findOneByEmail($uid);
55-
if ($subscriber === null) {
56-
throw new MessageNotReceivedException();
57-
}
56+
if ($uid === Attachment::FORWARD) {
57+
return;
58+
}
59+
60+
$subscriber = $this->subscriberRepository->findOneByEmail($uid);
61+
if ($subscriber === null) {
62+
throw new SubscriberNotFoundException();
5863
}
5964
}
6065

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public function testTextModePrependsNoticeAndLinks(): void
9191
);
9292
$this->assertStringContainsString('Doc description', $body);
9393
$this->assertStringContainsString('Location', $body);
94-
$this->assertStringContainsString('https://dl.example/42/?uid=user@example.com', $body);
94+
$this->assertStringContainsString('https://dl.example/42/?uid=' . urlencode('user@example.com'), $body);
9595
}
9696

9797
public function testHtmlUsesRepositoryFileIfExists(): void

0 commit comments

Comments
 (0)