From f65b90fb7dec475db0ffc26a7e048a88241a0097 Mon Sep 17 00:00:00 2001 From: Tatevik Date: Mon, 9 Feb 2026 14:27:59 +0400 Subject: [PATCH 1/3] AttachmentDownloadService --- .../AttachmentFileNotFoundException.php | 13 +++++ .../Model/Dto/DownloadableAttachment.php | 18 +++++++ .../Messaging/Service/AttachmentAdder.php | 3 +- .../Service/AttachmentDownloadService.php | 53 +++++++++++++++++++ .../Messaging/Service/AttachmentAdderTest.php | 2 +- 5 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 src/Domain/Messaging/Exception/AttachmentFileNotFoundException.php create mode 100644 src/Domain/Messaging/Model/Dto/DownloadableAttachment.php create mode 100644 src/Domain/Messaging/Service/AttachmentDownloadService.php diff --git a/src/Domain/Messaging/Exception/AttachmentFileNotFoundException.php b/src/Domain/Messaging/Exception/AttachmentFileNotFoundException.php new file mode 100644 index 00000000..b924b39a --- /dev/null +++ b/src/Domain/Messaging/Exception/AttachmentFileNotFoundException.php @@ -0,0 +1,13 @@ +getTo()[0]->getAddress(); - // todo: add endpoint in rest-api project - $viewUrl = $this->attachmentDownloadUrl . '/?id=' . $att->getId() . '&uid=' . $hash; + $viewUrl = $this->attachmentDownloadUrl . '/' . $att->getId() . '/?uid=' . $hash; $email->text( $email->getTextBody() diff --git a/src/Domain/Messaging/Service/AttachmentDownloadService.php b/src/Domain/Messaging/Service/AttachmentDownloadService.php new file mode 100644 index 00000000..7a816b25 --- /dev/null +++ b/src/Domain/Messaging/Service/AttachmentDownloadService.php @@ -0,0 +1,53 @@ +getFilename(); + if ($filename === null || $filename === '') { + throw new AttachmentFileNotFoundException(); + } + + $path = rtrim($this->attachmentRepositoryPath, '/'); + $filePath = $path . '/' . $filename; + + if (!is_file($filePath) || !is_readable($filePath)) { + throw new AttachmentFileNotFoundException(); + } + + $mimeType = $attachment->getMimeType() + ?? MimeTypes::getDefault()->guessMimeType($filePath) + ?? 'application/octet-stream'; + + $size = filesize($filePath); + $size = $size === false ? null : $size; + + /** @var StreamInterface $stream */ + $stream = Utils::streamFor(Utils::tryFopen($filePath, 'rb')); + + return new DownloadableAttachment( + filename: $filename, + mimeType: $mimeType, + size: $size, + content: $stream, + ); + } +} diff --git a/tests/Unit/Domain/Messaging/Service/AttachmentAdderTest.php b/tests/Unit/Domain/Messaging/Service/AttachmentAdderTest.php index 9356eb3d..511833cb 100644 --- a/tests/Unit/Domain/Messaging/Service/AttachmentAdderTest.php +++ b/tests/Unit/Domain/Messaging/Service/AttachmentAdderTest.php @@ -91,7 +91,7 @@ public function testTextModePrependsNoticeAndLinks(): void ); $this->assertStringContainsString('Doc description', $body); $this->assertStringContainsString('Location', $body); - $this->assertStringContainsString('https://dl.example/?id=42&uid=user@example.com', $body); + $this->assertStringContainsString('https://dl.example/42/?uid=user@example.com', $body); } public function testHtmlUsesRepositoryFileIfExists(): void From 14c47a3b4d1441a80df77fe231a34708e4673616 Mon Sep 17 00:00:00 2001 From: Tatevik Date: Mon, 9 Feb 2026 14:33:57 +0400 Subject: [PATCH 2/3] Add tests --- .../Service/AttachmentDownloadServiceTest.php | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 tests/Unit/Domain/Messaging/Service/AttachmentDownloadServiceTest.php diff --git a/tests/Unit/Domain/Messaging/Service/AttachmentDownloadServiceTest.php b/tests/Unit/Domain/Messaging/Service/AttachmentDownloadServiceTest.php new file mode 100644 index 00000000..64bf2551 --- /dev/null +++ b/tests/Unit/Domain/Messaging/Service/AttachmentDownloadServiceTest.php @@ -0,0 +1,101 @@ +tempDir = sys_get_temp_dir() . '/phplist-att-dl-' . bin2hex(random_bytes(5)); + if (!is_dir($this->tempDir)) { + mkdir($this->tempDir, 0777, true); + } + } + + protected function tearDown(): void + { + // cleanup temp directory + if (is_dir($this->tempDir)) { + $files = scandir($this->tempDir) ?: []; + foreach ($files as $f) { + if ($f === '.' || $f === '..') { + continue; + } + unlink($this->tempDir . '/' . $f); + } + rmdir($this->tempDir); + } + } + + public function testThrowsWhenFilenameIsEmpty(): void + { + $service = new AttachmentDownloadService($this->tempDir); + + $attachment = $this->createMock(Attachment::class); + $attachment->method('getFilename')->willReturn(''); + + $this->expectException(AttachmentFileNotFoundException::class); + $service->getDownloadable($attachment); + } + + public function testThrowsWhenFileDoesNotExist(): void + { + $service = new AttachmentDownloadService($this->tempDir); + + $attachment = $this->createMock(Attachment::class); + $attachment->method('getFilename')->willReturn('missing-file.pdf'); + + $this->expectException(AttachmentFileNotFoundException::class); + $service->getDownloadable($attachment); + } + + public function testReturnsDownloadableWithExplicitMimeType(): void + { + $service = new AttachmentDownloadService($this->tempDir); + + $filename = 'doc.pdf'; + $content = '%PDF-1.4\n'; + file_put_contents($this->tempDir . '/' . $filename, $content); + + $attachment = $this->createMock(Attachment::class); + $attachment->method('getFilename')->willReturn($filename); + $attachment->method('getMimeType')->willReturn('application/pdf'); + + $dl = $service->getDownloadable($attachment); + + $this->assertSame($filename, $dl->filename); + $this->assertSame('application/pdf', $dl->mimeType); + $this->assertSame(strlen($content), $dl->size); + $this->assertSame($content, (string)$dl->content); + } + + public function testGuessesMimeTypeAndProvidesStream(): void + { + $service = new AttachmentDownloadService($this->tempDir); + + $filename = 'note.txt'; + $content = "Hello, world!\n"; + file_put_contents($this->tempDir . '/' . $filename, $content); + + $attachment = $this->createMock(Attachment::class); + $attachment->method('getFilename')->willReturn($filename); + $attachment->method('getMimeType')->willReturn(null); + + $dl = $service->getDownloadable($attachment); + + $this->assertSame($filename, $dl->filename); + // Symfony MimeTypes should detect text/plain for .txt + $this->assertSame('text/plain', $dl->mimeType); + $this->assertSame(strlen($content), $dl->size); + $this->assertSame($content, (string)$dl->content); + } +} From 0838e277e05640bbeb2dde53c21699eecfa5187d Mon Sep 17 00:00:00 2001 From: Tatevik Date: Mon, 9 Feb 2026 14:47:33 +0400 Subject: [PATCH 3/3] After review 0 --- composer.json | 4 ++-- .../Model/Dto/DownloadableAttachment.php | 2 +- .../Service/AttachmentDownloadService.php | 21 ++++++++++++++----- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/composer.json b/composer.json index 0a39fd1b..f49193ab 100644 --- a/composer.json +++ b/composer.json @@ -86,11 +86,11 @@ "ext-curl": "*", "ext-fileinfo": "*", "setasign/fpdf": "^1.8", - "phpdocumentor/reflection-docblock": "^5.2" + "phpdocumentor/reflection-docblock": "^5.2", + "guzzlehttp/guzzle": "^6.3.0" }, "require-dev": { "phpunit/phpunit": "^9.5", - "guzzlehttp/guzzle": "^6.3.0", "squizlabs/php_codesniffer": "^3.2.0", "phpstan/phpstan": "^1.10", "nette/caching": "^3.0.0", diff --git a/src/Domain/Messaging/Model/Dto/DownloadableAttachment.php b/src/Domain/Messaging/Model/Dto/DownloadableAttachment.php index 93b258aa..f02265fb 100644 --- a/src/Domain/Messaging/Model/Dto/DownloadableAttachment.php +++ b/src/Domain/Messaging/Model/Dto/DownloadableAttachment.php @@ -11,7 +11,7 @@ final class DownloadableAttachment public function __construct( public readonly string $filename, public readonly string $mimeType, - public readonly int $size, + public readonly ?int $size, public readonly StreamInterface $content, ) { } diff --git a/src/Domain/Messaging/Service/AttachmentDownloadService.php b/src/Domain/Messaging/Service/AttachmentDownloadService.php index 7a816b25..97302284 100644 --- a/src/Domain/Messaging/Service/AttachmentDownloadService.php +++ b/src/Domain/Messaging/Service/AttachmentDownloadService.php @@ -21,15 +21,26 @@ public function __construct( public function getDownloadable(Attachment $attachment): DownloadableAttachment { - $filename = $attachment->getFilename(); - if ($filename === null || $filename === '') { + $original = $attachment->getFilename(); + $filename = basename($original); + + if ($filename === '' || $filename !== $original) { throw new AttachmentFileNotFoundException(); } - $path = rtrim($this->attachmentRepositoryPath, '/'); - $filePath = $path . '/' . $filename; + $baseDir = realpath($this->attachmentRepositoryPath); + if ($baseDir === false) { + throw new \RuntimeException('Attachment repository path does not exist.'); + } + + $filePath = $baseDir . DIRECTORY_SEPARATOR . $filename; + $realPath = realpath($filePath); - if (!is_file($filePath) || !is_readable($filePath)) { + if ($realPath === false || + !str_starts_with($realPath, $baseDir . DIRECTORY_SEPARATOR) || + !is_file($realPath) || + !is_readable($realPath) + ) { throw new AttachmentFileNotFoundException(); }